search

RustAudio / audrey

A crate to simplify reading, writing and converting between a variety of audio formats.
Other
132 stars 16 forks source link

Interim support for MP3 #24

Open Cocalus opened 4 years ago

Cocalus commented 4 years ago

Since there hasn't been much movement on https://github.com/RustAudio/mp3. I could make a PR to add mp3 support via https://crates.io/crates/minimp3. Since that is a C wrapper and against the stated goal of Audrey, perhaps making it a non default feature would be acceptable. Once a production ready Rust mp3 decoder is made that can be swapped in and the feature enabled by default. The API should stay the same with the exception of the types wrapped by the Reader and Error Enums. Maybe calling the non default mp3 feature "mp3_unstable", with a README comment about that expected breakage would be enough. I don't think most users will need to access the inner values anyway.

I currently have a a Enum wrapper around Audrey and minimp3 abstracting the two in some code I'm using. The only slightly weird thing is that mp3's can change sample rate (it's apparently set per block) and maybe channels. So I'll need to extend minimp3's errors with another wrapper type in the middle, to error if those values change in the middle of a file.

est31 commented 4 years ago

See also https://github.com/RustAudio/rodio/issues/256

Cocalus commented 4 years ago

I missed those. Doesn't look like either claims to be complete for mp3 yet. Sonata is testing mp3 and puremp3 is version 0.1.0. Sonata looks like a self contained replacement for Audrey, too bad it isn't finished yet. If either of those are more finished than they advertise I could look at how hard it would be to add them to Audrey. Right now minimp3 wouldn't be that hard for me to do.

est31 commented 4 years ago

@Cocalus could you study the puremp3 crate in a production setting and assemble a list of stuff it has to implement/fix until we can consider switching to it?

Cocalus commented 4 years ago

I'm not that familiar with the MP3 standard but I'd start with a more complete test vectors and fuzzing. https://github.com/lieff/minimp3 seems to have a pretty good set of vectors and a fuzz harness. At least a lot more than the one file of puremp3. Maybe port the test harness to Rust then verify that minimp3-sys (assuming it covers the test API, if not make one that does) of minimp3-rs still works under the Rust harness. Personally I'd probably try to do the whole test driven C -> Rust conversion on the 2K lines of minimp3, with some criterion benchmarks against minimp3-sys. Then it's just small mechanical code transformations and I could avoid having grok the standard.

Shnatsel commented 3 years ago

The state of MP3 decoding in Symphonia is advertised as follows:

Most media streams play. Inaudible glitches may be present. Most common features are supported.

I suppose the easiest way to test it is to get a bunch of MP3s from archive.org or some such, decode them and compare the results to a reference decoder. But right now I'm busy doing that very thing to 8 HTTP clients.

Shnatsel commented 2 years ago

Symphonia v0.5 has shipped just recently with a much improved MP3 decoder!

I have personally tested it on over 500,000 MP3 files and compared the output to mpg123, ffmpeg and libmad. All the bugs that this uncovered were fixed prior to v0.5 release. In fact, this testing has found quite a few bugs in ffmpeg!

So Symphonia v0.5 should me more than good enough for use in Audrey.