Move MIDI parsing up from ALSA driver to platform independent driver.

Aims for more consistent MIDI support across Windows, MacOS, Linux and
to provide a base for adding MIDI drivers for other platforms.
Reworks the MIDIDriverALSAMidi MIDI parsing implementation as a platform
independent version in MIDIDriver::Parser.
Uses MIDIDriver::Parser to provide running status support in MacOS
MIDIDriverCoreMidi.
Collects connected input names at open, ensuring devices indices reported
in events match names in array returned from get_connected_inputs.

Fixes #77035.
Fixes #79811.

With code review changes by: A Thousand Ships (she/her)
<[email protected]>

core/os/midi_driver.cpp

@@ -38,88 +38,167 @@ MIDIDriver *MIDIDriver::get_singleton() {
 	return singleton;
 }
 
-void MIDIDriver::set_singleton() {
+MIDIDriver::MIDIDriver() {
 	singleton = this;
 }
 
-void MIDIDriver::receive_input_packet(int device_index, uint64_t timestamp, uint8_t *data, uint32_t length) {
-	Ref<InputEventMIDI> event;
-	event.instantiate();
-	event->set_device(device_index);
-	uint32_t param_position = 1;
-
-	if (length >= 1) {
-		if (data[0] >= 0xF0) {
-			// channel does not apply to system common messages
-			event->set_channel(0);
-			event->set_message(MIDIMessage(data[0]));
-			last_received_message = data[0];
-		} else if ((data[0] & 0x80) == 0x00) {
-			// running status
-			event->set_channel(last_received_message & 0xF);
-			event->set_message(MIDIMessage(last_received_message >> 4));
-			param_position = 0;
+MIDIDriver::MessageCategory MIDIDriver::Parser::category(uint8_t p_midi_fragment) {
+	if (p_midi_fragment >= 0xf8) {
+		return MessageCategory::RealTime;
+	} else if (p_midi_fragment >= 0xf0) {
+		// System Exclusive begin/end are specified as System Common Category
+		// messages, but we separate them here and give them their own categories
+		// as their behavior is significantly different.
+		if (p_midi_fragment == 0xf0) {
+			return MessageCategory::SysExBegin;
+		} else if (p_midi_fragment == 0xf7) {
+			return MessageCategory::SysExEnd;
+		}
+		return MessageCategory::SystemCommon;
+	} else if (p_midi_fragment >= 0x80) {
+		return MessageCategory::Voice;
+	}
+	return MessageCategory::Data;
+}
+
+MIDIMessage MIDIDriver::Parser::status_to_msg_enum(uint8_t p_status_byte) {
+	if (p_status_byte & 0x80) {
+		if (p_status_byte < 0xf0) {
+			return MIDIMessage(p_status_byte >> 4);
 		} else {
-			event->set_channel(data[0] & 0xF);
-			event->set_message(MIDIMessage(data[0] >> 4));
-			param_position = 1;
-			last_received_message = data[0];
+			return MIDIMessage(p_status_byte);
 		}
 	}
+	return MIDIMessage::NONE;
+}
 
-	switch (event->get_message()) {
-		case MIDIMessage::AFTERTOUCH:
-			if (length >= 2 + param_position) {
-				event->set_pitch(data[param_position]);
-				event->set_pressure(data[param_position + 1]);
-			}
-			break;
+size_t MIDIDriver::Parser::expected_data(uint8_t p_status_byte) {
+	return expected_data(status_to_msg_enum(p_status_byte));
+}
 
+size_t MIDIDriver::Parser::expected_data(MIDIMessage p_msg_type) {
+	switch (p_msg_type) {
+		case MIDIMessage::NOTE_OFF:
+		case MIDIMessage::NOTE_ON:
+		case MIDIMessage::AFTERTOUCH:
 		case MIDIMessage::CONTROL_CHANGE:
-			if (length >= 2 + param_position) {
-				event->set_controller_number(data[param_position]);
-				event->set_controller_value(data[param_position + 1]);
-			}
-			break;
+		case MIDIMessage::PITCH_BEND:
+		case MIDIMessage::SONG_POSITION_POINTER:
+			return 2;
+		case MIDIMessage::PROGRAM_CHANGE:
+		case MIDIMessage::CHANNEL_PRESSURE:
+		case MIDIMessage::QUARTER_FRAME:
+		case MIDIMessage::SONG_SELECT:
+			return 1;
+		default:
+			return 0;
+	}
+}
 
-		case MIDIMessage::NOTE_ON:
+uint8_t MIDIDriver::Parser::channel(uint8_t p_status_byte) {
+	if (category(p_status_byte) == MessageCategory::Voice) {
+		return p_status_byte & 0x0f;
+	}
+	return 0;
+}
+
+void MIDIDriver::send_event(int p_device_index, uint8_t p_status,
+		const uint8_t *p_data, size_t p_data_len) {
+	const MIDIMessage msg = Parser::status_to_msg_enum(p_status);
+	ERR_FAIL_COND(p_data_len < Parser::expected_data(msg));
+
+	Ref<InputEventMIDI> event;
+	event.instantiate();
+	event->set_device(p_device_index);
+	event->set_channel(Parser::channel(p_status));
+	event->set_message(msg);
+	switch (msg) {
 		case MIDIMessage::NOTE_OFF:
-			if (length >= 2 + param_position) {
-				event->set_pitch(data[param_position]);
-				event->set_velocity(data[param_position + 1]);
-			}
+		case MIDIMessage::NOTE_ON:
+			event->set_pitch(p_data[0]);
+			event->set_velocity(p_data[1]);
 			break;
-
-		case MIDIMessage::PITCH_BEND:
-			if (length >= 2 + param_position) {
-				event->set_pitch((data[param_position + 1] << 7) | data[param_position]);
-			}
+		case MIDIMessage::AFTERTOUCH:
+			event->set_pitch(p_data[0]);
+			event->set_pressure(p_data[1]);
+			break;
+		case MIDIMessage::CONTROL_CHANGE:
+			event->set_controller_number(p_data[0]);
+			event->set_controller_value(p_data[1]);
 			break;
-
 		case MIDIMessage::PROGRAM_CHANGE:
-			if (length >= 1 + param_position) {
-				event->set_instrument(data[param_position]);
-			}
+			event->set_instrument(p_data[0]);
 			break;
-
 		case MIDIMessage::CHANNEL_PRESSURE:
-			if (length >= 1 + param_position) {
-				event->set_pressure(data[param_position]);
-			}
+			event->set_pressure(p_data[0]);
+			break;
+		case MIDIMessage::PITCH_BEND:
+			event->set_pitch((p_data[1] << 7) | p_data[0]);
 			break;
+		// QUARTER_FRAME, SONG_POSITION_POINTER, and SONG_SELECT not yet implemented.
 		default:
 			break;
 	}
-
-	Input *id = Input::get_singleton();
-	id->parse_input_event(event);
+	Input::get_singleton()->parse_input_event(event);
 }
 
-PackedStringArray MIDIDriver::get_connected_inputs() {
-	PackedStringArray list;
-	return list;
+void MIDIDriver::Parser::parse_fragment(uint8_t p_fragment) {
+	switch (category(p_fragment)) {
+		case MessageCategory::RealTime:
+			// Real-Time messages are single byte messages that can
+			// occur at any point and do not interrupt other messages.
+			// We pass them straight through.
+			MIDIDriver::send_event(device_index, p_fragment);
+			break;
+
+		case MessageCategory::SysExBegin:
+			status_byte = p_fragment;
+			skipping_sys_ex = true;
+			break;
+
+		case MessageCategory::SysExEnd:
+			status_byte = 0;
+			skipping_sys_ex = false;
+			break;
+
+		case MessageCategory::Voice:
+		case MessageCategory::SystemCommon:
+			skipping_sys_ex = false; // If we were in SysEx, assume it was aborted.
+			received_data_len = 0;
+			status_byte = 0;
+			ERR_FAIL_COND(expected_data(p_fragment) > DATA_BUFFER_SIZE);
+			if (expected_data(p_fragment) == 0) {
+				// No data bytes needed, post it now.
+				MIDIDriver::send_event(device_index, p_fragment);
+			} else {
+				status_byte = p_fragment;
+			}
+			break;
+
+		case MessageCategory::Data:
+			// We don't currently process SysEx messages, so ignore their data.
+			if (!skipping_sys_ex) {
+				const size_t expected = expected_data(status_byte);
+				if (received_data_len < expected) {
+					data_buffer[received_data_len] = p_fragment;
+					received_data_len++;
+					if (received_data_len == expected) {
+						MIDIDriver::send_event(device_index, status_byte,
+								data_buffer, expected);
+						received_data_len = 0;
+						// Voice messages can use 'running status', sending further
+						// messages without resending their status byte.
+						// For other messages types we clear the cached status byte.
+						if (category(status_byte) != MessageCategory::Voice) {
+							status_byte = 0;
+						}
+					}
+				}
+			}
+			break;
+	}
 }
 
-MIDIDriver::MIDIDriver() {
-	set_singleton();
+PackedStringArray MIDIDriver::get_connected_inputs() const {
+	return connected_input_names;
 }

core/os/midi_driver.h

@@ -42,19 +42,73 @@ class MIDIDriver {
 	static MIDIDriver *singleton;
 	static uint8_t last_received_message;
 
+protected:
+	// Categories of message for parser logic.
+	enum class MessageCategory {
+		Data,
+		Voice,
+		SysExBegin,
+		SystemCommon, // excluding System Exclusive Begin/End
+		SysExEnd,
+		RealTime,
+	};
+
+	// Convert midi data to InputEventMIDI and send it to Input.
+	// p_data_len is the length of the buffer passed at p_data, this must be
+	// at least equal to the data required by the passed message type, but
+	// may be larger. Only the required data will be read.
+	static void send_event(int p_device_index, uint8_t p_status,
+			const uint8_t *p_data = nullptr, size_t p_data_len = 0);
+
+	class Parser {
+	public:
+		Parser() = default;
+		Parser(int p_device_index) :
+				device_index{ p_device_index } {}
+		virtual ~Parser() = default;
+
+		// Push a byte of MIDI stream. Any completed messages will be
+		// forwarded to MIDIDriver::send_event.
+		void parse_fragment(uint8_t p_fragment);
+
+		static MessageCategory category(uint8_t p_midi_fragment);
+
+		// If the byte is a Voice Message status byte return the contained
+		// channel number, otherwise zero.
+		static uint8_t channel(uint8_t p_status_byte);
+
+		// If the byte is a status byte for a message with a fixed number of
+		// additional data bytes, return the number expected, otherwise zero.
+		static size_t expected_data(uint8_t p_status_byte);
+		static size_t expected_data(MIDIMessage p_msg_type);
+
+		// If the fragment is a status byte return the message type
+		// represented, otherwise MIDIMessage::NONE.
+		static MIDIMessage status_to_msg_enum(uint8_t p_status_byte);
+
+	private:
+		int device_index = 0;
+
+		static constexpr size_t DATA_BUFFER_SIZE = 2;
+
+		uint8_t status_byte = 0;
+		uint8_t data_buffer[DATA_BUFFER_SIZE] = { 0 };
+		size_t received_data_len = 0;
+		bool skipping_sys_ex = false;
+	};
+
+	PackedStringArray connected_input_names;
+
 public:
 	static MIDIDriver *get_singleton();
-	void set_singleton();
+
+	MIDIDriver();
+	virtual ~MIDIDriver() = default;
 
 	virtual Error open() = 0;
 	virtual void close() = 0;
 
-	virtual PackedStringArray get_connected_inputs();
-
-	static void receive_input_packet(int device_index, uint64_t timestamp, uint8_t *data, uint32_t length);
-
-	MIDIDriver();
-	virtual ~MIDIDriver() {}
+	PackedStringArray get_connected_inputs() const;
 };
 
 #endif // MIDI_DRIVER_H

drivers/alsamidi/midi_driver_alsamidi.cpp

@@ -37,137 +37,36 @@
 
 #include <errno.h>
 
-MIDIDriverALSAMidi::MessageCategory MIDIDriverALSAMidi::msg_category(uint8_t msg_part) {
-	if (msg_part >= 0xf8) {
-		return MessageCategory::RealTime;
-	} else if (msg_part >= 0xf0) {
-		// System Exclusive begin/end are specified as System Common Category messages,
-		// but we separate them here and give them their own categories as their
-		// behavior is significantly different.
-		if (msg_part == 0xf0) {
-			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;
-}
-
-size_t MIDIDriverALSAMidi::msg_expected_data(uint8_t status_byte) {
-	if (msg_category(status_byte) == MessageCategory::Voice) {
-		// Voice messages have a channel number in the status byte, mask it out.
-		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 0xC0: // Program Change
-		case 0xD0: // Channel Pressure (Aftertouch)
-		case 0xF1: // MIDI Time Code Quarter Frame
-		case 0xF3: // Song Select
-			return 1;
-	}
+MIDIDriverALSAMidi::InputConnection::InputConnection(int p_device_index,
+		snd_rawmidi_t *p_rawmidi) :
+		parser(p_device_index), rawmidi_ptr(p_rawmidi) {}
 
-	return 0;
-}
-
-void MIDIDriverALSAMidi::InputConnection::parse_byte(uint8_t byte, MIDIDriverALSAMidi &driver,
-		uint64_t timestamp, int device_index) {
-	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(device_index, 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(device_index, 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(device_index, timestamp, &byte, 1);
-			}
-			break;
-	}
-}
-
-int MIDIDriverALSAMidi::InputConnection::read_in(MIDIDriverALSAMidi &driver, uint64_t timestamp, int device_index) {
-	int ret;
+void MIDIDriverALSAMidi::InputConnection::read() {
+	int read_count;
 	do {
-		uint8_t byte = 0;
-		ret = snd_rawmidi_read(rawmidi_ptr, &byte, 1);
+		uint8_t buffer[32];
+		read_count = snd_rawmidi_read(rawmidi_ptr, buffer, sizeof(buffer));
 
-		if (ret < 0) {
-			if (ret != -EAGAIN) {
-				ERR_PRINT("snd_rawmidi_read error: " + String(snd_strerror(ret)));
+		if (read_count < 0) {
+			if (read_count != -EAGAIN) {
+				ERR_PRINT("snd_rawmidi_read error: " + String(snd_strerror(read_count)));
 			}
 		} else {
-			parse_byte(byte, driver, timestamp, device_index);
+			for (int i = 0; i < read_count; i++) {
+				parser.parse_fragment(buffer[i]);
+			}
 		}
-	} while (ret > 0);
-
-	return ret;
+	} while (read_count > 0);
 }
 
 void MIDIDriverALSAMidi::thread_func(void *p_udata) {
 	MIDIDriverALSAMidi *md = static_cast<MIDIDriverALSAMidi *>(p_udata);
-	uint64_t timestamp = 0;
 
 	while (!md->exit_thread.is_set()) {
 		md->lock();
-
-		InputConnection *connections = md->connected_inputs.ptrw();
-		size_t connection_count = md->connected_inputs.size();
-
-		for (size_t i = 0; i < connection_count; i++) {
-			connections[i].read_in(*md, timestamp, (int)i);
+		for (InputConnection &conn : md->connected_inputs) {
+			conn.read();
 		}
-
 		md->unlock();
 
 		OS::get_singleton()->delay_usec(1000);
@@ -181,15 +80,25 @@ Error MIDIDriverALSAMidi::open() {
 		return ERR_CANT_OPEN;
 	}
 
-	int i = 0;
-	for (void **n = hints; *n != nullptr; n++) {
-		char *name = snd_device_name_get_hint(*n, "NAME");
+	lock();
+	int device_index = 0;
+	for (void **h = hints; *h != nullptr; h++) {
+		char *name = snd_device_name_get_hint(*h, "NAME");
 
 		if (name != nullptr) {
 			snd_rawmidi_t *midi_in;
 			int ret = snd_rawmidi_open(&midi_in, nullptr, name, SND_RAWMIDI_NONBLOCK);
 			if (ret >= 0) {
-				connected_inputs.insert(i++, InputConnection(midi_in));
+				// Get display name.
+				snd_rawmidi_info_t *info;
+				snd_rawmidi_info_malloc(&info);
+				snd_rawmidi_info(midi_in, info);
+				connected_input_names.push_back(snd_rawmidi_info_get_name(info));
+				snd_rawmidi_info_free(info);
+
+				connected_inputs.push_back(InputConnection(device_index, midi_in));
+				// Only increment device_index for successfully connected devices.
+				device_index++;
 			}
 		}
 
@@ -198,6 +107,7 @@ Error MIDIDriverALSAMidi::open() {
 		}
 	}
 	snd_device_name_free_hint(hints);
+	unlock();
 
 	exit_thread.clear();
 	thread.start(MIDIDriverALSAMidi::thread_func, this);
@@ -211,11 +121,12 @@ void MIDIDriverALSAMidi::close() {
 		thread.wait_to_finish();
 	}
 
-	for (int i = 0; i < connected_inputs.size(); i++) {
-		snd_rawmidi_t *midi_in = connected_inputs[i].rawmidi_ptr;
-		snd_rawmidi_close(midi_in);
+	for (const InputConnection &conn : connected_inputs) {
+		snd_rawmidi_close(conn.rawmidi_ptr);
 	}
+
 	connected_inputs.clear();
+	connected_input_names.clear();
 }
 
 void MIDIDriverALSAMidi::lock() const {
@@ -226,24 +137,6 @@ void MIDIDriverALSAMidi::unlock() const {
 	mutex.unlock();
 }
 
-PackedStringArray MIDIDriverALSAMidi::get_connected_inputs() {
-	PackedStringArray list;
-
-	lock();
-	for (int i = 0; i < connected_inputs.size(); i++) {
-		snd_rawmidi_t *midi_in = connected_inputs[i].rawmidi_ptr;
-		snd_rawmidi_info_t *info;
-
-		snd_rawmidi_info_malloc(&info);
-		snd_rawmidi_info(midi_in, info);
-		list.push_back(snd_rawmidi_info_get_name(info));
-		snd_rawmidi_info_free(info);
-	}
-	unlock();
-
-	return list;
-}
-
 MIDIDriverALSAMidi::MIDIDriverALSAMidi() {
 	exit_thread.clear();
 }

drivers/alsamidi/midi_driver_alsamidi.h

@@ -51,24 +51,15 @@ class MIDIDriverALSAMidi : public MIDIDriver {
 	Thread thread;
 	Mutex mutex;
 
-	class InputConnection {
-	public:
+	struct InputConnection {
 		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, int device_index);
+		InputConnection(int p_device_index, snd_rawmidi_t *p_rawmidi);
 
+		Parser parser;
 		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, int device_index);
+		// Read in and parse available data, forwarding complete events to Input.
+		void read();
 	};
 
 	Vector<InputConnection> connected_inputs;
@@ -77,30 +68,12 @@ class MIDIDriverALSAMidi : public MIDIDriver {
 
 	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;
 
 public:
-	virtual Error open();
-	virtual void close();
-
-	virtual PackedStringArray get_connected_inputs();
+	virtual Error open() override;
+	virtual void close() override;
 
 	MIDIDriverALSAMidi();
 	virtual ~MIDIDriverALSAMidi();

drivers/coremidi/midi_driver_coremidi.cpp

@@ -37,16 +37,30 @@
 #import <CoreAudio/HostTime.h>
 #import <CoreServices/CoreServices.h>
 
+Mutex MIDIDriverCoreMidi::mutex;
+bool MIDIDriverCoreMidi::core_midi_closed = false;
+
+MIDIDriverCoreMidi::InputConnection::InputConnection(int p_device_index, MIDIEndpointRef p_source) :
+		parser(p_device_index), source(p_source) {}
+
 void MIDIDriverCoreMidi::read(const MIDIPacketList *packet_list, void *read_proc_ref_con, void *src_conn_ref_con) {
-	MIDIPacket *packet = const_cast<MIDIPacket *>(packet_list->packet);
-	int *device_index = static_cast<int *>(src_conn_ref_con);
-	for (UInt32 i = 0; i < packet_list->numPackets; i++) {
-		receive_input_packet(*device_index, packet->timeStamp, packet->data, packet->length);
-		packet = MIDIPacketNext(packet);
+	MutexLock lock(mutex);
+	if (!core_midi_closed) {
+		InputConnection *source = static_cast<InputConnection *>(src_conn_ref_con);
+		const MIDIPacket *packet = packet_list->packet;
+		for (UInt32 packet_index = 0; packet_index < packet_list->numPackets; packet_index++) {
+			for (UInt16 data_index = 0; data_index < packet->length; data_index++) {
+				source->parser.parse_fragment(packet->data[data_index]);
+			}
+			packet = MIDIPacketNext(packet);
+		}
 	}
 }
 
 Error MIDIDriverCoreMidi::open() {
+	ERR_FAIL_COND_V_MSG(client || core_midi_closed, FAILED,
+			"MIDIDriverCoreMidi cannot be reopened.");
+
 	CFStringRef name = CFStringCreateWithCString(nullptr, "Godot", kCFStringEncodingASCII);
 	OSStatus result = MIDIClientCreate(name, nullptr, nullptr, &client);
 	CFRelease(name);
@@ -61,12 +75,27 @@ Error MIDIDriverCoreMidi::open() {
 		return ERR_CANT_OPEN;
 	}
 
-	int sources = MIDIGetNumberOfSources();
-	for (int i = 0; i < sources; i++) {
+	int source_count = MIDIGetNumberOfSources();
+	int connection_index = 0;
+	for (int i = 0; i < source_count; i++) {
 		MIDIEndpointRef source = MIDIGetSource(i);
 		if (source) {
-			MIDIPortConnectSource(port_in, source, static_cast<void *>(&i));
-			connected_sources.insert(i, source);
+			InputConnection *conn = memnew(InputConnection(connection_index, source));
+			const OSStatus res = MIDIPortConnectSource(port_in, source, static_cast<void *>(conn));
+			if (res != noErr) {
+				memdelete(conn);
+			} else {
+				connected_sources.push_back(conn);
+
+				CFStringRef nameRef = nullptr;
+				char name[256];
+				MIDIObjectGetStringProperty(source, kMIDIPropertyDisplayName, &nameRef);
+				CFStringGetCString(nameRef, name, sizeof(name), kCFStringEncodingUTF8);
+				CFRelease(nameRef);
+				connected_input_names.push_back(name);
+
+				connection_index++; // Contiguous index for successfully connected inputs.
+			}
 		}
 	}
 
@@ -74,11 +103,17 @@ Error MIDIDriverCoreMidi::open() {
 }
 
 void MIDIDriverCoreMidi::close() {
-	for (int i = 0; i < connected_sources.size(); i++) {
-		MIDIEndpointRef source = connected_sources[i];
-		MIDIPortDisconnectSource(port_in, source);
+	mutex.lock();
+	core_midi_closed = true;
+	mutex.unlock();
+
+	for (InputConnection *conn : connected_sources) {
+		MIDIPortDisconnectSource(port_in, conn->source);
+		memdelete(conn);
 	}
+
 	connected_sources.clear();
+	connected_input_names.clear();
 
 	if (port_in != 0) {
 		MIDIPortDispose(port_in);
@@ -91,26 +126,6 @@ void MIDIDriverCoreMidi::close() {
 	}
 }
 
-PackedStringArray MIDIDriverCoreMidi::get_connected_inputs() {
-	PackedStringArray list;
-
-	for (int i = 0; i < connected_sources.size(); i++) {
-		MIDIEndpointRef source = connected_sources[i];
-		CFStringRef ref = nullptr;
-		char name[256];
-
-		MIDIObjectGetStringProperty(source, kMIDIPropertyDisplayName, &ref);
-		CFStringGetCString(ref, name, sizeof(name), kCFStringEncodingUTF8);
-		CFRelease(ref);
-
-		list.push_back(name);
-	}
-
-	return list;
-}
-
-MIDIDriverCoreMidi::MIDIDriverCoreMidi() {}
-
 MIDIDriverCoreMidi::~MIDIDriverCoreMidi() {
 	close();
 }

drivers/coremidi/midi_driver_coremidi.h

@@ -34,6 +34,7 @@
 #ifdef COREMIDI_ENABLED
 
 #include "core/os/midi_driver.h"
+#include "core/os/mutex.h"
 #include "core/templates/vector.h"
 
 #import <CoreMIDI/CoreMIDI.h>
@@ -43,17 +44,25 @@ class MIDIDriverCoreMidi : public MIDIDriver {
 	MIDIClientRef client = 0;
 	MIDIPortRef port_in;
 
-	Vector<MIDIEndpointRef> connected_sources;
+	struct InputConnection {
+		InputConnection() = default;
+		InputConnection(int p_device_index, MIDIEndpointRef p_source);
+		Parser parser;
+		MIDIEndpointRef source;
+	};
+
+	Vector<InputConnection *> connected_sources;
+
+	static Mutex mutex;
+	static bool core_midi_closed;
 
 	static void read(const MIDIPacketList *packet_list, void *read_proc_ref_con, void *src_conn_ref_con);
 
 public:
-	virtual Error open();
-	virtual void close();
-
-	PackedStringArray get_connected_inputs();
+	virtual Error open() override;
+	virtual void close() override;
 
-	MIDIDriverCoreMidi();
+	MIDIDriverCoreMidi() = default;
 	virtual ~MIDIDriverCoreMidi();
 };
 

drivers/winmidi/midi_driver_winmidi.cpp

@@ -36,26 +36,42 @@
 
 void MIDIDriverWinMidi::read(HMIDIIN hMidiIn, UINT wMsg, DWORD_PTR dwInstance, DWORD_PTR dwParam1, DWORD_PTR dwParam2) {
 	if (wMsg == MIM_DATA) {
-		receive_input_packet((int)dwInstance, (uint64_t)dwParam2, (uint8_t *)&dwParam1, 3);
+		// For MIM_DATA: dwParam1 = wMidiMessage, dwParam2 = dwTimestamp.
+		// Windows implementation has already unpacked running status and dropped any SysEx,
+		// so we can just forward straight to the event.
+		const uint8_t *midi_msg = (uint8_t *)&dwParam1;
+		send_event((int)dwInstance, midi_msg[0], &midi_msg[1], 2);
 	}
 }
 
 Error MIDIDriverWinMidi::open() {
+	int device_index = 0;
 	for (UINT i = 0; i < midiInGetNumDevs(); i++) {
 		HMIDIIN midi_in;
+		MIDIINCAPS caps;
 
-		MMRESULT res = midiInOpen(&midi_in, i, (DWORD_PTR)read, (DWORD_PTR)i, CALLBACK_FUNCTION);
-		if (res == MMSYSERR_NOERROR) {
+		MMRESULT open_res = midiInOpen(&midi_in, i, (DWORD_PTR)read,
+				(DWORD_PTR)device_index, CALLBACK_FUNCTION);
+		MMRESULT caps_res = midiInGetDevCaps(i, &caps, sizeof(MIDIINCAPS));
+
+		if (open_res == MMSYSERR_NOERROR) {
 			midiInStart(midi_in);
-			connected_sources.insert(i, midi_in);
+			connected_sources.push_back(midi_in);
+			if (caps_res == MMSYSERR_NOERROR) {
+				connected_input_names.push_back(caps.szPname);
+			} else {
+				// Should push something even if we don't get a name,
+				// so that the IDs line up correctly on the script side.
+				connected_input_names.push_back("ERROR");
+			}
+			// Only increment device index for successfully connected devices.
+			device_index++;
 		} else {
 			char err[256];
-			midiInGetErrorText(res, err, 256);
+			midiInGetErrorText(open_res, err, 256);
 			ERR_PRINT("midiInOpen error: " + String(err));
 
-			MIDIINCAPS caps;
-			res = midiInGetDevCaps(i, &caps, sizeof(MIDIINCAPS));
-			if (res == MMSYSERR_NOERROR) {
+			if (caps_res == MMSYSERR_NOERROR) {
 				ERR_PRINT("Can't open MIDI device \"" + String(caps.szPname) + "\", is it being used by another application?");
 			}
 		}
@@ -64,25 +80,6 @@ Error MIDIDriverWinMidi::open() {
 	return OK;
 }
 
-PackedStringArray MIDIDriverWinMidi::get_connected_inputs() {
-	PackedStringArray list;
-
-	for (int i = 0; i < connected_sources.size(); i++) {
-		HMIDIIN midi_in = connected_sources[i];
-		UINT id = 0;
-		MMRESULT res = midiInGetID(midi_in, &id);
-		if (res == MMSYSERR_NOERROR) {
-			MIDIINCAPS caps;
-			res = midiInGetDevCaps(i, &caps, sizeof(MIDIINCAPS));
-			if (res == MMSYSERR_NOERROR) {
-				list.push_back(caps.szPname);
-			}
-		}
-	}
-
-	return list;
-}
-
 void MIDIDriverWinMidi::close() {
 	for (int i = 0; i < connected_sources.size(); i++) {
 		HMIDIIN midi_in = connected_sources[i];
@@ -90,9 +87,7 @@ void MIDIDriverWinMidi::close() {
 		midiInClose(midi_in);
 	}
 	connected_sources.clear();
-}
-
-MIDIDriverWinMidi::MIDIDriverWinMidi() {
+	connected_input_names.clear();
 }
 
 MIDIDriverWinMidi::~MIDIDriverWinMidi() {

drivers/winmidi/midi_driver_winmidi.h

@@ -48,12 +48,10 @@ class MIDIDriverWinMidi : public MIDIDriver {
 	static void CALLBACK read(HMIDIIN hMidiIn, UINT wMsg, DWORD_PTR dwInstance, DWORD_PTR dwParam1, DWORD_PTR dwParam2);
 
 public:
-	virtual Error open();
-	virtual void close();
+	virtual Error open() override;
+	virtual void close() override;
 
-	virtual PackedStringArray get_connected_inputs();
-
-	MIDIDriverWinMidi();
+	MIDIDriverWinMidi() = default;
 	virtual ~MIDIDriverWinMidi();
 };