Opened 19 months ago

Closed 13 months ago

Last modified 12 months ago

#15369 closed defect (fixed)

SCUMM: MI (Sega MegaCD) - audio pops at the end of every SBL sound effect

Reported by: dwatteau Owned by: eriktorbjorn
Priority: normal Component: Engine: SCUMM
Version: Keywords: segacd, sega, audio
Cc: Game: Monkey Island 1

Description

This happens with current Git HEAD, but AFAICS it's been this way for many years.

Whenever a sound effect is played in the SegaCD release of Monkey1, a "pop" sound can be heard at the end of it. It happens on all the systems I have tested.

I don't my game game resources have any problem, GAME.000 is the one in the detection tables, and GAME.001 even matches the BACKUP.001 that's included in the CD:

c13225cb1bbd3bc9fe578301696d8021  GAME.000
808f192970d39b0e9a7db0664b647372  GAME.001
808f192970d39b0e9a7db0664b647372  BACKUP.001

(The sound resources are *not* the Redbook audio CD tracks used for the background music.)

Running the original game with OpenEmu/GenesisPlusGX, I can confirm that the original game doesn't have this behavior.

Maybe there's something inaccurate in the way the SBL sound effects are handled for the SegaCD platform?

Attachments (1)

door-closing.png (58.8 KB ) - added by eriktorbjorn 14 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 by dwatteau, 19 months ago

Ah, I forgot to give two quick examples

  1. At the front of the SCUMM bar, open/close the door multiple times
  2. Go in front of the Governor's Mansion. The dogs bark; it's easy to hear the sound effect pops every time

comment:2 by athrxx, 19 months ago

You can see it in sound.cpp, line 449 - 465. Apparently, it is just guesswork test code that was never finished.

comment:3 by eriktorbjorn, 14 months ago

I just got my hand on an affordable loose CD of this.

I think there's something funny going on here... Take the door opening sound as an example, since it happens so early in the game.

The resource size is 4,534 bytes.

The first four bytes is the "sound tag". In this case, it's SBL .

The size of the SBL chunk is 4,526 bytes, which makes sense because we've read the first eight bytes to find this. But we ignore the first 27 bytes, and go directly to the VOC header. The VOC part of the resource is therefore assumed to be 4,499 bytes.

This part of the resource is then unscrambled.

The size of the VOC header is 6 bytes. The audio data begins after that, so there should be at most 4,493 bytes. But according to the VOC header there is 4,500 bytes.

We get something similar for the door closing:

The resource is 122 bytes. The SBL chunk is 114 bytes (8 bytes less), and the VOC part is assumed to be 27 bytes less than that so 87 bytes. But the VOC header then says the audio data is 88 bytes.

comment:4 by eriktorbjorn, 14 months ago

