[Scummvm-git-logs] scummvm master -> 74609d7fa59a937c0fd3eda271a88cd2c7a1843c

[athrxx]

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

Summary:
74609d7fa5 SCUMM: (IMS/MT-32) - minor fixes for playback on real hardware


Commit: 74609d7fa59a937c0fd3eda271a88cd2c7a1843c
    https://github.com/scummvm/scummvm/commit/74609d7fa59a937c0fd3eda271a88cd2c7a1843c
Author: athrxx (athrxx at scummvm.org)
Date: 2022-10-22T22:20:41+02:00

Commit Message:
SCUMM: (IMS/MT-32) - minor fixes for playback on real hardware

- avoid sending unnecessary sysex messages
- add a delay after certain sysex messages
(but without calling OSystem::delayMillis(), since
that has negative impact on the playback).

Changed paths:
    engines/scumm/imuse/drivers/midi.cpp
    engines/scumm/imuse/imuse_internal.h
    engines/scumm/imuse/imuse_player.cpp


diff --git a/engines/scumm/imuse/drivers/midi.cpp b/engines/scumm/imuse/drivers/midi.cpp
index 456570785d5..2d18b43f90e 100644
--- a/engines/scumm/imuse/drivers/midi.cpp
+++ b/engines/scumm/imuse/drivers/midi.cpp
@@ -643,6 +643,7 @@ public:
 	~IMuseChannel_MT32() override {}
 
 	bool allocate() override;
+	void reset();
 
 	// Regular messages
 	void programChange(byte program) override;
@@ -698,15 +699,17 @@ IMuseChannel_MT32::IMuseChannel_MT32(IMuseDriver_MT32 *drv, int number) : IMuseC
 bool IMuseChannel_MT32::allocate() {
 	bool res = IMuseChannel_Midi::allocate();
 
-	if (res && !_newSystem) {
-		byte msg[] = { (byte)(_timbre >> 6), (byte)(_timbre & 0x3F), 0x18, 0x32, 0x10, 0x00, _reverbSwitch};
-		sendSysexPatchData(0, msg, sizeof(msg));
+	if (res && !_newSystem)
 		_program = _number;
-	}
 
 	return res;
 }
 
+void IMuseChannel_MT32::reset() {
+	byte msg[] = { (byte)(_timbre >> 6), (byte)(_timbre & 0x3F), 0x18, 0x32, 0x10, 0x00, _reverbSwitch};
+	sendSysexPatchData(0, msg, sizeof(msg));
+}
+
 void IMuseChannel_MT32::programChange(byte program)  {
 	if (program > 127)
 		return;
@@ -895,6 +898,11 @@ void IMuseDriver_MT32::initDevice() {
 		send(0x3F0AB0 | i);
 		send(0x4000E0 | i);
 	}
+
+	for (int i = 0; i < _numChannels; ++i) {
+		static_cast<IMuseChannel_MT32*>(_imsParts[i])->reset();
+		g_system->delayMillis(5);
+	}
 }
 
 void IMuseDriver_MT32::deinitDevice() {
diff --git a/engines/scumm/imuse/imuse_internal.h b/engines/scumm/imuse/imuse_internal.h
index 57385062d2a..967e1c9fe5f 100644
--- a/engines/scumm/imuse/imuse_internal.h
+++ b/engines/scumm/imuse/imuse_internal.h
@@ -296,6 +296,7 @@ public:
 	// MidiDriver 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;
 };
 
diff --git a/engines/scumm/imuse/imuse_player.cpp b/engines/scumm/imuse/imuse_player.cpp
index ac224ec8c7d..a848d875778 100644
--- a/engines/scumm/imuse/imuse_player.cpp
+++ b/engines/scumm/imuse/imuse_player.cpp
@@ -435,6 +435,33 @@ void Player::sysEx(const byte *p, uint16 len) {
 		(*_se->_sysex)(this, p, len);
 }
 
+uint16 Player::sysExNoDelay(const byte *msg, uint16 length) {
+	sysEx(msg, length);
+
+	// The reason for adding this delay was the music track in the MI2 start scene (on the bridge, with Largo) when
+	// played on real hardware (in my case a Roland CM32L). The track starts with several sysex messages (mostly
+	// iMuse control messages, but also a Roland custom timbre sysex message). When played through the Munt emulator
+	// this works totally fine, but the real hardware seems to still "choke" on the sysex data, when the actual song
+	// playback has already started. This will cause a skipping of the first couple of notes, since the midi parser
+	// will not wait, but strictly enforce sync on the next time stamps.
+	// My tests with the dreamm emulator on that scene did sometimes show the same issue (although to a weaker extent),
+	// but most of the time not. So it seems to be rather a delicate and race-condition prone matter. The original
+	// parser handles the timing differently than our general purpose parser and the code execution is also expected
+	// to be much slower, so that might make all the difference here. It is really a flaw of the track. The time stamps
+	// after the sysex messages should have been made a bit more generous. 
+	// Now, I have added some delays here that I have taken from the original DOTT MT-32 driver's sysex function which
+	// are supposed to handle the situation when _scanning is enabled. For non-_scanning situations there is no delay in
+	// the original driver, since apparently is wasn't necessary.
+	// We only need to intercept actual hardware sysex messages here. So, for the iMuse control messages, we intercept
+	// just type 0, since that one leads to hardware messages. This is not a perfect solution, but it seems to work
+	// as intended.
+
+	if (_isMT32 && ((msg[0] == IMUSE_SYSEX_ID && msg[1] == 0) || msg[0] == ROLAND_SYSEX_ID))
+		return length >= 25 ? 70 : 20;
+
+	return 0;
+}
+
 void Player::decode_sysex_bytes(const byte *src, byte *dst, int len) {
 	while (len >= 0) {
 		*dst++ = ((src[0] << 4) & 0xFF) | (src[1] & 0xF);