search

Thunderforge / scummvm-issues

ScummVM main repository
https://www.scummvm.org
GNU General Public License v3.0
0 stars 0 forks source link

BACKENDS: IOS: MP3 playback failing, libmad writing out of bounds #26

Open Thunderforge opened 2 years ago

Thunderforge commented 2 years ago

I've ripped and encoded the King's Quest 6 end credits CD track to track1.mp3 but the IOS build crashes immediately upon playback. Windows and Mac builds work. I've tried several unrelated mp3s. Failure occurs during construction of MP3Stream due to libmad's mad_synth_frame() writing beyond its struct and corrupting adjacent values. This looks unrelated to the game or CD-audio context in which it's being used. I think any mp3 playback with MP3Stream would crash.

class BaseMP3Stream : public virtual AudioStream {
    ...
    mad_synth _synth;
    uint _channels; // overwritten with 0 by mad_synth_frame()
    uint _rate;     // overwritten with 0 by mad_synth_frame()

I debugged this sequence:

  1. MP3Stream constructor correctly sets _rate to 441000
  2. MP3Stream constructor calls BaseMP3Stream::decodeMP3Data()
  3. BaseMP3Stream::decodeMP3Data() calls mad_synth_frame(&_synth, &_frame) which overwrites _channels and _rate with zero
  4. Audio::Timestamp fails an assertion because 0 is an illegal rate

I'm building with Xcode 12.4 (latest) on an M1 with macos 11.2.3 (latest) for an iphone 12 mini with ios 14.4.1. I'm using the instructions on the wiki and scummvm-ios7-libs-v2.zip. I tripped this a few months ago with earlier versions and now got around to debugging it.

To reproduce with attached KQ6 CD save:

  1. Name any mp3 "track1.mp3" and put it in the game directory
  2. Set "Text and speech" to Speech in ScummVM Audio options (Subtitles will trigger alternate credits without music)
  3. Load the save and wait three seconds

I suspect that any MP3Stream usage from any engine will trigger this. I'm hoping something is just off about the prebuilt libmad. Happy to help test.

kq6-cd.025.zip

Thunderforge commented 2 years ago

This is indeed a known issue with any game that use mp3. This was reported on the forum a few times, and I have seen the issue myself. However it looks like we never created a bug report for it. So thank you for doing it.

My conclusion were the same as yours, and I am indeed hoping that recompiling libmad for iOS will fix the issue. I just never go around to doing it. This is listed as a task in our iOS/Android Trello board.

Drascula is a good game to use for testing as it is available from our download page with a mp3 music addon available, and it crashes right at the start.

Thunderforge commented 2 years ago

Oh very good, I'm glad it wasn't just me.

Thanks for keeping up the ios support, criezy. I love having Dagger of Amon-Ra on me at all times; it's a security blanket and genuinely hilarious.

There should be a competition to see who has the funniest organization codesigning their phone's ScummVM. =)

Thunderforge commented 2 years ago

This would be nice to get fixed before the release.

Thunderforge commented 2 years ago

I'm not sure what builds scummvm-ios7-libs-v2.zip, but the md_size.diff and length-check.patch patches for libmad in ​https://deb.debian.org/debian/pool/main/libm/libmad/libmad_0.15.1b-10.diff.gz could help, I believe.

Thunderforge commented 2 years ago

libmad.a is a fat MachO with 4 architectures: arm, arm64, x86, x86_64. libmad.h is an include file with platform specific defines which seem to come from x86 build (note the FPM_INTEL define).

I suspect that the overflow comes from this discrepancy between the header used by ScummVM code to allocate the structure and the code included in libmad.a.