MP3 decoder access violation when attempting to read this file

[domsmart]

Hi all

We’re trying to debug a crash in Loopcloud, which results from trying to read certain MP3 files. This affects PC release only. Here’s an example of an offending MP3:

[https://www.dropbox.com/s/e7yo5iypzwzdkmy/beastie_boys__intergalactic_.mp3?dl=0]

When reading this file on a debug build, reading will repeatedly trigger the following assert in juce_MP3AudioFormat.cpp, but will continue to execute (albeit with corrupted audio data being output):

    if (bitrateIndex == 0)
    {
        jassertfalse; // This means the file is using "free format". Apparently very few decoders
                      // support this mode, and this one certainly doesn't handle it correctly!
        frameSize = 0;
    }

In PC release however, an access violation occurs in MP3Stream::layer2Step1, somewhere around line 2016:

        for (int i = 0; i < jsbound; ++i)
        {
            auto step = allocTable->bits;
            allocTable += (static_cast<intptr_t> (1) << step);
            si.allocation[i][0] = getBitsUint8 (step);
            si.allocation[i][1] = getBitsUint8 (step);
        }

This with optimisation level /O2. With optimisations disabled, it works just fine.

Anybody else encountered this, or can any of the Juce team give us some pointers (or preferably a fix) please? We don’t need to read valid audio data, but the crash is a problem for us, since we’re scanning users’ entire sample libraries, and one duff file lurking on a hard disk somewhere can cause the app to fall over.

[jimc]

Just bumping this one.

It’s causing problems. Any bright ideas anyone?

[jimc]

Dear JUCE team,

This crash is can be replicated in the AudioPlaybackDemo.

Steps -

1. Load audio playback demo (using JUCE 5.4.6 here)
2. Set ENABLE MP3AudioFormat in the projucer.
3. Build 64-bit release.
4. Load MP3 file provided above.
5. Crash.

[jimc]

Has anyone from JUCE seen this?!

[fefanto]

Hi Jim,
used to work on mpeg audio a lot a while back and out of curiosity took a look at the file you posted.

First of all , is this an error occurring on a lot of files or is it a rare problem?

Some random testing follows…

(1) Checking the file with a frame analyzer (e.g. GitHub - Sjord/checkmate: Checkmate MP3 Checker is a free program that checks MP3 files for errors.), reports it as “broken”, here’s the cli tool report output

mpck-0.21.exe beastie_boys__intergalactic_.mp3

SUMMARY: beastie_boys__intergalactic_.mp3
    version                       MPEG v1.0
    layer                         3
    average bitrate               139333 bps (VBR)
    samplerate                    48000 Hz
    frames                        7882
    time                          3:09.168
    unidentified                  1745 b (0%)
    errors                        unidentified bytes
                                  inconsistent frame headers
                                  invalid header values
                                  CRC error
    result                        Bad

This can be due to some headers not having correct values , or to the file implementing some edge cases of the mpeg standard that the decoder has no valid implementation for.

(2) On the other hand Opened the same file with Adobe Audition and it did not flinch an eye … even tough lots of products that manage mpeg audio may have a more “liberal” approach
e.g. read the first header and if the bitrate is constant assume all following frames never change.

(3) then tried to set up some kind of workaround e.g. decodec and reencode, using lame, and got this, which at first sight tells me this file is really problematic: lame is a very mature player in the mpeg audio player space, even tough again this is up a lot ot how “liberal” the encoder is.

C:\Users\Michelangelo\Downloads>lame --decode beastie_boys__intergalactic_.mp3
input:  beastie_boys__intergalactic_.mp3
        (48 kHz, 2 channels, MPEG-1 Layer II)
output: beastie_boys__intergalactic_.wav  (16 bit, Microsoft WAVE)
skipping initial 241 samples (encoder+decoder delay)
Frame#     2/2864   384 kbps  L  R  ihip: bitstream problem, resyncing skipping 18227 bytes...
Frame#     3/2864    32 kbps  LMSR  Ihip: bitstream problem, resyncing skipping 22264 bytes...
Frame#     4/2864   112 kbps         hip: bitstream problem, resyncing skipping 5965 bytes...
Frame#     5/2864   320 kbps         hip: bitstream problem, resyncing skipping 9842 bytes...
Frame#     6/2864   256 kbps         hip: bitstream problem, resyncing skipping 30882 bytes...
Frame#     7/2864   160 kbps         hip: bitstream problem, resyncing skipping 72 bytes...
Frame#     8/2864   320 kbps  LMSR   hip: bitstream problem, resyncing skipping 13497 bytes...
Frame#     9/2864    56 kbps         hip: bitstream problem, resyncing skipping 28027 bytes...
Frame#    10/2864    48 kbps  LMSR  ihip: bitstream problem, resyncing skipping 12844 bytes...
Frame#    11/2864    80 kbps  LMSR  Ihip: bitstream problem, resyncing skipping 3287 bytes...
Frame#    12/2864   112 kbps  LMSR  ihip: bitstream problem, resyncing skipping 24884 bytes...
Frame#    13/2864   160 kbps         hip: bitstream problem, resyncing skipping 8820 bytes...
Frame#    14/2864    96 kbps  LMSR   hip: bitstream problem, resyncing skipping 5946 bytes...
Frame#    15/2864   320 kbps         hip: bitstream problem, resyncing skipping 10177 bytes...
Frame#    16/2864   112 kbps  L  R  ihip: bitstream problem, resyncing skipping 4010 bytes...
Frame#    17/2864   112 kbps  LMSR  ihip: bitstream problem, resyncing skipping 8245 bytes...
Frame#    18/2864    64 kbps         hip: bitstream problem, resyncing skipping 23357 bytes...
Frame#    19/2864    96 kbps  L  R   hip: bitstream problem, resyncing skipping 327 bytes...
Error: sample frequency has changed in MP3 file - not supported

(4) doing some more research, one decoder that seems to play friendly with the file is madplay (it’s open source so it would be possible to dive deep and see how it’s taking info out of this file without failing like juce decoder or lame decoder):

windows version here : RareWares - Other MP3 and MP2 encoders and decoders

[Conclusion] (while you wait for Juce fix)

in case this error is a lone error, my advice is to decode it / reencode it by hand

command line to generate a wav file:

madplay.exe beastie_boys__intergalactic_.mp3 -o wave:beastieboys_mad_decode.wav

then re-encode with lame

lame beastieboys_mad_decode.wav

which creates a beastieboys_mad_decode.mp3 file that now is playable by juce demorunner

On the other side, if you get a ton of errors like this one, you could setup a check+re-encoding process in a win or linux cloud machine using some combination of the mp3 checker above || a juce CLI decoder test || lame || madplay || ffmpeg || whatever cli tool you can get your hands on…

Of course in this case you’d run the risk of publishing audio which is formally correct but has glitches or errors of whatever nature, so you have to measure the benefit/cost ratio of this.

[jimc]

Amazing analysis. Sadly this is part of our sample management tool with thousands of installations and it’s causing crashes on user systems that are very hard for the user to diagnose (as we are analysis loads of files in parallel and the user probably isn’t even near their computer). So we are just going to need to patch the MP3 decoder so it doesn’t take the application down

[fefanto]

Ok so if that file is not under your control from the start (e.g. dragged on your client app by the user) then yes your options can be :

• having a functional decoder (“fixed” juce decoder, or you could get a commercial license of the library used by madplay) Even tough I suspect there is nothing to “fix” in juce decoder, it’s just not as liberal as some…that file is indeed broken in some way.
• have a graceful way to fail : implement some mp3 frame analysis like the mp3chek tool above and if the file is not “sane” just report it as unusable by the app.

[jimc]

We could switch to another library for decoding. But I’d rather someone from JUCE just patched JUCE

[asimilon]

@jimc

I’d rather someone from JUCE just patched JUCE

I second that, I have hit this assert once or twice in macOS, but it didn’t seem to cause a problem (maybe because of debug build?), but hearing it causes crashes in Windows release builds is a bit concerning.

[ed95]

Thanks for raising this and for the example to reproduce the crash. We’ve got a fix which will avoid crashing when reading dodgy mp3s like the one you provided and it’ll make its way onto the public JUCE branches shortly.

[jimc]

Thanks Ed!

That’ll make everyone happy. Pop the commit number here when you’re done and we will cherry pick it onto our branch.

J.

[domsmart]

Amazing! Thank you @fefanto for your investigation and @ed95 for the fix

[ed95]

MP3: Fixed a global buffer overflow and constexpr'd a few things