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

recording to wav/aiff causes very distorted audio if it saturates #8674

Closed
mixxxbot opened this issue Aug 23, 2022 · 9 comments
Closed

recording to wav/aiff causes very distorted audio if it saturates #8674

mixxxbot opened this issue Aug 23, 2022 · 9 comments
Milestone

Comments

@mixxxbot
Copy link
Collaborator

Reported by: JosepMaJAZ
Date: 2016-10-30T00:55:35Z
Status: Fix Released
Importance: Undecided
Launchpad Issue: lp1637786


Mixxx 2.0 and above.

Recording the mix to wav or aiff generates audio files that distort in the places where audio saturates (goes above full scale).

The root of the cause is that the conversion from float to integer does not clip the values and they overflow.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2016-10-30T22:25:25Z


Thank you for adopting this!

Here you will find some more info to th issue:
https://bugs.launchpad.net/mixxx/+bug/1415758
https://bugs.launchpad.net/mixxx/+bug/1415720

Mixxx clamps the samples when passing them to the soundcard here:

SampleUtil::copyClampBuffer(outputBuffer, pAudioOutputBuffer,

The broken normalization in sndfile is enabled here:

sf_command(m_pSndfile, SFC_SET_NORM_FLOAT, NULL, SF_TRUE);

@mixxxbot
Copy link
Collaborator Author

Commented by: JosepMaJAZ
Date: 2016-10-31T18:19:17Z


Thanks. I knew the but was already reported, but didn't find the correct words to find them.

The bug itself is fixed ( libsndfile normalization is ok, but it needs the clip option too ).

Manual clamping was the first natural idea, but one that I could not implement, because the buffer that is received in process is constant, so the compiler prevents it to be modified. Dynamically creating a buffer just for clamping seemed like a bad idea too.

And I will definitely try to implement the different bitsize saving. Should be trivial.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2016-10-31T23:09:58Z


Fix Committed = Pull Request is merged.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2016-10-31T23:29:35Z


The libsndfile normalization is IMHO heavily broken by design.

It is up to the user to level the gain of the recoded samples before he starts recording.
All normalization during the recording does not reflect the original samples passed to the sound card, which is probably intended. This means, clipping at the sound-card should IMHO be recorded to the file as it is. The libsndfile normalization would apply an instant gain to the signal, which introduces a click sound and leads to a recording where the first half is louder than the second half.

You may apply the clipping in
EngineSideChain::run() or
EngineSideChain::writeSamples()
to get around the const issue

This would prevent to record samples above 1 in a float recording
So finally a second buffer seams to be required.

@mixxxbot
Copy link
Collaborator Author

Commented by: JosepMaJAZ
Date: 2016-11-01T00:03:03Z


I believe we are not on the same page.

As I understand libsndfile option of normalize, it simply means that it translates the full scale. It does not try to guess which is the peak value.

That is, if normalization is not selected, in order to get a 16 bit PCM audio with -32768 to 32767 range, the floating point data provided should be -32768.0 to 32767.0, whereas if normalization is used, that same PCM range is obtained from a floating point data between -1.0 to 1.0.

And that's it, no more.

So, what if the floating point data is -1.3 to 1.2 due to excessive gain? Then you need to tell it to clip at full scale. And that's what I've just changed in the code.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2016-11-01T10:05:36Z


Oh yes, you are right, I have misread the docs. You proposed solution is correct.

Reading it again, I noticed the dither topic.

Could you check if dithering is enabled on write?
This is required to remove the quantization noise from the recorded fixed point samples.

I have found noting in the API docs about it. Only the test code:
https://github.com/erikd/libsndfile/blob/2fcf531ac940829cf350f86786fcff5160a32143/tests/dither_test.c#L157

@mixxxbot
Copy link
Collaborator Author

Commented by: JosepMaJAZ
Date: 2016-11-01T10:59:56Z


Mmmm... Indeed, it's not documented.

But the code exists since many years ago: https://github.com/erikd/libsndfile/blob/master/src/dither.c

I've written there an issue so that they are aware of this.

Going to try the code and see.

@mixxxbot
Copy link
Collaborator Author

Commented by: JosepMaJAZ
Date: 2017-01-19T21:30:47Z


#1035

@mixxxbot
Copy link
Collaborator Author

Issue closed with status Fix Released.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
@mixxxbot mixxxbot added this to the 2.1.0 milestone Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant