soundsourcemp3 is clamping > 2.5 dB

Bug #1408100 reported by Daniel Schürmann
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
Fix Released
Low
Unassigned

Bug Description

It turns out that soundsourcemp3 cuts up to 2.5dB of high leveled track.

The range of libmad samples is -8.0 to +7.9999999962747097015380859375
But it is clamped t -1 .. +1 in soundsorcemp3 to have the maximum resolution of 16 bits
The original output samples of libmad have a 32 bit resolution.

In my test, I have collected the maximum values for some tracks and they are in the range of
-343734695 .. 351910901
-1.28 .. 1.31

This mean that Mixxx just cuts 2.5 dB (flat top) of the dynamic range.

Revision history for this message
Owen Williams (ywwg) wrote :

I doubt this is right, or we would have had tons of reports of problems with mp3s being badly clipped. And our scale function was copied basically from madplay, and that code also clips in a similar way.

If you can use something like mp3split to make a small, fair-use-compatible portion of an audio file that demonstrates these large numbers, that would help.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Well, this is a bummer. What I'm confused about is that madplay itself uses roughly the same method we do. madplay is the reference example implementation of how to use libmad by the author -- so why would they do this?

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

mpg321 does roughly the same, too:
https://github.com/e3c/mpg321/blob/master/mad.c#L808

Also, reposting my comment from Bug #1408011

"""
Looking at how others manage converting libmad's fixed point numbers to s16 -- I think our madScale function is fine.

madlld: https://github.com/bbcrd/audiowaveform/blob/master/src/madlld-1.1p1/madlld.c#L165

madplay: https://github.com/Distrotech/madplay/blob/master/audio.c#L436

It seems our madScale method is right out of madplay:
https://github.com/Distrotech/madplay/blob/master/audio.c#L213

Except our clipping is slightly different:
https://github.com/Distrotech/madplay/blob/master/audio.c#L176

We don't do the "signal statistics" that madplay does (which conditionally applies the clipping?). Not sure I understand why that is -- -- perhaps there's a post-processing scaling based on the signal statistics?
"""

tags: added: mp3
Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

I read through madplay and found no evidence of using the signal statistics for anything except user feedback.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

@Owen, you have the file already. Many of my files have these issues, so I assume you should have some others to too, just analyse your library and store max and min samples.

Maybe it is just noise, which should be clamped?

Once we have the float interface, we have to deal with this "noise" in Mixxx.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Or it is noise due to our seek code, (which is also broken Bug #1407394)

Revision history for this message
Daniel Schürmann (daschuer) wrote :

this is the patch I use for testing, just beak after the analysis is finished

Revision history for this message
Daniel Schürmann (daschuer) wrote :

after checking 50 tracks, I am at
-388078489 .. 363767620
-1,44 .. 1,35
and 86 clipping samples in a row

We should as a mad developer about this.

Revision history for this message
Owen Williams (ywwg) wrote :

I played back the whole track you sent me and it didn't clip once. Under what circumstances is it clipping?

Revision history for this message
Owen Williams (ywwg) wrote :

I can trigger clipping if I fling the track with the mouse, but the values are only 1.00001 and probably due to floating point scaling and/or dithering errors.

Revision history for this message
Owen Williams (ywwg) wrote :

oh now I see, clipping in the madscale, not soundutil. Have you tried patching madplay to see if it also clips these samples? I don't know enough about mp3 to know if this clipping is an expected outcome of the encoding process.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

This is a useful post about clipping samples:
http://www.mars.org/pipermail/mad-user/2001-February/000113.html

Conclusion; They can occur and clipping them will puts distortion on the file.

This means for us:
* Wait for the floating point soundsource API,
* Add a gain or compressor inside sounsourcemp3
* Quickhack a sloution to get the original 32 bit mad_samples from the soundsource

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Nice post! Rob makes it sound like a pretty rare problem yet your experience does not match that.

Can you two investigate why you one of you sees clipping and one of you doesn't on the same file? Can you both share your libmad package information (or if you built from source, how you did it)?

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Since it's been this way for years and nobody has come banging on our door about audio quality issues with MP3 -- I'm inclined to do the safe thing and change nothing in 1.12.0.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

It looks like the problem gets more important over the years:
http://www.mars.org/pipermail/mad-user/2001-February/000114.html

It is OK with me, to fix it after 1.12 release by using Uwe's float API.

However after knowing this the pressure gets higher.

We may consider the following:
What doe we expect issues with the API changes?
Isn't the hopefully soon started beta phase ideal for finding out remaining issues?
Isn't a pure floating point engine worth a try?

By the way: the faad decoder is effected too.

Revision history for this message
Uwe Klotz (uklotzde-deactivatedaccount) wrote :

The new floating-point API converts the 28-bit integer sample output from libmad by multiplying with the following factor:

const float kMadScale = 1.0f / float(mad_fixed_t(1) << MAD_F_FRACBITS)

I just noticed that this expression can be simplified to 1.0f / float(MAD_F_ONE)

Conversion results:
kMadScale * (-MAD_F_ONE) = -1.0f
kMadScale * (MAD_F_ONE - 1) = 1.0f
kMadScale * (MAD_F_ONE) = 1.0f

This is the symmetric 2^n method that we also use in SampleUtil now and that is discussed here:
http://blog.bjornroche.com/2009/12/linearity-and-dynamic-range-in-int.html
http://blog.bjornroche.com/2009/12/int-float-int-its-jungle-out-there.html

As far as I know clipping might occur when decoding an MP3 encoded signal even if the original signal before encoding does not clip. I don't think that we can do anything about it.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

> I don't think that we can do anything about it.

Your float API helps here, because the > 1.0 samples will not be clamped at the 16 bit gate and are processed in the ReplayGain detection and than a suitable gain is picked hopefully without clipping and loss of resolution since we are in the float domain.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

@Daniel -- my take is that Uwe's PR is quite large and the SoundSource code quality is the lowest in our codebase. The risk for subtle regressions is high due to the general code quality there and it's been in a pretty stable state for a couple releases now.

The mild refactors Uwe did already had some regressions that we didn't catch until we had users reporting that Ogg, FLAC, and WAVs didn't load at all anymore. Dealing with these (on top of the swarm of bugs I predict the beta will uncover) would delay the ultimate release.

Changed in mixxx:
status: New → Confirmed
importance: Undecided → Low
Revision history for this message
Uwe Klotz (uklotzde-deactivatedaccount) wrote :

Of course, libmad delivers 32-bits divided into 28-bits fractional precision and 4-bits non-fractional digits. The output range covered by mad_fixed_t is [-8.0, 8.0).

The current 16-bit sample API of Mixxx clamps the libmad output to [-1.0, 1.0) immediately after decoding which might lead to distortions if the decoded output of libmad exceeds this range. As you already mentioned the new floating-point Audio-/SoundSource API will hopefully fix this issue.

Revision history for this message
Uwe Klotz (uklotzde-deactivatedaccount) wrote :

Should be fixed now on master (post 1.12 release).

With the new floating-point SoundSource API the decoded output from libmad is no longer clamped at [-1.0, 1.0)

Changed in mixxx:
assignee: nobody → Uwe Klotz (uklotzde)
status: Confirmed → Fix Committed
Changed in mixxx:
milestone: none → 2.1
tags: added: soundsource
Changed in mixxx:
status: Fix Committed → Fix Released
Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7798

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.