Message-Id: <20250313-cme-fix-v1-1-d404889e4de8@butoba.net>
Date: Thu, 13 Mar 2025 23:16:17 +0100
From: Ricard Wanderlof <ricard2013@...oba.net>
To: Clemens Ladisch <clemens@...isch.de>,
	Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>
Cc: Ricard Wanderlof <ricard2013@...oba.net>,
	linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] ALSA: usb-audio: Fix CME quirk for UF series keyboards

Fix quirk for CME master keyboards so it not only handles
sysex but also song position pointer, MIDI timing clock, start
and stop messages, and active sensing. All of these can be
output by the CME UF series master keyboards.

Tested with a CME UF6 in a desktop Linux environment as
well as on the Zynthian Raspberry Pi based platform.

Signed-off-by: Ricard Wanderlof <ricard2013@...oba.net>
---
The CME UF6 not only sends sysex messages using CIN 0xf which
in the standard is only intended for single-byte unparsed
messages, but also song position pointer, as well as ordinary
MIDI timing clock, start, stop and active sensing.

The old quirk sent all four bytes of the USB MIDI message through
when CIN 0xf was received, which caused trailing bytes in the case
of single byte data, which would be interpreted as zero-valued
repeats of the previosly received channel message due to the
running status feature of MIDI. (In practice, this causes
continuous controllers to be reset to 0 as soon as a new value is
set. For note messages, it basically results in a note off for
note number 0, which is an annoyance although harmless in 
practice.)

Furthermore, the UF6 can send multiple messages in the same
packet. Because of this, and the 0xf quirk, we need to parse each
such message individually to get the length right, before sending
it on.

Finally, the UF6 sends messages with length 0 when it has nothing
else to send; don't forward these useless messages.

According to the original comment in the code, the old
functionality was intended to handle sysex messages only, and this
commit will still handle that case properly, so should be
compatible with all devices utilizing this quirk. I have only
tested this with a CME UF6 as don't have access to any other
CME devices (which are pretty old and rare these days).
---
 sound/usb/midi.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 6 deletions(-)

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 737dd00e97b142148fc8b9c997067528d86e6543..aa2b7c589c019d8ec1e92dfe54e0e33ac4132709 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -489,16 +489,84 @@ static void ch345_broken_sysex_input(struct snd_usb_midi_in_endpoint *ep,
 
 /*
  * CME protocol: like the standard protocol, but SysEx commands are sent as a
- * single USB packet preceded by a 0x0F byte.
+ * single USB packet preceded by a 0x0F byte, as are system realtime
+ * messages and MIDI Active Sensing.
+ * Also, multiple messages can be sent in the same packet.
  */
 static void snd_usbmidi_cme_input(struct snd_usb_midi_in_endpoint *ep,
 				  uint8_t *buffer, int buffer_length)
 {
-	if (buffer_length < 2 || (buffer[0] & 0x0f) != 0x0f)
-		snd_usbmidi_standard_input(ep, buffer, buffer_length);
-	else
-		snd_usbmidi_input_data(ep, buffer[0] >> 4,
-				       &buffer[1], buffer_length - 1);
+	int remaining = buffer_length;
+
+	/*
+	 * CME send sysex, song position pointer, system realtime
+	 * and active sensing using CIN 0x0f, which in the standard
+	 * is only intended for single byte unparsed data.
+	 * So we need to interpret these here before sending them on.
+	 * By default, we assume single byte data, which is true
+	 * for system realtime (midi clock, start, stop and continue)
+	 * and active sensing, and handle the other (known) cases
+	 * separately.
+	 * In contrast to the standard, CME does not split sysex
+	 * into multiple 4-byte packets, but lumps everything together
+	 * into one. In addition, CME can string multiple messages
+	 * together in the same packet; pressing the Record button
+	 * on an UF6 sends a sysex message directly followed
+	 * by a song position pointer in the same packet.
+	 * For it to have any reasonable meaning, a sysex message
+	 * needs to be at least 3 bytes in length (0xf0, id, 0xf7),
+	 * corresponding to a packet size of 4 bytes, and the ones sent
+	 * by CME devices are 6 or 7 bytes, making the packet fragments
+	 * 7 or 8 bytes long (six or seven bytes plus preceding CN+CIN byte).
+	 * For the other types, the packet size is always 4 bytes,
+	 * as per the standard, with the data size being 3 for SPP
+	 * and 1 for the others.
+	 * Thus all packet fragments are at least 4 bytes long, so we can
+	 * skip anything that is shorter; this also conveniantly skips
+	 * packets with size 0, which CME devices continuously send when
+	 * they have nothing better to do.
+	 * Another quirk is that sometimes multiple messages are sent
+	 * in the same packet. This has been observed for midi clock
+	 * and active sensing i.e. 0x0f 0xf8 0x00 0x00 0x0f 0xfe 0x00 0x00,
+	 * but also multiple note ons/offs, and control change together
+	 * with MIDI clock. Similarly, some sysex messages are followed by
+	 * the song position pointer in the same packet, and occasionally
+	 * additionally by a midi clock or active sensing.
+	 * We handle this by looping over all data and parsing it along the way.
+	 */
+	while (remaining >= 4) {
+		int source_length = 4; /* default */
+
+		if ((buffer[0] & 0x0f) == 0x0f) {
+			int data_length = 1; /* default */
+
+			if (buffer[1] == 0xf0) {
+				/* Sysex: Find EOX and send on whole message. */
+				/* To kick off the search, skip the first
+				 * two bytes (CN+CIN and SYSEX (0xf0).
+				 */
+				uint8_t *tmp_buf = buffer + 2;
+				int tmp_length = remaining - 2;
+
+				while (tmp_length > 1 && *tmp_buf != 0xf7) {
+					tmp_buf++;
+					tmp_length--;
+				}
+				data_length = tmp_buf - buffer;
+				source_length = data_length + 1;
+			} else if (buffer[1] == 0xf2) {
+				/* Three byte song position pointer */
+				data_length = 3;
+			}
+			snd_usbmidi_input_data(ep, buffer[0] >> 4,
+					       &buffer[1], data_length);
+		} else {
+			/* normal channel events */
+			snd_usbmidi_standard_input(ep, buffer, source_length);
+		}
+		buffer += source_length;
+		remaining -= source_length;
+	}
 }
 
 /*

---
base-commit: 520a563b9a162d8a7484a32086de8e7c84d69945
change-id: 20250313-cme-fix-087b7321ed46

Best regards,
-- 
--
Ricard Wanderlöf                              [ http://www.butoba.net ]

Analog synth, railroad, magnetic recording and Citroen GS/GSA enthusiast
        -- "Ich bin der Musikant mit Taschenrechner in der Hand" --

Powered by blists - more mailing lists