Add support for MP3

[dexter3k]

The longest-running mp3- related patent is expired as of 16th April, 2017. This makes .mp3 patent-free in US too. Also licensing program has been terminated.
So, why not to add support for it in the Audio module?

• Implement decoder (reader): With minimp3, see MP3 reader support using minimp3 #1851 MP3 reader support #1796
• Implement encoder (writer): See Implement MP3 Encoder #2679

[j4cobgarby]

Correct me if I'm wrong but as far as I can tell, the actual specification for the mp3 format costs almost 200 euros.

[minirop]

you speak about the ISO document? not really useful since there are already LAME for decoding mp3.

[j4cobgarby]

Oh okay, if someone assigns me to this I'll have a go

[MarioLiebisch]

Just do it, then create a pull request and refer to this issue by mentioning #1232 in the text.

[j4cobgarby]

Okay, on it

[ricanteja]

How is development on this coming along @j4cobgarby? I might take it on myself if you take too long :)

[j4cobgarby]

@ricanteja Feel free to have a go yourself - I've got a lot going on and probably won't be able to make much progress for a couple of weeks :)

[ricanteja]

Okay so I've been spending way too much time on this and not really making any progress (while surviving a hurricane). I've looked into MAD, mpg123, LAME to try and figure this out but they all have their challenges.
Challenge no.1 is getting these libraries to build. Damn auto make. The build system is so convoluted good luck trying to figure out what broke.
Challenge no.2 is licensing. MAD is GPL, while mpg123 and LAME are LGPL, but it doesn't matter because I can't build either one of them!

If anyone has any experience with GNU auto tools or has built mpg123 before please ping me in this thread. I'd love to get MP3 support in SFML but I'm starting to wonder if it is worth the headache. All this just to support 1 more file type?

[eXpl0it3r]

So we only got GPL or LGPL as option? Adding another must have shared library dependency doesn't sound very exciting...

I've played around with GNU auto tools on Windows a bit for the other dependencies, but it really isn't very nice, but still doable if their automake file is useful.

[ricanteja]

I don't even want to think about what compiling these libraries for mobile platforms will look like... shudders

I just don't think it is worth the headache at this point but I'm curious what others think.

[eXpl0it3r]

IMHO as long as we can't find a non GPL/LGPL MP3 decoder I wouldn't want to add it. Vorbis and soon opus offer better compression vs file size, but I do understand the wish for MP3 support as a multimedia library.

[feliwir]

I have used mpg123 in the past and it was working very nicely. I guess @ricanteja i struggeling to compile because of the assembler parts?

[lieff]

@eXpl0it3r Hi, I'm working on non GPL MP3 decoder: https://github.com/lieff/minimp3

[eXpl0it3r]

@lieff Interesting! 🙂
Any plans on encoding support? 😉

[lieff]

Currently no plans about encoder, but I can reconsider if it's needed =)

[eXpl0it3r]

Fact is that SFML currently provides encoding and decoding for all the supported formats, as such the argument for adding a new format in an asymmetrical way, is harder to get accepted.

[MarioLiebisch]

I'd claim that MP3 would be popular/useful enough to even accept asynchronous handling for now, especially considering it would cover most popular use cases.

[LaurentGomila]

Audio encoders and decoders are clearly split exactly for this purpose: to allow some formats to only have one. In my opinion, it's totally fine to be able to read MP3 but not write it. It should not block its integration into SFML.

[eXpl0it3r]

@ricanteja Do you feel motivated to give this another go with minimp3?

[LaurentGomila]

@lieff what's the status of your library? It looks complete but you said you were working on it, so I'm wondering if it's ready to be used.

[lieff]

@LaurentGomila It's fully feature complete. I'm still working on test procedure checking, check more compilers and further optimization. I will look at encoder later, I think it's not so hard to create quality/speed unefficient encoder.

[lieff]

I found BSD 2-Clause mp3 encoder from Andreas Johansson. Is it good enough license?

[LaurentGomila]

Sure it is.

[lieff]

Well then, I've created repo with it https://github.com/lieff/mp3-enc-bsd
Now we have both, decoder and encoder.

[LaurentGomila]

Do we have more information about this encoder? Has it been tested? Is it bug-free? etc.

[lieff]

Encoder testing is more complicated than decoder (because we have no precise reference to compare with). I've tested it briefly and found over-read problem ([mp3 @ 0x7fc72c002700] overread, skip -4 enddists: -1 -1), but files are playable and quality is acceptable. I will fix this problem later.

[lieff]

About encoder - I think It's from old freebsd/linux. Mentions can be found on old sites http://linux-sound.org/mpeg.html http://sunsite.univie.ac.at/Linux-soundapp/mpeg.html but archive is hard to find.

[lieff]

Well, yes. Will try get to this in next weekend.

[lieff]

