Fix MIDI input with ALSA.

Reworked the handling of ALSA RawMidi input to support:
- Running Status.
- RealTime Category messages arriving during other messages data.
- Multiple connected RawMidi interfaces.

drivers/alsamidi/midi_driver_alsamidi.cpp

@@ -37,85 +37,135 @@
 
 #include <errno.h>
 
-static int get_message_size(uint8_t message) {
+MIDIDriverALSAMidi::MessageCategory MIDIDriverALSAMidi::msg_category(uint8_t msg_part) {
-	switch (message & 0xF0) {
+	if (msg_part >= 0xf8) {
-		case 0x80: // note off
+		return MessageCategory::RealTime;
-		case 0x90: // note on
+	} else if (msg_part >= 0xf0) {
-		case 0xA0: // aftertouch
+		// System Exclusive begin/end are specified as System Common Category messages,
-		case 0xB0: // continuous controller
+		// but we separate them here and give them their own categories as their
-		case 0xE0: // pitch bend
+		// behaviour is significantly different.
-		case 0xF2: // song position pointer
+		if (msg_part == 0xf0) {
-			return 3;
+			return MessageCategory::SysExBegin;
+		} else if (msg_part == 0xf7) {
+			return MessageCategory::SysExEnd;
+		}
+		return MessageCategory::SystemCommon;
+	} else if (msg_part >= 0x80) {
+		return MessageCategory::Voice;
+	}
+	return MessageCategory::Data;
+}
 
-		case 0xC0: // patch change
+size_t MIDIDriverALSAMidi::msg_expected_data(uint8_t status_byte) {
-		case 0xD0: // channel pressure
+	if (msg_category(status_byte) == MessageCategory::Voice) {
-		case 0xF1: // time code quarter frame
+		// Voice messages have a channel number in the status byte, mask it out.
-		case 0xF3: // song select
+		status_byte &= 0xf0;
+	}
+
+	switch (status_byte) {
+		case 0x80: // Note Off
+		case 0x90: // Note On
+		case 0xA0: // Polyphonic Key Pressure (Aftertouch)
+		case 0xB0: // Control Change (CC)
+		case 0xE0: // Pitch Bend Change
+		case 0xF2: // Song Position Pointer
 			return 2;
 
-		case 0xF0: // SysEx start
+		case 0xC0: // Program Change
-		case 0xF4: // reserved
+		case 0xD0: // Channel Pressure (Aftertouch)
-		case 0xF5: // reserved
+		case 0xF1: // MIDI Time Code Quarter Frame
-		case 0xF6: // tune request
+		case 0xF3: // Song Select
-		case 0xF7: // SysEx end
-		case 0xF8: // timing clock
-		case 0xF9: // reserved
-		case 0xFA: // start
-		case 0xFB: // continue
-		case 0xFC: // stop
-		case 0xFD: // reserved
-		case 0xFE: // active sensing
-		case 0xFF: // reset
 			return 1;
 	}
 
-	return 256;
+	return 0;
+}
+
+void MIDIDriverALSAMidi::InputConnection::parse_byte(uint8_t byte, MIDIDriverALSAMidi &driver,
+		uint64_t timestamp) {
+	switch (msg_category(byte)) {
+		case MessageCategory::RealTime:
+			// Real-Time messages are single byte messages that can
+			// occur at any point.
+			// We pass them straight through.
+			driver.receive_input_packet(timestamp, &byte, 1);
+			break;
+
+		case MessageCategory::Data:
+			// We don't currently forward System Exclusive messages so skip their data.
+			// Collect any expected data for other message types.
+			if (!skipping_sys_ex && expected_data > received_data) {
+				buffer[received_data + 1] = byte;
+				received_data++;
+
+				// Forward a complete message and reset relevant state.
+				if (received_data == expected_data) {
+					driver.receive_input_packet(timestamp, buffer, received_data + 1);
+					received_data = 0;
+
+					if (msg_category(buffer[0]) != MessageCategory::Voice) {
+						// Voice Category messages can be sent with "running status".
+						// This means they don't resend the status byte until it changes.
+						// For other categories, we reset expected data, to require a new status byte.
+						expected_data = 0;
+					}
+				}
+			}
+			break;
+
+		case MessageCategory::SysExBegin:
+			buffer[0] = byte;
+			skipping_sys_ex = true;
+			break;
+
+		case MessageCategory::SysExEnd:
+			expected_data = 0;
+			skipping_sys_ex = false;
+			break;
+
+		case MessageCategory::Voice:
+		case MessageCategory::SystemCommon:
+			buffer[0] = byte;
+			received_data = 0;
+			expected_data = msg_expected_data(byte);
+			skipping_sys_ex = false;
+			if (expected_data == 0) {
+				driver.receive_input_packet(timestamp, &byte, 1);
+			}
+			break;
+	}
+}
+
+int MIDIDriverALSAMidi::InputConnection::read_in(MIDIDriverALSAMidi &driver, uint64_t timestamp) {
+	int ret;
+	do {
+		uint8_t byte = 0;
+		ret = snd_rawmidi_read(rawmidi_ptr, &byte, 1);
+
+		if (ret < 0) {
+			if (ret != -EAGAIN) {
+				ERR_PRINT("snd_rawmidi_read error: " + String(snd_strerror(ret)));
+			}
+		} else {
+			parse_byte(byte, driver, timestamp);
+		}
+	} while (ret > 0);
+
+	return ret;
 }
 
 void MIDIDriverALSAMidi::thread_func(void *p_udata) {
 	MIDIDriverALSAMidi *md = static_cast<MIDIDriverALSAMidi *>(p_udata);
 	uint64_t timestamp = 0;
-	uint8_t buffer[256];
-	int expected_size = 255;
-	int bytes = 0;
 
 	while (!md->exit_thread.is_set()) {
-		int ret;
-
 		md->lock();
 
-		for (int i = 0; i < md->connected_inputs.size(); i++) {
+		InputConnection *connections = md->connected_inputs.ptrw();
-			snd_rawmidi_t *midi_in = md->connected_inputs[i];
+		size_t connection_count = md->connected_inputs.size();
-			do {
-				uint8_t byte = 0;
-				ret = snd_rawmidi_read(midi_in, &byte, 1);
-				if (ret < 0) {
-					if (ret != -EAGAIN) {
-						ERR_PRINT("snd_rawmidi_read error: " + String(snd_strerror(ret)));
-					}
-				} else {
-					if (byte & 0x80) {
-						// Flush previous packet if there is any
-						if (bytes) {
-							md->receive_input_packet(timestamp, buffer, bytes);
-							bytes = 0;
-						}
-						expected_size = get_message_size(byte);
-						// After a SysEx start, all bytes are data until a SysEx end, so
-						// we're going to end the command at the SES, and let the common
-						// driver ignore the following data bytes.
-					}
 
-					if (bytes < 256) {
+		for (size_t i = 0; i < connection_count; i++) {
-						buffer[bytes++] = byte;
+			connections[i].read_in(*md, timestamp);
-						// If we know the size of the current packet receive it if it reached the expected size
-						if (bytes >= expected_size) {
-							md->receive_input_packet(timestamp, buffer, bytes);
-							bytes = 0;
-						}
-					}
-				}
-			} while (ret > 0);
 		}
 
 		md->unlock();
@@ -139,7 +189,7 @@ Error MIDIDriverALSAMidi::open() {
 			snd_rawmidi_t *midi_in;
 			int ret = snd_rawmidi_open(&midi_in, nullptr, name, SND_RAWMIDI_NONBLOCK);
 			if (ret >= 0) {
-				connected_inputs.insert(i++, midi_in);
+				connected_inputs.insert(i++, InputConnection(midi_in));
 			}
 		}
 
@@ -160,7 +210,7 @@ void MIDIDriverALSAMidi::close() {
 	thread.wait_to_finish();
 
 	for (int i = 0; i < connected_inputs.size(); i++) {
-		snd_rawmidi_t *midi_in = connected_inputs[i];
+		snd_rawmidi_t *midi_in = connected_inputs[i].rawmidi_ptr;
 		snd_rawmidi_close(midi_in);
 	}
 	connected_inputs.clear();
@@ -179,7 +229,7 @@ PackedStringArray MIDIDriverALSAMidi::get_connected_inputs() {
 
 	lock();
 	for (int i = 0; i < connected_inputs.size(); i++) {
-		snd_rawmidi_t *midi_in = connected_inputs[i];
+		snd_rawmidi_t *midi_in = connected_inputs[i].rawmidi_ptr;
 		snd_rawmidi_info_t *info;
 
 		snd_rawmidi_info_malloc(&info);

drivers/alsamidi/midi_driver_alsamidi.h

@@ -46,12 +46,48 @@ class MIDIDriverALSAMidi : public MIDIDriver {
 	Thread thread;
 	Mutex mutex;
 
-	Vector<snd_rawmidi_t *> connected_inputs;
+	class InputConnection {
+	public:
+		InputConnection() = default;
+		InputConnection(snd_rawmidi_t *midi_in) :
+				rawmidi_ptr{ midi_in } {}
+
+		// Read in and parse available data, forwarding any complete messages through the driver.
+		int read_in(MIDIDriverALSAMidi &driver, uint64_t timestamp);
+
+		snd_rawmidi_t *rawmidi_ptr = nullptr;
+
+	private:
+		static const size_t MSG_BUFFER_SIZE = 3;
+		uint8_t buffer[MSG_BUFFER_SIZE] = { 0 };
+		size_t expected_data = 0;
+		size_t received_data = 0;
+		bool skipping_sys_ex = false;
+		void parse_byte(uint8_t byte, MIDIDriverALSAMidi &driver, uint64_t timestamp);
+	};
+
+	Vector<InputConnection> connected_inputs;
 
 	SafeFlag exit_thread;
 
 	static void thread_func(void *p_udata);
 
+	enum class MessageCategory {
+		Data,
+		Voice,
+		SysExBegin,
+		SystemCommon, // excluding System Exclusive Begin/End
+		SysExEnd,
+		RealTime,
+	};
+
+	// If the passed byte is a status byte, return the associated message category,
+	// else return MessageCategory::Data.
+	static MessageCategory msg_category(uint8_t msg_part);
+
+	// Return the number of data bytes expected for the provided status byte.
+	static size_t msg_expected_data(uint8_t status_byte);
+
 	void lock() const;
 	void unlock() const;