SCUMM: Indy3 (MAC) boxing bell rings 2 times instead of 3

[dwatteau]

With today's Git f7bdce7671691 HEAD, and the old Macintosh 16-color release of Indy3.

If you:

1. Start a new game.
2. Go next to the box ring, use the mallet with the bell.
3. You can see that Indy is hitting the bell 3 times, but you can only hear the sound of the bell twice (it looks like the second one is muted?).

When playing with the original interpreter on a real G3 running Mac OS 9.2, the bell does ring 3 times.

Object 76-835 handles this this way:

[004F] (48) if (Local[0] == 836) {
[0056] (40)   cutscene([]);
[0058] (01)   putActor(1,174,125);
[005E] (80)   breakHere();
[005F] (1A)   Local[0] = 1;
[0064] (11)   animateCostume(1,10);
[0067] (1C)   startSound(44);
[0069] (80)   breakHere();
[006A] (11)   animateCostume(1,3);
[006D] (2E)   delay(30);
[0071] (46)   Local[0]++;
[0074] (44)   unless (Local[0] > 3) goto 0064;
[007B] (C0)   endCutscene();
[007C] (18) }

so AFAICS the sound is meant to be heard 3 times. Maybe the delay is a bit short?

[eriktorbjorn]

If I hack o5_startSound() to wait for sound 44 to end before playing it again, I get all three bell sounds. But I don't know if that's the appropriate workaround.

[AndywinXp]

Personally I would avoid inserting new workarounds/hacks. We've began removing those :-P

To me this sounds like one of two things: a timing bug (does slowing down the game timer change anything?), or a sound driver bug (should newer sounds interrupt older ones)?

[dwatteau]

Yeah, it's a very minor issue in that game, so I agree that it's probably better not to add a script workaround for it if the problem is in our audio or timer code. Reminds me a bit of the issues regarding Maniac Mansion NES audio being a bit off, too.

[eriktorbjorn]

Fair enough. But I have no idea what the timing should be, and it's MixerImpl::playStream() itself that blocks the sound for being a duplicate of an already playing one.

[dwatteau]

I wonder if this isn't a matter of reimplementing the Amiga ​https://github.com/scummvm/scummvm/pull/3598 changes there, since Indy3 Mac and Indy3 Amiga apparently have similar sound structures.

[eriktorbjorn]

This isn't a Mac music player issue, as far as I understand. It's just a sound effect that gets passed on to Sound::playSound(), where it's played as one of these:

	else if ((_vm->_game.platform == Common::kPlatformMacintosh) && (_vm->_game.id == GID_INDY3) && READ_BE_UINT16(ptr + 8) == 0x1C) {
		// Sound format as used in Indy3 EGA Mac.
		// It seems to be closely related to the Amiga format, see player_v3a.cpp
		// The following is known:
		// offset 0, 16 LE: total size
		// offset 2-7: ?
		// offset 8, 16BE: offset to sound data (0x1C = 28 -> header size 28?)
		// offset 10-11: ? another offset, maybe related to looping?
		// offset 12, 16BE: size of sound data
		// offset 14-15: ? often the same as 12-13: maybe loop size/end?
		// offset 16-19: ? all 0?
		// offset 20, 16BE: rate divisor
		// offset 22-23: ? often identical to the rate divisior? (but not in sound 8, which loops)
		// offset 24, byte (?): volume
		// offset 25: ? same as volume -- maybe left vs. right channel?
		// offset 26: ?  if != 0: stop current sound?
		// offset 27: ?  loopcount? 0xff == -1 for infinite?

[dwatteau]

Sorry, I should have been clearer about what I was thinking about!

In the part that you are quoting, there's this:

// Sound format as used in Indy3 EGA Mac.
// It seems to be closely related to the Amiga format, see player_v3a.cpp
...
// offset 26: ?  if != 0: stop current sound?
...

and then in the PR above, there's now that in player_v3a.cpp, for Indy3 Amiga:

// Offset 0x2C with nonzero value delays until reading the next packet
// Offset 0x2C with zero value stops playback immediately
// The other offsets are unknown, but they are never used

// Indy3 always uses 0x18, 0x2C-nonzero, then 0x2C-zero
// Loom doesn't use these at all
...

(​https://github.com/scummvm/scummvm/blob/c2821ab37a129239279ccb1afb0f2ce0fc9d4c11/engines/scumm/players/player_v3a.cpp#L238-L263)

i.e. I'm wondering if we're not missing or misinterpreting something regarding offset 26 in Indy3 Mac sound format (e.g. maybe some samples are meant to interrupt any previous ones).

Anyway, this is just a small idea I had in mind. It likely is a wrong one :p

[AndywinXp]

It might actually be the correct assumption, good catch! Since we have no disasm to check, it would be nice to try and ask someone in the Discord server (maybe athrxx might know something about mac drivers?).

Otherwise we could limit this theoretical change to indy3, and have it as a PR (something like this would need extensive testing, to ensure that nothing else is broken).

[AndywinXp]

A good place to start would be to check if INDY3 does subsequent calls of those opcodes, in the fashion of the Amiga driver.

[eriktorbjorn]

Replying to dwatteau:

i.e. I'm wondering if we're not missing or misinterpreting something regarding offset 26 in Indy3 Mac sound format (e.g. maybe some samples are meant to interrupt any previous ones).

Anyway, this is just a small idea I had in mind. It likely is a wrong one :p

Ah, I see now. Well, it doesn't not make sense at least. :-)

I guess it might be interesting to extract all the sound resources, check what they're likely used for, and compare their headers. Comparing them to the Amiga version's sounds may be interesting too, but I don't have that one.

[dwatteau]

Thanks. I don't have the Amiga version either, unfortunately…

However, for what it's worth, with a very naive change like this one:

diff --git a/engines/scumm/sound.cpp b/engines/scumm/sound.cpp
index bca8581cc30..458bd3c976a 100644
--- a/engines/scumm/sound.cpp
+++ b/engines/scumm/sound.cpp
@@ -548,7 +548,7 @@ void Sound::playSound(int soundID) {
 		// offset 22-23: ? often identical to the rate divisior? (but not in sound 8, which loops)
 		// offset 24, byte (?): volume
 		// offset 25: ? same as volume -- maybe left vs. right channel?
-		// offset 26: ?  if != 0: stop current sound?
+		// offset 26: ?  if == 0: stop current sound? (bug #13887)
 		// offset 27: ?  loopcount? 0xff == -1 for infinite?
 
 		size = READ_BE_UINT16(ptr + 12);
@@ -574,6 +574,9 @@ void Sound::playSound(int soundID) {
 			stream = plainStream;
 		}
 
+		if (!ptr[26])
+			_mixer->stopID(soundID);
+
 		_mixer->playStream(Audio::Mixer::kSFXSoundType, nullptr, stream, soundID, vol, 0);
 	}
 	else {

the 3 sound effects for the bell can be heard again in the Macintosh version. I haven't done a full gameplay to check for any regression, but I've loaded a few dozen saves and it all seems fine. There's a similar trick around line 851.

[dwatteau]

The change above was submitted with this PR and was merged:
​https://github.com/scummvm/scummvm/pull/4547

It also fixes the thunder sound effect when Indy is outside the windows of Castle Brunwald (room 13), as checked (by ear) against the original Macintosh intepreter. No regression was found during a full gameplay with this change.