As far as I can tell, after unscrambling the door opening/closing sound resources are identical to the ones from the VGA CD version (I didn't compare to any other), except for the last eight bytes.

So now I'm going to assume that my above analysis is slightly incorrect in some way, and that we just need to unscramble more bytes.

by eriktorbjorn, 14 months ago

Attachment: door-closing.png added

comment:5 by eriktorbjorn, 14 months ago

This is my attempt at dissecting the "close door" sound, since it's small enough to visualize as text:


  • The SBL tag (4 bytes)
  • The size of the SBL block (4 bytes). This is 114, and covers every single byte of the remaining resource, including padding.
  • The AUhd and AUdt data that we don't care about (19 bytes). This is where the +27 comes from.
  • This is where the scrambled data begins. The first byte should always be 1 after unscrambling.
  • The next 3 bytes are the size of the audio data, plus 2. 90 - 2 = 88. It's probably to account for the next two bytes not being part of the audio data.
  • The next byte is the sample rate.
  • This is followed by 1 byte of padding.
  • The next 88 bytes is the audio data.
  • The final byte is probably just to pad it to an even number of bytes.

So we need to unscramble 6 bytes of VOC header data, and 88 bytes of audio data. 96 bytes in total.

But what we actually unscramble is 114 - 27 = 87 bytes.

I think I see now! We're adding 27 to the resource pointer to get to the VOC header, which is correct. But we also subtract 27 from the SBL size, which is incorrect. Because the SBL data does not include the first 8 bytes of the resource. We need to subtract by 19 instead. As far as I can tell, this bug has been there for 20+ years.

diff --git a/engines/scumm/sound.cpp b/engines/scumm/sound.cpp
index 3fb7b6e2050..55e1dbf609a 100644
--- a/engines/scumm/sound.cpp
+++ b/engines/scumm/sound.cpp
@@ -272,7 +272,7 @@ void Sound::triggerSound(int soundID) {
                // 80 80 80 80 80 80 80 80  |........|
                // 80 80 80 80 80 80 80 80  |........|

-               size = READ_BE_UINT32(ptr + 4) - 27;
+               size = READ_BE_UINT32(ptr + 4) - 19;
                ptr += 27;

                // Fingolfin says: after eyeballing a single SEGA

This size is only used for the descrambling, so changing it should not affect anything else. I can't commit right now, but I'll do it later today unless there are any objections.

comment:6 by dwatteau, 14 months ago

I absolutely love how well you've documented your findings here, Erik. Thanks a lot. It's really informative for me to know how you proceeded and how that part works.

I can load my various saves and/or a full SegaCD gameplay with your diff if you’d like.

in reply to:  6 comment:7 by eriktorbjorn, 14 months ago

Replying to dwatteau:

I absolutely love how well you've documented your findings here, Erik. Thanks a lot. It's really informative for me to know how you proceeded and how that part works.

I actually didn't find the problem until I had drawn most of that image. :-)

I can load my various saves and/or a full SegaCD gameplay with your diff if you’d like.

That would be great! I only got the game yesterday, so I haven't had much time to look at it myself.

comment:8 by Torbjörn Andersson <eriktorbjorn@…>, 14 months ago

In e96f022c:

SCUMM: Fix audio pops at the end of Sega CD MI1 (bug #15369)

A error in how the size of the audio data was calculated meant that the
last eight bytes were never decrypted. As far as I can tell, this bug
was there for more than twenty years! Other than the encryption, the
sound resources appear to be identical to the VGA CD ones.

comment:9 by eriktorbjorn, 14 months ago

Owner: set to eriktorbjorn
Resolution: fixed
Status: newpending

I've committed my fix, and added a comment to make sure no one changes it back by mistake.

comment:10 by dwatteau, 14 months ago

Thanks, so far the results sound very good. I don't have access to all my old SegaCD saves this week, but I'll report any issue once I get back to them.

comment:11 by dwatteau, 13 months ago

So, I've played my own SegaCD copy of Monkey1 with today's Git HEAD, and it all looks good -- even with my 2009, 2010 or 2019 saves.

For some strange reason, though, I had to delete and add my SegaCD folder twice for the enable_sega_shadow_mode GUI option to appear. Weird (and I do have commit 5b6e9ca40f55c03bd4b6275af2f2d32bd6fdc004 in my build). First time, Extra was set to "SEGA". Then, it was empty, but in both cases, platform was always Common::kPlatformSegaCD.

That's not the first time that the SCUMM detection code has strange sync issues like those.

comment:12 by eriktorbjorn, 13 months ago

Status: pendingclosed

Odd. The presence of the CD version settings should depend on if "extra" is "CD", or if "platform" is FM-Towns or Sega CD, and the presence of the shadow mode checkbox in those settings should depend only on if "platform" is Sega CD. My first attempt based it on "extra", but that was unreliable as I recall it. The detection is still a bit of a mystery to me.

Anyway, I'm closing this bug report. Thanks for testing!

comment:13 by Torbjörn Andersson <eriktorbjorn@…>, 12 months ago

In f40b4850:

SCUMM: Fix audio pops at the end of Sega CD MI1 (bug #15369)

A error in how the size of the audio data was calculated meant that the
last eight bytes were never decrypted. As far as I can tell, this bug
was there for more than twenty years! Other than the encryption, the
sound resources appear to be identical to the VGA CD ones.

(cherry picked from commit e96f022cfdf0bca276fc304c098260419ccd870c)

Note: See TracTickets for help on using tickets.