[Scummvm-git-logs] scummvm master -> 11c74aaef85c85e56c76f8b4f380d971923f7b33

sev- sev at scummvm.org
Fri Jul 24 22:35:57 UTC 2020


This automated email contains information about 12 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
dd0090dc7a GROOVIE: Use new Miles MIDI driver functionality
2f4937bd6d MIDI: Fix XMIDI SysEx Final Data controller
50772c1fbc MIDI: Fix XMIDI custom timbre loading
b38f1f00ac MIDI/GROOVIE: Fix MT-32 XMIDI timbre init delays
848d8315a6 MIDI/GROOVIE: Refactor XMIDI timbre chunk loading
0856c04bfa GROOVIE: Remove dead music code
315bbc7b5b MIDI: Use driver-specific impl. for stopping all notes
3ad8c85af2 MIDI: Prevent duplicate MIDI messages
5da83d8ea0 GROOVIE: Pause music when ScummVM menu is open
da64fdc3d1 MIDI: Fix MIDI parser tracker overflow
dcdcca3d31 GROOVIE: Initialize MIDI on direct game load
11c74aaef8 MIDI: Updated MidiParser documentation


Commit: dd0090dc7a9d9219584064e07c5d24c7007a25c2
    https://github.com/scummvm/scummvm/commit/dd0090dc7a9d9219584064e07c5d24c7007a25c2
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
GROOVIE: Use new Miles MIDI driver functionality

This adds using the updated Miles MIDI driver for GM music and using the driver
for source volume scaling.

This reduces the amount of MIDI volume messages when fading, because they are
now only sent on active MIDI channels and if they are different from the
current channel volume. This fixes problems with hardware MIDI devices like the
MT-32 and SC-55, which have trouble dealing with the high number of messages.

Changed paths:
    engines/groovie/music.cpp
    engines/groovie/music.h


diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index a489b22037..a9c83e7d3e 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -385,11 +385,14 @@ bool MusicPlayerMidi::loadParser(Common::SeekableReadStream *stream, bool loop)
 // MusicPlayerXMI
 
 MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName) :
-	MusicPlayerMidi(vm) {
+	MusicPlayerMidi(vm),
+	_milesMidiDriver(NULL) {
 
 	// Create the driver
 	MidiDriver::DeviceHandle dev = MidiDriver::detectDevice(MDT_MIDI | MDT_ADLIB | MDT_PREFER_GM);
 	MusicType musicType = MidiDriver::getMusicType(dev);
+	if (musicType == MT_GM && ConfMan.getBool("native_mt32"))
+		musicType = MT_MT32;
 	_driver = NULL;
 
 	// new Miles Audio support, to disable set milesAudioEnabled to false
@@ -404,24 +407,22 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 		// 11th Hour uses SAMPLE.AD/SAMPLE.OPL/SAMPLE.MT
 		switch (musicType) {
 		case MT_ADLIB:
+			// TODO Would be nice if the Miles AdLib and MIDI drivers shared
+			// a common interface, then we can use only _milesMidiDriver in
+			// this class.
 			_driver = Audio::MidiDriver_Miles_AdLib_create(gtlName + ".AD", gtlName + ".OPL");
 			break;
 		case MT_MT32:
-			_driver = Audio::MidiDriver_Miles_MT32_create(gtlName + ".MT");
+			_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, gtlName + ".MT");
+			newTimbreListProc = Audio::MidiDriver_Miles_MT32_processXMIDITimbreChunk;
 			break;
 		case MT_GM:
-			if (ConfMan.getBool("native_mt32")) {
-				_driver = Audio::MidiDriver_Miles_MT32_create(gtlName + ".MT");
-				musicType = MT_MT32;
-			}
+			_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, "");
 			break;
 		default:
 			break;
 		}
