SCUMM: MONKEY2: Wrong instruments used in MT-32 playback

[gabberhead]

here is 1 mp3 file with the problem with the newest daily build and 1 file with the right instruments used with stable 2.8.1. i mean the short part in the middle.
i am using munt 2.7.0 for mt32 emulation. roms are cm32l. i noticed it today. i dont know, when this problem began and also i dont know, if other games are also effected by this problem.

[eriktorbjorn]

It only seems to happen when you skip the first part, not if you let it play uninterrupted. Bisecting points to this commit:

Author: athrxx <athrxx@scummvm.org>
Date:   Fri Mar 22 18:18:58 2024 +0100

    SCUMM: (IMS/MI2/FOA) - implement old style channel management

    (fix bug #14618: Inaccurate fades in INDY4,  "Test 1")

    This part of the bug ticket is not about fades. The reason
    for the glitch is that the driver has no unused channels left
    and the newer allocation mechanism is not able to free up
    some of the used channels here.

 engines/scumm/imuse/drivers/fmtowns.cpp | 12 +++---
 engines/scumm/imuse/drivers/fmtowns.h   |  1 +
 engines/scumm/imuse/imuse.cpp           | 76 ++++++++++++++++++++++++++++++---
 engines/scumm/imuse/imuse.h             |  7 +--
 engines/scumm/imuse/imuse_internal.h    | 12 +++++-
 engines/scumm/imuse/imuse_part.cpp      | 11 ++++-
 engines/scumm/imuse/imuse_player.cpp    | 12 ++++--
 engines/scumm/imuse/sysex_scumm.cpp     | 15 +++++--
 engines/scumm/scumm.cpp                 |  8 +---
 9 files changed, 119 insertions(+), 35 deletions(-)

[athrxx]

It can be fixed by removing the call to player->uninit_parts(); in sysex_scumm.cpp, line 122. The Indy 4 imuse driver seems to make that call, though, so I am not yet convinced that the removal is a good idea. I'll check what the MI2 driver does (should be the same as Indy 4, but who knows...).

[athrxx]

I have made a fix. Please test if it works as intended...

[eriktorbjorn]

I'm not the original reporter, of course, but it seems to work for me. But the condition in sysExNoDelay() looks a bit funny, and GCC warns about it:

if (_isMT32 && (!_scanning && (msg[0] == IMUSE_SYSEX_ID && msg[1] == 0) || msg[0] == ROLAND_SYSEX_ID))
    return length >= 25 ? 70 : 20;

engines/scumm/imuse/imuse_player.cpp: In member function ‘virtual uint16 Scumm::Player::sysExNoDelay(const byte*, uint16)’:
engines/scumm/imuse/imuse_player.cpp:480:36: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
  480 |         if (_isMT32 && (!_scanning && (msg[0] == IMUSE_SYSEX_ID && msg[1] == 0) || msg[0] == ROLAND_SYSEX_ID))
      |                         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So as long as msg[0] == ROLAND_SYSEX_ID the whole condition will be true, regardless of _isMT32 and !_scanning, right? Was it supposed to look like this instead?

if (_isMT32 && !_scanning && ((msg[0] == IMUSE_SYSEX_ID && msg[1] == 0) || msg[0] == ROLAND_SYSEX_ID))
    return length >= 25 ? 70 : 20;

[gabberhead]

its working again.

[athrxx]

Thanks for testing again!