From cf9be31ce11be42715f07e1cd634798fbdb635d0 Mon Sep 17 00:00:00 2001 From: Charles the Thobe Date: Wed, 25 Mar 2026 15:36:46 +0200 Subject: [PATCH 1/3] CoreMIDIDevice: Fix potential crash when enumerating device names --- source/zmusic/configuration.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/source/zmusic/configuration.cpp b/source/zmusic/configuration.cpp index de125a7..2d13e97 100644 --- a/source/zmusic/configuration.cpp +++ b/source/zmusic/configuration.cpp @@ -251,16 +251,27 @@ struct MidiDeviceList } } #elif __APPLE__ - for (int i = 0; i < MIDIGetNumberOfDestinations(); i++) + CFStringRef cfName; + char string_buffer[128]; + auto destCount = MIDIGetNumberOfDestinations(); + for (int i = 0; i < destCount; i++) { - uint32_t endpoint = MIDIGetDestination(i); + auto endpoint = MIDIGetDestination(i); if (!endpoint) { continue; } - CFStringRef cfName; + cfName = nullptr; MIDIObjectGetStringProperty(endpoint, kMIDIPropertyName, &cfName); - devices.push_back({ strdup(CFStringGetCStringPtr(cfName, kCFStringEncodingUTF8)), i, MIDIDEV_MAPPER }); + if (!CFStringGetCString(cfName, string_buffer, sizeof(string_buffer), kCFStringEncodingUTF8)) + { + strcpy(string_buffer, "CoreMidi device"); + } + if (cfName != nullptr) + { + CFRelease(cfName); + } + devices.push_back({ strdup(string_buffer), i, MIDIDEV_MAPPER }); } #endif #endif From 624e19f5d29e5300d9325d883caecbcf564dac04 Mon Sep 17 00:00:00 2001 From: Charles the Thobe Date: Wed, 25 Mar 2026 15:37:07 +0200 Subject: [PATCH 2/3] AlsaMIDIDevice and CoreMIDIDevice small refactor --- source/mididevices/music_alsa_mididevice.cpp | 164 ++++++------- .../mididevices/music_coremidi_mididevice.mm | 225 ++++++++---------- 2 files changed, 182 insertions(+), 207 deletions(-) diff --git a/source/mididevices/music_alsa_mididevice.cpp b/source/mididevices/music_alsa_mididevice.cpp index 142c903..9364a22 100644 --- a/source/mididevices/music_alsa_mididevice.cpp +++ b/source/mididevices/music_alsa_mididevice.cpp @@ -57,16 +57,16 @@ class AlsaMIDIDevice : public MIDIDevice int GetTechnology() const override; int SetTempo(int tempo) override; int SetTimeDiv(int timediv) override; - int StreamOut(MidiHeader *data) override; - int StreamOutSync(MidiHeader *data) override; + int StreamOut(MidiHeader* data) override; + int StreamOutSync(MidiHeader* data) override; int Resume() override; void Stop() override; - bool FakeVolume() override { return true; }; //Not sure if we even can control the volume this way with Alsa, so make it fake. + bool FakeVolume() override { return true; }; // Not sure if we even can control the volume this way with Alsa, so make it fake. bool Pause(bool paused) override; void InitPlayback() override; bool Update() override; - bool CanHandleSysex() const override { return true; } //Assume we can, let Alsa sort it out. - void PrecacheInstruments(const uint16_t *instruments, int count) override; + bool CanHandleSysex() const override { return true; } // Assume we can, let Alsa sort it out. + void PrecacheInstruments(const uint16_t* instruments, int count) override; protected: bool PullEvent(); @@ -74,12 +74,13 @@ class AlsaMIDIDevice : public MIDIDevice void HandleEvent(snd_seq_event_t &event, uint tick); AlsaSequencer &sequencer; - MidiHeader *Events = nullptr; + MidiHeader* Events = nullptr; snd_seq_event_t Event; + std::array ShortMsgBuffer; snd_midi_event_t* Coder = nullptr; uint32_t Position = 0; uint32_t PositionOffset; - uint NextEventTickDelta; + uint32_t CurrentEventTickDelta; const static int IntendedPortId = 0; bool Connected = false; @@ -91,12 +92,12 @@ class AlsaMIDIDevice : public MIDIDevice int Technology; bool Precache; - int InitialTempo = 480000; + int InitialTempo = 500000; int Tempo; - int TimeDiv = 480; + int Division = 100; // PPQN std::thread PlayerThread; - volatile bool Exit = false; + volatile bool Exit; std::mutex Mutex; std::condition_variable ExitCond; }; @@ -125,7 +126,7 @@ int AlsaMIDIDevice::Open() if (PortId < 0) { - snd_seq_port_info_t *pinfo; + snd_seq_port_info_t* pinfo; snd_seq_port_info_alloca(&pinfo); snd_seq_port_info_set_port(pinfo, IntendedPortId); @@ -200,12 +201,12 @@ int AlsaMIDIDevice::SetTempo(int tempo) int AlsaMIDIDevice::SetTimeDiv(int timediv) { - TimeDiv = timediv; + Division = timediv; return 0; } // This is meant to mirror WinMIDIDevice::PrecacheInstruments -void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count) +void AlsaMIDIDevice::PrecacheInstruments(const uint16_t* instruments, int count) { // Setting snd_midiprecache to false disables this precaching, since it // does involve sleeping for more than a miniscule amount of time. @@ -215,7 +216,6 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count) } uint8_t bank[16] = {0}; uint8_t i, chan; - std::array message; for (i = 0, chan = 0; i < count; ++i) { @@ -227,29 +227,29 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count) { if (bank[9] != banknum) { - message = { MIDI_CTRLCHANGE | 9, 0, banknum }; - snd_midi_event_encode(Coder, message.data(), 3, &Event); + ShortMsgBuffer = { MIDI_CTRLCHANGE | 9, 0, banknum }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); HandleEvent(Event, 0); bank[9] = banknum; } - message = { MIDI_NOTEON | 9, instr, 1 }; - snd_midi_event_encode(Coder, message.data(), 3, &Event); + ShortMsgBuffer = { MIDI_NOTEON | 9, instr, 1 }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); HandleEvent(Event, 0); } else { // Melodic if (bank[chan] != banknum) { - message = { MIDI_CTRLCHANGE | 9, 0, banknum }; - snd_midi_event_encode(Coder, message.data(), 3, &Event); + ShortMsgBuffer = { MIDI_CTRLCHANGE | 9, 0, banknum }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); HandleEvent(Event, 0); bank[chan] = banknum; } - message = { (uint8_t)(MIDI_PRGMCHANGE | chan), (uint8_t)instruments[i] }; - snd_midi_event_encode(Coder, message.data(), 2, &Event); + ShortMsgBuffer = { (uint8_t)(MIDI_PRGMCHANGE | chan), (uint8_t)instruments[i] }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 2, &Event); HandleEvent(Event, 0); - message = { (uint8_t)(MIDI_NOTEON | chan), 60, 1 }; - snd_midi_event_encode(Coder, message.data(), 3, &Event); + ShortMsgBuffer = { (uint8_t)(MIDI_NOTEON | chan), 60, 1 }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); HandleEvent(Event, 0); if (++chan == 9) { // Skip the percussion channel @@ -265,8 +265,8 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count) for (chan = 15; chan-- != 0; ) { // Turn all notes off - message = { (uint8_t)(MIDI_CTRLCHANGE | chan), 123, 0 }; - snd_midi_event_encode(Coder, message.data(), 3, &Event); + ShortMsgBuffer = { (uint8_t)(MIDI_CTRLCHANGE | chan), 123, 0 }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); HandleEvent(Event, 0); } // And now chan is back at 0, ready to start the cycle over. @@ -277,13 +277,11 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count) { if (bank[i] != 0) { - message = { MIDI_CTRLCHANGE | 9, 0, 0 }; - snd_midi_event_encode(Coder, message.data(), 3, &Event); + ShortMsgBuffer = { MIDI_CTRLCHANGE | 9, 0, 0 }; + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); HandleEvent(Event, 0); } } - // Wait until all events are processed - snd_seq_sync_output_queue(sequencer.handle); } bool AlsaMIDIDevice::PullEvent() @@ -299,10 +297,10 @@ bool AlsaMIDIDevice::PullEvent() } if (Position >= Events->dwBytesRecorded) - { // All events in the "Events" buffer were used, point to next buffer + { // All events in the buffer were used, point to next buffer Events = Events->lpNext; Position = 0; - if (Callback != NULL) + if (Callback) { // This ensures that we always have 2 unused buffers after 1 is used up. // omit this nested "if" block if you want to use up the 2 buffers before requesting new buffers Callback(CallbackData); @@ -314,16 +312,16 @@ bool AlsaMIDIDevice::PullEvent() return false; } - uint32_t *event = (uint32_t *)(Events->lpData + Position); - NextEventTickDelta = event[0]; + uint32_t* event = (uint32_t*)(Events->lpData + Position); + CurrentEventTickDelta = event[0]; // First 4 bytes of event // Get event size to advance Position - if (event[2] < 0x80000000) - { // Short message - PositionOffset = 12; + if (event[2] < 0x80000000) // Short message (event[2] is the combined status/data bytes) + { + PositionOffset = 12; // 4 bytes delta time, 4 bytes reserved, 4 bytes MIDI message (up to 3 bytes + padding) } - else - { // Long message + else // Long message or meta-event (event[2] holds type and parameter length) + { PositionOffset = 12 + ((MEVENT_EVENTPARM(event[2]) + 3) & ~3); } @@ -331,34 +329,29 @@ bool AlsaMIDIDevice::PullEvent() switch (MEVENT_EVENTTYPE(event[2])) { case MEVENT_TEMPO: - { - int tempo = MEVENT_EVENTPARM(event[2]); - snd_seq_ev_set_queue_tempo(&Event, QueueId, tempo); + snd_seq_ev_set_queue_tempo(&Event, QueueId, MEVENT_EVENTPARM(event[2])); break; - } - case MEVENT_LONGMSG: // SysEx messages... + case MEVENT_LONGMSG: // SysEx message... { - uint8_t* data = (uint8_t *)&event[3]; - int len = MEVENT_EVENTPARM(event[2]); - if (len > 2 && data[0] == 0xF0 && data[len - 1] == 0xF7) + int long_msg_len = MEVENT_EVENTPARM(event[2]); + uint8_t* long_msg_data = (uint8_t*)&event[3]; + // Ensure valid sysex message + if (long_msg_len > 2 && long_msg_data[0] == 0xF0 && long_msg_data[long_msg_len - 1] == 0xF7) { - snd_seq_ev_set_sysex(&Event, len, (void*)data); + snd_seq_ev_set_sysex(&Event, long_msg_len, (void*)long_msg_data); break; } } case 0: // Short MIDI event - { - uint8_t status = event[2] & 0xFF; - uint8_t param1 = (event[2] >> 8) & 0x7f; - uint8_t param2 = (event[2] >> 16) & 0x7f; - uint8_t message[] = {status, param1, param2}; - // This silently ignore extra bytes, so no message length logic is needed. - snd_midi_event_encode(Coder, message, 3, &Event); + ShortMsgBuffer = { (uint8_t)(event[2] & 0xff), // Status + (uint8_t)((event[2] >> 8) & 0xff), // Data 1 + (uint8_t)((event[2] >> 16) & 0xff) }; // Data 2 + + // This silently ignores extra bytes, so no message length logic is needed. + snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event); break; - } default: // We didn't really recognize the event, treat it as a NOP Event.type = SND_SEQ_EVENT_NONE; - snd_seq_ev_set_fixed(&Event); } return true; } @@ -374,10 +367,10 @@ void AlsaMIDIDevice::PlayerLoop() const std::chrono::microseconds buffer_step(40000); // TODO: fill in error handling throughout this. - snd_seq_queue_tempo_t *tempo; + snd_seq_queue_tempo_t* tempo; snd_seq_queue_tempo_alloca(&tempo); snd_seq_queue_tempo_set_tempo(tempo, InitialTempo); - snd_seq_queue_tempo_set_ppq(tempo, TimeDiv); + snd_seq_queue_tempo_set_ppq(tempo, Division); snd_seq_set_queue_tempo(sequencer.handle, QueueId, tempo); snd_seq_start_queue(sequencer.handle, QueueId, NULL); @@ -386,7 +379,7 @@ void AlsaMIDIDevice::PlayerLoop() Tempo = InitialTempo; int buffered_ticks = 0; - snd_seq_queue_status_t *status; + snd_seq_queue_status_t* status; snd_seq_queue_status_malloc(&status); while (!Exit) @@ -399,26 +392,28 @@ void AlsaMIDIDevice::PlayerLoop() } // Figure out if we should sleep (the event is too far in the future for us to care), and for how long - int next_event_tick = buffered_ticks + NextEventTickDelta; + int current_event_tick = buffered_ticks + CurrentEventTickDelta; snd_seq_get_queue_status(sequencer.handle, QueueId, status); int queue_tick = snd_seq_queue_status_get_tick_time(status); - int tick_delta = next_event_tick - queue_tick; - auto usecs = std::chrono::microseconds(tick_delta * Tempo / TimeDiv); - auto schedule_time = std::max(std::chrono::microseconds(0), usecs - buffer_step); + int ticks_until_current_ev = current_event_tick - queue_tick; + auto time_until_current_ev = std::chrono::microseconds(ticks_until_current_ev * Tempo / Division); + auto schedule_time = time_until_current_ev - buffer_step; if (schedule_time >= buffer_step) { - ExitCond.wait_for(lock, schedule_time); - continue; + if (ExitCond.wait_for(lock, schedule_time) == std::cv_status::no_timeout) + { + break; + } } - if (tick_delta < 0) - { // Can be triggered on rare occasions on playback start. + if (ticks_until_current_ev < 0) + { // Can be triggered on playback start. // Message shouldn't be shown by default like other midi backends here. - ZMusic_Printf(ZMUSIC_MSG_NOTIFY, "Alsa sequencer underrun: %d ticks!\n", tick_delta); + ZMusic_Printf(ZMUSIC_MSG_NOTIFY, "Alsa sequencer underrun: %d ticks!\n", ticks_until_current_ev); } // We found an event worthy of sending to the sequencer - HandleEvent(Event, next_event_tick); - buffered_ticks = next_event_tick; + HandleEvent(Event, current_event_tick); + buffered_ticks = current_event_tick; Position += PositionOffset; } @@ -426,9 +421,14 @@ void AlsaMIDIDevice::PlayerLoop() snd_seq_queue_status_free(status); } -// Requires QueueId to be started first for non-zero tick position +// Requires QueueId to be started first for non-zero tick positioned events. void AlsaMIDIDevice::HandleEvent(snd_seq_event_t &event, uint tick) { + if (event.type == SND_SEQ_EVENT_NONE) + { // NOP event, clear event handle and return. + snd_seq_ev_clear(&event); + return; + } snd_seq_ev_set_source(&event, PortId); snd_seq_ev_set_subs(&event); if (event.type == SND_SEQ_EVENT_TEMPO) @@ -468,7 +468,10 @@ void AlsaMIDIDevice::Stop() { Exit = true; ExitCond.notify_all(); - PlayerThread.join(); + if (PlayerThread.joinable()) + { + PlayerThread.join(); + } snd_seq_drop_output(sequencer.handle); // This drops events in the sequencer, the sequencer is still usable // Reset all channels to prevent hanging notes @@ -489,19 +492,18 @@ bool AlsaMIDIDevice::Pause(bool paused) } -int AlsaMIDIDevice::StreamOut(MidiHeader *header) +int AlsaMIDIDevice::StreamOut(MidiHeader* header) { - header->lpNext = NULL; - if (Events == NULL) + header->lpNext = nullptr; + if (Events == nullptr) { Events = header; Position = 0; } else { - MidiHeader **p; - - for (p = &Events; *p != NULL; p = &(*p)->lpNext) + MidiHeader** p; + for (p = &Events; *p != nullptr; p = &(*p)->lpNext) { } *p = header; } @@ -509,7 +511,7 @@ int AlsaMIDIDevice::StreamOut(MidiHeader *header) } -int AlsaMIDIDevice::StreamOutSync(MidiHeader *header) +int AlsaMIDIDevice::StreamOutSync(MidiHeader* header) { return StreamOut(header); } @@ -519,7 +521,7 @@ bool AlsaMIDIDevice::Update() return true; } -MIDIDevice *CreateAlsaMIDIDevice(int mididevice) +MIDIDevice* CreateAlsaMIDIDevice(int mididevice) { return new AlsaMIDIDevice(mididevice, miscConfig.snd_midiprecache); } diff --git a/source/mididevices/music_coremidi_mididevice.mm b/source/mididevices/music_coremidi_mididevice.mm index 5a78ed4..c904951 100644 --- a/source/mididevices/music_coremidi_mididevice.mm +++ b/source/mididevices/music_coremidi_mididevice.mm @@ -42,10 +42,10 @@ #include #include -#include "../zmusic/zmusic_internal.h" #include "mididevice.h" #include "zmusic/mididefs.h" #include "zmusic/mus2midi.h" +#include "zmusic/zmusic_internal.h" //========================================================================== // @@ -77,7 +77,6 @@ void PrecacheInstruments(const uint16_t* instruments, int count) override; protected: - void CalcTickRate(); bool PullEvent(); // CoreMIDI handles @@ -90,8 +89,8 @@ enum EventType { TempoEv, MidiMsgEv, NoEvent }; union EventMsg { - uint32_t Tempo; - uint8_t* MidiMsg; + uint32_t tempo; + uint8_t* data; }; struct CurrentEvent { @@ -108,9 +107,9 @@ // Threading std::thread PlayerThread; - volatile bool ExitRequested; - std::condition_variable EventCV; // Still needed for pause/resume - std::mutex EventMutex; // Still needed for pause/resume + volatile bool Exit; + std::condition_variable ExitCond; + std::mutex Mutex; bool isOpen; bool Precache; @@ -119,12 +118,10 @@ int Tempo; int InitialTempo; int Division; - MIDITimeStamp CurrentEvTimeStamp; // This will track the host time of the current event being processed. - MIDITimeStamp NextEvTimeStamp; - double NanoSecsPerTick; // Conversion factor: Host Time Units per MIDI Tick. MidiHeader* Events; // Linked list of MIDI headers uint32_t Position; // Current position in the MidiHeader buffer uint32_t PositionOffset; + uint32_t CurrentEventTickDelta; // Thread functions static void PlayerThreadProc(CoreMIDIDevice* device); @@ -142,11 +139,9 @@ , midiClient(0) , midiOutPort(0) , midiDestination(0) - , ExitRequested(false) , isOpen(false) - , Tempo(500000) // Default: 120 BPM (500,000 µs per quarter note) - , Division(96) // Default PPQN - , CurrentEvTimeStamp(0) + , InitialTempo(500000) // Default: 120 BPM (500,000 µs per quarter note) + , Division(100) // Default PPQN , Events(nullptr) , Position(0) , Precache(precache) @@ -180,7 +175,7 @@ OSStatus status; // Create MIDI client - status = MIDIClientCreate(CFSTR("GZDoom"), nullptr, nullptr, &midiClient); + status = MIDIClientCreate(CFSTR("ZMusic"), nullptr, nullptr, &midiClient); if (status != noErr) { ZMusic_Printf(ZMUSIC_MSG_ERROR, "CoreMIDI: Failed to create MIDI client (error %d)\n", (int)status); @@ -188,7 +183,7 @@ } // Create output port - status = MIDIOutputPortCreate(midiClient, CFSTR("GZDoom Output"), &midiOutPort); + status = MIDIOutputPortCreate(midiClient, CFSTR("ZMusic Program Music"), &midiOutPort); if (status != noErr) { ZMusic_Printf(ZMUSIC_MSG_ERROR, "CoreMIDI: Failed to create output port (error %d)\n", (int)status); @@ -220,17 +215,6 @@ return -1; } - // Get device name for logging - CFStringRef deviceName = nullptr; - MIDIObjectGetStringProperty(midiDestination, kMIDIPropertyName, &deviceName); - if (deviceName != nullptr) - { - char nameBuf[256]; - CFStringGetCString(deviceName, nameBuf, sizeof(nameBuf), kCFStringEncodingUTF8); - ZMusic_Printf(ZMUSIC_MSG_DEBUG, "CoreMIDI: Opened device %d: %s\n", deviceID, nameBuf); - CFRelease(deviceName); - } - isOpen = true; return 0; } @@ -295,20 +279,6 @@ return MIDIDEV_MIDIPORT; } -//========================================================================== -// -// CoreMIDIDevice :: CalcTickRate -// -//========================================================================== - -void CoreMIDIDevice::CalcTickRate() -{ - // Tempo is in microseconds per quarter note. Division is PPQN. - // (Tempo / PPQN) what the midi tick time is in microseconds. - // CoreAudio and CoreMidi work in nano seconds so multiply by 1000. - NanoSecsPerTick = Tempo / Division * 1000; -} - //========================================================================== // // CoreMIDIDevice :: SetTempo @@ -333,7 +303,7 @@ int CoreMIDIDevice::SetTimeDiv(int timediv) { - Division = timediv > 0 ? timediv : 96; + Division = timediv; return 0; } @@ -345,23 +315,20 @@ // //========================================================================== -int CoreMIDIDevice::StreamOut(MidiHeader* data) +int CoreMIDIDevice::StreamOut(MidiHeader* header) { - if (!isOpen) { return -1; }; - - data->lpNext = nullptr; + header->lpNext = nullptr; if (Events == nullptr) { - Events = data; + Events = header; Position = 0; } else { MidiHeader** p; for (p = &Events; *p != nullptr; p = &(*p)->lpNext) - { - } - *p = data; + { } + *p = header; } return 0; } @@ -374,9 +341,9 @@ // //========================================================================== -int CoreMIDIDevice::StreamOutSync(MidiHeader* data) +int CoreMIDIDevice::StreamOutSync(MidiHeader* header) { - return StreamOut(data); + return StreamOut(header); } //========================================================================== @@ -389,14 +356,12 @@ int CoreMIDIDevice::Resume() { - if (!isOpen) { return -1; }; - - if (!PlayerThread.joinable()) + if (!isOpen || PlayerThread.joinable()) { - ExitRequested = false; - PlayerThread = std::thread(PlayerThreadProc, this); + return -1; } - + Exit = false; + PlayerThread = std::thread(PlayerThreadProc, this); return 0; } @@ -410,26 +375,23 @@ void CoreMIDIDevice::Stop() { - if (!isOpen) { return; } - + Exit = true; + ExitCond.notify_all(); if (PlayerThread.joinable()) { - ExitRequested = true; - EventCV.notify_all(); PlayerThread.join(); } + MIDIFlushOutput(midiDestination); // Drop pending events. + // Send All Notes Off and Reset All Controllers for (int channel = 0; channel < 16; ++channel) { - uint8_t msg1[3] = { (uint8_t)(0xB0 | channel), 123, 0 }; - SendMIDIData(msg1, 3, 0); // All Notes Off - uint8_t msg2[3] = { (uint8_t)(0xB0 | channel), 121, 0 }; - SendMIDIData(msg2, 3, 0); // Reset All Controllers + ShortMsgBuffer = { (uint8_t)(0xB0 | channel), 123, 0 }; + SendMIDIData(ShortMsgBuffer.data(), 3, 0); // All Notes Off + ShortMsgBuffer = { (uint8_t)(0xB0 | channel), 121, 0 }; + SendMIDIData(ShortMsgBuffer.data(), 3, 0); // Reset All Controllers } - - // Clear event queue - Events = nullptr; } //========================================================================== @@ -468,11 +430,7 @@ void CoreMIDIDevice::InitPlayback() { - CurrentEvTimeStamp = AudioConvertHostTimeToNanos(AudioGetCurrentHostTime()); // Initialize with current host time - Position = 0; - Events = nullptr; - Tempo = InitialTempo; - CalcTickRate(); + Exit = false; } //========================================================================== @@ -482,6 +440,7 @@ // This is meant to mirror WinMIDIDevice::PrecacheInstruments // //========================================================================== + void CoreMIDIDevice::PrecacheInstruments(const uint16_t* instruments, int count) { // Setting snd_midiprecache to false disables this precaching, since it @@ -574,7 +533,7 @@ } if (Position >= Events->dwBytesRecorded) - { // All events in the "Events" buffer were used, point to next buffer + { // All events in the buffer were used, point to next buffer Events = Events->lpNext; Position = 0; if (Callback) @@ -589,35 +548,30 @@ return false; } - // Read the delta time (first 4 bytes of the event) - uint32_t* event_ptr = (uint32_t*)(Events->lpData + Position); - uint32_t tick_delta = event_ptr[0]; // Assuming delta time is the first uint32_t + uint32_t* event = (uint32_t*)(Events->lpData + Position); + CurrentEventTickDelta = event[0]; // First 4 bytes of event - // Advance CurrentEventHostTime based on delta ticks. - // This timestamp will be used for the current event, accurate to the 0.5 millisecond. - NextEvTimeStamp = CurrentEvTimeStamp + tick_delta * NanoSecsPerTick; - - uint32_t midi_event_type_param = event_ptr[2]; // This is the actual MIDI event or meta-event info - - if (midi_event_type_param < 0x80000000) // Short message (midi_event_type_param is the combined status/data bytes) + // Get event size to advance Position + if (event[2] < 0x80000000) // Short message (event[2] is the combined status/data bytes) { PositionOffset = 12; // 4 bytes delta time, 4 bytes reserved, 4 bytes MIDI message (up to 3 bytes + padding) } - else // Long message or meta-event (midi_event_type_param holds type and parameter length) + else // Long message or meta-event (event[2] holds type and parameter length) { - PositionOffset = 12 + ((MEVENT_EVENTPARM(midi_event_type_param) + 3) & ~3); + PositionOffset = 12 + ((MEVENT_EVENTPARM(event[2]) + 3) & ~3); } - switch (MEVENT_EVENTTYPE(midi_event_type_param)) + // Pulling event out of buffer + switch (MEVENT_EVENTTYPE(event[2])) { case MEVENT_TEMPO: // Tempo change event, update our internal calculation for future events - PrepareTempo(MEVENT_EVENTPARM(midi_event_type_param)); + PrepareTempo(MEVENT_EVENTPARM(event[2])); break; case MEVENT_LONGMSG: - { // Long MIDI message (SysEx, etc.), data starts after event_ptr[3] - int long_msg_len = MEVENT_EVENTPARM(midi_event_type_param); - uint8_t* long_msg_data = (uint8_t*)&event_ptr[3]; + { // Long MIDI message (SysEx, etc.), data starts after event[3] + int long_msg_len = MEVENT_EVENTPARM(event[2]); + uint8_t* long_msg_data = (uint8_t*)&event[3]; // Ensure valid sysex message if (long_msg_len > 2 && long_msg_data[0] == 0xF0 && long_msg_data[long_msg_len - 1] == 0xF7) { @@ -627,10 +581,10 @@ } case 0: // Short MIDI message (note on/off, control change, etc.) { - // midi_event_type_param contains the 1, 2, or 3 byte MIDI message - ShortMsgBuffer = { (uint8_t)(midi_event_type_param & 0xff), // Status - (uint8_t)((midi_event_type_param >> 8) & 0xff), // Data 1 - (uint8_t)((midi_event_type_param >> 16) & 0xff) }; // Data 2 + // event[2] contains the 1, 2, or 3 byte MIDI message + ShortMsgBuffer = { (uint8_t)(event[2] & 0xff), // Status + (uint8_t)((event[2] >> 8) & 0xff), // Data 1 + (uint8_t)((event[2] >> 16) & 0xff) }; // Data 2 int msgLen = 0; if (ShortMsgBuffer[0] >= 0xF0) // System messages @@ -683,59 +637,78 @@ void CoreMIDIDevice::PlayerLoop() { - std::unique_lock lock(EventMutex); - std::chrono::nanoseconds buffer_time_limit(40000000); + std::unique_lock lock(Mutex); + std::chrono::nanoseconds buffer_step(40000000); + + Tempo = InitialTempo; + // Initialize midi clock with current host time + MIDITimeStamp buffer_timestamp = AudioConvertHostTimeToNanos(AudioGetCurrentHostTime()); + // Process all available events and schedule them with CoreMIDI - while (!ExitRequested) //while (Events != nullptr && !Paused && !ExitRequested) + while (!Exit) { if (!PullEvent()) { - EventCV.wait_for(lock, buffer_time_limit); + ExitCond.wait_for(lock, buffer_step); continue; } - std::chrono::nanoseconds next_ev_time_delta(NextEvTimeStamp - AudioConvertHostTimeToNanos(AudioGetCurrentHostTime())); - std::chrono::nanoseconds schedule_time = next_ev_time_delta - buffer_time_limit; - if (schedule_time >= buffer_time_limit) + // CoreAudio and CoreMidi work in nano seconds so multiply by 1000. + MIDITimeStamp current_ev_timestamp = buffer_timestamp + CurrentEventTickDelta * Tempo / Division * 1000; + + auto time_until_current_ev = std::chrono::nanoseconds(current_ev_timestamp - AudioConvertHostTimeToNanos(AudioGetCurrentHostTime())); + auto schedule_time = time_until_current_ev - buffer_step; + if (schedule_time >= buffer_step) + { // Try to keep buffered events under 2x buffer_step + if (ExitCond.wait_for(lock, schedule_time) == std::cv_status::no_timeout) + { + break; + } + } + if (time_until_current_ev < std::chrono::nanoseconds::zero()) + { // Can be triggered on playback start. + // Message shouldn't be shown by default like other midi backends here. + ZMusic_Printf(ZMUSIC_MSG_NOTIFY, "CoreMidi backend underrun by %d nanoseconds!\n", time_until_current_ev.count()); + } + + // Handle CurrentEvent + switch (CurrentEvent.EventType) { - // Try to keep events under 2x time limit - EventCV.wait_for(lock, schedule_time); - continue; + case TempoEv: + Tempo = CurrentEvent.EventMsg.tempo; + break; + case MidiMsgEv: + SendMIDIData(CurrentEvent.EventMsg.data, CurrentEvent.length, AudioConvertNanosToHostTime(current_ev_timestamp)); + break; + case NoEvent: + default: + ; } - CurrentEvTimeStamp = NextEvTimeStamp; + buffer_timestamp = current_ev_timestamp; Position += PositionOffset; - HandleCurrentEvent(); } - std::this_thread::sleep_for(buffer_time_limit * 2); } +//========================================================================== +// +// CoreMIDIDevice :: PrepareTempo and PrepareMidiMsg +// +// Prepare pulled event to be handled later +// +//========================================================================== + void CoreMIDIDevice::PrepareTempo(const uint32_t tempo) { CurrentEvent.EventType = TempoEv; - CurrentEvent.EventMsg.Tempo = tempo; + CurrentEvent.EventMsg.tempo = tempo; } void CoreMIDIDevice::PrepareMidiMsg(uint8_t* msg, uint32_t length) { CurrentEvent.EventType = MidiMsgEv; - CurrentEvent.EventMsg.MidiMsg = msg; + CurrentEvent.EventMsg.data = msg; CurrentEvent.length = length; } -void CoreMIDIDevice::HandleCurrentEvent() -{ - switch (CurrentEvent.EventType) - { - case TempoEv: - Tempo = CurrentEvent.EventMsg.Tempo; - CalcTickRate(); - break; - case MidiMsgEv: - SendMIDIData(CurrentEvent.EventMsg.MidiMsg, CurrentEvent.length, AudioConvertNanosToHostTime(CurrentEvTimeStamp)); - break; - default: - } -} - //========================================================================== // // CoreMIDIDevice :: SendMIDIData From 3eef72774b79b383caeb1577aa0e58469b5c9ea7 Mon Sep 17 00:00:00 2001 From: Charles the Thobe Date: Fri, 3 Apr 2026 02:07:45 +0200 Subject: [PATCH 3/3] AlsaMIDIDevice and CoreMIDIDevice protect against spurious wakeups --- source/mididevices/music_alsa_mididevice.cpp | 2 +- source/mididevices/music_coremidi_mididevice.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/mididevices/music_alsa_mididevice.cpp b/source/mididevices/music_alsa_mididevice.cpp index 9364a22..e301e8b 100644 --- a/source/mididevices/music_alsa_mididevice.cpp +++ b/source/mididevices/music_alsa_mididevice.cpp @@ -402,7 +402,7 @@ void AlsaMIDIDevice::PlayerLoop() { if (ExitCond.wait_for(lock, schedule_time) == std::cv_status::no_timeout) { - break; + continue; } } if (ticks_until_current_ev < 0) diff --git a/source/mididevices/music_coremidi_mididevice.mm b/source/mididevices/music_coremidi_mididevice.mm index c904951..1a5aa15 100644 --- a/source/mididevices/music_coremidi_mididevice.mm +++ b/source/mididevices/music_coremidi_mididevice.mm @@ -662,7 +662,7 @@ { // Try to keep buffered events under 2x buffer_step if (ExitCond.wait_for(lock, schedule_time) == std::cv_status::no_timeout) { - break; + continue; } } if (time_until_current_ev < std::chrono::nanoseconds::zero())