From 46908fb63a9894d467b4d42c10fa64e92792f51f Mon Sep 17 00:00:00 2001 From: Niels Martin Hansen Date: Thu, 4 Jul 2019 21:27:18 +0200 Subject: [PATCH] Fix: Various reliability and correctness improvements to MIDI on Windows. (#7620) --- src/music/dmusic.cpp | 50 +++++------- src/music/midi.h | 15 ++++ src/music/midifile.cpp | 31 ++++++++ src/music/win32_m.cpp | 167 ++++++++++++++++++++++------------------- 4 files changed, 151 insertions(+), 112 deletions(-) diff --git a/src/music/dmusic.cpp b/src/music/dmusic.cpp index fece709fda..47b3db19ae 100644 --- a/src/music/dmusic.cpp +++ b/src/music/dmusic.cpp @@ -538,19 +538,19 @@ static void TransmitChannelMsg(IDirectMusicBuffer *buffer, REFERENCE_TIME rt, by } } -static void TransmitSysex(IDirectMusicBuffer *buffer, REFERENCE_TIME rt, byte *&msg_start, size_t &remaining) +static void TransmitSysex(IDirectMusicBuffer *buffer, REFERENCE_TIME rt, const byte *&msg_start, size_t &remaining) { /* Find end of message. */ - byte *msg_end = msg_start; + const byte *msg_end = msg_start; while (*msg_end != MIDIST_ENDSYSEX) msg_end++; msg_end++; // Also include SysEx end byte. - if (buffer->PackUnstructured(rt, 0, msg_end - msg_start, msg_start) == E_OUTOFMEMORY) { + if (buffer->PackUnstructured(rt, 0, msg_end - msg_start, const_cast(msg_start)) == E_OUTOFMEMORY) { /* Buffer is full, clear it and try again. */ _port->PlayBuffer(buffer); buffer->Flush(); - buffer->PackUnstructured(rt, 0, msg_end - msg_start, msg_start); + buffer->PackUnstructured(rt, 0, msg_end - msg_start, const_cast(msg_start)); } /* Update position in buffer. */ @@ -558,9 +558,11 @@ static void TransmitSysex(IDirectMusicBuffer *buffer, REFERENCE_TIME rt, byte *& msg_start = msg_end; } -static void TransmitSysexConst(IDirectMusicBuffer *buffer, REFERENCE_TIME rt, byte *msg_start, size_t length) +static void TransmitStandardSysex(IDirectMusicBuffer *buffer, REFERENCE_TIME rt, MidiSysexMessage msg) { - TransmitSysex(buffer, rt, msg_start, length); + size_t length = 0; + const byte *data = MidiGetStandardSysexMessage(msg, length); + TransmitSysex(buffer, rt, data, length); } /** Transmit 'Note off' messages to all MIDI channels. */ @@ -571,27 +573,16 @@ static void TransmitNotesOff(IDirectMusicBuffer *buffer, REFERENCE_TIME block_ti TransmitChannelMsg(_buffer, block_time + 10, MIDIST_CONTROLLER | ch, MIDICT_SUSTAINSW, 0); TransmitChannelMsg(_buffer, block_time + 10, MIDIST_CONTROLLER | ch, MIDICT_MODE_RESETALLCTRL, 0); } - /* Explicitly flush buffer to make sure the note off messages are processed - * before we send any additional control messages. */ + + /* Performing a GM reset stops all sound and resets all parameters. */ + TransmitStandardSysex(_buffer, block_time + 20, MidiSysexMessage::ResetGM); + TransmitStandardSysex(_buffer, block_time + 30, MidiSysexMessage::RolandSetReverb); + + /* Explicitly flush buffer to make sure the messages are processed, + * as we want sound to stop immediately. */ _port->PlayBuffer(_buffer); _buffer->Flush(); - /* Some songs change the "Pitch bend range" registered parameter. If - * this doesn't get reset, everything else will start sounding wrong. */ - for (int ch = 0; ch < 16; ch++) { - /* Running status, only need status for first message - * Select RPN 00.00, set value to 02.00, and de-select again */ - TransmitChannelMsg(_buffer, block_time + 10, MIDIST_CONTROLLER | ch, MIDICT_RPN_SELECT_LO, 0x00); - TransmitChannelMsg(_buffer, block_time + 10, MIDICT_RPN_SELECT_HI, 0x00); - TransmitChannelMsg(_buffer, block_time + 10, MIDICT_DATAENTRY, 0x02); - TransmitChannelMsg(_buffer, block_time + 10, MIDICT_DATAENTRY_LO, 0x00); - TransmitChannelMsg(_buffer, block_time + 10, MIDICT_RPN_SELECT_LO, 0x7F); - TransmitChannelMsg(_buffer, block_time + 10, MIDICT_RPN_SELECT_HI, 0x7F); - - _port->PlayBuffer(_buffer); - _buffer->Flush(); - } - /* Wait until message time has passed. */ Sleep(Clamp((block_time - cur_time) / MS_TO_REFTIME, 5, 1000)); } @@ -616,13 +607,6 @@ static void MidiThreadProc(void *) REFERENCE_TIME cur_time; clock->GetTime(&cur_time); - /* Standard "Enable General MIDI" message */ - static byte gm_enable_sysex[] = { 0xF0, 0x7E, 0x00, 0x09, 0x01, 0xF7 }; - TransmitSysexConst(_buffer, cur_time, &gm_enable_sysex[0], sizeof(gm_enable_sysex)); - /* Roland-specific reverb room control, used by the original game */ - static byte roland_reverb_sysex[] = { 0xF0, 0x41, 0x10, 0x42, 0x12, 0x40, 0x01, 0x30, 0x02, 0x04, 0x00, 0x40, 0x40, 0x00, 0x00, 0x09, 0xF7 }; - TransmitSysexConst(_buffer, cur_time, &roland_reverb_sysex[0], sizeof(roland_reverb_sysex)); - _port->PlayBuffer(_buffer); _buffer->Flush(); @@ -665,7 +649,7 @@ static void MidiThreadProc(void *) _playback.do_start = false; } - /* Turn all notes off in case we are seeking between music titles. */ + /* Reset playback device between songs. */ clock->GetTime(&cur_time); TransmitNotesOff(_buffer, block_time, cur_time); @@ -751,7 +735,7 @@ static void MidiThreadProc(void *) block_time = playback_start_time + block.realtime * MIDITIME_TO_REFTIME; DEBUG(driver, 9, "DMusic thread: Streaming block " PRINTF_SIZE " (cur=" OTTD_PRINTF64 ", block=" OTTD_PRINTF64 ")", current_block, (long long)(current_time / MS_TO_REFTIME), (long long)(block_time / MS_TO_REFTIME)); - byte *data = block.data.Begin(); + const byte *data = block.data.Begin(); size_t remaining = block.data.Length(); byte last_status = 0; while (remaining > 0) { diff --git a/src/music/midi.h b/src/music/midi.h index 473f7f18bb..90f04435e6 100644 --- a/src/music/midi.h +++ b/src/music/midi.h @@ -141,4 +141,19 @@ enum MidiController { MIDICT_MODE_POLY = 127, }; + +/** Well-known MIDI system exclusive message values for use with the MidiGetStandardSysexMessage function. */ +enum class MidiSysexMessage { + /** Reset device to General MIDI defaults */ + ResetGM, + /** Reset device to (Roland) General Standard defaults */ + ResetGS, + /** Reset device to (Yamaha) XG defaults */ + ResetXG, + /** Set up Roland SoundCanvas reverb room as TTD does */ + RolandSetReverb, +}; + +const byte *MidiGetStandardSysexMessage(MidiSysexMessage msg, size_t &length); + #endif /* MUSIC_MIDI_H */ diff --git a/src/music/midifile.cpp b/src/music/midifile.cpp index 91f83c529d..eb5499426d 100644 --- a/src/music/midifile.cpp +++ b/src/music/midifile.cpp @@ -27,6 +27,37 @@ static MidiFile *_midifile_instance = NULL; +/** + * Retrieve a well-known MIDI system exclusive message. + * @param msg Which sysex message to retrieve + * @param[out] length Receives the length of the returned buffer + * @return Pointer to byte buffer with sysex message + */ +const byte *MidiGetStandardSysexMessage(MidiSysexMessage msg, size_t &length) +{ + static byte reset_gm_sysex[] = { 0xF0, 0x7E, 0x7F, 0x09, 0x01, 0xF7 }; + static byte reset_gs_sysex[] = { 0xF0, 0x41, 0x10, 0x42, 0x12, 0x40, 0x00, 0x7F, 0x00, 0x41, 0xF7 }; + static byte reset_xg_sysex[] = { 0xF0, 0x43, 0x10, 0x4C, 0x00, 0x00, 0x7E, 0x00, 0xF7 }; + static byte roland_reverb_sysex[] = { 0xF0, 0x41, 0x10, 0x42, 0x12, 0x40, 0x01, 0x30, 0x02, 0x04, 0x00, 0x40, 0x40, 0x00, 0x00, 0x09, 0xF7 }; + + switch (msg) { + case MidiSysexMessage::ResetGM: + length = lengthof(reset_gm_sysex); + return reset_gm_sysex; + case MidiSysexMessage::ResetGS: + length = lengthof(reset_gs_sysex); + return reset_gs_sysex; + case MidiSysexMessage::ResetXG: + length = lengthof(reset_xg_sysex); + return reset_xg_sysex; + case MidiSysexMessage::RolandSetReverb: + length = lengthof(roland_reverb_sysex); + return roland_reverb_sysex; + default: + NOT_REACHED(); + } +} + /** * Owning byte buffer readable as a stream. * RAII-compliant to make teardown in error situations easier. diff --git a/src/music/win32_m.cpp b/src/music/win32_m.cpp index a32318db12..abe2aa31b4 100644 --- a/src/music/win32_m.cpp +++ b/src/music/win32_m.cpp @@ -35,7 +35,7 @@ static struct { CRITICAL_SECTION lock; ///< synchronization for playback status fields bool playing; ///< flag indicating that playback is active - bool do_start; ///< flag for starting playback of next_file at next opportunity + int do_start; ///< flag for starting playback of next_file at next opportunity bool do_stop; ///< flag for stopping playback at next opportunity byte current_volume; ///< current effective volume setting byte new_volume; ///< volume setting to change to @@ -73,10 +73,10 @@ static void TransmitChannelMsg(byte status, byte p1, byte p2 = 0) midiOutShortMsg(_midi.midi_out, status | (p1 << 8) | (p2 << 16)); } -static void TransmitSysex(byte *&msg_start, size_t &remaining) +static void TransmitSysex(const byte *&msg_start, size_t &remaining) { /* find end of message */ - byte *msg_end = msg_start; + const byte *msg_end = msg_start; while (*msg_end != MIDIST_ENDSYSEX) msg_end++; msg_end++; /* also include sysex end byte */ @@ -97,9 +97,11 @@ static void TransmitSysex(byte *&msg_start, size_t &remaining) msg_start = msg_end; } -static void TransmitSysexConst(byte *msg_start, size_t length) +static void TransmitStandardSysex(MidiSysexMessage msg) { - TransmitSysex(msg_start, length); + size_t length = 0; + const byte *data = MidiGetStandardSysexMessage(msg, length); + TransmitSysex(data, length); } /** @@ -108,82 +110,94 @@ static void TransmitSysexConst(byte *msg_start, size_t length) */ void CALLBACK TimerCallback(UINT uTimerID, UINT, DWORD_PTR dwUser, DWORD_PTR, DWORD_PTR) { - /* Try to check playback status changes. - * If _midi is already locked, skip checking for this cycle and try again - * next cycle, instead of waiting for locks in the realtime callback. */ - if (TryEnterCriticalSection(&_midi.lock)) { - /* check for stop */ - if (_midi.do_stop) { - DEBUG(driver, 2, "Win32-MIDI: timer: do_stop is set"); - midiOutReset(_midi.midi_out); - _midi.playing = false; - _midi.do_stop = false; + /* Ensure only one timer callback is running at once, and prevent races on status flags */ + if (!TryEnterCriticalSection(&_midi.lock)) return; + + /* check for stop */ + if (_midi.do_stop) { + DEBUG(driver, 2, "Win32-MIDI: timer: do_stop is set"); + midiOutReset(_midi.midi_out); + _midi.playing = false; + _midi.do_stop = false; + LeaveCriticalSection(&_midi.lock); + return; + } + + /* check for start/restart/change song */ + if (_midi.do_start != 0) { + /* Have a delay between playback start steps, prevents jumbled-together notes at the start of song */ + if (timeGetTime() - _midi.playback_start_time < 50) { LeaveCriticalSection(&_midi.lock); return; } + DEBUG(driver, 2, "Win32-MIDI: timer: do_start step %d", _midi.do_start); - /* check for start/restart/change song */ - if (_midi.do_start) { - DEBUG(driver, 2, "Win32-MIDI: timer: do_start is set"); - if (_midi.playing) { - midiOutReset(_midi.midi_out); - /* Some songs change the "Pitch bend range" registered - * parameter. If this doesn't get reset, everything else - * will start sounding wrong. */ - for (int ch = 0; ch < 16; ch++) { - /* Running status, only need status for first message */ - /* Select RPN 00.00, set value to 02.00, and unselect again */ - TransmitChannelMsg(MIDIST_CONTROLLER | ch, MIDICT_RPN_SELECT_LO, 0x00); - TransmitChannelMsg(MIDICT_RPN_SELECT_HI, 0x00); - TransmitChannelMsg(MIDICT_DATAENTRY, 0x02); - TransmitChannelMsg(MIDICT_DATAENTRY_LO, 0x00); - TransmitChannelMsg(MIDICT_RPN_SELECT_LO, 0x7F); - TransmitChannelMsg(MIDICT_RPN_SELECT_HI, 0x7F); - } - } + if (_midi.do_start == 1) { + /* Send "all notes off" */ + midiOutReset(_midi.midi_out); + _midi.playback_start_time = timeGetTime(); + _midi.do_start = 2; + + LeaveCriticalSection(&_midi.lock); + return; + } else if (_midi.do_start == 2) { + /* Reset the device to General MIDI defaults */ + TransmitStandardSysex(MidiSysexMessage::ResetGM); + _midi.playback_start_time = timeGetTime(); + _midi.do_start = 3; + + LeaveCriticalSection(&_midi.lock); + return; + } else if (_midi.do_start == 3) { + /* Set up device-specific effects */ + TransmitStandardSysex(MidiSysexMessage::RolandSetReverb); + _midi.playback_start_time = timeGetTime(); + _midi.do_start = 4; + + LeaveCriticalSection(&_midi.lock); + return; + } else if (_midi.do_start == 4) { + /* Load the new file */ _midi.current_file.MoveFrom(_midi.next_file); std::swap(_midi.next_segment, _midi.current_segment); _midi.current_segment.start_block = 0; _midi.playback_start_time = timeGetTime(); _midi.playing = true; - _midi.do_start = false; + _midi.do_start = 0; _midi.current_block = 0; MemSetT(_midi.channel_volumes, 127, lengthof(_midi.channel_volumes)); - } else if (!_midi.playing) { - /* not playing, stop the timer */ - DEBUG(driver, 2, "Win32-MIDI: timer: not playing, stopping timer"); - timeKillEvent(uTimerID); - _midi.timer_id = 0; - LeaveCriticalSection(&_midi.lock); - return; } - - /* check for volume change */ - static int volume_throttle = 0; - if (_midi.current_volume != _midi.new_volume) { - if (volume_throttle == 0) { - DEBUG(driver, 2, "Win32-MIDI: timer: volume change"); - _midi.current_volume = _midi.new_volume; - volume_throttle = 20 / _midi.time_period; - for (int ch = 0; ch < 16; ch++) { - int vol = ScaleVolume(_midi.channel_volumes[ch], _midi.current_volume); - TransmitChannelMsg(MIDIST_CONTROLLER | ch, MIDICT_CHANVOLUME, vol); - } - } - else { - volume_throttle--; - } - } - + } else if (!_midi.playing) { + /* not playing, stop the timer */ + DEBUG(driver, 2, "Win32-MIDI: timer: not playing, stopping timer"); + timeKillEvent(uTimerID); + _midi.timer_id = 0; LeaveCriticalSection(&_midi.lock); + return; + } + + /* check for volume change */ + static int volume_throttle = 0; + if (_midi.current_volume != _midi.new_volume) { + if (volume_throttle == 0) { + DEBUG(driver, 2, "Win32-MIDI: timer: volume change"); + _midi.current_volume = _midi.new_volume; + volume_throttle = 20 / _midi.time_period; + for (int ch = 0; ch < 16; ch++) { + byte vol = ScaleVolume(_midi.channel_volumes[ch], _midi.current_volume); + TransmitChannelMsg(MIDIST_CONTROLLER | ch, MIDICT_CHANVOLUME, vol); + } + } else { + volume_throttle--; + } } /* skip beginning of file? */ if (_midi.current_segment.start > 0 && _midi.current_block == 0 && _midi.current_segment.start_block == 0) { /* find first block after start time and pretend playback started earlier - * this is to allow all blocks prior to the actual start to still affect playback, - * as they may contain important controller and program changes */ + * this is to allow all blocks prior to the actual start to still affect playback, + * as they may contain important controller and program changes */ uint preload_bytes = 0; for (size_t bl = 0; bl < _midi.current_file.blocks.size(); bl++) { MidiFile::DataBlock &block = _midi.current_file.blocks[bl]; @@ -200,7 +214,7 @@ void CALLBACK TimerCallback(UINT uTimerID, UINT, DWORD_PTR dwUser, DWORD_PTR, DW * The delay compensation is needed to avoid time-compression of following messages. */ DEBUG(driver, 2, "Win32-MIDI: timer: start from block %d (ticktime %d, realtime %.3f, bytes %d)", (int)bl, (int)block.ticktime, ((int)block.realtime) / 1000.0, (int)preload_bytes); - _midi.playback_start_time -= block.realtime / 1000 - preload_bytes * 1000 / 3125; + _midi.playback_start_time -= block.realtime / 1000 - (DWORD)(preload_bytes * 1000 / 3125); break; } } @@ -229,7 +243,7 @@ void CALLBACK TimerCallback(UINT uTimerID, UINT, DWORD_PTR dwUser, DWORD_PTR, DW break; } - byte *data = block.data.Begin(); + const byte *data = block.data.Begin(); size_t remaining = block.data.Length(); byte last_status = 0; while (remaining > 0) { @@ -308,25 +322,28 @@ void CALLBACK TimerCallback(UINT uTimerID, UINT, DWORD_PTR dwUser, DWORD_PTR, DW _midi.do_stop = true; } } + + LeaveCriticalSection(&_midi.lock); } void MusicDriver_Win32::PlaySong(const MusicSongInfo &song) { DEBUG(driver, 2, "Win32-MIDI: PlaySong: entry"); + + MidiFile new_song; + if (!new_song.LoadSong(song)) return; + DEBUG(driver, 2, "Win32-MIDI: PlaySong: Loaded song"); + EnterCriticalSection(&_midi.lock); - if (!_midi.next_file.LoadSong(song)) { - LeaveCriticalSection(&_midi.lock); - return; - } - + _midi.next_file.MoveFrom(new_song); _midi.next_segment.start = song.override_start; _midi.next_segment.end = song.override_end; _midi.next_segment.loop = song.loop; DEBUG(driver, 2, "Win32-MIDI: PlaySong: setting flag"); _midi.do_stop = _midi.playing; - _midi.do_start = true; + _midi.do_start = 1; if (_midi.timer_id == 0) { DEBUG(driver, 2, "Win32-MIDI: PlaySong: starting timer"); @@ -347,7 +364,7 @@ void MusicDriver_Win32::StopSong() bool MusicDriver_Win32::IsSongPlaying() { - return _midi.playing || _midi.do_start; + return _midi.playing || (_midi.do_start != 0); } void MusicDriver_Win32::SetVolume(byte vol) @@ -381,14 +398,6 @@ const char *MusicDriver_Win32::Start(const char * const *parm) midiOutReset(_midi.midi_out); - /* Standard "Enable General MIDI" message */ - static byte gm_enable_sysex[] = { 0xF0, 0x7E, 0x00, 0x09, 0x01, 0xF7 }; - TransmitSysexConst(&gm_enable_sysex[0], sizeof(gm_enable_sysex)); - - /* Roland-specific reverb room control, used by the original game */ - static byte roland_reverb_sysex[] = { 0xF0, 0x41, 0x10, 0x42, 0x12, 0x40, 0x01, 0x30, 0x02, 0x04, 0x00, 0x40, 0x40, 0x00, 0x00, 0x09, 0xF7 }; - TransmitSysexConst(&roland_reverb_sysex[0], sizeof(roland_reverb_sysex)); - /* prepare multimedia timer */ TIMECAPS timecaps; if (timeGetDevCaps(&timecaps, sizeof(timecaps)) == MMSYSERR_NOERROR) {