Here rough mp3 reader support lieff@2e0d617 .

Currently whole file readed to memory at open() time. Need to finish mp3dec_ex_* api for proper streaming and seek to sample. Sample added to SFML/examples/sound/.

Is it enough to make first PR?

[LaurentGomila]

Is it enough to make first PR?

If it's not finished (proper streaming) then you should probably complete it first 😉

[lieff]

Not like it's not finished, It's dependent on usage pattern. Disadvantages of pre-decode whole file is memory usage and decode time. But since we need to fill info.sampleCount at open() time - this is whole file scan for mp3 anyway and there will be lag depending on hdd speed. So it may be acceptable in some cases and may be not in others.

Here simple benchmark: one hour mp3 file (88 megabytes), 48000 Hz, stereo, 192 kb/s.
Decode time - 3.4 seconds on i7-6700K CPU. So for more common ~8mb ~6min track it will be 0.34 seconds which can be acceptable lag. This scheme also can be further optimized using multi-threaded decode (which can't be used in streaming mode since frames dependent from previous and it needs big chunks of file).

Streaming mode brings less memory usage and open() lag decreases to file load time, without decode part. If we do not need precise track length knowledge - we can also omit file scan, but it needs API change?

[LaurentGomila]

Not like it's not finished, It's dependent on usage pattern.

The low-level decoding classes are supposed to be implemented using streaming; then it's up to higher-level classes to decide what to do (pre-load everything or stream content). Isn't streaming possible with MiniMP3?

But since we need to fill info.sampleCount at open() time

We can usually know the total sample count without decoding the whole file (using metadata). I would be surprised if the MP3 format didn't allow that.

So, it would really be better if the MP3 back-end was implemented like all others. If the library is too limited then we can have a look at others.

[lieff]

OK, I will finish streaming mode first, just bit more code (probably next weekend again).

We can usually know the total sample count without decoding the whole file (using metadata). I would be surprised if the MP3 format didn't allow that.

Yes, mp3 is more like elementary stream, not container format like ogg. Track length also can be stored in ID3 (TLEN Text information frame), but in milliseconds precision and may not present. There also can be "Info"/"Xing" VBR tags, but again it may not present in file and needed fields like num frames also optional. So we can avoid scan only if VBRTag found, needed fields present and this information is correct.

[lieff]

Here initial streaming version: lieff@7330cae

Still some improvements can be done:

• Handle VBR Tag and omit file scan (still needed if seek happens).
• Use binary search for seek instead of index scan (not sure if it helps much in real scenarios).
• Do not cache input file and re-read it after initial scan & rewinds?
• Tune skip samples on seek. In mp3 not all frames can be decoded form start because of bit-reservoir.

Also seek() needs some testing. How to add seek test in Sound.cpp example properly?

[lieff]

All major improvements have been done. I'll wait for feedback a bit and then make a PR.

[ipatix]

Are there any updates on the progress (if there is at all)?

I was looking for a library that can do sound including MP3 without having to combine multiple libraries manually.

[lieff]

@ipatix You can try https://github.com/lieff/SFML master branch. Currently I do not know any issues related to SFML integration, but there 2 things which stops me from release a PR:

• Big APEv2 tag at end of file prevents it's removal in streaming manner, i.e. we need seek at the end of file, (virtually) remove the tag and seek back. This is the only case when we need such jumps, id3v1 and small APEv2 can be safely removed by reading file chunks linearly. I think how better support this case with minimal API changes, for callback support we need ftell() function, or provide file size on open.
• I want to add scan limit to not read whole big file in case if it's not mp3 file (should not happen with SFML, because format detected first, but can happen if detection failed and detection can be improved).

Otherwise, it's should be ready to use.

[ipatix]

@lieff
I'll give it a try.

[1aam2am1]

Is there mp3 support now in 2021 or are there some problems with it?

[lieff]

https://github.com/lieff/SFML branch should work, with issues mentioned above.

[eXpl0it3r]

@lieff For the APEv2 issue, what's the impact from the view point of SFML? Would it crash? Fail to stop?

I think, even if there are some limits with certain flavors of MP3 formats, it might be beneficial to add it for now, maybe with a warning regarding APEv2 tags.

Just tested it locally and it worked great. 🙂

[lieff]

@eXpl0it3r It should not crash, worst case APEv2 data will be treated as mp3 and very low chance some garbage decoded.

I guess you're right, it's probably already good enough. Here PR #1796 .
I'll just create another one when the new functionality is ready.

[dnr7]

I'm not sure if mp3 support for sfml is working or not. If it isn't miniaudio is public domain or mit which really means public domain, and it has an mp3 decoder.

[eXpl0it3r]

Closing this issue, to reduce confusion on the more commonly association of "MP3 support" with "playing MP3 files" and have opened #2679 instead.