Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Secret of Monkey Island audio bug #385

Closed
flashmasta opened this issue May 21, 2020 · 7 comments
Closed

The Secret of Monkey Island audio bug #385

flashmasta opened this issue May 21, 2020 · 7 comments
Assignees
Labels

Comments

@flashmasta
Copy link

@flashmasta flashmasta commented May 21, 2020

When you go to talk to a pirate using dosbox-staging 0.75 final the audio goes crazy using the CD version of the game. I tested the same scenario in dosbox-x 0.83.1 and dosbox 0.74-3 and the audio is fine so it seems to be a dosbox-staging specific bug. I created a video to show what I am talking about here https://www.youtube.com/watch?v=s3l4MMQTIEw&feature=youtu.be (start @ 1:30 for dosbox-staging and 3:00 for dosbox-x, I didn't record my test of dosbox but it was the same as dosbox-x).

You can also tell with dosbox-staging there are loud audio "pops" that seem to occur when audio tracks are changing? You can hear them when you listen closely, they are more obvious in person and it seems like OBS made it quieter. The popping may be related to the "pirate" bug since you can tell the music cuts out and comes back in, which is normal, but on staging at least when it comes back in it is static.

@kcgen
Copy link
Member

@kcgen kcgen commented May 21, 2020

Thanks @flashmasta! I'll look into this.

@kcgen kcgen self-assigned this May 21, 2020
@kcgen kcgen added the bug label May 21, 2020
@kcgen
Copy link
Member

@kcgen kcgen commented May 21, 2020

@flashmasta, thanks for the detailed report and careful listening!

When you go to talk to a pirate using dosbox-staging 0.75 final the audio goes crazy using the CD version of the game.

One of the optimizations in the CD-DA code is to perform sequential audio decode in the audio tracks, without seeking (if possible). In this scenario with a bin/cue, we're essentially using a single binary track, however when we speak to the pirate the game pauses the audio, loads that dialog as data (performing a seek and binary read), and then unpauses the audio.

This revealed a bug in this sequential decode: when the pirate scene resumes playback, the last position used happens to be where we left off loading the pirate's data, causing the garbled audio. This pause--load-data--resume pattern appears to be an extremely rare behavior (even though it makes perfect sense) so I'm very thankful you noticed and reported it!

The branch below now keeps track of the last audio position in all audio-related events, and now confirms and re-positions the track on decode (only if needed).

You can also tell with dosbox-staging there are loud audio "pops" that seem to occur when audio tracks are changing? You can hear them when you listen closely, they are more obvious in person and it seems like OBS made it quieter.

Yes, with the BIN/CUE, I noticed this when leaving the tavern. As you exit through the door, the game requests that the current track be stopped, and then moments later starts playing the outside track. When the game asks the track to stop, staging halts the CD-DA mixer channel.

The pop is due to audio data going from 0 to an audible magnitude or vice-versa. There are ways to address this (some games rapidly ramp the CD-DA volume up when enabling playback and ramp down before stopping playback). This is something we could add so it always happens - however it would introduce small delays on the front and back of these start and stop events. This isn't how real hardware behaves though (physical CDROM can contain discontinuous steps in magnitude across tracks, likewise I haven't heard of CDROM players ramping the volume on the front and back of playback or pause events).

For now, I've added a small change to inject "silence filler" around these events. Does it help? Another option is to rip to FLAC or Opus; I haven't heard any pops when using these audio tracks.

Test branch: kc/bin-pos-fix-1, and test binaries:

@flashmasta
Copy link
Author

@flashmasta flashmasta commented May 21, 2020

Thanks, that test build fixes the audio bug when talking to the pirate. I didn't notice a reduction in the popping noise (there is a loud one right when the game starts, right before the Lucasfilm logo, for example).

The popping noise, at least to some degree or frequency, is something that I am not 100% sure was or was not something I remember hearing when playing it originally. I did another short playthrough in dosbox-x to compare a notice a tiny pop during the game for the first time and now I am really second guessing myself. I also did another test in doxbox 0.74-3 and heard a pop before the Lucasfilm logo.

It was one of the first games I got with the Soundblaster 16/CD-ROM combo around 1995 (it might have been a pack-in) and the way memories work the more I think about it the more I question myself if I heard any popping 25 years ago. Without that original hardware I can't be sure if it isn't supposed to pop at all or just a small amount.

I would be concerned that the "fix" might create unintended side effects in other software. I really wish I still had my old 486SX/25 around for a sanity check. I would use ripped audio but I'm weird about playing it exactly how I did back in '95. I have the remaster that adds Talkie but just isn't the same.

@dreamer
Copy link
Member

@dreamer dreamer commented May 21, 2020

@flashmasta Thank you for confirmation. Can you share the info, how do you have CD audio tracks mounted? ISO, cue/bin, cue/ogg, cue/mp3 (or other format)?

@kcgen About ramping volume up/down - I heard it in physical CD players (in towers or car CD players), but I don't remember if I've ever heard it during playback from PC attached CD-ROM. I am quite sure I remember those "pops" when playing Worms in 0.74-3, but your CD-DA work removed that problem.

Perhaps this fix would be a candidate for backport to be released in 0.75.1?

@flashmasta
Copy link
Author

@flashmasta flashmasta commented May 21, 2020

I have it mounted as bin/cue. I have used the following commands with the same results:

imgmount d "location of file.cue" -t cdrom
imgmount d "location of file.cue" -t iso -fs iso

@kcgen
Copy link
Member

@kcgen kcgen commented May 21, 2020

I heard it in physical CD players (in towers or car CD players

@dreamer, very interesting; in that case, given (probably high-end) hardware did this, I think we should also try.

Perhaps this fix would be a candidate for backport to be released in 0.75.1

Yes; especially the bug fix. If we find a sweet spot with the CD-DA fade-in/out, that's probably a nice addition too.

@kcgen
Copy link
Member

@kcgen kcgen commented May 21, 2020

I've added CD-DA volume fade-in and fade-out around start, stop, and pause-toggle events. Right now it's set to complete the transition within 15ms, which is roughly one video frame.

Here are the discrete volume steps on playback start and un-pausing (150 steps), followed by the same steps down when pausing and stopping (another 150 steps):

Screenshot at 2020-05-21 11-47-58

Edit: Hmm.. the fading occurs serially in the same thread as the playback - so the volume ramps are happening before and after playback, instead of in parallel to the playback (which is what we need).

One option is to add a thread to perform the fade in parallel to decode process. These two processes share the volume multiplier, which the fade would incrementally change. Another (likely cleaner) option would be to have the CD-DA player's callback drive the ramp-down - however this would be inline in the decode process, so we'd want this as minimal as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants