Opened 3 years ago

Closed 3 years ago

#12513 closed defect (fixed)

MOHAWK: Static in Myst Sounds (GOG ME Release)

Reported by: squigley Owned by: SupSuper
Priority: normal Component: Engine: Mohawk
Version: Keywords:
Cc: Game: Myst


I have just compiled the latest git code (ScummVM 2.3.0git15967-g6378f6d675 (May 3 2021 23:17:34)
Features compiled in: Vorbis FLAC MP3 ALSA SEQ sndio TiMidity RGB zLib MPEG2 FluidSynth Theora FreeType2 JPEG PNG TinyGL OpenGL). I also tried with ScummVM 2.0.0 (Mar 11 2018 21:56:27) from dpkg.

I am using Linux Mint 19, kernel 4.15.0-140-generic #144-Ubuntu SMP Fri Mar 19 14:12:35 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

I used to wine to install Myst ME from (setup_myst_masterpiece_edition_1.0_svm_update_4_(22598)-1.bin).

It detects the game, it loads and runs fine, from a graphics perspective, however when moving around between screens, there are horrible loud white noise/static blasts for a few seconds when moving to a new screen or new page of a book etc.

Attachments (5)

scummvm.log (94.6 KB ) - added by ctoroman 3 years ago.
wave.cpp (7.1 KB ) - added by ctoroman 3 years ago.
screen1.PNG (79.8 KB ) - added by ctoroman 3 years ago.
screen2.PNG (163.4 KB ) - added by ctoroman 3 years ago.
screen3.PNG (77.3 KB ) - added by ctoroman 3 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by ctoroman, 3 years ago

I Confirm It. The game is interrupted. Log file in the attachment.

by ctoroman, 3 years ago

Attachment: scummvm.log added

comment:2 by ctoroman, 3 years ago

Workaround: Try to replace wave.cpp file in audio \ decoders folder and rebuild ScummVM

by ctoroman, 3 years ago

Attachment: wave.cpp added

comment:3 by squigley, 3 years ago

Sorry for the delay, I just checked this yesterday and saw it was updated (not sure if I should have received an email).

I rebuilt with your attached file, and I tested for 15 minutes or so, and I didn't see (hear) the issue at all, so it seems like your version attached above resolves this issue.

comment:4 by digitall, 3 years ago

Keywords: white noise static removed
Owner: set to digitall
Resolution: outdated
Status: newpending
Summary: Myst sounds white noise staticMOHAWK: Static in Myst Sounds (GOG ME Release)

squigley: I think ctorman's workaround is basically to apply the recent fixes to WAVE decoder in the latest git master. Please try with the latest nightly build as I think this is fixed in the current development master, and confirm.

comment:5 by ctoroman, 3 years ago

digitall, I tried on ScummVM 2.3.0git17408-g8e4369cde3 (May 19 2021 05:27:06).
The bug remained, nothing was fixed.

comment:6 by digitall, 3 years ago

@squigley: OK. Thanks for confirming.

I have had a look at the modified wave.cpp provided by ctoroman and it is basically the state of wave.cpp prior to the following commit (but with a few spurious white space changes and minor bits leftover as well):

That commit migrated the wave decoder to using substream rather than locally manually allocating memory buffers so probably is the cause of the issue i.e. if it deallocates
before it should, invalid memory could be played i.e. static.

It is possible that the later change is also causing this.

Since you can compile your own git version, I would suggest trying bisecting for this using git-bisect to confirm... or just manually trying the commit prior to fed5608 and fed5608 to see if this is the cause.

comment:7 by digitall, 3 years ago

Resolution: outdated
Status: pendingnew

comment:8 by digitall, 3 years ago

Ah, it appears it is as this bug had been previously reported as: by ctorman who had determined that commit as the cause. Have notified the author of that commit so hopefully this should be fixed.

comment:9 by eriktorbjorn, 3 years ago

As an experiment, I made the following change to my local copy:

diff --git a/audio/decoders/wave.cpp b/audio/decoders/wave.cpp
index 67e0b31d83..c34cc3f64d 100644
--- a/audio/decoders/wave.cpp
+++ b/audio/decoders/wave.cpp
@@ -205,7 +205,7 @@ SeekableAudioStream *makeWAVStream(Common::SeekableReadStream *stream, DisposeAf
                        size &= ~(sampleSize - 1);
-       Common::SeekableReadStream *dataStream = new Common::SeekableSubReadStream(stream, stream->pos(), stream->pos() + size, disposeAfterUse);
+       Common::SeekableReadStream *dataStream = new Common::SafeSeekableSubReadStream(stream, stream->pos(), stream->pos() + size, disposeAfterUse);
        switch (type) {
        case kWaveFormatMSIMAADPCM:

This seems to fix the problem. My only guess is that the different sound resources point to the same file, and using the plain SeekableSubReadStream you can't be sure if the stream is still pointing to the correct position when it resumes playing a sound.

But I'm not at all sure.

comment:10 by eriktorbjorn, 3 years ago

Changing the Mohawk engine so that getResource() creates a SafeSeekableSubReadStream() instead of a SeekableSubReadStream() seems to work too.

comment:11 by ctoroman, 3 years ago

The sound was fixed, but other bugs appeared:

by ctoroman, 3 years ago

Attachment: screen1.PNG added

by ctoroman, 3 years ago

Attachment: screen2.PNG added

by ctoroman, 3 years ago

Attachment: screen3.PNG added

comment:12 by eriktorbjorn, 3 years ago

As far as I know, nothing has been changed to fix the sound yet. But come to think of it, if it can leave the audio stream at the wrong position it can probably leave other resource streams in the wrong position as well, which I guess could lead to the errors you describe.

If so, it probably won't help to use SafeSeekableSubReadStream, because unless I'm mistaken audio runs in a separate thread, and this class is explicitly documented as "*not* threading safe".

I wonder if this is a problem only with Myst, or if other engines are impacted as well?

comment:13 by eriktorbjorn, 3 years ago

Bud Tucker in Double Trouble seems to be affect too, but I don't yet understand why. That one seems to just play a looping WAV directly from a file.

But that's what bisecting pointed to:

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[fed5608e439e9e60a454e06fa27246ea5844bf9d] AUDIO: Wrap raw streams in SeekableSubReadStream instead of allocating them
Last edited 3 years ago by eriktorbjorn (previous) (diff)

comment:14 by SupSuper, 3 years ago

Owner: changed from digitall to SupSuper
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.