[PATCH BlueZ 1/2] profiles/midi: Fix running status across channels

[PATCH BlueZ 1/2] profiles/midi: Fix running status across channels

[RasmusML]

From: RasmusML <rasmusml@hotmail.dk>

When two MIDI messages of the same type but on different channels end
up in the same BLE-MIDI packet, read_ev_others() writes the second
status byte without the timestampLow byte that must precede every
status byte. A receiver then reads that status byte as the timestamp
and applies the following data bytes to the previous message's channel
through running status, e.g.:

  sent:    ch1 NoteOff 60, ch2 NoteOff 64, ch3 CC123, ch4 CC123
  packet:  B5 94 81 3C 00 82 40 00 94 B3 7B 00 B4 7B 00
  decoded: ch1 NoteOff 60, ch1 NoteOff 64, ch3 CC123, ch3 CC123

This happens because rstatus holds the ALSA event type, which carries
no channel, so the writer assumes running status while
snd_midi_event_decode() correctly emits a new status byte.

Fix this by decoding each message into a local buffer first and
appending timestampLow whenever the decoder actually wrote a status
byte. If the message does not fit, flush the packet and, if it relied
on running status, decode it again with a reset decoder so the new
packet starts with a full status byte. This replaces the -ENOMEM retry
path.

Closes: https://github.com/bluez/bluez/issues/2630
Assisted-by: Claude:claude-opus-5-5
---
 profiles/midi/libmidi.c | 59 +++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/profiles/midi/libmidi.c b/profiles/midi/libmidi.c
index ef33f7dfd..349f5f7b2 100644
--- a/profiles/midi/libmidi.c
+++ b/profiles/midi/libmidi.c
@@ -191,40 +191,47 @@ static void read_ev_sysex(struct midi_write_parser *parser, const snd_seq_event_
 static void read_ev_others(struct midi_write_parser *parser, const snd_seq_event_t *ev,
                            midi_read_ev_cb write_cb, void *user_data)
 {
-	int length;
+	uint8_t msg[MIDI_MSG_MAX_SIZE];
+	long length;
+	bool status;
 
-	/* check for running status */
-	if (parser->rstatus != ev->type) {
+	/* running status only ever continues the same event type */
+	if (parser->rstatus != ev->type)
 		snd_midi_event_reset_decode(parser->midi_ev);
-		append_timestamp_low(parser);
-	}
 
-	/* each midi message has timestampLow byte to follow */
-	length = snd_midi_event_decode(parser->midi_ev,
-	                               parser->midi_stream.data +
-	                               parser->midi_stream.len,
-	                               parser_get_available_size(parser),
-	                               ev);
-
-	if (length == -ENOMEM) {
-		/* remove previously added timestampLow */
-		if (parser->rstatus != ev->type)
-			parser->midi_stream.len--;
+	length = snd_midi_event_decode(parser->midi_ev, msg, sizeof(msg), ev);
+	if (length <= 0)
+		return;
+
+	/*
+	 * The decoder leaves the status byte out only when it repeats the
+	 * previous one, channel included, and every status byte it does write
+	 * needs a timestampLow before it. Decide that from the decoded message,
+	 * not from the event type: two note-ons on different channels share a
+	 * type but not a status byte.
+	 */
+	status = msg[0] & 0x80;
+
+	if (parser_get_available_size(parser) < (size_t) length + status) {
 		write_cb(parser, user_data);
-		/* cleanup state for next packet */
-		snd_midi_event_reset_decode(parser->midi_ev);
 		midi_write_reset(parser);
 		append_timestamp_high_maybe(parser);
-		append_timestamp_low(parser);
-		length = snd_midi_event_decode(parser->midi_ev,
-		                               parser->midi_stream.data +
-		                               parser->midi_stream.len,
-		                               parser_get_available_size(parser),
-		                               ev);
+
+		/* a new packet cannot continue running status */
+		if (!status) {
+			snd_midi_event_reset_decode(parser->midi_ev);
+			length = snd_midi_event_decode(parser->midi_ev, msg,
+							sizeof(msg), ev);
+			if (length <= 0)
+				return;
+			status = true;
+		}
 	}
 
-	if (length > 0)
-		parser->midi_stream.len += length;
+	if (status)
+		append_timestamp_low(parser);
+
+	buffer_append_data(&parser->midi_stream, msg, length);
 }
 
 void midi_read_ev(struct midi_write_parser *parser, const snd_seq_event_t *ev,
-- 
2.43.0

[PATCH BlueZ 2/2] unit/test-midi: Add multi-channel writer test

[RasmusML]

From: RasmusML <rasmusml@hotmail.dk>

Add a writer test with messages of the same type on different channels
for Note On, Note Off, Control Change and Pitch Bend. Each of these
needs its own status byte preceded by timestampLow, and running status
must only be used when the channel matches too.

The existing writer tests use a single channel for every event, which
is why the missing timestampLow was not caught before.

Assisted-by: Claude:claude-opus-5-5
---
 unit/test-midi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/unit/test-midi.c b/unit/test-midi.c
index 702133e59..79cb03123 100644
--- a/unit/test-midi.c
+++ b/unit/test-midi.c
@@ -525,6 +525,24 @@ static const snd_seq_event_t event5_expect[] = {
 
 static const struct midi_write_test midi5 = BLE_WRITE_TEST_INIT(event5, event5_expect);
 
+/*
+ * Same event type on different channels: each needs its own status byte and
+ * timestampLow, so running status must not carry over between them.
+ */
+static const snd_seq_event_t event6[] = {
+	NOTE_EVENT(NOTEON, 0, 60, 100),       /* Note On */
+	NOTE_EVENT(NOTEON, 1, 64, 100),       /* Note On, other channel */
+	NOTE_EVENT(NOTEON, 1, 67, 100),       /* Note On, running status */
+	NOTE_EVENT(NOTEOFF, 0, 60, 0),        /* Note Off */
+	NOTE_EVENT(NOTEOFF, 1, 64, 0),        /* Note Off, other channel */
+	CONTROL_EVENT(CONTROLLER, 2, 0, 123), /* All Notes Off */
+	CONTROL_EVENT(CONTROLLER, 3, 0, 123), /* All Notes Off, other channel */
+	CONTROL_EVENT(PITCHBEND, 4, 0, 0),    /* Pitch Bend */
+	CONTROL_EVENT(PITCHBEND, 5, 0, 0),    /* Pitch Bend, other channel */
+};
+
+static const struct midi_write_test midi6 = BLE_WRITE_TEST_INIT_BASIC(event6);
+
 struct midi_data {
 	size_t events_tested;
 	const struct midi_write_test *midi_test;
@@ -618,6 +636,8 @@ int main(int argc, char *argv[])
 	           &midi4, NULL, test_midi_writer, NULL);
 	tester_add("Split ALSA SysEx events to raw BLE packets",
 	           &midi5, NULL, test_midi_writer, NULL);
+	tester_add("ALSA Seq events on several channels to Raw BLE packets",
+			&midi6, NULL, test_midi_writer, NULL);
 
 	return tester_run();
 }
-- 
2.43.0

RE: [BlueZ,1/2] profiles/midi: Fix running status across channels

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1199554/

---Test result---

Test Summary:
CheckPatch                    PASS      0.73 seconds
GitLint                       PASS      0.51 seconds
BuildEll                      PASS      14.55 seconds
BluezMake                     PASS      193.26 seconds
MakeCheck                     PASS      13.40 seconds
MakeDistcheck                 PASS      110.90 seconds
CheckValgrind                 PASS      168.74 seconds
CheckSmatch                   PASS      192.40 seconds
bluezmakeextell               PASS      67.73 seconds
TestFunctional                PASS      846.42 seconds
IncrementalBuild              PASS      185.30 seconds
ScanBuild                     PASS      646.37 seconds



https://github.com/bluez/bluez/pull/2750

---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/2] profiles/midi: Fix running status across channels

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon,  2 Nov 2026 23:12:22 +0100 you wrote:
> From: RasmusML <rasmusml@hotmail.dk>
> 
> When two MIDI messages of the same type but on different channels end
> up in the same BLE-MIDI packet, read_ev_others() writes the second
> status byte without the timestampLow byte that must precede every
> status byte. A receiver then reads that status byte as the timestamp
> and applies the following data bytes to the previous message's channel
> through running status, e.g.:
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/2] profiles/midi: Fix running status across channels
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=737e84da251a
  - [BlueZ,2/2] unit/test-midi: Add multi-channel writer test
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ae27e4a2baf4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html