Replace minimp3 with dr_mp3

[DeeJayLSP]

Last time minimp3 got updated was in November 2021 (almost 3 years ago by the time this PR was opened).

Meanwhile, dr_mp3, a piece of work done on top of minimp3, has been updated quite recently.

Within Godot, it fixes a few issues:

• No need for patches;
• No more MSVC warnings (removed system header treatment);
• Possibility of overriding the memory allocator to use Godot's own.
• No more link-time optimization warnings (supersedes Revert "MP3: Remove unnecessary memory allocation" #109788).

Template release binary sizes remain the same.

Since minimp3 is removed, the following are also changed in this PR:

• minimp3 module renamed to mp3, to better reflect its purpose rather than naming its dependency;
• minimp3_extra_formats build variable renamed to mp3_extra_formats.

This should not break compatibility unless module/build option name changes count (does not affect existing projects™).

[DeeJayLSP]

Fixed an issue that caused single channel MP3 files to play badly.

This fix could also be applied to #80160 if it was viable

[DeeJayLSP]

It looks like a fix is coming: mackron/dr_libs@0463355

[DeeJayLSP]

Current version has the issue fixed. Despite being a dev version I didn't notice any problems.

Maybe it's safe to mark as ready now. I'll keep it updated while it's open.

[DeeJayLSP]

Considering that I'm also trying to make a module named dr_wav in #96545, should the module in this PR be named dr_mp3 instead of just mp3?

mp3 makes it clearer this is for MP3 support, while dr_mp3 has the name of the dependency, like it is already with minimp3.

[Calinou]

I would rename the module to just mp3. It's more intuitive for people using SCons options to disable features, and it follows the naming scheme of most existing modules more closely (gltf, fbx).

[DeeJayLSP]

Rebased on top of master. Added a dr_alloc_calls.h file also used by dr_wav in #96545.

Due to renaming an entire module folder, any minor changes to the minimp3 module require a rebase.

[adamscott]

Reviewed during the Audio team meeting. We will test the PR just to make sure and individually review it in the next days. But this PR should be approved and merged early during 4.6 dev cycle.

[Calinou]

Tested locally on the Run-time saving and loading demo, it works as expected. Code looks good to me.

I can confirm building with LTO also no longer reproduces warnings for MP3 code on GCC 15.2.1. There are still plenty of other LTO warnings produced though (e.g. by polypartition, VHACD, libjpeg-turbo, …).

Edit: There are some bugs with the PR, see #96547 (comment).

[Calinou]

I actually found that MP3 playback breaks on some longer files (resulting in audible stuttering and slowed down playback), even though it appears fine in the first 5 seconds of playback. It plays correctly in master.

Try playing this MP3 in the run-time loading/saving demo, for instance: Marc A. Pullen - Aeroembolism.zip

[mp3 @ 0x55f1d5a4a940] Estimating duration from bitrate, this may be inaccurate
Input #0, mp3, from 'Fanatic/Marc A. Pullen - Aeroembolism.mp3':
  Metadata:
    title           : Aeroembolism
    artist          : Marc A. Pullen
    album           : RTC-3057
    track           : 8
    comment         : Free download from http://www.last.fm/music/Marc+A.+Pullen and http://MP3.com
  Duration: 00:04:00.04, start: 0.000000, bitrate: 128 kb/s
  Stream #0:0: Audio: mp3 (mp3float), 44100 Hz, stereo, fltp, 128 kb/s

This one is fine though, in comparison: Crimson Blue - Iceland -02.zip

Input #0, mp3, from 'Crimson Blue/Iceland/02.mp3':
  Metadata:
    title           : Ave Sensorium
    album           : Iceland
    track           : 2/5
    copyright       : 2011 Crimson Blue. Licensed to the public under http://creativecommons.org/licenses/by-sa/3.0/ verify at http://www.jamendo.com/album/87605/
    artist          : Crimson Blue
    encoded_by      : Jamendo : http://www.jamendo.com/ | LAME
    TLEN            : 257000
    TOFN            : 02 - Ave Sensorium.mp3
    creation_time   : 2011-3-19
    TDOR            : 2011-03-19
    comment         : http://www.jamendo.com/
    genre           : Classic Rock
    lyrics-eng      : [...]
    http://www.jamendo.com/: http://www.jamendo.com/
    date            : 2011
  Duration: 00:04:16.42, start: 0.025056, bitrate: 188 kb/s
  Stream #0:0: Audio: mp3 (mp3float), 44100 Hz, stereo, fltp, 188 kb/s
      Metadata:
        encoder         : LAME3.96r
      Side data:
        replaygain: track gain - -7.500000, track peak - unknown, album gain - unknown, album peak - unknown,
  Stream #0:1: Video: mjpeg (Baseline), yuvj420p(pc, bt470bg/unknown/unknown), 400x400 [SAR 72:72 DAR 1:1], 90k tbr, 90k tbn (attached pic)
      Metadata:
        title           : Crimson Blue : Iceland
        comment         : Cover (front)
  Stream #0:2: Video: png, rgb24(pc, gbr/unknown/unknown), 200x50, 90k tbr, 90k tbn (attached pic)
      Metadata:
        title           : http://www.jamendo.com
        comment         : Lyricist/text writer
  Stream #0:3: Video: png, rgba(pc, gbr/bt709/bt470m), 88x31 [SAR 2835:2835 DAR 88:31], 90k tbr, 90k tbn (attached pic)
      Metadata:
        title           : http://creativecommons.org/licenses/by-sa/3.0/
        comment         : Other

Notice how the first one has no bitrate information, so it has to be guessed from the file by ffprobe.

[DeeJayLSP]

Rebased.

I actually found that MP3 playback breaks on some longer files (resulting in audible stuttering and slowed down playback), even though it appears fine in the first 5 seconds of playback. It plays correctly in master.

I tried to modify the PR back then so the code would be similar to the Ogg Vorbis player. Reverting that seems to have solved this issue (given file plays normally).

[goatchurchprime]

I just compiled and checked that this PR works on nixos (linux) after the audio meeting, so it looks good to me.

Do you think it's possible to write down a "Review date" in the future in the code of frozen third-party libraries like this, when developer teams are encouraged to check upstream for any important versions that should be pulled in?

[DeeJayLSP]

Do you think it's possible to write down a "Review date" in the future in the code of frozen third-party libraries like this, when developer teams are encouraged to check upstream for any important versions that should be pulled in?

Because a single repository had tags for a MP3, WAV and FLAC library, I was unsure of how to inform the tags correctly in the thirdparty README so I just pulled it from the latest git.

But I recalled: the three tend to have a release at the same time. I have modified so it uses the latest dr_mp3 tag instead. In case we implement the others in the future, we can just the correct tag for the latest used one.

[Calinou]

I can confirm the playback issue I mentioned above is fixed now.

[DeeJayLSP]

Rebased after recent changes.

[adamscott]

Didn't test any MP3 files, but the code seems right. Thanks @DeeJayLSP !

[DeeJayLSP]

Removed the public-domain declaration ahead of #112006