-
-		if (musicType == MT_MT32) {
-			newTimbreListProc = Audio::MidiDriver_Miles_MT32_processXMIDITimbreChunk;
-		}
+		_musicType = musicType;
 	}
 
 	if (_driver) {
@@ -436,9 +437,11 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 	assert(_driver);
 
 	// Create the parser
-	_midiParser = MidiParser::createParser_XMIDI(NULL, NULL, newTimbreListProc, _driver);
+	_midiParser = MidiParser::createParser_XMIDI(NULL, NULL, newTimbreListProc, _milesMidiDriver, 0);
 
-	_driver->open();	// TODO: Handle return value != 0 (indicating an error)
+	int result = _driver->open();
+	if (result > 0 && result != MidiDriver::MERR_ALREADY_OPEN)
+		error("Opening MidiDriver failed with error code %i", result);
 
 	// Set the parser's driver
 	_midiParser->setMidiDriver(this);
@@ -540,6 +543,33 @@ void MusicPlayerXMI::send(uint32 b) {
 	MusicPlayerMidi::send(bytesToSend);
 }
 
+void MusicPlayerXMI::send(int8 source, uint32 b) {
+	if (_milesMidiDriver) {
+		_milesMidiDriver->send(source, b);
+	} else {
+		MusicPlayerMidi::send(b);
+	}
+}
+
+void MusicPlayerXMI::metaEvent(int8 source, byte type, byte *data, uint16 length) {
+	if (_milesMidiDriver) {
+		if (type == 0x2F) // End Of Track
+			MusicPlayerMidi::endTrack();
+		_milesMidiDriver->metaEvent(source, type, data, length);
+	} else {
+		MusicPlayerMidi::metaEvent(type, data, length);
+	}
+}
+
+void MusicPlayerXMI::updateVolume() {
+	if (_milesMidiDriver) {
+		uint16 val = (_userVolume * _gameVolume) / 100;
+		_milesMidiDriver->setSourceVolume(0, val);
+	} else {
+		MusicPlayerMidi::updateVolume();
+	}
+}
+
 bool MusicPlayerXMI::load(uint32 fileref, bool loop) {
 	debugC(1, kDebugMIDI, "Groovie::Music: Starting the playback of song: %04X", fileref);
 
@@ -553,6 +583,13 @@ bool MusicPlayerXMI::load(uint32 fileref, bool loop) {
 	return loadParser(file, loop);
 }
 
+void MusicPlayerXMI::unload() {
+	MusicPlayerMidi::unload();
+	if (_milesMidiDriver) {
+		_milesMidiDriver->deinitSource(0);
+	}
+}
+
 void MusicPlayerXMI::loadTimbres(const Common::String &filename) {
 	// Load the Global Timbre Library format as documented in AIL2
 	debugC(1, kDebugMIDI, "Groovie::Music: Loading the GTL file %s", filename.c_str());
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index 73194e264f..04d7d98ac2 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -27,6 +27,7 @@
 #include "common/mutex.h"
 #include "audio/mididrv.h"
 #include "audio/mixer.h"
+#include "audio/miles.h"
 
 class MidiParser;
 
@@ -107,8 +108,6 @@ private:
 	byte _chanVolumes[0x10];
 	void updateChanVolume(byte channel);
 
-	void endTrack();
-
 protected:
 	byte *_data;
 	MidiParser *_midiParser;
@@ -117,6 +116,7 @@ protected:
 	void onTimerInternal() override;
 	void updateVolume() override;
 	void unload() override;
+	void endTrack();
 
 	bool loadParser(Common::SeekableReadStream *stream, bool loop);
 };
@@ -127,9 +127,13 @@ public:
 	~MusicPlayerXMI() override;
 
 	void send(uint32 b) override;
+	void send(int8 source, uint32 b) override;
+	void metaEvent(int8 source, byte type, byte *data, uint16 length) override;
 
 protected:
+	void updateVolume() override;
 	bool load(uint32 fileref, bool loop) override;
+	void unload() override;
 
 private:
 	// Channel banks
@@ -139,6 +143,7 @@ private:
 	uint8 _musicType;
 
 	bool _milesAudioMode;
+	Audio::MidiDriver_Miles_Midi *_milesMidiDriver;
 
 	// Timbres
 	class Timbre {


Commit: 2f4937bd6d9e1fd5680f732411a3b2725ba34140
    https://github.com/scummvm/scummvm/commit/2f4937bd6d9e1fd5680f732411a3b2725ba34140
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI: Fix XMIDI SysEx Final Data controller

XMIDI defines controllers which allow the MIDI data to send controller changes
that build up SysEx messages. The last SysEx data byte is specified using the
Final Data controller, which should append the final byte to the SysEx message
and send it to the MIDI device. The old implementation did not append the last
byte. Additionally, when increasing the memory address for the SysEx, it did
not take into account that the MIDI bytes are 7 bit.

This commit fixes these issues. This restores a missing data byte in a SySex in
the MIDI initialization of The 7th Guest.

Changed paths:
    audio/miles.h
    audio/miles_midi.cpp


diff --git a/audio/miles.h b/audio/miles.h
index 25ad2ad9a8..98f33c394a 100644
--- a/audio/miles.h
+++ b/audio/miles.h
@@ -70,11 +70,11 @@ namespace Audio {
 #define MILES_CONTROLLER_SYSEX_QUEUE_COUNT 3
 #define MILES_CONTROLLER_SYSEX_QUEUE_SIZE 32
 
-#define MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS1 0
-#define MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS2 1
-#define MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS3 2
-#define MILES_CONTROLLER_SYSEX_COMMAND_DATA     3
-#define MILES_CONTROLLER_SYSEX_COMMAND_SEND     4
+#define MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS1   0
+#define MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS2   1
+#define MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS3   2
+#define MILES_CONTROLLER_SYSEX_COMMAND_DATA       3
+#define MILES_CONTROLLER_SYSEX_COMMAND_FINAL_DATA 4
 
 #define MILES_CONTROLLER_XMIDI_RANGE_BEGIN 110
 #define MILES_CONTROLLER_XMIDI_RANGE_END 120
diff --git a/audio/miles_midi.cpp b/audio/miles_midi.cpp
index 6b6cb79af3..948af1f576 100644
--- a/audio/miles_midi.cpp
+++ b/audio/miles_midi.cpp
@@ -479,9 +479,9 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 
 		// figure out which queue is accessed
 		controllerNumber -= MILES_CONTROLLER_SYSEX_RANGE_BEGIN;
-		while (controllerNumber > MILES_CONTROLLER_SYSEX_COMMAND_SEND) {
+		while (controllerNumber > MILES_CONTROLLER_SYSEX_COMMAND_FINAL_DATA) {
 			sysExQueueNr++;
-			controllerNumber -= (MILES_CONTROLLER_SYSEX_COMMAND_SEND + 1);
+			controllerNumber -= (MILES_CONTROLLER_SYSEX_COMMAND_FINAL_DATA + 1);
 		}
 		assert(sysExQueueNr < MILES_CONTROLLER_SYSEX_QUEUE_COUNT);
 
@@ -513,8 +513,15 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 				}
 			}
 			break;
-		case MILES_CONTROLLER_SYSEX_COMMAND_SEND:
-			sysExSend = true;
+		case MILES_CONTROLLER_SYSEX_COMMAND_FINAL_DATA:
+			if (sysExPos < MILES_CONTROLLER_SYSEX_QUEUE_SIZE) {
+				// Space left? put current byte into queue
+				_sysExQueues[sysExQueueNr].data[sysExPos] = controllerValue;
+				sysExPos++;
+				// Do not increment dataPos. Subsequent Final Data commands will
+				// re-send the last address byte with the new controller value.
+				sysExSend = true;
+			}
 			break;
 		default:
 			assert(0);
@@ -528,8 +535,23 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 				// Execute SysEx
 				MT32SysEx(_sysExQueues[sysExQueueNr].targetAddress, _sysExQueues[sysExQueueNr].data);
 
-				// adjust target address to point at the end of the current data
-				_sysExQueues[sysExQueueNr].targetAddress += sysExPos;
+				// Adjust target address to point at the final data byte, or at the
+				// end of the current data in case of an overflow
+				// Note that the address bytes are actually 7 bits
+				byte addressByte1 = (_sysExQueues[sysExQueueNr].targetAddress & 0xFF0000) >> 16;
+				byte addressByte2 = (_sysExQueues[sysExQueueNr].targetAddress & 0x00FF00) >> 8;
+				byte addressByte3 = _sysExQueues[sysExQueueNr].targetAddress & 0x0000FF;
+				addressByte3 += _sysExQueues[sysExQueueNr].dataPos;
+				if (addressByte3 > 0x7F) {
+					addressByte3 -= 0x80;
+					addressByte2++;
+				}
+				if (addressByte2 > 0x7F) {
+					addressByte2 -= 0x80;
+					addressByte1++;
+				}
+				_sysExQueues[sysExQueueNr].targetAddress = addressByte1 << 16 | addressByte2 << 8 | addressByte3;
+
 				// reset queue data buffer
 				_sysExQueues[sysExQueueNr].dataPos = 0;
 			}
@@ -1112,7 +1134,7 @@ MidiDriver_Miles_Midi *MidiDriver_Miles_MIDI_create(MusicType midiType, const Co
 		uint16                    instrumentDataSize;
 
 		if (!fileStream->open(instrumentDataFilename))
-			error("MILES-MDI: could not open instrument file '%s'", instrumentDataFilename.c_str());
+			error("MILES-MIDI: could not open instrument file '%s'", instrumentDataFilename.c_str());
 
 		fileSize = fileStream->size();
 


Commit: 50772c1fbcee92546db56e83fd82b8b3886e40fe
    https://github.com/scummvm/scummvm/commit/50772c1fbcee92546db56e83fd82b8b3886e40fe
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI: Fix XMIDI custom timbre loading

XMIDI files have an optional header defining custom timbres used for each
track. Before playback these timbres have to be sent to the MIDI device. The
old implementation of this functionality had two problems:

- The custom timbres were only sent during loading of the XMIDI data, for the
first track. If another track was played, the custom timbres for that track
would not be loaded.
- The custom timbres were sent after starting playback of the track. This
caused the sending of the timbres to sometimes overlap with the start of the
playback of the track, causing wrong instruments and bad timing.

If fixed these issues by moving the loading of the timbres to a new event
handler which is triggered during the setTrack function, before playback is
started.

This fixes problems with some MT-32 tracks in The 7th Guest, f.e. the second
track that plays on the main menu after starting the game.

Changed paths:
    audio/midiparser.cpp
    audio/midiparser.h
    audio/midiparser_xmidi.cpp


diff --git a/audio/midiparser.cpp b/audio/midiparser.cpp
index 15de9cab5d..0fecc5e048 100644
--- a/audio/midiparser.cpp
+++ b/audio/midiparser.cpp
@@ -372,6 +372,9 @@ bool MidiParser::setTrack(int track) {
 	memset(_activeNotes, 0, sizeof(_activeNotes));
 	if (_disableAutoStartPlayback)
 		_doParse = false;
+
+	onTrackStart(track);
+
 	_activeTrack = track;
 	_position._playPos = _tracks[track];
 	parseNextEvent(_nextEvent);
diff --git a/audio/midiparser.h b/audio/midiparser.h
index 94ef5ad366..0a18c2c3e0 100644
--- a/audio/midiparser.h
+++ b/audio/midiparser.h
@@ -307,6 +307,13 @@ protected:
 	void hangingNote(byte channel, byte note, uint32 ticksLeft, bool recycle = true);
 	void hangAllActiveNotes();
 
+	/**
+	 * Called before starting playback of a track.
+	 * Can be implemented by subclasses if they need to
+	 * perform actions at this point.
+	 */
+	virtual void onTrackStart(uint8 track) { };
+
 	virtual void sendToDriver(uint32 b);
 	void sendToDriver(byte status, byte firstOp, byte secondOp) {
 		sendToDriver(status | ((uint32)firstOp << 8) | ((uint32)secondOp << 16));
diff --git a/audio/midiparser_xmidi.cpp b/audio/midiparser_xmidi.cpp
index 2be4472941..1a6acd5857 100644
--- a/audio/midiparser_xmidi.cpp
+++ b/audio/midiparser_xmidi.cpp
@@ -73,8 +73,6 @@ protected:
 
 	byte  *_tracksTimbreList[120]; ///< Timbre-List for each track.
 	uint32 _tracksTimbreListSize[120]; ///< Size of the Timbre-List for each track.
-	byte  *_activeTrackTimbreList;
-	uint32 _activeTrackTimbreListSize;
 
 protected:
 	uint32 readVLQ2(byte * &data);
@@ -92,6 +90,7 @@ protected:
 		MidiParser::resetTracking();
 		_loopCount = -1;
 	}
+	void onTrackStart(uint8 track) override;
 
 	void sendToDriver(uint32 b) override;
 	void sendMetaEventToDriver(byte type, byte *data, uint16 length) override;
@@ -102,9 +101,7 @@ public:
 			_newTimbreListProc(newTimbreListProc),
 			_newTimbreListDriver(newTimbreListDriver),
 			_source(source),
-			_loopCount(-1),
-			_activeTrackTimbreList(NULL),
-			_activeTrackTimbreListSize(0) {
+			_loopCount(-1) {
 		memset(_loop, 0, sizeof(_loop));
 		memset(_trackBranches, 0, sizeof(_trackBranches));
 		memset(_tracksTimbreList, 0, sizeof(_tracksTimbreList));
@@ -438,6 +435,9 @@ bool MidiParser_XMIDI::loadMusic(byte *data, uint32 size) {
 		int tracksRead = 0;
 		uint32 branchOffsets[128];
 		memset(branchOffsets, 0, sizeof(branchOffsets));
+		memset(_trackBranches, 0, sizeof(_trackBranches));
+		memset(_tracksTimbreList, 0, sizeof(_tracksTimbreList));
+		memset(_tracksTimbreListSize, 0, sizeof(_tracksTimbreListSize));
 		while (tracksRead < _numTracks) {
 			if (!memcmp(pos, "FORM", 4)) {
 				// Skip this plus the 4 bytes after it.
@@ -511,12 +511,9 @@ bool MidiParser_XMIDI::loadMusic(byte *data, uint32 size) {
 		_ppqn = 60;
 		resetTracking();
 		setTempo(500000);
-		setTrack(0);
-		_activeTrackTimbreList = _tracksTimbreList[0];
-		_activeTrackTimbreListSize = _tracksTimbreListSize[0];
 
-		if (_newTimbreListProc)
-			_newTimbreListProc(_newTimbreListDriver, _activeTrackTimbreList, _activeTrackTimbreListSize);
+		// Start playback of the first track.
+		setTrack(0);
 
 		return true;
 	}
@@ -524,6 +521,12 @@ bool MidiParser_XMIDI::loadMusic(byte *data, uint32 size) {
 	return false;
 }
 
+void MidiParser_XMIDI::onTrackStart(uint8 track) {
+	// Load custom timbres
+	if (_newTimbreListProc && _tracksTimbreListSize[track] > 0)
+		_newTimbreListProc(_newTimbreListDriver, _tracksTimbreList[track], _tracksTimbreListSize[track]);
+}
+
 void MidiParser_XMIDI::sendToDriver(uint32 b) {
 	if (_source < 0) {
 		MidiParser::sendToDriver(b);


Commit: b38f1f00accf5939365ab8c3eedcfddb7321e1ff
    https://github.com/scummvm/scummvm/commit/b38f1f00accf5939365ab8c3eedcfddb7321e1ff
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI/GROOVIE: Fix MT-32 XMIDI timbre init delays

XMIDI data for the MT-32 can contain a timbre chunk which contains the custom
timbre numbers used for a track. Before starting playback of the track, the
timbres must be loaded into the MT-32 using SysEx messages. Each SysEx message
needs a delay before sending the next message to give the MT-32 enough time to
process it.

The delays between the messages were generated using the OSystem::delayMillis
function. This throws off the timing of the MidiParser, which causes MIDI
messages to "pile up", in this case at the start of the track. This change adds
a queue to the Miles MIDI driver, which can store the SysEx messages that must
be executed. The MidiParser suspends playback until the driver has sent all
messages in the queue.

This fixes the start of tracks in The 7th Guest, which had incorrect timing.

Because of the new SysEx queue, MT-32 initialization of The 7th Guest takes a
bit longer than it did before. The game plays an animation of a fixed length
and aborts the initialization if it is not done by the end of the animation.
This problem was already fixed for the GM initialization, so I've applied the
same fix for the MT-32.

Changed paths:
    audio/mididrv.h
    audio/midiparser.cpp
    audio/miles.h
    audio/miles_midi.cpp
    engines/groovie/music.cpp
    engines/groovie/music.h
    engines/groovie/script.cpp


diff --git a/audio/mididrv.h b/audio/mididrv.h
index 6c8a2f2d5d..73040ebef5 100644
--- a/audio/mididrv.h
+++ b/audio/mididrv.h
@@ -159,6 +159,14 @@ public:
 	 * ignored.
 	 */
 	virtual void metaEvent(int8 source, byte type, byte *data, uint16 length) { metaEvent(type, data, length); }
+
+	/**
+	 * A driver implementation might need time to prepare playback of
+	 * a track. Use this function to check if the driver is ready to
+	 * receive MIDI events.
+	 */
+	virtual bool isReady() { return true; }
+
 protected:
 
 	/**
diff --git a/audio/midiparser.cpp b/audio/midiparser.cpp
index 0fecc5e048..4e56f5896c 100644
--- a/audio/midiparser.cpp
+++ b/audio/midiparser.cpp
@@ -187,7 +187,7 @@ void MidiParser::onTimer() {
 	// even if the parser does not parse events.
 	_sysExDelay -= (_sysExDelay > _timerRate) ? _timerRate : _sysExDelay;
 
-	if (!_position._playPos || !_driver || !_doParse || _pause)
+	if (!_position._playPos || !_driver || !_doParse || _pause || !_driver->isReady())
 		return;
 
 	_abortParse = false;
diff --git a/audio/miles.h b/audio/miles.h
index 98f33c394a..8b9bab3d4a 100644
--- a/audio/miles.h
+++ b/audio/miles.h
@@ -26,6 +26,7 @@
 #include "audio/mididrv.h"
 #include "common/error.h"
 #include "common/mutex.h"
+#include "common/queue.h"
 #include "common/stream.h"
 
 namespace Audio {
@@ -161,10 +162,12 @@ public:
 	}
 
 	void setTimerCallback(void *timer_param, Common::TimerManager::TimerProc timer_proc) override {
-		if (_driver)
-			_driver->setTimerCallback(timer_param, timer_proc);
+		_timer_param = timer_param;
+		_timer_proc = timer_proc;
 	}
 
+	void onTimer();
+
 	uint32 getBaseTempo() override {
 		if (_driver) {
 			return _driver->getBaseTempo();
@@ -188,26 +191,39 @@ protected:
 	uint16 _outputChannelMask;
 
 	int _baseFreq;
+	uint32 _timerRate;
 
 public:
+	/**
+	 * Processes the timbre chunk specified for a track
+	 * in an XMIDI file. This will load the necessary
+	 * timbres into the MIDI device using SysEx messages.
+	 *
+	 * This function will likely return before all SysEx
+	 * messages have been sent. Use the isReady method to
+	 * check if the driver has finished preparing for
+	 * playback. Playback should not be started before
+	 * this process has finished.
+	 */
 	void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize);
+	bool isReady() override { return _sysExQueue.empty(); }
 
 private:
 	void initMidiDevice();
 
-	void MT32SysEx(const uint32 targetAddress, const byte *dataPtr);
+	void MT32SysEx(const uint32 targetAddress, const byte *dataPtr, bool useSysExQueue = false);
 
 	uint32 calculateSysExTargetAddress(uint32 baseAddress, uint32 index);
 
 	void writeRhythmSetup(byte note, byte customTimbreId);
-	void writePatchTimbre(byte patchId, byte timbreGroup, byte timbreId);
+	void writePatchTimbre(byte patchId, byte timbreGroup, byte timbreId, bool useSysExQueue = false);
 	void writePatchByte(byte patchId, byte index, byte patchValue);
 	void writeToSystemArea(byte index, byte value);
 
 	const MilesMT32InstrumentEntry *searchCustomInstrument(byte patchBank, byte patchId);
 	int16 searchCustomTimbre(byte patchBank, byte patchId);
 
-	void setupPatch(byte patchBank, byte patchId);
+	void setupPatch(byte patchBank, byte patchId, bool useSysExQueue = false);
 	int16 installCustomTimbre(byte patchBank, byte patchId);
 
 	bool isOutputChannelUsed(uint8 outputChannel) { return _outputChannelMask & (1 << outputChannel); }
@@ -388,8 +404,8 @@ private:
 
 	uint32 _noteCounter; // used to figure out, which timbres are outdated
 
-	// SysEx Queues
-	MilesMT32SysExQueueEntry _sysExQueues[MILES_CONTROLLER_SYSEX_QUEUE_COUNT];
+	// Queues for Miles SysEx controllers
+	MilesMT32SysExQueueEntry _milesSysExQueues[MILES_CONTROLLER_SYSEX_QUEUE_COUNT];
 
 	// MIDI sources sending messages to this driver.
 	MidiSource _sources[MILES_MAXIMUM_SOURCES];
@@ -410,6 +426,56 @@ private:
 	uint8 _maximumActiveNotes;
 	// Tracks the notes being played by the MIDI device.
 	ActiveNote *_activeNotes;
+
+	/**
+	 * Stores data which is to be transmitted as a SysEx message
+	 * to a MIDI device. Neither data nor length should include
+	 * the SysEx start and stop bytes.
+	 */
+	struct SysExData {
+		byte data[270];
+		uint16 length;
+		SysExData() : length(0) {
+			memset(data, 0, sizeof(data));
+		}
+	};
+
+	// The number of microseconds to wait before sending the
+	// next SysEx message.
+	uint32 _sysExDelay;
+
+	/**
+	 * Queue of SysEx messages that must be sent to the
+	 * MIDI device. Used by processXMIDITimbreChunk to
+	 * send SysEx messages before starting playback of
+	 * a track.
+	 *
+	 * Sending other MIDI messages to the driver should
+	 * be suspended until all SysEx messages in the
+	 * queue have been sent to the MIDI device. Use the
+	 * isReady function to check if the driver is ready
+	 * to receive other messages.
+	 */
+	Common::Queue<SysExData> _sysExQueue;
+
+	// Mutex for write access to the SysEx queue.
+	Common::Mutex _sysExQueueMutex;
+
+	// External timer callback
+	void *_timer_param;
+	Common::TimerManager::TimerProc _timer_proc;
+
+public:
+	// Callback hooked up to the driver wrapped by the
+	// Miles MIDI driver object. Executes onTimer and
+	// the external callback set by the
+	// setTimerCallback function.
+	static void timerCallback(void *data) {
+		MidiDriver_Miles_Midi *driver = (MidiDriver_Miles_Midi *) data;
+		driver->onTimer();
+		if (driver->_timer_proc && driver->_timer_param)
+			driver->_timer_proc(driver->_timer_param);
+	}
 };
 
 extern MidiDriver *MidiDriver_Miles_AdLib_create(const Common::String &filenameAdLib, const Common::String &filenameOPL3, Common::SeekableReadStream *streamAdLib = nullptr, Common::SeekableReadStream *streamOPL3 = nullptr);
diff --git a/audio/miles_midi.cpp b/audio/miles_midi.cpp
index 948af1f576..6cfc3541ef 100644
--- a/audio/miles_midi.cpp
+++ b/audio/miles_midi.cpp
@@ -61,7 +61,11 @@ MidiDriver_Miles_Midi::MidiDriver_Miles_Midi(MusicType midiType, MilesMT32Instru
 		_enableGS(false),
 		_outputChannelMask(65535), // Channels 1-16
 		_baseFreq(250),
-		_noteCounter(0) {
+		_timerRate(0),
+		_noteCounter(0),
+		_sysExDelay(0), 
+		_timer_param(0),
+		_timer_proc(0) {
 	switch (midiType) {
 	case MT_MT32:
 		_midiType = MT_MT32;
@@ -140,6 +144,9 @@ int MidiDriver_Miles_Midi::open(MidiDriver *driver, bool nativeMT32) {
 	if (ret != MidiDriver::MERR_ALREADY_OPEN && ret != 0)
 		return ret;
 
+	_timerRate = _driver->getBaseTempo();
+	_driver->setTimerCallback(this, timerCallback);
+
 	initMidiDevice();
 
 	return 0;
@@ -233,7 +240,7 @@ uint16 MidiDriver_Miles_Midi::sysExNoDelay(const byte *msg, uint16 length) {
 	return delay;
 }
 
-void MidiDriver_Miles_Midi::MT32SysEx(const uint32 targetAddress, const byte *dataPtr) {
+void MidiDriver_Miles_Midi::MT32SysEx(const uint32 targetAddress, const byte *dataPtr, bool useSysExQueue) {
 	if (!_nativeMT32)
 		// MT-32 SysExes have no effect on GM devices.
 		return;
@@ -277,7 +284,17 @@ void MidiDriver_Miles_Midi::MT32SysEx(const uint32 targetAddress, const byte *da
 	assert(sysExPos < sizeof(sysExMessage));
 	sysExMessage[sysExPos++] = sysExChecksum & 0x7f;
 
-	sysEx(sysExMessage, sysExPos);
+	if (useSysExQueue) {
+		SysExData sysEx;
+		memcpy(sysEx.data, sysExMessage, sysExPos);
+		sysEx.length = sysExPos;
+
+		_sysExQueueMutex.lock();
+		_sysExQueue.push(sysEx);
+		_sysExQueueMutex.unlock();
+	} else {
+		sysEx(sysExMessage, sysExPos);
+	}
 }
 
 void MidiDriver_Miles_Midi::metaEvent(int8 source, byte type, byte *data, uint16 length) {
@@ -485,28 +502,28 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 		}
 		assert(sysExQueueNr < MILES_CONTROLLER_SYSEX_QUEUE_COUNT);
 
-		byte sysExPos = _sysExQueues[sysExQueueNr].dataPos;
+		byte sysExPos = _milesSysExQueues[sysExQueueNr].dataPos;
 		bool sysExSend = false;
 
 		switch(controllerNumber) {
 		case MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS1:
-			_sysExQueues[sysExQueueNr].targetAddress &= 0x00FFFF;
-			_sysExQueues[sysExQueueNr].targetAddress |= (controllerValue << 16);
+			_milesSysExQueues[sysExQueueNr].targetAddress &= 0x00FFFF;
+			_milesSysExQueues[sysExQueueNr].targetAddress |= (controllerValue << 16);
 			break;
 		case MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS2:
-			_sysExQueues[sysExQueueNr].targetAddress &= 0xFF00FF;
-			_sysExQueues[sysExQueueNr].targetAddress |= (controllerValue << 8);
+			_milesSysExQueues[sysExQueueNr].targetAddress &= 0xFF00FF;
+			_milesSysExQueues[sysExQueueNr].targetAddress |= (controllerValue << 8);
 			break;
 		case MILES_CONTROLLER_SYSEX_COMMAND_ADDRESS3:
-			_sysExQueues[sysExQueueNr].targetAddress &= 0xFFFF00;
-			_sysExQueues[sysExQueueNr].targetAddress |= controllerValue;
+			_milesSysExQueues[sysExQueueNr].targetAddress &= 0xFFFF00;
+			_milesSysExQueues[sysExQueueNr].targetAddress |= controllerValue;
 			break;
 		case MILES_CONTROLLER_SYSEX_COMMAND_DATA:
 			if (sysExPos < MILES_CONTROLLER_SYSEX_QUEUE_SIZE) {
 				// Space left? put current byte into queue
-				_sysExQueues[sysExQueueNr].data[sysExPos] = controllerValue;
+				_milesSysExQueues[sysExQueueNr].data[sysExPos] = controllerValue;
 				sysExPos++;
-				_sysExQueues[sysExQueueNr].dataPos = sysExPos;
+				_milesSysExQueues[sysExQueueNr].dataPos = sysExPos;
 				if (sysExPos >= MILES_CONTROLLER_SYSEX_QUEUE_SIZE) {
 					// overflow? -> send it now
 					sysExSend = true;
@@ -516,7 +533,7 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 		case MILES_CONTROLLER_SYSEX_COMMAND_FINAL_DATA:
 			if (sysExPos < MILES_CONTROLLER_SYSEX_QUEUE_SIZE) {
 				// Space left? put current byte into queue
-				_sysExQueues[sysExQueueNr].data[sysExPos] = controllerValue;
+				_milesSysExQueues[sysExQueueNr].data[sysExPos] = controllerValue;
 				sysExPos++;
 				// Do not increment dataPos. Subsequent Final Data commands will
 				// re-send the last address byte with the new controller value.
@@ -530,18 +547,18 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 		if (sysExSend) {
 			if (sysExPos > 0) {
 				// data actually available? -> send it
-				_sysExQueues[sysExQueueNr].data[sysExPos] = MILES_MT32_SYSEX_TERMINATOR; // put terminator
+				_milesSysExQueues[sysExQueueNr].data[sysExPos] = MILES_MT32_SYSEX_TERMINATOR; // put terminator
 
 				// Execute SysEx
-				MT32SysEx(_sysExQueues[sysExQueueNr].targetAddress, _sysExQueues[sysExQueueNr].data);
+				MT32SysEx(_milesSysExQueues[sysExQueueNr].targetAddress, _milesSysExQueues[sysExQueueNr].data);
 
 				// Adjust target address to point at the final data byte, or at the
 				// end of the current data in case of an overflow
 				// Note that the address bytes are actually 7 bits
-				byte addressByte1 = (_sysExQueues[sysExQueueNr].targetAddress & 0xFF0000) >> 16;
-				byte addressByte2 = (_sysExQueues[sysExQueueNr].targetAddress & 0x00FF00) >> 8;
-				byte addressByte3 = _sysExQueues[sysExQueueNr].targetAddress & 0x0000FF;
-				addressByte3 += _sysExQueues[sysExQueueNr].dataPos;
+				byte addressByte1 = (_milesSysExQueues[sysExQueueNr].targetAddress & 0xFF0000) >> 16;
+				byte addressByte2 = (_milesSysExQueues[sysExQueueNr].targetAddress & 0x00FF00) >> 8;
+				byte addressByte3 = _milesSysExQueues[sysExQueueNr].targetAddress & 0x0000FF;
+				addressByte3 += _milesSysExQueues[sysExQueueNr].dataPos;
 				if (addressByte3 > 0x7F) {
 					addressByte3 -= 0x80;
 					addressByte2++;
@@ -550,10 +567,10 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 					addressByte2 -= 0x80;
 					addressByte1++;
 				}
-				_sysExQueues[sysExQueueNr].targetAddress = addressByte1 << 16 | addressByte2 << 8 | addressByte3;
+				_milesSysExQueues[sysExQueueNr].targetAddress = addressByte1 << 16 | addressByte2 << 8 | addressByte3;
 
 				// reset queue data buffer
-				_sysExQueues[sysExQueueNr].dataPos = 0;
+				_milesSysExQueues[sysExQueueNr].dataPos = 0;
 			}
 		}
 		return;
@@ -868,7 +885,7 @@ const MilesMT32InstrumentEntry *MidiDriver_Miles_Midi::searchCustomInstrument(by
 	return NULL;
 }
 
-void MidiDriver_Miles_Midi::setupPatch(byte patchBank, byte patchId) {
+void MidiDriver_Miles_Midi::setupPatch(byte patchBank, byte patchId, bool useSysExQueue) {
 	_patchesBank[patchId] = patchBank;
 
 	if (patchBank) {
@@ -876,7 +893,7 @@ void MidiDriver_Miles_Midi::setupPatch(byte patchBank, byte patchId) {
 		int16 customTimbreId = searchCustomTimbre(patchBank, patchId);
 		if (customTimbreId >= 0) {
 			// now available? -> use this timbre
-			writePatchTimbre(patchId, 2, customTimbreId); // Group MEMORY
+			writePatchTimbre(patchId, 2, customTimbreId, useSysExQueue); // Group MEMORY
 			return;
 		}
 	}
@@ -884,9 +901,9 @@ void MidiDriver_Miles_Midi::setupPatch(byte patchBank, byte patchId) {
 	// for built-in bank (or timbres, that are not available) use default MT32 timbres
 	byte timbreId = patchId & 0x3F;
 	if (!(patchId & 0x40)) {
-		writePatchTimbre(patchId, 0, timbreId); // Group A
+		writePatchTimbre(patchId, 0, timbreId, useSysExQueue); // Group A
 	} else {
-		writePatchTimbre(patchId, 1, timbreId); // Group B
+		writePatchTimbre(patchId, 1, timbreId, useSysExQueue); // Group B
 	}
 }
 
@@ -1023,14 +1040,14 @@ int16 MidiDriver_Miles_Midi::installCustomTimbre(byte patchBank, byte patchId) {
 #endif
 
 	// upload common parameter data
-	MT32SysEx(targetAddressCommon, instrumentPtr->commonParameter);
+	MT32SysEx(targetAddressCommon, instrumentPtr->commonParameter, true);
 	// upload partial parameter data
-	MT32SysEx(targetAddressPartial1, instrumentPtr->partialParameters[0]);
-	MT32SysEx(targetAddressPartial2, instrumentPtr->partialParameters[1]);
-	MT32SysEx(targetAddressPartial3, instrumentPtr->partialParameters[2]);
-	MT32SysEx(targetAddressPartial4, instrumentPtr->partialParameters[3]);
+	MT32SysEx(targetAddressPartial1, instrumentPtr->partialParameters[0], true);
+	MT32SysEx(targetAddressPartial2, instrumentPtr->partialParameters[1], true);
+	MT32SysEx(targetAddressPartial3, instrumentPtr->partialParameters[2], true);
+	MT32SysEx(targetAddressPartial4, instrumentPtr->partialParameters[3], true);
 
-	setupPatch(patchBank, patchId);
+	setupPatch(patchBank, patchId, true);
 
 	return customTimbreId;
 }
@@ -1072,7 +1089,7 @@ void MidiDriver_Miles_Midi::writeRhythmSetup(byte note, byte customTimbreId) {
 	MT32SysEx(targetAddress, sysExData);
 }
 
-void MidiDriver_Miles_Midi::writePatchTimbre(byte patchId, byte timbreGroup, byte timbreId) {
+void MidiDriver_Miles_Midi::writePatchTimbre(byte patchId, byte timbreGroup, byte timbreId, bool useSysExQueue) {
 	byte   sysExData[3];
 	uint32 targetAddress = 0;
 
@@ -1083,7 +1100,7 @@ void MidiDriver_Miles_Midi::writePatchTimbre(byte patchId, byte timbreGroup, byt
 	sysExData[1] = timbreId;    // timbre number (0-63)
 	sysExData[2] = MILES_MT32_SYSEX_TERMINATOR; // terminator
 
-	MT32SysEx(targetAddress, sysExData);
+	MT32SysEx(targetAddress, sysExData, useSysExQueue);
 }
 
 void MidiDriver_Miles_Midi::writePatchByte(byte patchId, byte index, byte patchValue) {
@@ -1291,4 +1308,16 @@ void MidiDriver_Miles_Midi::setSourceVolume(uint8 source, uint16 volume) {
 	}
 }
 
+void MidiDriver_Miles_Midi::onTimer() {
+	Common::StackLock lock(_sysExQueueMutex);
+
+	_sysExDelay -= (_sysExDelay > _timerRate) ? _timerRate : _sysExDelay;
+
+	if (!_sysExQueue.empty() && _sysExDelay == 0) {
+		// Ready to send next SysEx message to the MIDI device
+		SysExData sysEx = _sysExQueue.pop();
+		_sysExDelay = sysExNoDelay(sysEx.data, sysEx.length) * 1000;
+	}
+}
+
 } // End of namespace Audio
diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index a9c83e7d3e..771fad5769 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -296,6 +296,10 @@ void MusicPlayerMidi::sysEx(const byte *msg, uint16 length) {
 		_driver->sysEx(msg, length);
 }
 
+uint16 MusicPlayerMidi::sysExNoDelay(const byte *msg, uint16 length) {
+	return _driver ? _driver->sysExNoDelay(msg, length) : 0;
+}
+
 void MusicPlayerMidi::metaEvent(byte type, byte *data, uint16 length) {
 	switch (type) {
 	case 0x2F:
@@ -561,6 +565,10 @@ void MusicPlayerXMI::metaEvent(int8 source, byte type, byte *data, uint16 length
 	}
 }
 
+bool MusicPlayerXMI::isReady() {
+	return _driver ? _driver->isReady() : false;
+}
+
 void MusicPlayerXMI::updateVolume() {
 	if (_milesMidiDriver) {
 		uint16 val = (_userVolume * _gameVolume) / 100;
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index 04d7d98ac2..f79dd02d9b 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -101,6 +101,7 @@ public:
 	// MidiDriver_BASE interface
 	void send(uint32 b) override;
 	void sysEx(const byte* msg, uint16 length) override;
+	uint16 sysExNoDelay(const byte *msg, uint16 length) override;
 	void metaEvent(byte type, byte *data, uint16 length) override;
 
 private:
@@ -129,6 +130,7 @@ public:
 	void send(uint32 b) override;
 	void send(int8 source, uint32 b) override;
 	void metaEvent(int8 source, byte type, byte *data, uint16 length) override;
+	bool isReady() override;
 
 protected:
 	void updateVolume() override;
diff --git a/engines/groovie/script.cpp b/engines/groovie/script.cpp
index 1252e97448..354078ce1a 100644
--- a/engines/groovie/script.cpp
+++ b/engines/groovie/script.cpp
@@ -577,16 +577,17 @@ void Script::o_videofromref() {			// 0x09
 		debugCN(1, kDebugScript, "\n");
 	}
 
-	// Determine if the GM initialization video is being played
-	bool gmInitVideo = (_version == kGroovieT7G && fileref == 0x2460);
+	// Determine if the MT-32 or GM initialization video is being played
+	bool gmInitVideo = _version == kGroovieT7G && fileref == 0x2460;
+	bool mt32InitVideo = _version == kGroovieT7G && fileref == 0x2461;
 	// Play the video
-	// If the GM init video is being played, loop it until the "audio"
+	// If a MIDI init video is being played, loop it until the "audio"
 	// (init commands) has finished playing
-	if (!playvideofromref(fileref, gmInitVideo)) {
+	if (!playvideofromref(fileref, gmInitVideo || mt32InitVideo)) {
 		// Move _currentInstruction back
 		_currentInstruction -= 3;
 	} else if (gmInitVideo) {
-		// The script plays the GM init video twice to give the "audio"
+		// The script plays the MIDI init video twice to give the "audio"
 		// enough time to play. It has just looped until the audio finished,
 		// so the second play is no longer necessary.
 		// Skip the next instruction.


Commit: 848d8315a68876292ea9c273ec92cd518a5ba491
    https://github.com/scummvm/scummvm/commit/848d8315a68876292ea9c273ec92cd518a5ba491
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI/GROOVIE: Refactor XMIDI timbre chunk loading

XMIDI timbre chunk processing was implemented using a callback. This
reimplements this using an interface for the necessary driver method, which is
a somewhat cleaner solution.

Changed paths:
    audio/midiparser.h
    audio/midiparser_xmidi.cpp
    audio/miles.h
    audio/miles_midi.cpp
    engines/groovie/music.cpp
    engines/groovie/music.h
    engines/kyra/sound/sound_pc_midi.cpp


diff --git a/audio/midiparser.h b/audio/midiparser.h
index 0a18c2c3e0..2d675ffe75 100644
--- a/audio/midiparser.h
+++ b/audio/midiparser.h
@@ -293,7 +293,7 @@ protected:
 	                        ///< simulated events in certain formats.
 	bool   _abortParse;    ///< If a jump or other operation interrupts parsing, flag to abort.
 	bool   _jumpingToTick; ///< True if currently inside jumpToTick
-	bool   _doParse;       ///< True if the parser should be parsing; false if it should be active
+	bool   _doParse;       ///< True if the parser should be parsing; false if it should not be active
 	bool   _pause;		   ///< True if the parser has paused parsing
 
 protected:
@@ -402,7 +402,6 @@ public:
 
 public:
 	typedef void (*XMidiCallbackProc)(byte eventData, void *refCon);
-	typedef void (*XMidiNewTimbreListProc)(MidiDriver_BASE *driver, const byte *timbreListPtr, uint32 timbreListSize);
 
 	MidiParser();
 	virtual ~MidiParser() { allNotesOff(); }
@@ -411,7 +410,7 @@ public:
 	virtual void unloadMusic();
 	virtual void property(int prop, int value);
 
-	void setMidiDriver(MidiDriver_BASE *driver) { _driver = driver; }
+	virtual void setMidiDriver(MidiDriver_BASE *driver) { _driver = driver; }
 	void setTimerRate(uint32 rate) { _timerRate = rate; }
 	void setTempo(uint32 tempo);
 	void onTimer();
@@ -470,7 +469,7 @@ public:
 	static void defaultXMidiCallback(byte eventData, void *refCon);
 
 	static MidiParser *createParser_SMF();
-	static MidiParser *createParser_XMIDI(XMidiCallbackProc proc = defaultXMidiCallback, void *refCon = 0, XMidiNewTimbreListProc newTimbreListProc = NULL, MidiDriver_BASE *newTimbreListDriver = NULL, int source = -1);
+	static MidiParser *createParser_XMIDI(XMidiCallbackProc proc = defaultXMidiCallback, void *refCon = 0, int source = -1);
 	static MidiParser *createParser_QT();
 	static void timerCallback(void *data) { ((MidiParser *) data)->onTimer(); }
 };
diff --git a/audio/midiparser_xmidi.cpp b/audio/midiparser_xmidi.cpp
index 1a6acd5857..f916a703ec 100644
--- a/audio/midiparser_xmidi.cpp
+++ b/audio/midiparser_xmidi.cpp
@@ -22,6 +22,7 @@
 
 #include "audio/midiparser.h"
 #include "audio/mididrv.h"
+#include "audio/miles.h"
 #include "common/textconsole.h"
 #include "common/util.h"
 
@@ -60,16 +61,12 @@ protected:
 	XMidiCallbackProc _callbackProc;
 	void *_callbackData;
 
-	// TODO:
-	// This should possibly get cleaned up at some point, but it's very tricks.
 	// We need to support XMIDI TIMB for 7th guest, which uses
 	// Miles Audio drivers. The MT32 driver needs to get the TIMB chunk, so that it
 	// can install all required timbres before the song starts playing.
-	// But we can't easily implement this directly like for example creating
-	// a special Miles Audio class for usage in this XMIDI-class, because other engines use this
-	// XMIDI-parser but w/o using Miles Audio drivers.
-	XMidiNewTimbreListProc _newTimbreListProc;
-	MidiDriver_BASE       *_newTimbreListDriver;
+	// This contains a pointer to _driver if it supports the required
+	// interface; otherwise it is null.
+	Audio::MidiDriver_Miles_Xmidi_Timbres *_newTimbreListDriver;
 
 	byte  *_tracksTimbreList[120]; ///< Timbre-List for each track.
 	uint32 _tracksTimbreListSize[120]; ///< Size of the Timbre-List for each track.
@@ -95,11 +92,10 @@ protected:
 	void sendToDriver(uint32 b) override;
 	void sendMetaEventToDriver(byte type, byte *data, uint16 length) override;
 public:
-	MidiParser_XMIDI(XMidiCallbackProc proc, void *data, XMidiNewTimbreListProc newTimbreListProc, MidiDriver_BASE *newTimbreListDriver, int8 source = -1) :
+	MidiParser_XMIDI(XMidiCallbackProc proc, void *data, int8 source = -1) :
 			_callbackProc(proc),
 			_callbackData(data),
-			_newTimbreListProc(newTimbreListProc),
-			_newTimbreListDriver(newTimbreListDriver),
+			_newTimbreListDriver(0),
 			_source(source),
 			_loopCount(-1) {
 		memset(_loop, 0, sizeof(_loop));
@@ -109,6 +105,7 @@ public:
 	}
 	~MidiParser_XMIDI() { }
 
+	void setMidiDriver(MidiDriver_BASE *driver) override;
 	bool loadMusic(byte *data, uint32 size) override;
 	bool hasJumpIndex(uint8 index) override;
 	bool jumpToIndex(uint8 index, bool stopNotes) override;
@@ -331,6 +328,11 @@ void MidiParser_XMIDI::parseNextEvent(EventInfo &info) {
 	}
 }
 
+void MidiParser_XMIDI::setMidiDriver(MidiDriver_BASE *driver) {
+	MidiParser::setMidiDriver(driver);
+	_newTimbreListDriver = dynamic_cast<Audio::MidiDriver_Miles_Xmidi_Timbres *>(driver);
+}
+
 bool MidiParser_XMIDI::loadMusic(byte *data, uint32 size) {
 	uint32 i = 0;
 	byte *start;
@@ -523,8 +525,8 @@ bool MidiParser_XMIDI::loadMusic(byte *data, uint32 size) {
 
 void MidiParser_XMIDI::onTrackStart(uint8 track) {
 	// Load custom timbres
-	if (_newTimbreListProc && _tracksTimbreListSize[track] > 0)
-		_newTimbreListProc(_newTimbreListDriver, _tracksTimbreList[track], _tracksTimbreListSize[track]);
+	if (_newTimbreListDriver && _tracksTimbreListSize[track] > 0)
+		_newTimbreListDriver->processXMIDITimbreChunk(_tracksTimbreList[track], _tracksTimbreListSize[track]);
 }
 
 void MidiParser_XMIDI::sendToDriver(uint32 b) {
@@ -547,6 +549,6 @@ void MidiParser::defaultXMidiCallback(byte eventData, void *data) {
 	warning("MidiParser: defaultXMidiCallback(%d)", eventData);
 }
 
-MidiParser *MidiParser::createParser_XMIDI(XMidiCallbackProc proc, void *data, XMidiNewTimbreListProc newTimbreListProc, MidiDriver_BASE *newTimbreListDriver, int source) {
-	return new MidiParser_XMIDI(proc, data, newTimbreListProc, newTimbreListDriver, source);
+MidiParser *MidiParser::createParser_XMIDI(XMidiCallbackProc proc, void *data, int source) {
+	return new MidiParser_XMIDI(proc, data, source);
 }
diff --git a/audio/miles.h b/audio/miles.h
index 8b9bab3d4a..e1db3fecf7 100644
--- a/audio/miles.h
+++ b/audio/miles.h
@@ -111,7 +111,29 @@ struct MilesMT32InstrumentEntry {
 	byte partialParameters[MILES_MT32_PATCHDATA_PARTIALPARAMETERS_COUNT][MILES_MT32_PATCHDATA_PARTIALPARAMETER_SIZE + 1];
 };
 
-class MidiDriver_Miles_Midi : public MidiDriver {
+/**
+ * Abstract class containing the interface for loading
+ * the XMIDI timbres specified in the timbre chunks of
+ * an XMIDI file.
+ */
+class MidiDriver_Miles_Xmidi_Timbres {
+public:
+	/**
+	 * Processes the timbre chunk specified for a track
+	 * in an XMIDI file. This will load the necessary
+	 * timbres into the MIDI device using SysEx messages.
+	 *
+	 * This function will likely return before all SysEx
+	 * messages have been sent. Use the isReady method to
+	 * check if the driver has finished preparing for
+	 * playback. Playback should not be started before
+	 * this process has finished.
+	 */
+	virtual void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize) = 0;
+	virtual ~MidiDriver_Miles_Xmidi_Timbres() { }
+};
+
+class MidiDriver_Miles_Midi : public MidiDriver, public MidiDriver_Miles_Xmidi_Timbres {
 public:
 	MidiDriver_Miles_Midi(MusicType midiType, MilesMT32InstrumentEntry *instrumentTablePtr, uint16 instrumentTableCount);
 	virtual ~MidiDriver_Miles_Midi();
@@ -194,18 +216,7 @@ protected:
 	uint32 _timerRate;
 
 public:
-	/**
-	 * Processes the timbre chunk specified for a track
-	 * in an XMIDI file. This will load the necessary
-	 * timbres into the MIDI device using SysEx messages.
-	 *
-	 * This function will likely return before all SysEx
-	 * messages have been sent. Use the isReady method to
-	 * check if the driver has finished preparing for
-	 * playback. Playback should not be started before
-	 * this process has finished.
-	 */
-	void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize);
+	void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize) override;
 	bool isReady() override { return _sysExQueue.empty(); }
 
 private:
@@ -484,8 +495,6 @@ extern MidiDriver_Miles_Midi *MidiDriver_Miles_MT32_create(const Common::String
 
 extern MidiDriver_Miles_Midi *MidiDriver_Miles_MIDI_create(MusicType midiType, const Common::String &instrumentDataFilename);
 
-extern void MidiDriver_Miles_MT32_processXMIDITimbreChunk(MidiDriver_BASE *driver, const byte *timbreListPtr, uint32 timbreListSize);
-
 } // End of namespace Audio
 
 #endif // AUDIO_MILES_MIDIDRIVER_H
diff --git a/audio/miles_midi.cpp b/audio/miles_midi.cpp
index 6cfc3541ef..6abbedded6 100644
--- a/audio/miles_midi.cpp
+++ b/audio/miles_midi.cpp
@@ -449,7 +449,7 @@ void MidiDriver_Miles_Midi::controlChange(byte outputChannel, byte controllerNum
 	}
 
 	// XMIDI MT-32 specific controllers
-	if (_nativeMT32) {
+	if (_midiType == MT_MT32 && _nativeMT32) {
 		switch (controllerNumber) {
 		case MILES_CONTROLLER_PATCH_REVERB:
 			writePatchByte(controlData.program, 6, controllerValue);
@@ -908,6 +908,11 @@ void MidiDriver_Miles_Midi::setupPatch(byte patchBank, byte patchId, bool useSys
 }
 
 void MidiDriver_Miles_Midi::processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize) {
+	if (_midiType != MT_MT32)
+		// Some GM files contain timbre chunks, but custom patches cannot
+		// be loaded on a GM device.
+		return;
+
 	uint16 timbreCount = 0;
 	uint32 expectedSize = 0;
 	const byte *timbreListSeeker = timbreListPtr;
@@ -1235,14 +1240,6 @@ MidiDriver_Miles_Midi *MidiDriver_Miles_MIDI_create(MusicType midiType, const Co
 	return new MidiDriver_Miles_Midi(midiType, instrumentTablePtr, instrumentTableCount);
 }
 
-void MidiDriver_Miles_MT32_processXMIDITimbreChunk(MidiDriver_BASE *driver, const byte *timbreListPtr, uint32 timbreListSize) {
-	MidiDriver_Miles_Midi *driverMT32 = dynamic_cast<MidiDriver_Miles_Midi *>(driver);
-
-	if (driverMT32) {
-		driverMT32->processXMIDITimbreChunk(timbreListPtr, timbreListSize);
-	}
-}
-
 void MidiDriver_Miles_Midi::deinitSource(uint8 source) {
 	assert(source < MILES_MAXIMUM_SOURCES);
 
diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index 771fad5769..290cd0ad4f 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -402,7 +402,6 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 	// new Miles Audio support, to disable set milesAudioEnabled to false
 	_milesAudioMode = false;
 	bool milesAudioEnabled = true;
-	MidiParser::XMidiNewTimbreListProc newTimbreListProc = NULL;
 
 	_musicType = 0;
 
@@ -418,7 +417,6 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 			break;
 		case MT_MT32:
 			_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, gtlName + ".MT");
-			newTimbreListProc = Audio::MidiDriver_Miles_MT32_processXMIDITimbreChunk;
 			break;
 		case MT_GM:
 			_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, "");
@@ -441,7 +439,7 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 	assert(_driver);
 
 	// Create the parser
-	_midiParser = MidiParser::createParser_XMIDI(NULL, NULL, newTimbreListProc, _milesMidiDriver, 0);
+	_midiParser = MidiParser::createParser_XMIDI(NULL, NULL, 0);
 
 	int result = _driver->open();
 	if (result > 0 && result != MidiDriver::MERR_ALREADY_OPEN)
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index f79dd02d9b..1cdc31132b 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -122,7 +122,7 @@ protected:
 	bool loadParser(Common::SeekableReadStream *stream, bool loop);
 };
 
-class MusicPlayerXMI : public MusicPlayerMidi {
+class MusicPlayerXMI : public MusicPlayerMidi, public Audio::MidiDriver_Miles_Xmidi_Timbres {
 public:
 	MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName);
 	~MusicPlayerXMI() override;
@@ -130,6 +130,10 @@ public:
 	void send(uint32 b) override;
 	void send(int8 source, uint32 b) override;
 	void metaEvent(int8 source, byte type, byte *data, uint16 length) override;
+	void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize) override {
+		if (_milesMidiDriver)
+			_milesMidiDriver->processXMIDITimbreChunk(timbreListPtr, timbreListSize);
+	};
 	bool isReady() override;
 
 protected:
diff --git a/engines/kyra/sound/sound_pc_midi.cpp b/engines/kyra/sound/sound_pc_midi.cpp
index c6d4f2cc90..dd125ef154 100644
--- a/engines/kyra/sound/sound_pc_midi.cpp
+++ b/engines/kyra/sound/sound_pc_midi.cpp
@@ -40,12 +40,12 @@ SoundMidiPC::SoundMidiPC(KyraEngine_v1 *vm, Audio::Mixer *mixer, MidiDriver *dri
 	_currentResourceSet = 0;
 	memset(&_resInfo, 0, sizeof(_resInfo));
 
-	_music = MidiParser::createParser_XMIDI(MidiParser::defaultXMidiCallback, NULL, NULL, NULL, 0);
+	_music = MidiParser::createParser_XMIDI(MidiParser::defaultXMidiCallback, NULL, 0);
 	assert(_music);
 	_music->property(MidiParser::mpDisableAllNotesOffMidiEvents, true);
 	_music->property(MidiParser::mpDisableAutoStartPlayback, true);
 	for (int i = 0; i < 3; ++i) {
-		_sfx[i] = MidiParser::createParser_XMIDI(MidiParser::defaultXMidiCallback, NULL, NULL, NULL, i + 1);
+		_sfx[i] = MidiParser::createParser_XMIDI(MidiParser::defaultXMidiCallback, NULL, i + 1);
 		assert(_sfx[i]);
 		_sfx[i]->property(MidiParser::mpDisableAllNotesOffMidiEvents, true);
 		_sfx[i]->property(MidiParser::mpDisableAutoStartPlayback, true);


Commit: 0856c04bfaea1abbf6acf41591786b890f2bc850
    https://github.com/scummvm/scummvm/commit/0856c04bfaea1abbf6acf41591786b890f2bc850
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
GROOVIE: Remove dead music code

The Groovie engine implemented some functionality which was later reimplemented
in the generic Miles drivers. These are now used by the engine, but the old
code is still present. I've removed this to clean up the audio code.

Changed paths:
    engines/groovie/music.cpp
    engines/groovie/music.h


diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index 290cd0ad4f..137c0ea3d8 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -399,42 +399,30 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 		musicType = MT_MT32;
 	_driver = NULL;
 
-	// new Miles Audio support, to disable set milesAudioEnabled to false
-	_milesAudioMode = false;
-	bool milesAudioEnabled = true;
-
 	_musicType = 0;
 
-	if (milesAudioEnabled) {
-		// 7th Guest uses FAT.AD/FAT.OPL/FAT.MT
-		// 11th Hour uses SAMPLE.AD/SAMPLE.OPL/SAMPLE.MT
-		switch (musicType) {
-		case MT_ADLIB:
-			// TODO Would be nice if the Miles AdLib and MIDI drivers shared
-			// a common interface, then we can use only _milesMidiDriver in
-			// this class.
-			_driver = Audio::MidiDriver_Miles_AdLib_create(gtlName + ".AD", gtlName + ".OPL");
-			break;
-		case MT_MT32:
-			_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, gtlName + ".MT");
-			break;
-		case MT_GM:
-			_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, "");
-			break;
-		default:
-			break;
-		}
-		_musicType = musicType;
-	}
-
-	if (_driver) {
-		_milesAudioMode = true;
-	}
-
-	if (!_driver) {
-		// No driver yet? create a generic one
+	// 7th Guest uses FAT.AD/FAT.OPL/FAT.MT
+	// 11th Hour uses SAMPLE.AD/SAMPLE.OPL/SAMPLE.MT
+	switch (musicType) {
+	case MT_ADLIB:
+		// TODO Would be nice if the Miles AdLib and MIDI drivers shared
+		// a common interface, then we can use only _milesMidiDriver in
+		// this class.
+		_driver = Audio::MidiDriver_Miles_AdLib_create(gtlName + ".AD", gtlName + ".OPL");
+		break;
+	case MT_MT32:
+		_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, gtlName + ".MT");
+		break;
+	case MT_GM:
+		_driver = _milesMidiDriver = Audio::MidiDriver_Miles_MIDI_create(musicType, "");
+		break;
+	case MT_NULL:
 		_driver = MidiDriver::createMidi(dev);
+		break;
+	default:
+		break;
 	}
+	_musicType = musicType;
 
 	assert(_driver);
 
@@ -450,99 +438,6 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 
 	// Set the timer rate
 	_midiParser->setTimerRate(_driver->getBaseTempo());
-
-	// Initialize the channel banks
-	for (int i = 0; i < 0x10; i++) {
-		_chanBanks[i] = 0;
-	}
-
-	if (_milesAudioMode)
-		return;
-
-	// Load the Global Timbre Library
-	if (MidiDriver::getMusicType(dev) == MT_ADLIB) {
-		// MIDI through AdLib
-		_musicType = MT_ADLIB;
-		loadTimbres(gtlName + ".ad");
-
-		// Setup the percussion channel
-		for (uint i = 0; i < _timbres.size(); i++) {
-			if (_timbres[i].bank == 0x7F)
-				setTimbreAD(9, _timbres[i]);
-		}
-	} else if ((MidiDriver::getMusicType(dev) == MT_MT32) || ConfMan.getBool("native_mt32")) {
-		_driver->sendMT32Reset();
-
-		// MT-32
-		_musicType = MT_MT32;
-		loadTimbres(gtlName + ".mt");
-	} else {
-		_driver->sendGMReset();
-
-		// GM
-		_musicType = 0;
-	}
-}
-
-MusicPlayerXMI::~MusicPlayerXMI() {
-	//~MusicPlayer();
-
-	// Unload the timbres
-	clearTimbres();
-}
-
-void MusicPlayerXMI::send(uint32 b) {
-	if (_milesAudioMode) {
-		MusicPlayerMidi::send(b);
-		return;
-	}
-
-	uint32 bytesToSend = b;
-	if ((b & 0xFFF0) == 0x72B0) { // XMIDI Patch Bank Select 114
-		// From AIL2's documentation: XMIDI Patch Bank Select controller (114)
-		// selects a bank to be used when searching the next patches
-		byte chan = b & 0xF;
-		byte bank = (b >> 16) & 0xFF;
-
-		debugC(5, kDebugMIDI, "Groovie::Music: Selecting bank %X for channel %X", bank, chan);
-		_chanBanks[chan] = bank;
-		return;
-	} else if ((b & 0xF0) == 0xC0) { // Program change
-		// We intercept the program change when using AdLib or MT32 drivers,
-		// since we have custom timbres for them.  The command is sent
-		// unchanged to GM drivers.
-		byte chan = b & 0xF;
-		byte patch = (b >> 8) & 0xFF;
-		if (_musicType != 0) {
-
-			debugC(5, kDebugMIDI, "Groovie::Music: Setting custom patch %X from bank %X to channel %X", patch, _chanBanks[chan], chan);
-
-			// Try to find the requested patch from the previously
-			// specified bank
-			int numTimbres = _timbres.size();
-			for (int i = 0; i < numTimbres; i++) {
-				if ((_timbres[i].bank == _chanBanks[chan]) &&
-					(_timbres[i].patch == patch)) {
-					if (_musicType == MT_ADLIB) {
-						setTimbreAD(chan, _timbres[i]);
-					} else if (_musicType == MT_MT32) {
-						setTimbreMT(chan, _timbres[i]);
-					}
-					return;
-				}
-			}
-
-			// If we got here we couldn't find the patch, and the
-			// received message will be sent unchanged.
-		} else if (chan == 0x9) {
-			// GM program change on the rhythm channel (drumkit selection).
-			// Apply drumkit fallback to correct invalid drumkit numbers.
-			byte correctedPatch = _driver->_gsDrumkitFallbackMap[patch];
-			debugC(5, kDebugMIDI, "Groovie::Music: Selected drumkit %X (requested %X)", correctedPatch, patch);
-			bytesToSend = 0xC0 | chan | (correctedPatch << 8);
-		}
-	}
-	MusicPlayerMidi::send(bytesToSend);
 }
 
 void MusicPlayerXMI::send(int8 source, uint32 b) {
@@ -596,197 +491,6 @@ void MusicPlayerXMI::unload() {
 	}
 }
 
-void MusicPlayerXMI::loadTimbres(const Common::String &filename) {
-	// Load the Global Timbre Library format as documented in AIL2
-	debugC(1, kDebugMIDI, "Groovie::Music: Loading the GTL file %s", filename.c_str());
-
-	// Does it exist?
-	if (!Common::File::exists(filename)) {
-		error("Groovie::Music: %s not found", filename.c_str());
-		return;
-	}
-
-	// Open the GTL
-	Common::File *gtl = new Common::File();
-	if (!gtl->open(filename.c_str())) {
-		delete gtl;
-		error("Groovie::Music: Couldn't open %s", filename.c_str());
-		return;
-	}
-
-	// Clear the old timbres before loading the new ones
-	clearTimbres();
-
-	// Get the list of timbres
-	while (true) {
-		Timbre t;
-		t.patch = gtl->readByte();
-		t.bank = gtl->readByte();
-		if ((t.patch == 0xFF) && (t.bank == 0xFF)) {
-			// End of list
-			break;
-		}
-		// We temporarily use the size field to store the offset
-		t.size = gtl->readUint32LE();
-
-		// Add it to the list
-		_timbres.push_back(t);
-	}
-
-	// Read the timbres
-	for (unsigned int i = 0; i < _timbres.size(); i++) {
-		// Seek to the start of the timbre
-		gtl->seek(_timbres[i].size);
-
-		// Read the size
-		_timbres[i].size = gtl->readUint16LE() - 2;
-
-		// Allocate memory for the timbre data
-		_timbres[i].data = new byte[_timbres[i].size];
-
-		// Read the timbre data
-		gtl->read(_timbres[i].data, _timbres[i].size);
-		debugC(5, kDebugMIDI, "Groovie::Music: Loaded patch %x in bank %x with size %d",
-			_timbres[i].patch, _timbres[i].bank, _timbres[i].size);
-	}
-
-	// Close the file
-	delete gtl;
-}
-
-void MusicPlayerXMI::clearTimbres() {
-	// Delete the allocated data
-	int num = _timbres.size();
-	for (int i = 0; i < num; i++) {
-		delete[] _timbres[i].data;
-	}
-
-	// Erase the array entries
-	_timbres.clear();
-}
-
-void MusicPlayerXMI::setTimbreAD(byte channel, const Timbre &timbre) {
-	// Verify the timbre size
-	if (timbre.size != 12) {
-		error("Groovie::Music: Invalid size for an AdLib timbre: %d", timbre.size);
-	}
-
-	// Prepare the AdLib Instrument array from the GTL entry
-	//
-	// struct AdLibInstrument used by our AdLib MIDI synth is 30 bytes.
-	// Since we pass data + 2 for non percussion instruments we need to
-	// have a buffer of size 32, so there are no invalid memory reads,
-	// when setting up an AdLib instrument.
-	byte data[32];
-	memset(data, 0, sizeof(data));
-
-	data[2] = timbre.data[1];        // mod_characteristic
-	data[3] = timbre.data[2] ^ 0x3F; // mod_scalingOutputLevel
-	data[4] = ~timbre.data[3];       // mod_attackDecay
-	data[5] = ~timbre.data[4];       // mod_sustainRelease
-	data[6] = timbre.data[5];        // mod_waveformSelect
-	data[7] = timbre.data[7];        // car_characteristic
-	data[8] = timbre.data[8] ^ 0x3F; // car_scalingOutputLevel
-	data[9] = ~timbre.data[9];       // car_attackDecay
-	data[10] = ~timbre.data[10];     // car_sustainRelease
-	data[11] = timbre.data[11];      // car_waveformSelect
-	data[12] = timbre.data[6];       // feedback
-
-	// Send the instrument to the driver
-	if (timbre.bank == 0x7F) {
-		// This is a Percussion instrument, this will always be set on the same note
-		data[0] = timbre.patch;
-
-		// From AIL2's documentation: If the instrument is to be played in MIDI
-		// channel 10, num specifies its desired absolute MIDI note number.
-		data[1] = timbre.data[0];
-
-		_driver->getPercussionChannel()->sysEx_customInstrument('ADLP', data);
-	} else {
-		// Some tweaks for non-percussion instruments
-		byte mult1 = timbre.data[1] & 0xF;
-		if (mult1 < 4)
-			mult1 = 1 << mult1;
-		data[2] = (timbre.data[1] & 0xF0) + (mult1 & 0xF);
-		byte mult2 = timbre.data[7] & 0xF;
-		if (mult2 < 4)
-			mult2 = 1 << mult2;
-		data[7] = (timbre.data[7] & 0xF0) + (mult2 & 0xF);
-		// TODO: Fix CHARACTERISTIC: 0xF0: pitch_vib, amp_vib, sustain_sound, env_scaling  0xF: freq_mult
-		// TODO: Fix KSL_TL: 0xC: key_scale_lvl  0x3F: out_lvl
-
-		// From AIL2's documentation: num specifies the number of semitones
-		// by which to transpose notes played with the instrument.
-		if (timbre.data[0] != 0)
-			warning("Groovie::Music: AdLib instrument's transposing not supported");
-
-		_driver->sysEx_customInstrument(channel, 'ADL ', data + 2);
-	}
-}
-
-
-#include "common/pack-start.h"	// START STRUCT PACKING
-
-struct RolandInstrumentSysex {
-	byte roland_id;
-	byte device_id;
-	byte model_id;
-	byte command;
-	byte address[3];
-	byte instrument[0xF6];
-	byte checksum;
-} PACKED_STRUCT;
-
-#include "common/pack-end.h"	// END STRUCT PACKING
-
-void setRolandInstrument(MidiDriver *drv, byte channel, byte *instrument) {
-	RolandInstrumentSysex sysex;
-	memcpy(&sysex.instrument, instrument, 0xF6);
-
-	// Show the timbre name as extra debug information
-	Common::String name((char *)instrument, 10);
-	debugC(5, kDebugMIDI, "Groovie::Music: Setting MT32 timbre '%s' to channel %d", name.c_str(), channel);
-
-	sysex.roland_id = 0x41;
-	sysex.device_id = channel; // Unit#
-	sysex.model_id = 0x16; // MT32
-	sysex.command = 0x12; // Data set
-
-	// Remap instrument to appropriate address space.
-	int address = 0x008000;
-	sysex.address[0] = (address >> 14) & 0x7F;
-	sysex.address[1] = (address >>  7) & 0x7F;
-	sysex.address[2] = (address      ) & 0x7F;
-
-	// Compute the checksum.
-	byte checksum = 0;
-	byte *ptr = sysex.address;
-	for (int i = 4; i < (int)sizeof(RolandInstrumentSysex) - 1; ++i)
-		checksum -= *ptr++;
-	sysex.checksum = checksum & 0x7F;
-
-	// Send sysex
-	drv->sysEx((byte *)&sysex, sizeof(RolandInstrumentSysex));
-
-
-	// Wait the time it takes to send the SysEx data
-	uint32 delay = (sizeof(RolandInstrumentSysex) + 2) * 1000 / 3125;
-
-	// Plus an additional delay for the MT-32 rev00
-	delay += 40;
-
-	g_system->delayMillis(delay);
-}
-
-void MusicPlayerXMI::setTimbreMT(byte channel, const Timbre &timbre) {
-	// Verify the timbre size
-	if (timbre.size != 0xF6)
-		error("Groovie::Music: Invalid size for an MT-32 timbre: %d", timbre.size);
-
-	setRolandInstrument(_driver, channel, timbre.data);
-}
-
-
 // MusicPlayerMac_t7g
 
 MusicPlayerMac_t7g::MusicPlayerMac_t7g(GroovieEngine *vm) : MusicPlayerMidi(vm) {
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index 1cdc31132b..75cab7e832 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -125,9 +125,7 @@ protected:
 class MusicPlayerXMI : public MusicPlayerMidi, public Audio::MidiDriver_Miles_Xmidi_Timbres {
 public:
 	MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName);
-	~MusicPlayerXMI() override;
 
-	void send(uint32 b) override;
 	void send(int8 source, uint32 b) override;
 	void metaEvent(int8 source, byte type, byte *data, uint16 length) override;
 	void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize) override {
@@ -142,29 +140,10 @@ protected:
 	void unload() override;
 
 private:
-	// Channel banks
-	byte _chanBanks[0x10];
-
 	// Output music type
 	uint8 _musicType;
 
-	bool _milesAudioMode;
 	Audio::MidiDriver_Miles_Midi *_milesMidiDriver;
-
-	// Timbres
-	class Timbre {
-	public:
-		Timbre() : data(NULL), patch(0), bank(0), size(0) {}
-		byte patch;
-		byte bank;
-		uint32 size;
-		byte *data;
-	};
-	Common::Array<Timbre> _timbres;
-	void loadTimbres(const Common::String &filename);
-	void clearTimbres();
-	void setTimbreAD(byte channel, const Timbre &timbre);
-	void setTimbreMT(byte channel, const Timbre &timbre);
 };
 
 class MusicPlayerMac_t7g : public MusicPlayerMidi {


Commit: 315bbc7b5b314807249a4fd27bfe6dfb57da9641
    https://github.com/scummvm/scummvm/commit/315bbc7b5b314807249a4fd27bfe6dfb57da9641
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI: Use driver-specific impl. for stopping all notes

When the MIDI parser wanted to stop all notes on a MIDI device, it would send
an All Notes Off message and, optionally, a Sustain off message on all 16 MIDI
channels. This is inefficient for devices like the MT-32, which uses only 9
channels. The MT-32 hardware is also quite slow, so it is more responsive if it
has to process less messages.

I've added a generic stopAllNotes message to the MIDI driver using the old
method, which can be overriden by more specific implementations in subclasses.
The Miles MIDI driver implementation sends All Notes Off only on the channels
used by the MT-32. There are probably also more efficient implementations
possible for f.e. AdLib.

Changed paths:
    audio/mididrv.cpp
    audio/mididrv.h
    audio/midiparser.cpp
    audio/miles.h
    audio/miles_midi.cpp
    engines/groovie/music.cpp
    engines/groovie/music.h
    engines/kyra/sound/sound_pc_midi.cpp


diff --git a/audio/mididrv.cpp b/audio/mididrv.cpp
index 97799161be..74d1b88823 100644
--- a/audio/mididrv.cpp
+++ b/audio/mididrv.cpp
@@ -818,6 +818,14 @@ void MidiDriver_BASE::send(int8 source, byte status, byte firstOp, byte secondOp
 	send(source, status | ((uint32)firstOp << 8) | ((uint32)secondOp << 16));
 }
 
+void MidiDriver_BASE::stopAllNotes(bool stopSustainedNotes) {
+	for (int i = 0; i < 16; ++i) {
+		send(0xB0 | i, 0x7B, 0); // All notes off
+		if (stopSustainedNotes)
+			send(0xB0 | i, 0x40, 0); // Also send a sustain off event (bug #3116608)
+	}
+}
+
 void MidiDriver::midiDriverCommonSend(uint32 b) {
 	if (_midiDumpEnable) {
 		midiDumpDo(b);
diff --git a/audio/mididrv.h b/audio/mididrv.h
index 73040ebef5..199bce8084 100644
--- a/audio/mididrv.h
+++ b/audio/mididrv.h
@@ -160,6 +160,24 @@ public:
 	 */
 	virtual void metaEvent(int8 source, byte type, byte *data, uint16 length) { metaEvent(type, data, length); }
 
+	/**
+	 * Stops all currently active notes. Specify stopSustainedNotes if
+	 * the MIDI data makes use of the sustain controller to also stop
+	 * sustained notes.
+	 *
+	 * Usually, the MIDI parser tracks active notes and terminates them
+	 * when playback is stopped. This method should be used as a backup
+	 * to silence the MIDI output in case the MIDI parser makes a
+	 * mistake when tracking acive notes. It can also be used when
+	 * quitting or pausing a game.
+	 *
+	 * By default, this method sends an All Notes Off message and, if
+	 * stopSustainedNotes is true, a Sustain off message on all MIDI
+	 * channels. Driver implementations can override this if they want
+	 * to implement this functionality in a different way.
+	 */
+	virtual void stopAllNotes(bool stopSustainedNotes = false);
+
 	/**
 	 * A driver implementation might need time to prepare playback of
 	 * a track. Use this function to check if the driver is ready to
diff --git a/audio/midiparser.cpp b/audio/midiparser.cpp
index 4e56f5896c..94716f6336 100644
--- a/audio/midiparser.cpp
+++ b/audio/midiparser.cpp
@@ -330,12 +330,7 @@ void MidiParser::allNotesOff() {
 	if (!_disableAllNotesOffMidiEvents) {
 		// To be sure, send an "All Note Off" event (but not all MIDI devices
 		// support this...).
-
-		for (i = 0; i < 16; ++i) {
-			sendToDriver(0xB0 | i, 0x7b, 0); // All notes off
-			if (_sendSustainOffOnNotesOff)
-				sendToDriver(0xB0 | i, 0x40, 0); // Also send a sustain off event (bug #3116608)
-		}
+		_driver->stopAllNotes(_sendSustainOffOnNotesOff);
 	}
 
 	memset(_activeNotes, 0, sizeof(_activeNotes));
diff --git a/audio/miles.h b/audio/miles.h
index e1db3fecf7..8337dcd982 100644
--- a/audio/miles.h
+++ b/audio/miles.h
@@ -169,8 +169,7 @@ public:
 	 */
 	void setSourceVolume(uint8 source, uint16 volume);
 
-	/** Stops all notes currently playing on the MIDI device. */
-	void allNotesOff();
+	void stopAllNotes(bool stopSustainedNotes = false) override;
 
 	MidiChannel *allocateChannel() override {
 		if (_driver)
diff --git a/audio/miles_midi.cpp b/audio/miles_midi.cpp
index 6abbedded6..5976835ea6 100644
--- a/audio/miles_midi.cpp
+++ b/audio/miles_midi.cpp
@@ -784,12 +784,15 @@ void MidiDriver_Miles_Midi::stopNotesOnChannel(uint8 outputChannelNumber) {
 	}
 }
 
-void MidiDriver_Miles_Midi::allNotesOff() {
+void MidiDriver_Miles_Midi::stopAllNotes(bool stopSustainedNotes) {
 	for (int i = 0; i < MILES_MIDI_CHANNEL_COUNT; ++i) {
 		if (!isOutputChannelUsed(i))
 			continue;
-		_driver->send(0xB0 | i, MILES_CONTROLLER_SUSTAIN, 0);
-		_midiChannels[i].currentData.sustain = false;
+
+		if (stopSustainedNotes) {
+			_driver->send(0xB0 | i, MILES_CONTROLLER_SUSTAIN, 0);
+			_midiChannels[i].currentData.sustain = false;
+		}
 		_driver->send(0xB0 | i, MILES_CONTROLLER_ALL_NOTES_OFF, 0);
 		_midiChannels[i].activeNotes = 0;
 	}
diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index 137c0ea3d8..9282f3f962 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -458,6 +458,11 @@ void MusicPlayerXMI::metaEvent(int8 source, byte type, byte *data, uint16 length
 	}
 }
 
+void MusicPlayerXMI::stopAllNotes(bool stopSustainedNotes) {
+	if (_driver)
+		_driver->stopAllNotes(stopSustainedNotes);
+}
+
 bool MusicPlayerXMI::isReady() {
 	return _driver ? _driver->isReady() : false;
 }
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index 75cab7e832..8475a62f5d 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -128,6 +128,7 @@ public:
 
 	void send(int8 source, uint32 b) override;
 	void metaEvent(int8 source, byte type, byte *data, uint16 length) override;
+	void stopAllNotes(bool stopSustainedNotes) override;
 	void processXMIDITimbreChunk(const byte *timbreListPtr, uint32 timbreListSize) override {
 		if (_milesMidiDriver)
 			_milesMidiDriver->processXMIDITimbreChunk(timbreListPtr, timbreListSize);
diff --git a/engines/kyra/sound/sound_pc_midi.cpp b/engines/kyra/sound/sound_pc_midi.cpp
index dd125ef154..0aed316dbd 100644
--- a/engines/kyra/sound/sound_pc_midi.cpp
+++ b/engines/kyra/sound/sound_pc_midi.cpp
@@ -89,7 +89,7 @@ SoundMidiPC::~SoundMidiPC() {
 	delete _music;
 	for (int i = 0; i < 3; ++i)
 		delete _sfx[i];
-	_output->allNotesOff();
+	_output->stopAllNotes();
 
 	delete _output; // This automatically frees _driver (!)
 
@@ -356,7 +356,7 @@ void SoundMidiPC::pause(bool paused) {
 		for (int i = 0; i < 3; i++)
 			_sfx[i]->pausePlaying();
 		if (_output)
-			_output->allNotesOff();
+			_output->stopAllNotes();
 	} else {
 		_music->resumePlaying();
 		for (int i = 0; i < 3; ++i)


Commit: 3ad8c85af209ac43b38f6d83f58436b24f428261
    https://github.com/scummvm/scummvm/commit/3ad8c85af209ac43b38f6d83f58436b24f428261
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI: Prevent duplicate MIDI messages

The MIDI parser would send All Notes Off events on all MIDI channels as a
safety measure when stopping playback in various situations without first
checking if a track was actually playing. Depending on the specifics of the
code driving the MIDI parser, this could generate many duplicate All Notes Off,
Sustain off and/or Pitch Bend center messages. This is a problem for slow MIDI
hardware like the MT-32, which needs some time to process all these messages.
As a result, the timing at the beginning of the next track could be wrong.

I've introduced checks in stopPlaying, setTrack, unloadMusic and the MIDI
parser destructor so that allNotesOff is called once when a track is playing,
and not a second time when playback has already stopped. Similarly, unloadMusic
does not execute when no music is loaded to prevent duplicate Pitch Bend
messages.

Changed paths:
    audio/midiparser.cpp
    audio/midiparser.h
    audio/midiparser_xmidi.cpp
    engines/groovie/music.cpp
    engines/groovie/music.h


diff --git a/audio/midiparser.cpp b/audio/midiparser.cpp
index 94716f6336..af5b6519aa 100644
--- a/audio/midiparser.cpp
+++ b/audio/midiparser.cpp
@@ -359,7 +359,7 @@ bool MidiParser::setTrack(int track) {
 
 	if (_smartJump)
 		hangAllActiveNotes();
-	else
+	else if (isPlaying())
 		allNotesOff();
 
 	resetTracking();
@@ -377,7 +377,8 @@ bool MidiParser::setTrack(int track) {
 }
 
 void MidiParser::stopPlaying() {
-	allNotesOff();
+	if (isPlaying())
+		allNotesOff();
 	resetTracking();
 	_pause = false;
 }
@@ -508,12 +509,14 @@ bool MidiParser::jumpToTick(uint32 tick, bool fireEvents, bool stopNotes, bool d
 }
 
 void MidiParser::unloadMusic() {
-	resetTracking();
-	allNotesOff();
+	if (_numTracks == 0)
+		// No music data loaded
+		return;
+
+	stopPlaying();
 	_numTracks = 0;
 	_activeTrack = 255;
 	_abortParse = true;
-	_pause = false;
 
 	if (_centerPitchWheelOnUnload) {
 		// Center the pitch wheels in preparation for the next piece of
diff --git a/audio/midiparser.h b/audio/midiparser.h
index 2d675ffe75..6cf8b7aa98 100644
--- a/audio/midiparser.h
+++ b/audio/midiparser.h
@@ -404,7 +404,7 @@ public:
 	typedef void (*XMidiCallbackProc)(byte eventData, void *refCon);
 
 	MidiParser();
-	virtual ~MidiParser() { allNotesOff(); }
+	virtual ~MidiParser() { stopPlaying(); }
 
 	virtual bool loadMusic(byte *data, uint32 size) = 0;
 	virtual void unloadMusic();
diff --git a/audio/midiparser_xmidi.cpp b/audio/midiparser_xmidi.cpp
index f916a703ec..958c2f9358 100644
--- a/audio/midiparser_xmidi.cpp
+++ b/audio/midiparser_xmidi.cpp
@@ -103,7 +103,7 @@ public:
 		memset(_tracksTimbreList, 0, sizeof(_tracksTimbreList));
 		memset(_tracksTimbreListSize, 0, sizeof(_tracksTimbreListSize));
 	}
-	~MidiParser_XMIDI() { }
+	~MidiParser_XMIDI() { stopPlaying(); }
 
 	void setMidiDriver(MidiDriver_BASE *driver) override;
 	bool loadMusic(byte *data, uint32 size) override;
diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index 9282f3f962..00c839b457 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -440,6 +440,10 @@ MusicPlayerXMI::MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName)
 	_midiParser->setTimerRate(_driver->getBaseTempo());
 }
 
+MusicPlayerXMI::~MusicPlayerXMI() {
+	_midiParser->stopPlaying();
+}
+
 void MusicPlayerXMI::send(int8 source, uint32 b) {
 	if (_milesMidiDriver) {
 		_milesMidiDriver->send(source, b);
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index 8475a62f5d..2ce5ab895a 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -125,6 +125,7 @@ protected:
 class MusicPlayerXMI : public MusicPlayerMidi, public Audio::MidiDriver_Miles_Xmidi_Timbres {
 public:
 	MusicPlayerXMI(GroovieEngine *vm, const Common::String &gtlName);
+	~MusicPlayerXMI();
 
 	void send(int8 source, uint32 b) override;
 	void metaEvent(int8 source, byte type, byte *data, uint16 length) override;


Commit: 5da83d8ea07311e13eab1e7b4e75842ad1b1dbc1
    https://github.com/scummvm/scummvm/commit/5da83d8ea07311e13eab1e7b4e75842ad1b1dbc1
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
GROOVIE: Pause music when ScummVM menu is open

The MIDI music would keep playing while the ScummVM menu was open. This would
cause the music for cutscenes to go out of sync. I've added pausing to the
MusicPlayerMidi to fix this.

Changed paths:
    engines/groovie/groovie.cpp
    engines/groovie/groovie.h
    engines/groovie/music.cpp
    engines/groovie/music.h


diff --git a/engines/groovie/groovie.cpp b/engines/groovie/groovie.cpp
index 5ecd64eee8..94bfa26766 100644
--- a/engines/groovie/groovie.cpp
+++ b/engines/groovie/groovie.cpp
@@ -340,6 +340,12 @@ Common::Error GroovieEngine::run() {
 	return Common::kNoError;
 }
 
+void GroovieEngine::pauseEngineIntern(bool pause) {
+	Engine::pauseEngineIntern(pause);
+	if (_musicPlayer)
+		_musicPlayer->pause(pause);
+}
+
 Common::Platform GroovieEngine::getPlatform() const {
 	return _gameDescription->desc.platform;
 }
diff --git a/engines/groovie/groovie.h b/engines/groovie/groovie.h
index 7d36bce78f..c17ceb2445 100644
--- a/engines/groovie/groovie.h
+++ b/engines/groovie/groovie.h
@@ -99,6 +99,7 @@ protected:
 
 	// Engine APIs
 	Common::Error run() override;
+	void pauseEngineIntern(bool pause) override;
 
 	bool hasFeature(EngineFeature f) const override;
 
diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index 00c839b457..501c7e0a6f 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -313,6 +313,16 @@ void MusicPlayerMidi::metaEvent(byte type, byte *data, uint16 length) {
 	}
 }
 
+void MusicPlayerMidi::pause(bool pause) {
+	if (_midiParser) {
+		if (pause) {
+			_midiParser->pausePlaying();
+		} else {
+			_midiParser->resumePlaying();
+		}
+	}
+}
+
 void MusicPlayerMidi::updateChanVolume(byte channel) {
 	// Generate a MIDI Control change message for the volume
 	uint32 b = 0x7B0;
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index 2ce5ab895a..a051793b55 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -45,6 +45,10 @@ public:
 	void playCD(uint8 track);
 	void startBackground();
 	bool isPlaying() { return _isPlaying; }
+	// Pause or resume the music. Note that digital music
+	// already pauses when the ScummVM menu is open, so
+	// it does not seem to need an implementation.
+	virtual void pause(bool pause) { }
 
 	void frameTick();
 	void setBackgroundDelay(uint16 delay);
@@ -104,6 +108,8 @@ public:
 	uint16 sysExNoDelay(const byte *msg, uint16 length) override;
 	void metaEvent(byte type, byte *data, uint16 length) override;
 
+	void pause(bool pause) override;
+
 private:
 	// Channel volumes
 	byte _chanVolumes[0x10];


Commit: da64fdc3d1fd2bf6730dba339a94fba1afdb0fde
    https://github.com/scummvm/scummvm/commit/da64fdc3d1fd2bf6730dba339a94fba1afdb0fde
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI: Fix MIDI parser tracker overflow

The MIDI parser tracks the number of ticks and microseconds since the start of
playback of the current track. If a track loops infinitely, the tracker
eventually overflows (after about 71 minutes). The parser would then go haywire,
sending out the MIDI events at full speed and not terminating hanging notes,
resulting in loads of polyphony overrun errors. This does not affect looping
using jumpToTick or autoLoop, because that resets the tracker.

This change fixes this for the XMIDI parser. processEvent implementations can
now set a bool flag on the event, indicating that the event has caused a loop.
The MIDI parser will then reset the ticks and microseconds on the tracker to 0,
maintaining the deltas between the previous event values and current values.
The absolute number of ticks and microseconds since track start do not seem to
be needed anywhere; only the difference with the previous event is relevant.

This should fix bugs #6275 and #4354.

Changed paths:
    audio/midiparser.cpp
    audio/midiparser.h
    audio/midiparser_xmidi.cpp


diff --git a/audio/midiparser.cpp b/audio/midiparser.cpp
index af5b6519aa..f1b3c4e307 100644
--- a/audio/midiparser.cpp
+++ b/audio/midiparser.cpp
@@ -211,6 +211,7 @@ void MidiParser::onTimer() {
 		}
 	}
 
+	bool loopEvent = false;
 	while (!_abortParse) {
 		EventInfo &info = _nextEvent;
 
@@ -219,7 +220,6 @@ void MidiParser::onTimer() {
 			break;
 
 		// Process the next info.
-		_position._lastEventTick += info.delta;
 		if (info.event < 0x80) {
 			warning("Bad command or running status %02X", info.event);
 			_position._playPos = 0;
@@ -241,8 +241,11 @@ void MidiParser::onTimer() {
 		if (!ret)
 			return;
 
+		loopEvent |= info.loop;
+
 		if (!_abortParse) {
 			_position._lastEventTime = eventTime;
+			_position._lastEventTick += info.delta;
 			parseNextEvent(_nextEvent);
 		}
 	}
@@ -250,6 +253,15 @@ void MidiParser::onTimer() {
 	if (!_abortParse) {
 		_position._playTime = endTime;
 		_position._playTick = (_position._playTime - _position._lastEventTime) / _psecPerTick + _position._lastEventTick;
+		if (loopEvent) {
+			// One of the processed events has looped (part of) the MIDI data.
+			// Infinite looping will cause the tracker to overflow eventually.
+			// Reset the tracker positions to prevent this from happening.
+			_position._playTime -= _position._lastEventTime;
+			_position._lastEventTime = 0;
+			_position._playTick -= _position._lastEventTick;
+			_position._lastEventTick = 0;
+		}
 	}
 }
 
diff --git a/audio/midiparser.h b/audio/midiparser.h
index 6cf8b7aa98..7dfdc1c13c 100644
--- a/audio/midiparser.h
+++ b/audio/midiparser.h
@@ -104,9 +104,12 @@ struct EventInfo {
 	               ///< For Note On events, a non-zero value indicates that no Note Off event
 	               ///< will occur, and the MidiParser will have to generate one itself.
 	               ///< For all other events, this value should always be zero.
+	bool   loop;   ///< Indicates that this event loops (part of) the MIDI data.
 
 	byte channel() const { return event & 0x0F; } ///< Separates the MIDI channel from the event.
 	byte command() const { return event >> 4; }   ///< Separates the command code from the event.
+
+	EventInfo() : start(0), delta(0), event(0), length(0), loop(false) { basic.param1 = 0; basic.param2 = 0; ext.type = 0; ext.data = 0; }
 };
 
 /**
diff --git a/audio/midiparser_xmidi.cpp b/audio/midiparser_xmidi.cpp
index 958c2f9358..6c1d069ed6 100644
--- a/audio/midiparser_xmidi.cpp
+++ b/audio/midiparser_xmidi.cpp
@@ -166,6 +166,7 @@ bool MidiParser_XMIDI::jumpToIndex(uint8 index, bool stopNotes) {
 void MidiParser_XMIDI::parseNextEvent(EventInfo &info) {
 	info.start = _position._playPos;
 	info.delta = readVLQ2(_position._playPos);
+	info.loop = false;
 
 	// Process the next event.
 	info.event = *(_position._playPos++);
@@ -230,12 +231,15 @@ void MidiParser_XMIDI::parseNextEvent(EventInfo &info) {
 				} else {
 					// Repeat 0 means "loop forever".
 					if (_loop[_loopCount].repeat) {
-						if (--_loop[_loopCount].repeat == 0)
+						if (--_loop[_loopCount].repeat == 0) {
 							_loopCount--;
-						else
+						} else {
 							_position._playPos = _loop[_loopCount].pos;
+							info.loop = true;
+						}
 					} else {
 						_position._playPos = _loop[_loopCount].pos;
+						info.loop = true;
 					}
 				}
 			}


Commit: dcdcca3d31aa5d745740d298cc3bcf581eafe9a2
    https://github.com/scummvm/scummvm/commit/dcdcca3d31aa5d745740d298cc3bcf581eafe9a2
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
GROOVIE: Initialize MIDI on direct game load

The 7th Guest has MIDI initialization files for GM and the MT-32 that need to
run for these devices to sound correctly. When loading a game from the ScummVM
main menu, MIDI initialization was not performed.

This change adds this. It runs a slightly changed copy of the game's MIDI init
script when loading a game using the ScummVM menus. It also tracks if MIDI init
has been performed, so it doesn't run a second time when loading a game.

I don't know if the script works properly for the Mac and iOS versions of the
game (or if it is even needed), so I've only enabled it for the DOS versions of
the game for now.

Changed paths:
    engines/groovie/music.cpp
    engines/groovie/music.h
    engines/groovie/script.cpp


diff --git a/engines/groovie/music.cpp b/engines/groovie/music.cpp
index 501c7e0a6f..30d2008bf6 100644
--- a/engines/groovie/music.cpp
+++ b/engines/groovie/music.cpp
@@ -45,7 +45,7 @@ namespace Groovie {
 MusicPlayer::MusicPlayer(GroovieEngine *vm) :
 	_vm(vm), _isPlaying(false), _backgroundFileRef(0), _gameVolume(100),
 	_prevCDtrack(0), _backgroundDelay(0), _fadingStartTime(0), _fadingStartVolume(0),
-	_fadingEndVolume(0), _fadingDuration(0), _userVolume(0) {
+	_fadingEndVolume(0), _fadingDuration(0), _midiInit(false), _userVolume(0) {
 }
 
 MusicPlayer::~MusicPlayer() {
diff --git a/engines/groovie/music.h b/engines/groovie/music.h
index a051793b55..d36c52867f 100644
--- a/engines/groovie/music.h
+++ b/engines/groovie/music.h
@@ -50,6 +50,11 @@ public:
 	// it does not seem to need an implementation.
 	virtual void pause(bool pause) { }
 
+	// Set the MIDI initialization state
+	void setMidiInit(bool midiInit) { _midiInit = midiInit; }
+	// Returns true if MIDI has been fully initialized
+	bool isMidiInit() { return _midiInit; }
+
 	void frameTick();
 	void setBackgroundDelay(uint16 delay);
 
@@ -81,6 +86,9 @@ private:
 protected:
 	GroovieEngine *_vm;
 
+	// True if the MIDI initialization has completed
+	bool _midiInit;
+
 	// Callback
 	static void onTimer(void *data);
 	virtual void onTimerInternal() {}
diff --git a/engines/groovie/script.cpp b/engines/groovie/script.cpp
index 354078ce1a..c8ef6da802 100644
--- a/engines/groovie/script.cpp
+++ b/engines/groovie/script.cpp
@@ -45,6 +45,26 @@
 
 namespace Groovie {
 
+// Adapted from SCRIPT.GRV
+const byte t7gMidiInitScript[] = {
+	0x1A, 0x00, 0x01, 0xB1, 0x12, 0x00,		// strcmpnejmp (if (var 0100 != 01) jmp 0012)
+	0x02, 0x46, 0x4C,						// playsong 4C46 (GM init)
+	0x03,									// bf9on (fade-in)
+	0x09, 0x60, 0x24,						// videofromref 2460 (GM init video)
+	0x09, 0x60, 0x24,						// videofromref 2460 (GM init video)
+	0x04,									// palfadeout
+	0x29,									// stopmidi
+	0x1A, 0x00, 0x01, 0xB2, 0x21, 0x00,		// :0012 - strcmpnejmp (if (var 0100 != 02) jmp 0021)
+	0x02, 0x45, 0x4C,						// playsong 4C45 (MT-32 init)
+	0x03,									// bf9on (fade-in)
+	0x09, 0x61, 0x24,						// videofromref 2461 (MT-32 init video)
+	0x04,									// palfadeout
+	0x29,									// stopmidi
+	0x31, 0x63, 0x00, 0x00, 0x00,			// :0021 - midivolume 0063, 0000
+	0x3C,									// checkvalidsaves
+	0x43, 0x00								// returnscript 00
+};
+
 Script::Script(GroovieEngine *vm, EngineVersion version) :
 	_code(NULL), _savedCode(NULL), _stacktop(0), _debugger(NULL), _vm(vm),
 	_videoFile(NULL), _videoRef(0), _staufsMove(NULL), _lastCursor(0xff),
@@ -188,6 +208,10 @@ void Script::directGameLoad(int slot) {
 		_savedCode = nullptr;
 	}
 
+	uint16 targetInstruction;
+	const byte *midiInitScript = 0;
+	uint8 midiInitScriptSize = 0;
+
 	// HACK: We set the slot to load in the appropriate variable, and set the
 	// current instruction to the one that actually loads the saved game
 	// specified in that variable. This differs depending on the game and its
@@ -195,21 +219,46 @@ void Script::directGameLoad(int slot) {
 	if (_version == kGroovieT7G) {
 		// 7th Guest
 		setVariable(0x19, slot);
-		_currentInstruction = 0x287;
+		targetInstruction = 0x287;
+		// TODO Not sure if this works on or is necessary for Mac or iOS
+		// versions. Disabling it to prevent breaking game loading.
+		if (_vm->getPlatform() == Common::kPlatformDOS) {
+			midiInitScript = t7gMidiInitScript;
+			midiInitScriptSize = sizeof(t7gMidiInitScript);
+		}
 	} else {
 		// 11th Hour
 		setVariable(0xF, slot);
 		// FIXME: This bypasses a lot of the game's initialization procedure
-		_currentInstruction = 0xE78E;
+		targetInstruction = 0xE78E;
+	}
+
+	if (midiInitScript && !_vm->_musicPlayer->isMidiInit()) {
+		// Run the MIDI init script as a subscript.
+
+		// Backup the current script state
+		_savedCode = _code;
+		_savedCodeSize = _codeSize;
+		_savedStacktop = _stacktop;
+		_savedScriptFile = _scriptFile;
+		// Set the game load instruction as the backup instruction. This
+		// will run when the subscript returns.
+		_savedInstruction = targetInstruction;
+
+		// Set the MIDI init script as the current script.
+		_codeSize = midiInitScriptSize;
+		_code = new byte[_codeSize];
+		memcpy(_code, midiInitScript, _codeSize);
+		_stacktop = 0;
+		_currentInstruction = 0;
+	} else {
+		// No MIDI initialization necessary. Just jump to the game load
+		// instruction.
+		_currentInstruction = targetInstruction;
+		// Due to HACK above, the call to check valid save slots is not run.
+		// As this is where we load save names, manually call it here.
+		o_checkvalidsaves();
 	}
-
-	// TODO: We'll probably need to start by running the beginning of the
-	// script to let it do the soundcard initialization and then do the
-	// actual loading.
-
-	// Due to HACK above, the call to check valid save slots is not run.
-	// As this is where we load save names, manually call it here.
-	o_checkvalidsaves();
 }
 
 void Script::step() {
@@ -586,12 +635,16 @@ void Script::o_videofromref() {			// 0x09
 	if (!playvideofromref(fileref, gmInitVideo || mt32InitVideo)) {
 		// Move _currentInstruction back
 		_currentInstruction -= 3;
-	} else if (gmInitVideo) {
-		// The script plays the MIDI init video twice to give the "audio"
-		// enough time to play. It has just looped until the audio finished,
-		// so the second play is no longer necessary.
-		// Skip the next instruction.
-		_currentInstruction += 3;
+	} else if (gmInitVideo || mt32InitVideo) {
+		// MIDI initialization has completed. Set this on the music player,
+		// so that MIDI init will not be done again on game load.
+		_vm->_musicPlayer->setMidiInit(true);
+		if (gmInitVideo)
+			// The script plays the GM init video twice to give the "audio"
+			// enough time to play. It has just looped until the audio finished,
+			// so the second play is no longer necessary.
+			// Skip the next instruction.
+			_currentInstruction += 3;
 	}
 }
 


Commit: 11c74aaef85c85e56c76f8b4f380d971923f7b33
    https://github.com/scummvm/scummvm/commit/11c74aaef85c85e56c76f8b4f380d971923f7b33
Author: NMIError (crampen at gmail.com)
Date: 2020-07-25T00:35:47+02:00

Commit Message:
MIDI: Updated MidiParser documentation

Changed paths:
    audio/midiparser.h


diff --git a/audio/midiparser.h b/audio/midiparser.h
index 7dfdc1c13c..14a1f3a8f4 100644
--- a/audio/midiparser.h
+++ b/audio/midiparser.h
@@ -161,10 +161,17 @@ struct NoteTimer {
  * may also override the default MidiParser behavior for
  * the following methods:
  *   - resetTracking
+ *   - getTick
+ *   - jumpToIndex
+ *   - hasJumpIndex
  *   - allNotesOff
  *   - unloadMusic
  *   - property
- *   - getTick
+ *   - processEvent
+ *   - onTrackStart
+ *   - sendToDriver
+ *   - sendMetaEventToDriver
+ *   - setMidiDriver
  *
  * Please see the documentation for these individual
  * functions for more information on their use.
@@ -189,10 +196,12 @@ struct NoteTimer {
  * MidiDriver. In the simplest configuration, you can plug
  * a single MidiParser directly into the output MidiDriver
  * being used. However, you can only plug in one at a time;
- * otherwise channel conflicts will occur. Furthermore,
- * meta events that may be needed to interactively control
- * music flow cannot be handled because they are being
- * sent directly to the output device.
+ * otherwise channel conflicts will occur. Multiple parsers
+ * can be used if they do not use the same channels, or if
+ * they use some form of dynamic channel allocation.
+ * Furthermore, meta events that may be needed to
+ * interactively control music flow cannot be handled
+ * because they are being sent directly to the output device.
  *
  * If you need more control over the MidiParser while it's
  * playing, you can create your own "pseudo-MidiDriver" and
@@ -200,7 +209,10 @@ struct NoteTimer {
  * MidiDriver. The MidiParser will send events to your
  * pseudo-MidiDriver, which in turn must send them to the
  * output MidiDriver (or do whatever special handling is
- * required).
+ * required). Make sure to implement all functions which
+ * are necessary for proper functioning of the parser and
+ * forward the calls to the real driver (even if you do not
+ * want to customize the functionality).
  *
  * To specify the MidiDriver to send music output to,
  * use the MidiParser::setMidiDriver method.
@@ -233,13 +245,17 @@ struct NoteTimer {
  * to the music data and the size of the data. (NOTE: Some
  * MidiParser variants don't require a size, and 0 is fine.
  * However, when writing client code to use MidiParser, it is
- * best to assume that a valid size will be required.
+ * best to assume that a valid size will be required.)
  *
  * Convention requires that each implementation of
  * MidiParser::loadMusic automatically set up default tempo
  * and current track. This effectively means that the
  * MidiParser will start playing as soon as timer events
- * start coming in.
+ * start coming in. If you want to start playback at a later
+ * point, you can specify the mpDisableAutoStartPlayback
+ * property. You can then specify the track and/or starting
+ * point using setTrack, jumpToTick or jumpToIndex, and then
+ * call startPlaying to start playback.
  *
  * <b>STEP 6: Activate a timer source for the MidiParser.</b>
  * The easiest timer source to use is the timer of the
@@ -250,7 +266,7 @@ struct NoteTimer {
  * and timer_param will be a pointer to your MidiParser object.
  *
  * This configuration only allows one MidiParser to be driven
- * by the MidiDriver at a time. To drive more MidiDrivers, you
+ * by the MidiDriver at a time. To drive more MidiParsers, you
  * will need to create a "pseudo-MidiDriver" as described earlier,
  * In such a configuration, the pseudo-MidiDriver should be set
  * as the timer recipient in MidiDriver::setTimerCallback, and
@@ -258,9 +274,15 @@ struct NoteTimer {
  *
  * <b>STEP 7: Music shall begin to play!</b>
  * Congratulations! At this point everything should be hooked up
- * and the MidiParser should generate music. Note that there is
- * no way to "stop" the MidiParser. You can "pause" the MidiParser
- * simply by not sending timer events to it, or you can call
+ * and the MidiParser should generate music. You can pause
+ * playback and resume playing from the point you left off using
+ * the pausePlaying and resumePlaying functions. (Note that MIDI
+ * does not pause very well and active notes will be missing when
+ * you resume playback.) You can also "pause" the MidiParser
+ * simply by not sending timer events to it. You can stop
+ * playback using the stopPlaying function; you can then later
+ * play the track again from the start using startPlaying (or
+ * select a new track first using setTrack). You can call
  * MidiParser::unloadMusic to permanently stop the music. (This
  * method resets everything and detaches the MidiParser from the
  * memory block containing the music data.)
@@ -400,7 +422,7 @@ public:
 		  * or setting the track. Use startPlaying to start playback.
 		  * Note that not every parser implementation might support this.
 		  */
-		  mpDisableAutoStartPlayback = 7
+		 mpDisableAutoStartPlayback = 7
 	};
 
 public:




More information about the Scummvm-git-logs mailing list