Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the MP3 audio codec #85

Closed
davthedev opened this issue Sep 19, 2019 · 4 comments
Closed

Add support for the MP3 audio codec #85

davthedev opened this issue Sep 19, 2019 · 4 comments

Comments

@davthedev
Copy link

Describe the project you are working on:

A rhythm game (think Guitar Hero / Dance Dance Revolution / Audiosurf). There will be a feature where players will be able to play the game over their own songs.

Describe how this feature / enhancement will help your project:
As a developer, I have no issues sticking to OGG as preferred music format. It simplifies the task for licensing reasons and performance.

But, I would like users of my game to be able to easily import their own songs. And this is a different story. Asking them to convert their MP3 to OGG files can be a barrier. The in-game editor should be as easy as using an "open" command and locating the music file on the filesystem. It should work with whichever popular music formats (MP3, AAC...) are likely to be found.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

Describe implementation detail for your proposal (in code), if possible:

I am aware of the licensing issues that MP3 support brings around.

Offering audio codec modules in the form of standalone extensions could be a solution. The system could be extended to video codecs as well. Game that do not require licensed audio formats support may not need the extensions.

If this enhancement will not be used often, can it be worked around with a few lines of script?:

No. There is no additional audio codec support available at the moment.

Is there a reason why this should be core and not an add-on in the asset library?:

Competition (example : Unity) already support more formats out-of-the-box AFAIK. We also would need to have the API ready to support such extensions.

@Calinou
Copy link
Member

Calinou commented Sep 20, 2019

See also godotengine/godot#27236. There's a third-party module you can compile with Godot to get MP3 support: https://github.com/DeleteSystem32/godot-minimp3

I am aware of the licensing issues that MP3 support brings around.

All MP3 patents expired in April 2017, so it should be fine to use now.

Since patents related to AAC decoders haven't expired yet, we can't implement AAC in Godot's core. However, nothing prevents someone from providing a third-party module for it.

@akien-mga
Copy link
Member

akien-mga commented Oct 20, 2020

See also godotengine/godot#27236. There's a third-party module you can compile with Godot to get MP3 support: https://github.com/DeleteSystem32/godot-minimp3

@DeleteSystem32 We've discussed this with @reduz and @Calinou and we think that it would be good to have built-in MP3 decoding support, and minimp3 seems to be a good option. I tested your module on the current 3.2 branch and it seems to work fine in early testing.

The README says that it's experimental, what's the current state?

Would you be interested in contributing this module directly upstream to https://github.com/godotengine/godot?

It should be for the master branch in priority, though an additional PR with a 3.2 version can also be considered. Some comments from @reduz:

22:46 <reduz> Akien: the code looks good for minimp3, but it should be modified to use the proper seek API
22:47 <reduz> also in godot 4.0 the audio buffer memory thing is gone, regular memalloc/memfree  should do
23:05 <Akien> I tried the module on 3.2, it seems to work, including seeking
23:09 <reduz> yeah it has seeking but its not a good way to do it
23:10 <reduz> which is why i did not add it before
23:10 <reduz> but now minimp3 added proper seek support (https://github.com/lieff/minimp3) so it should use it

@DeleteSystem32
Copy link

Hello!

The README says that it's experimental, what's the current state?

I've been using it on a project without issues for quite a bit, and it's been stable. Essentially, I just haven't updated the readme to reflect this.

Would you be interested in contributing this module directly upstream to https://github.com/godotengine/godot?

Absolutely! I will take a look at fixing seeking as well as the changes for Godot 4.0.

@akien-mga
Copy link
Member

Fixed by godotengine/godot#43007.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants