From 607c5ec49f605966c6870e34cebcd2f967be12d3 Mon Sep 17 00:00:00 2001 From: Ibrahn Sahir Date: Wed, 10 Apr 2024 10:50:28 +0100 Subject: [PATCH] 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) <96648715+AThousandShips@users.noreply.github.com> --- core/os/midi_driver.cpp | 205 +++++++++++++++------- core/os/midi_driver.h | 68 ++++++- drivers/alsamidi/midi_driver_alsamidi.cpp | 177 ++++--------------- drivers/alsamidi/midi_driver_alsamidi.h | 41 +---- drivers/coremidi/midi_driver_coremidi.cpp | 79 +++++---- drivers/coremidi/midi_driver_coremidi.h | 21 ++- drivers/winmidi/midi_driver_winmidi.cpp | 55 +++--- drivers/winmidi/midi_driver_winmidi.h | 8 +- 8 files changed, 335 insertions(+), 319 deletions(-) diff --git a/core/os/midi_driver.cpp b/core/os/midi_driver.cpp index 6870c84b498..6c748b14980 100644 --- a/core/os/midi_driver.cpp +++ b/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 event; - event.instantiate(); - event->set_device(device_index); - uint32_t param_position = 1; +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; +} - 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; +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)); +} - 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::NOTE_ON: +size_t MIDIDriver::Parser::expected_data(MIDIMessage p_msg_type) { + switch (p_msg_type) { case MIDIMessage::NOTE_OFF: - if (length >= 2 + param_position) { - event->set_pitch(data[param_position]); - event->set_velocity(data[param_position + 1]); - } - break; - + case MIDIMessage::NOTE_ON: + case MIDIMessage::AFTERTOUCH: + case MIDIMessage::CONTROL_CHANGE: case MIDIMessage::PITCH_BEND: - if (length >= 2 + param_position) { - event->set_pitch((data[param_position + 1] << 7) | data[param_position]); - } - break; - + case MIDIMessage::SONG_POSITION_POINTER: + return 2; case MIDIMessage::PROGRAM_CHANGE: - if (length >= 1 + param_position) { - event->set_instrument(data[param_position]); - } - break; - case MIDIMessage::CHANNEL_PRESSURE: - if (length >= 1 + param_position) { - event->set_pressure(data[param_position]); - } + case MIDIMessage::QUARTER_FRAME: + case MIDIMessage::SONG_SELECT: + return 1; + default: + return 0; + } +} + +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 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: + case MIDIMessage::NOTE_ON: + event->set_pitch(p_data[0]); + event->set_velocity(p_data[1]); break; + 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: + event->set_instrument(p_data[0]); + break; + case MIDIMessage::CHANNEL_PRESSURE: + 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; } diff --git a/core/os/midi_driver.h b/core/os/midi_driver.h index cad3d8189e0..ddce63f9c83 100644 --- a/core/os/midi_driver.h +++ b/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 diff --git a/drivers/alsamidi/midi_driver_alsamidi.cpp b/drivers/alsamidi/midi_driver_alsamidi.cpp index b87be69cc59..445fc4a9936 100644 --- a/drivers/alsamidi/midi_driver_alsamidi.cpp +++ b/drivers/alsamidi/midi_driver_alsamidi.cpp @@ -37,137 +37,36 @@ #include -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; -} +MIDIDriverALSAMidi::InputConnection::InputConnection(int p_device_index, + snd_rawmidi_t *p_rawmidi) : + parser(p_device_index), rawmidi_ptr(p_rawmidi) {} -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; - } - - 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(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(); } diff --git a/drivers/alsamidi/midi_driver_alsamidi.h b/drivers/alsamidi/midi_driver_alsamidi.h index 95ded3b1c96..45811bec478 100644 --- a/drivers/alsamidi/midi_driver_alsamidi.h +++ b/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 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(); diff --git a/drivers/coremidi/midi_driver_coremidi.cpp b/drivers/coremidi/midi_driver_coremidi.cpp index 87fc7612f72..f6cc59471ed 100644 --- a/drivers/coremidi/midi_driver_coremidi.cpp +++ b/drivers/coremidi/midi_driver_coremidi.cpp @@ -37,16 +37,30 @@ #import #import +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(packet_list->packet); - int *device_index = static_cast(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(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(&i)); - connected_sources.insert(i, source); + InputConnection *conn = memnew(InputConnection(connection_index, source)); + const OSStatus res = MIDIPortConnectSource(port_in, source, static_cast(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(); } diff --git a/drivers/coremidi/midi_driver_coremidi.h b/drivers/coremidi/midi_driver_coremidi.h index 38fb5156646..02cbc6234c5 100644 --- a/drivers/coremidi/midi_driver_coremidi.h +++ b/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 @@ -43,17 +44,25 @@ class MIDIDriverCoreMidi : public MIDIDriver { MIDIClientRef client = 0; MIDIPortRef port_in; - Vector connected_sources; + struct InputConnection { + InputConnection() = default; + InputConnection(int p_device_index, MIDIEndpointRef p_source); + Parser parser; + MIDIEndpointRef source; + }; + + Vector 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(); + virtual Error open() override; + virtual void close() override; - PackedStringArray get_connected_inputs(); - - MIDIDriverCoreMidi(); + MIDIDriverCoreMidi() = default; virtual ~MIDIDriverCoreMidi(); }; diff --git a/drivers/winmidi/midi_driver_winmidi.cpp b/drivers/winmidi/midi_driver_winmidi.cpp index 07f0226c5d7..0f37f63ccd5 100644 --- a/drivers/winmidi/midi_driver_winmidi.cpp +++ b/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() { diff --git a/drivers/winmidi/midi_driver_winmidi.h b/drivers/winmidi/midi_driver_winmidi.h index f3e016f378d..7a752522330 100644 --- a/drivers/winmidi/midi_driver_winmidi.h +++ b/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(); };