MP2 support

[Ithamar]

Godot version

4.0beta17

System information

Windows 10, any renderer.

Issue description

For several game remakes (including Darkstone: The Evil Reigns) I need MP2 support since much of the original music is using that format.

Now,Godot uses minimp3 for MP3 support, which has MP2 support, but it is currently disabled at build time.

Would you guys be open to a patch to the build system that would allow choosing whether or not to include MP2 support? I can disable it by default, but it would be much nicer that way then having to include another copy of minimp3 just for adding MP2 support.

Steps to reproduce

Add an MP2 file to the project. Notice it doesn't get imported.

Minimal reproduction project

N/A

[Calinou]

Can you compare stripped binary size of release export templates (target=template_release production=yes) when compiling with and without MP2 support?

[Ithamar]

Can you compare stripped binary size of release export templates (target=template_release production=yes) when compiling with and without MP2 support?

Okay, so after running the two builds, the difference in size 3584 bytes, rather small I might say.

[Ithamar]

For anyone wondering, the following will enable mp2 (simply adding mp1 as extension will also add mp1 support):

diff --git a/modules/minimp3/audio_stream_mp3.cpp b/modules/minimp3/audio_stream_mp3.cpp
index 6af86a96dc..a64e0870cc 100644
--- a/modules/minimp3/audio_stream_mp3.cpp
+++ b/modules/minimp3/audio_stream_mp3.cpp
@@ -28,7 +28,7 @@
 /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
 /**************************************************************************/

-#define MINIMP3_ONLY_MP3
+//#define MINIMP3_ONLY_MP3
 #define MINIMP3_FLOAT_OUTPUT
 #define MINIMP3_IMPLEMENTATION
 #define MINIMP3_NO_STDIO
diff --git a/modules/minimp3/resource_importer_mp3.cpp b/modules/minimp3/resource_importer_mp3.cpp
index 6a04e40c73..2a622fee86 100644
--- a/modules/minimp3/resource_importer_mp3.cpp
+++ b/modules/minimp3/resource_importer_mp3.cpp
@@ -47,6 +47,7 @@ String ResourceImporterMP3::get_visible_name() const {
 }

 void ResourceImporterMP3::get_recognized_extensions(List<String> *p_extensions) const {
+       p_extensions->push_back("mp2");
        p_extensions->push_back("mp3");
 }

[Calinou]

For anyone wondering, the following will enable mp2 (simply adding mp1 as extension will also add mp1 support):

If disabling that define also enables MP1 support, we might as well expose it too 🙂

[Ithamar]

If disabling that define also enables MP1 support, we might as well expose it too 🙂

Yeah agreed, I just didn't realise it until I pasted the patch here 😛

So, should I create a PR to simply remove the MINIMP3_ONLY_MP3 (and add .mp1/.mp2 to the supported extensions list) or do we need someone else to okay that first?