Static MP3 source playing incorrectly

[Endaris]

One of my users approached me about a mp3 sound breaking in a user mod that played correctly with love 11.5.
This is a loud neighing sound so careful when using headphones.

In love 12 instead of the neighing, after some delay some light knocks followed by static noise can be heard.

Playback happens like this:

love.audio.play(love.audio.newSource(filePath, "static"))

The issue has been reproduced on Windows with SDL3 CI love as well as Linux with an older SDL2 CI love.

chain8.zip

A simple re-export in audacity fixes the problem for love 12 so it's not a big issue and I expect this to be a library issue like the LodePNG related crashes when moving from 11.3 to 11.4. If this indeed proves to not be fixable on love's end I think it would be good to document the potential breaking of audio files on the 12.0 wiki page.

[MikuAuahDark]

Handling MP3 is certainly difficult due to how they're detected technically. Not to mention that LOVE will try every possible decoders before proceeding. I did some debugging and here's what I found:

• The attached MP3 has invalid ID3v2 metadata. Specifically it says the metadata size is 0.
• LOVE happily consider the metadata length is 0 (this is invalid as per ID3 specification but this doesn't matter for this case, see below)
• LOVE start looking the first 128 bytes of the MP3 (after the ID3 metadata, if present). The reason why LOVE only looks 128 bytes is to prevent something like Garbled Audio In Some Cases #1803 happening. dr_mp3 look too much so delegating it to dr_mp3 is not an option. Sadly the first valid MP3 header in the file you attached is 960 bytes relative to the start of the file, outside the 128 bytes range LOVE tries to look at.
• LOVE doesn't found any valid MP3 header in the first 128 bytes of the data, bails out, proceed to give it to the next decoder.
• ModPlug decoder somehow can identify it as one of valid tracker formats, not throwing an error.
• LOVE happily accept the resulting ModPlug decoder, but the ModPlug decoder itself doesn't play the file properly.

As a user, what you can do is:

• Use better audio format, such as Vorbis (only if you have the original lossless audio file)
• Perform stream-copy of the MP3 file to correct the metadata (which is lossless fix), e.g. ffmpeg -i chain8.mp3 -c:a copy chain8_copy.mp3. Your approach of re-encoding using Audacity is half-step to the right direction. While it fixes the metadata, you're introducing double-compression, reducing the audio quality further.

[Endaris]

Thanks for the thorough analysis!
So I take it love 11.5 only parsed by file extension and as a tradeoff was more persistent when looking for the header and managed to identify it as a MP3.

From my side the issue is resolved but I assume you may want to keep it open to prevent false positives for ModPlug just like the current approach is intended to prevent false positives for MP3.

[MikuAuahDark]

To be honest, there's not much we can do for ModPlug.

[Endaris]

Fair enough.
I'm just spinning some thoughts so feel free to discard them:
As far as I can tell the current approach is to simply try decoders in a set order.
The issue you linked that caused the search range for an MP3 header to incorrectly detect a WAV as MP3 and successively play garbled audio references a commit in which the order of decoders had MP3 first, making WAV files go through the MP3 decoder before having a chance at the WAV decoder which surely would correctly identify them as WAV.
Since then the decoder array changed to MP3 moving to the second last spot before ModPlug so as I understand the reduction of search range may no longer be necessary to avoid incorrect decoding of WAV files because the WAV decoder is always tried first.
Unless there are concerns about ModPlug files being incorrectly detected as MP3 in a reverse of the issue with the present file, I imagine that reverting the reduction of search range may have no adverse effects.

Additionally, and I completely understand if you want to dismiss it, in a partial return to the previous approach, the MP3 decoder could be allowed an extended search range exclusively under the condition that the file extension is mp3. I understand if that is not feasible with the change to a stream-based approach.