GNOME Bugzilla – Bug 326734
[mad] Add 24-bit output support
Last modified: 2006-03-21 16:24:03 UTC
MAD MP3 decoder supports 24-bit output and it really is more accurate than your usual 16-bit. Especially if you have a 24-bit soundcard (which I do). It shouldn't be too much work to add the support to the plugin. In fact, maybe I'll do it on my own. Currently, mad plugin info says it supports only 16-bit output: gst-inspect-0.8 mad ... SRC template: 'src' Availability: Always Capabilities: audio/x-raw-int endianness: 1234 signed: true width: 16 depth: 16 rate: { 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000 } channels: [ 1, 2 ] ...
Cool. 0.8 is a bit unmaintained these days though; we'd probably be more inclined to accept a patch against the mad plugin in gst-plugins-ugly. So this bug is really against gst-plugins-ugly. Reclassifying so that when you submit your patch it goes to the right place :-)
I'll do the patch against both, but I'm testing it with 0.8.x, as that's what certain application I'm very interested in uses. And I think it's still default in many distributions. It will decode 24 bits of quality by default. Of course GStreamer will truncate to 16 bits later on if required. Side notes: I think GStreamer should do dithering when converting to lower bit depth. That's for another bug (maybe it's alreade there?). If I'm reading alsasink source code right, it uses 16 bits by default, not the highest bit depth possible (like 32 wrt my card). Alsasink mentions it as supported.
Ok, here's the patch for GStreamer 0.8.11. It should be extremely easy to bump it, and I'll do so in a while. I decided to decode every bit of quality available to 32 bit int. Also fixed the scaling function. Previous one was incorrect for MAD_F_FRACBITS < 16. The scaling function was taken from libmad, not from cheap example code. Hence the #if.
Created attachment 57312 [details] [review] Support full quality decoding in GStreamer MAD plugin Ok, here's the patch for GStreamer 0.8.11. It should be extremely easy to bump it, and I'll do so in a while. I decided to decode every bit of quality available to 32 bit int. Also fixed the scaling function. Previous one was incorrect for MAD_F_FRACBITS < 16. (But I don't know why anyone would want to set it so low and still use GStreamer.) The scaling function was taken from libmad, not from cheap example code. Hence the #if.
Created attachment 57314 [details] [review] Same patch as above, but for 0.10.0 release This is the second patch. Unlike the previous one, this wasn't tested in the least. It should work correctly anyway.
thanks for the patch. Would you have some 24bit mp3s to test this with ?
The patches I posted have gain problems (it's too quiet). I had to fix scale() routine. (Forgot to convert from MAD's 29 to 32 bits.) I will post final versions in a moment. *** OT? There's no need for 24-bit mp3's. MP3 is a lossy format, so synthesis of any MP3 file can be done with almost arbitrary precision. I won't delve into mathematical detail here. See http://mp3decoders.mp3-tech.org/24bit.html point 3. Check this comparison out (foobar2000 run under Wine): INFO (foo_bitcompare) : Comparing: INFO (foo_bitcompare) : location: "file://c:\gst-test.wav" (0) INFO (foo_bitcompare) : location: "file://c:\001 madplay-test-24.wav" (0) INFO (foo_bitcompare) : first different sample found INFO (foo_bitcompare) : differences found: 18085910 sample(s), starting at 2.2675737E-05 second(s), peak: 1.1548400E-07 at 6.6453243E+01 second(s), 1ch INFO (foo_bitcompare) : Finished successfully. INFO (foo_bitcompare) : Comparing: INFO (foo_bitcompare) : location: "file://C:\madplay-test.wav" (0) INFO (foo_bitcompare) : location: "file://c:\001 madplay-test-24.wav" (0) INFO (foo_bitcompare) : No differences in decoded data found. INFO (foo_bitcompare) : Finished successfully. Madplay doesn't provide full quality of underlying libmad. INFO (foo_bitcompare) : Comparing: INFO (foo_bitcompare) : location: "file://C:\madplay-test.wav" (0) INFO (foo_bitcompare) : location: "file://c:\001 madplay-test-16.wav" (0) INFO (foo_bitcompare) : first different sample found INFO (foo_bitcompare) : differences found: 18597029 sample(s), starting at 4.9886621E-04 second(s), peak: 3.0398369E-05 at 6.6453243E+01 second(s), 1ch INFO (foo_bitcompare) : Finished successfully. INFO (foo_bitcompare) : Comparing: INFO (foo_bitcompare) : location: "file://c:\gst-test.wav" (0) INFO (foo_bitcompare) : location: "file://c:\001 madplay-test-16.wav" (0) INFO (foo_bitcompare) : first different sample found INFO (foo_bitcompare) : differences found: 18666984 sample(s), starting at 2.2675737E-05 second(s), peak: 3.0513853E-05 at 6.6453243E+01 second(s), 1ch INFO (foo_bitcompare) : Finished successfully. Notice that in 32 bit madplay mode is in reality 24 bit, while MAD is internally 29 bit signed. gst-test.wav was obtained with these commands: gst-launch-0.8 filesrc location="$IN" ! mad ! filesink location=gst-test.raw rawtowav.py gst-test.raw gst-test.wav rawtowav is a throwaway script which uses Python's wave module to wrap raw data in RIFF format. madplay-test.wav was obtained with this command: madplay -b 32 -d -owave:madplay-test.wav $IN madplay-test-16.wav was obtained with this command: madplay -b 16 -d -owave:madplay-test.wav $IN madplay-test-24.wav was obtained with this command: madplay -b 24 -d -owave:madplay-test.wav $IN Versions 001 - * in front were converted to 32 bit with foobar's diskwriter to facilitate comparison.
Created attachment 57358 [details] [review] Full quality decoding support for GStreamer 0.8.x MAD plugin This is the version with missing bit shift fixed. (the tests were done with it)
Created attachment 57359 [details] [review] Full quality decoding support for GStreamer 0.10.x MAD plugin Also a fixed version. Was not even compile tested, but should work fine.
Patches look ok from a brief read, except this bit looks strange: - bytes_per_sample = MAD_NCHANNELS (&mad->frame.header) << 1; + bytes_per_sample = MAD_NCHANNELS (&mad->frame.header) * 32; Also, I'm not sure whether we need permission for the 'scale code taken from MAD'. Technically, the GStreamer plugin itself is LGPL, where mad code itself is GPL.
>Patches look ok from a brief read, except this bit looks strange: - bytes_per_sample = MAD_NCHANNELS (&mad->frame.header) << 1; + bytes_per_sample = MAD_NCHANNELS (&mad->frame.header) * 32; << 1 is a bitshift to left. This means that a single channel turns to 2 bytes, while 2 channels turn to 4 bytes. Well, I meant to write * 4 instead of * 32. I had 3 (24 bits) somewhere during development and messed it up, as you can see. Or change it to a left shift by 2. It makes no difference, except being less readable. Any sane compiler would optimise it to a bit shift. Should I refresh the patches?
>Patches look ok from a brief read, except this bit looks strange: - bytes_per_sample = MAD_NCHANNELS (&mad->frame.header) << 1; + bytes_per_sample = MAD_NCHANNELS (&mad->frame.header) * 32; 32 should be a 4. It was initially 3, meaning 24 bit samples, but I of course outsmarted myself. Of course, it could also be a bit shift by 2, which is much less readable and not faster at all. (when using any optimising compiler) As for permission: We apparently don't need any. If it would be needed, then previous code was also taken from minimad.c from libmad, so there's no change. Also LGPL.
Created attachment 57380 [details] [review] Patch against gst-plugins-0.8.11 - final Fixed patch attached.
Created attachment 57381 [details] [review] Patch against gst-plugins-ugly-0.10.0 - final Fixed patch attached.
Applied the 0.10 patch to CVS head: * ext/mad/gstmad.c: (gst_mad_convert_src), (scale), (gst_mad_check_caps_reset), (gst_mad_chain): Merge patch from Radoslaw Szkodzinski (bug 326734) Leaving open for the 0.8 patch - Ronald is cutting a release and I'm not sure whether gst-plugins 0.8 is frozen or not.
I don't think it would be wise to change the output caps of a common element like mad at this point in the 0.8 series. The risk of breaking apps that don't use audioconvert and assume mad will always output 16bit audio just seems to big IMHO.