Opened 4 years ago
Closed 4 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 |
Description
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 GOG.com (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)
Change History (19)
comment:1 by , 4 years ago
by , 4 years ago
Attachment: | scummvm.log added |
---|
comment:2 by , 4 years ago
Workaround: Try to replace wave.cpp file in audio \ decoders folder and rebuild ScummVM
by , 4 years ago
comment:3 by , 4 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 , 4 years ago
Keywords: | white noise static removed |
---|---|
Owner: | set to |
Resolution: | → outdated |
Status: | new → pending |
Summary: | Myst sounds white noise static → MOHAWK: 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 , 4 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 , 4 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):
https://github.com/scummvm/scummvm/commit/fed5608e439e9e60a454e06fa27246ea5844bf9d
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 https://github.com/scummvm/scummvm/commit/4ea4627b1187b343ed95da8fcaa9d5292e9d0217 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 , 4 years ago
Resolution: | outdated |
---|---|
Status: | pending → new |
comment:8 by , 4 years ago
Ah, it appears it is as this bug had been previously reported as: https://bugs.scummvm.org/ticket/12523 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 , 4 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 , 4 years ago
Changing the Mohawk engine so that getResource() creates a SafeSeekableSubReadStream() instead of a SeekableSubReadStream() seems to work too.
by , 4 years ago
Attachment: | screen1.PNG added |
---|
by , 4 years ago
Attachment: | screen2.PNG added |
---|
by , 4 years ago
Attachment: | screen3.PNG added |
---|
comment:12 by , 4 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 , 4 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
`
comment:14 by , 4 years ago
Owner: | changed from | to
---|---|
Resolution: | → fixed |
Status: | new → closed |
I Confirm It. The game is interrupted. Log file in the attachment.