Compressor effect: Adjust Makeup Time constant

[daschuer]

Bug Description

The Compressor introduced in #12523
Use the input RMS value to generate a makeup gain. This is required to allow fading and outros.
Unfortunately this value does not scale with the compressor timing nor the engine buffer.

Version

2.5~alpha

OS

No response

[fonsargo]

@daschuer Could you explain in more details, what you would like to have? Do you want to change makeupcoeff together with changing attack or release time?

In my first test I have tested with long attack and release times which was compensated by the auto markup gain. The kMakeUpAttackCoeff fits to the defaults.
Did you consider to adjust it with the buffer size and the attack and the release times?

Yes, I considered it, but the problem is that this auto make up algorithm is too simple. It doesn't have separate attack and release time, it has just "generic" coefficient. If I connect it to attack time, for example, someone may be surprised, that changing attack time has influence to make up gain. Moreover, it's fine to use fast attack time (~0.05ms) and slow release time (~250ms) for dance-pop songs, but I definitely don't want to use such fast attack time for auto make up algorithm, because it will lead to crackling.

Eventually I decided to leave this auto make up algorithm as simple as it is, and create another effect called "Auto Gain Control" in order to control gain more precisely. It will have it's own attack and release time and some other stuff. What do you think about it?

[daschuer]

Since auto makeup it "auto", it should maintain roughly the same sound when enabled, IMHO without making the user to think about it.

I have tested a bit more and I must admit that my conclusion when writing the bug was probably not perfect.

One issue is that the makeup gain will introduce unwanted noise, because the gain is not ramped here:

mixxx/src/effects/backends/builtin/compressoreffect.cpp

Line 187 in 8f64790

SampleUtil::applyGain(pOutput, makeUpGainState, numSamples);

The calculation of the makeUp can be optimized by sqrtf(sumSquared/sumSquared)

Can you introduce a PR for that?

The other issue is the buffer dependency, This could be solve by always building the RMS value for the same number of samples. Not sure if this is worth the effort, or if we find a smarter solution. Need to test.

[daschuer]

Wait, this makeUp gain calculation can be optimized a lot. Because the samples are the same, you can eliminate them from the calculation entirely:

    makeUp = SampleUtil::rms(pCompressoreGainBuffer, numSamples);
    // than apply the pCompressoreGainBuffer /  makeUp to the audio samples. 

[daschuer]

All buffers needs to be cleared when enabling this effect, to not apply values form a previous use:

mixxx/src/effects/backends/builtin/phasereffect.cpp

Line 124 in 8f64790

if (enableState == EffectEnableState::Enabling) {

[fonsargo]

Since auto makeup it "auto", it should maintain roughly the same sound when enabled, IMHO without making the user to think about it.

Yes, it's how it works now: it maintains output gain the same as input gain.

The calculation of the makeUp can be optimized by sqrtf(sumSquared/sumSquared)

Indeed, you're right

The other issue is the buffer dependency, This could be solve by always building the RMS value for the same number of samples. Not sure if this is worth the effort, or if we find a smarter solution. Need to test.

I guess the buffer size is always the same unless we don't change sample rate, isn't it?

Wait, this makeUp gain calculation can be optimized a lot. Because the samples are the same, you can eliminate them from the calculation entirely:

I don't understand it. Why do you think samples are the same? pInput isn't the same as pOutput, because applying make up goes after applying compression. Also I tried to use your expression in code and it just doesn't work, because sound becomes too quiet:

CSAMPLE_GAIN makeUp =  SampleUtil::rms(pOutput, numSamples);

[daschuer]

I guess the buffer size is always the same unless we don't change sample rate, isn't it?

We done even change the sample rate at this point. However the user might configure a different buffers size and sample rate in the hardware preferences. In this case, the Compressor effect should still sound the same or at least similar.

Also I tried to use your expression in code and it just doesn't work, because sound becomes too quiet.

You need to collect the calculated gain from the compressor stage in a buffer and then calc the rms from it (not from the samples).
Than divide the compressor gain by it.
Example for a buffer size of four:

• The compressor gain is 0.7 (ratio) for all four samples
• The RMS of four 0.7 values is 0.7
• The now divide all four gains by the RMS value, in this case 0.7
• 0.7/0.7 = 1

But how make it buffer size independent?
We may probably apply the smoothing of the RMS value before taking the squareroot. Not tested:

CSAMPLE smothedSqareGain = 1;
smothedSqareGain = (gain * gain) + kMakeUpAttackCoeff * (smothedSqareGain - (gain * gain));
makeup = 1 / sqrt(smothedSqareGain);

[fonsargo]

We done even change the sample rate at this point. However the user might configure a different buffers size and sample rate in the hardware preferences. In this case, the Compressor effect should still sound the same or at least similar.

Ok, I got it. I think we can easily fix it by calculating make up attack coeff from buffer size and sample rate. As I understand the buffer size is engineParameters.framesPerBuffer() / engineParameters.sampleRate() * 1000 (ms), isn't it?

You need to collect the calculated gain from the compressor stage in a buffer and then calc the rms from it (not from the samples).
Than divide the compressor gain by it.

oh, eventually I got it. So you suggest not to apply calculated gain to the buffer immediately, but firstly calculate make up from it, secondly apply make up to calculated gains, and finally apply these gains to the input buffer. It's not the same algorithm that I used, but I think it should work too. I need some time to implement it, I'll take a look.

But how make it buffer size independent?

I don't think it's so important to make it buffer size independent. As long as we calculate make up coeff from buffer size and sample rate, it will be good enough.

[daschuer]

As I understand the buffer size is engineParameters.framesPerBuffer() / engineParameters.sampleRate() * 1000 (ms), isn't it?

Yes.

I need some time to implement it, I'll take a look.

I have no strong demand. Consider how this matches to the other topic of buffer independence.

As long as we calculate make up coeff from buffer size and sample rate, it will be good enough.

I agree. There should be no significant different sound at least.

Can you also add your reasoning for the picked markup timing as comment?

[fonsargo]

Can you also add your reasoning for the picked markup timing as comment?

Yes, sure. I've done several fixes, which were discussed here. You can see it in #13261