From 350eb9a041fb187cc5a0bcc74b42ff7521851e77 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Mon, 1 Jun 2026 13:08:11 +0200 Subject: [PATCH] midi: don't write trailing continuation 0xf0 for SysEx Because our midi messages already have a size, we don't need the 0xf0 continuation terminator. Also having the terminator optionally requires you to check and strip it if it's there. The easiest algorithm is to check the first byte for start (0xf0) or continuation (0xf7) and the last byte for end (0xf7) and that should be enough to process the messages without having to ever stip the last byte. --- doc/dox/internals/midi.dox | 10 +++--- pipewire-jack/src/pipewire-jack.c | 4 +++ spa/plugins/alsa/alsa-seq.c | 2 -- src/modules/module-ffado-driver.c | 4 +++ src/modules/module-jack-tunnel.c | 4 +++ src/modules/module-netjack2/peer.c | 4 +++ src/modules/module-rtp/midi.c | 49 ++++++++++++++++++------------ src/modules/module-vban/midi.c | 2 +- src/tools/midifile.c | 3 -- src/tools/pw-cat.c | 5 --- 10 files changed, 52 insertions(+), 35 deletions(-) diff --git a/doc/dox/internals/midi.dox b/doc/dox/internals/midi.dox index 35cc1d634..496d1aad2 100644 --- a/doc/dox/internals/midi.dox +++ b/doc/dox/internals/midi.dox @@ -76,23 +76,25 @@ F0 byte and end with a F7 byte. Because of the buffer data length limitations, it might be necessary to split a MIDI1 SysEx message accross multiple buffers. -The stategy to implement this is specified in RFC 6295 (RTP Midi) section +The stategy to implement this is inspired by RFC 6295 (RTP Midi) section 3.2. Long SysEx messages can be split up into parts by using the following start/end bytes combinations: |-----------------------------------------------------------| | Sublist Position | Head Status Octet | Tail Status Octet | |-----------------------------------------------------------| - | first | 0xF0 | (0xF0) | + | first | 0xF0 | | |-----------------------------------------------------------| - | middle | 0xF7 | (0xF0) | + | middle | 0xF7 | | |-----------------------------------------------------------| | last | 0xF7 | 0xF7 | |-----------------------------------------------------------| | cancel | 0xF7 | 0xF4 | ----------------------------------------------------------- -The trailing 0xf0 byte can be omitted at the end of continuation packets. +Because control packets have a size, there is no need for a tail +status byte for incomplete first and middle/continuation packets. +They have a regular data byte as the last octet (contrary to RFC 6295). Nodes that require a complete SysEx message must be able to assemble the complete message from the parts before processing the message. diff --git a/pipewire-jack/src/pipewire-jack.c b/pipewire-jack/src/pipewire-jack.c index 71314c957..ee5356208 100644 --- a/pipewire-jack/src/pipewire-jack.c +++ b/pipewire-jack/src/pipewire-jack.c @@ -1647,6 +1647,10 @@ static void convert_to_event(struct mix_info **mix, uint32_t n_mix, void *midi, break; } } else { + if (size > 1 && data[0] == 0xf7) { + data++; + size--; + } res = midi_event_write(midi, control->offset, data, size, fix); } if (res < 0) diff --git a/spa/plugins/alsa/alsa-seq.c b/spa/plugins/alsa/alsa-seq.c index df6ceee88..893930464 100644 --- a/spa/plugins/alsa/alsa-seq.c +++ b/spa/plugins/alsa/alsa-seq.c @@ -886,8 +886,6 @@ static int process_write(struct seq_state *state) continue; if (body[0] == 0xf0 || body[0] == 0xf7) { - if (body[body_size-1] == 0xf0) - body_size -= 1; if (body[0] == 0xf7) { body += 1; body_size -= 1; diff --git a/src/modules/module-ffado-driver.c b/src/modules/module-ffado-driver.c index 69646f742..acec767f3 100644 --- a/src/modules/module-ffado-driver.c +++ b/src/modules/module-ffado-driver.c @@ -354,6 +354,10 @@ static void midi_to_ffado(struct port *p, float *src, uint32_t n_samples) if (index < c.offset) index = SPA_ROUND_UP_N(c.offset, 8); + if (size > 1 && data[0] == 0xf7) { + data++; + size--; + } for (j = 0; j < size; j++) { if (index >= n_samples) { /* keep events that don't fit for the next cycle */ diff --git a/src/modules/module-jack-tunnel.c b/src/modules/module-jack-tunnel.c index e4a8685ab..14f45744d 100644 --- a/src/modules/module-jack-tunnel.c +++ b/src/modules/module-jack-tunnel.c @@ -284,6 +284,10 @@ static void midi_to_jack(struct impl *impl, float *dst, float *src, uint32_t n_s data = tmp; size = 3; } + if (size > 1 && data[0] == 0xf7) { + data++; + size--; + } if ((res = jack.midi_event_write(dst, c.offset, data, size)) < 0) pw_log_warn("midi %p: can't write event: %s", dst, spa_strerror(res)); diff --git a/src/modules/module-netjack2/peer.c b/src/modules/module-netjack2/peer.c index 76f3a9ed1..61a999ef9 100644 --- a/src/modules/module-netjack2/peer.c +++ b/src/modules/module-netjack2/peer.c @@ -351,6 +351,10 @@ static void midi_to_netjack2(struct netjack2_peer *peer, buf->lost_events++; continue; } + if (size > 1 && data[0] == 0xf7) { + data++; + size--; + } n2j_midi_buffer_write(buf, c.offset, data, size, peer->fix_midi); } if (buf->write_pos > 0) diff --git a/src/modules/module-rtp/midi.c b/src/modules/module-rtp/midi.c index 7f97a7535..43e629acc 100644 --- a/src/modules/module-rtp/midi.c +++ b/src/modules/module-rtp/midi.c @@ -270,7 +270,7 @@ static int rtp_midi_receive_midi(struct impl *impl, uint8_t *packet, uint32_t ti while (offs < end) { uint32_t delta; - int size; + int size, tail_trim = 0; if (first && !hdr.z) delta = 0; @@ -290,9 +290,11 @@ static int rtp_midi_receive_midi(struct impl *impl, uint8_t *packet, uint32_t ti packet[offs], size, offs, end); return -EINVAL; } + if (packet[offs + size-1] == 0xf0) + tail_trim++; spa_pod_builder_control(&b, timestamp, SPA_CONTROL_Midi); - spa_pod_builder_bytes(&b, &packet[offs], size); + spa_pod_builder_bytes(&b, &packet[offs], size - tail_trim); offs += size; first = false; @@ -372,34 +374,41 @@ unexpected_ssrc: return -EINVAL; } -static int write_event(uint8_t *p, uint32_t buffer_size, uint32_t value, const void *ev, uint32_t size) +static int write_event(uint8_t *p, uint32_t buffer_size, uint32_t delta, const uint8_t *ev, uint32_t size) { - uint64_t buffer; - uint8_t b; + uint64_t buffer; + uint8_t b; unsigned int count = 0; + uint32_t total; - if (buffer_size <= size) + total = size; + if (ev[size-1] != 0xf7) + total++; + + if (buffer_size <= total) return -ENOSPC; - buffer = value & 0x7f; - while ((value >>= 7)) { + buffer = delta & 0x7f; + while ((delta >>= 7)) { if (buffer > (UINT64_MAX >> 8)) return -ERANGE; - buffer <<= 8; - buffer |= ((value & 0x7f) | 0x80); - } - do { - if (count >= buffer_size) - return -ENOSPC; + buffer <<= 8; + buffer |= ((delta & 0x7f) | 0x80); + } + do { + if (count >= buffer_size) + return -ENOSPC; b = buffer & 0xff; - p[count++] = b; - buffer >>= 8; - } while (b & 0x80); + p[count++] = b; + buffer >>= 8; + } while (b & 0x80); - if (buffer_size - size < count || - count + size > (unsigned int)INT_MAX) + if (buffer_size - total < count || + count + total > (unsigned int)INT_MAX) return -ENOSPC; memcpy(&p[count], ev, size); - return (int)(count + size); + if (size < total) + p[count+size] = 0xf0; + return (int)(count + total); } static void rtp_midi_flush_packets(struct impl *impl, diff --git a/src/modules/module-vban/midi.c b/src/modules/module-vban/midi.c index dae0b12f1..44c4cdaa0 100644 --- a/src/modules/module-vban/midi.c +++ b/src/modules/module-vban/midi.c @@ -51,7 +51,7 @@ static void vban_midi_process_playback(void *data) goto done; /* the ringbuffer contains series of sequences, one for each - * received packet. This is not share mem so we can use the + * received packet. This is not shared mem so we can use the * iterator. */ SPA_POD_SEQUENCE_FOREACH((struct spa_pod_sequence*)pod, c) { #if 0 diff --git a/src/tools/midifile.c b/src/tools/midifile.c index 5525af31f..f7a347a77 100644 --- a/src/tools/midifile.c +++ b/src/tools/midifile.c @@ -560,9 +560,6 @@ int midi_file_write_event(struct midi_file *mf, const struct midi_event *event) ev_size--; ev_data++; - if (ev_data[ev_size-1] == 0xf0) - ev_size--; - CHECK_RES(write_varlen(mf, tr, ev_size)); tr->size += 1; diff --git a/src/tools/pw-cat.c b/src/tools/pw-cat.c index 8a41cd37c..c45294a89 100644 --- a/src/tools/pw-cat.c +++ b/src/tools/pw-cat.c @@ -1569,11 +1569,6 @@ static int sysex_play(struct data *d, void *dst, uint32_t maxsize, unsigned int bytes[size] = 0xf7; size += 1; } - } else { - if (bytes[size-1] != 0xf0) { - bytes[size] = 0xf0; - size += 1; - } } if (d->sysex.first) { if (bytes[0] != 0xf0) {