Compressor effect #12523

fonsargo · 2024-01-06T17:57:45Z

This is a simple implementation of general single-band compressor, which was requested in this feature request #6453 years ago.
I'm not an expert of compressors, so I've just used general information about it from the Internet, mainly from this readme: https://github.com/p-hlp/CTAGDRC
It's the simpliest solution with gain computer and ballistics, without parameters automation. However in most cases it will be enough, for example, for radio broadcasting.
The only automation I did is the Auto makeup. I haven't found some good example of it, so I tried to introduce some "homemade" algorithm.

I've been using custom build with this effect for radio broadcasting for 2 weeks and it looks good. However, due to this bug #12451 I have some limitations of using this compressor.

Also I'm not a C++ developer, I've been working with Java for years, so there may be some stupid mistakes which I apologize for in advance :)

JoergAtGithub · 2024-01-06T18:06:02Z

Welcome at Mixxx!
As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

fonsargo · 2024-01-06T18:15:35Z

Welcome at Mixxx! As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

Done!

JoergAtGithub · 2024-01-06T19:45:27Z

The Ubuntu builds and the pre-commit check are failing. All these CI checks needs to be pass.
The best way to fix pre-commit issues is to install pre-commit locally on your system, as described here: https://github.com/mixxxdj/mixxx/wiki/Using-Git#set-up-automatic-code-checking
Than it will fix these issues automatically before every commit.
Alternatively, you can download the pre-commit.patch file from the artifacts of this PR

unzip it, and apply it using:
git apply pre-commit.patch

Swiftb0y

couple comments, thank you very much so far.

src/effects/backends/builtin/compressoreffect.h

src/effects/backends/builtin/compressoreffect.cpp

daschuer

Thank you. I have left some comments. Not yet tested.

src/effects/backends/builtin/compressoreffect.cpp

daschuer · 2024-01-09T14:44:33Z

src/effects/backends/builtin/compressoreffect.cpp

+    autoMakeUp->setName(QObject::tr("Auto Makeup Gain"));
+    autoMakeUp->setShortName(QObject::tr("Makeup"));
+    autoMakeUp->setDescription(QObject::tr(
+            "The AutoMakeup button enables automatic makeup gain to 0 db level"));


Suggested change

"The AutoMakeup button enables automatic makeup gain to 0 db level"));

"The Auto Makeup button enables automatic gain adjustment so that the perceived volume does not change. "));

In fact this description isn't true, because this algorithm adjusts volume to 0 dBFS (actually around -3 dBFS) in anyways. So it doesn't matter, what was the original level, output will be almost 0 dBFS (even if original input was -30 dBFS for example).

OK, than you may add a true description?

I think originally it was true, however your suggestion changes the meaning. If you don't like the original one, I can try to reformulate it. In fact it just adjusts volume to almost 0 dBFS level for any input.

I was thinking about it and it's just come to my mind that maybe it's better to call it "AGC (Auto Gain Coontrol)"? What do you think?

src/effects/backends/builtin/compressoreffect.cpp

src/util/sample.cpp

src/effects/backends/builtin/compressoreffect.cpp

daschuer · 2024-02-03T21:53:57Z

src/effects/backends/builtin/compressoreffect.cpp

+    double makeUpStateDB = pState->previousMakeUpGain;
+    CSAMPLE maxSample = SampleUtil::maxAbsAmplitude(pOutput, numSamples);
+    if (maxSample > 0) {
+        double maxSampleDB = ratio2db(maxSample);


I have a warning here:

/home/daniel/workspace/mixxx2/src/effects/backends/builtin/compressoreffect.cpp:165:48: required from here /home/daniel/workspace/mixxx2/src/util/math.h:78:21: warning: conversion from ‘double’ to ‘float’ may change value [-Wfloat-conversion] 78 | return log10(a) * 20; | ~~~~~~~~~^~~~

Actually I have no clue, why this warning is still here. I think I have convertion from float to double, but not double to float.

ratio2db is a template type. It uses the float implementation because of CSAMPLE maxSample
But log10() is double. So it looks like a static cast to T is missing.
return static_cast<T>(log10(a)) * 20;
Not you fault, but please fix it.

The other question is if we need double precision for maxSampleDB, nd can we avoid the DB round trip and instead use the ratio values?

Yes, it's a good question. I managed to avoid this DB round trip, so now I don't use ratio2db() and db2ratio() for applying make up.

daschuer · 2024-02-03T21:56:35Z

src/effects/backends/builtin/compressoreffect.h

+constexpr CSAMPLE_GAIN kMakeUpAttackCoeff = 0.03f;
+constexpr CSAMPLE_GAIN kMakeUpTarget = -3.0f;
+
+double calculateBallistics(double paramMs, const mixxx::EngineParameters& engineParameters) {


No, I use it on these lines: 117,119,193,201 in compressoreffect.cpp

Ah, the issue is that it is unused in very cpp file that includes the h file. Can we move the whole namespace to the CPP file?

Yes. sure. I've moved it to CPP file.

daschuer · 2024-02-03T23:27:14Z

I have tested this briefly and it works good. Thank you.
Some findings:

The tootlips are long and in a single line. Can you add reasonable line breaks (@ronso0 or is there a better solution?)
The effect has not meta binding, not strictly necessary, but you may consider to add one with center at unity to be able to be used as a quick effect.
Many controllers have only access to the first three or four knobs. Verify that these are the knobs that users may touch during the performance.
Even though the Gain is well described as output gain in tool tips, I initially was confused I had the input gain in mind. On some stomp-boxes it is called level. Would that work for you as well?
The default level is an issue when used as a deck effect, because it is way higher than the default -18 LUFS of replay gain and has zero head-room for mixing. To not destroy the gain level, the Output Gain should be ~-6 for modern pop music.
The Make-up switch is a tool to blow your ears/speakers, if you come form manual compensation. Did you consider to reset the gain knob to default while switching?
With the effect applied + Auto Make-Up, you are no longer able to mix, because the effect compensates your fade out. Do you have an idea how to fix that? Can we calculate the Make-Up gain form the other knobs alone?
The effect introduces crackling. This is sourced form rectangular gain changes. It can be fixed by fade all gains for one buffer size.

fonsargo · 2024-02-19T21:12:54Z

Thank you for testing and for the good points.
I'll try to answer to it:

The tootlips are long and in a single line. Can you add reasonable line breaks

Yes, I've added line breaks.

The effect has not meta binding, not strictly necessary, but you may consider to add one with center at unity to be able to be used as a quick effect.

Many controllers have only access to the first three or four knobs. Verify that these are the knobs that users may touch during the performance.

Actually I'm not sure, that it could be useful. Compressor it's not a sound effect, it's more about sound tuning. Usually you find the best settings for you and use it all the time. So it will be strange if someone tries to increase and decrease attack or release knob for example dureing the perfomance, because it just mess up the sound.

Even though the Gain is well described as output gain in tool tips, I initially was confused I had the input gain in mind. On some stomp-boxes it is called level. Would that work for you as well?

Personally I think it should be "Gain", because it's the same in meaning as "Main Output Gain" knob in the main mixer for example.

The default level is an issue when used as a deck effect, because it is way higher than the default -18 LUFS of replay gain and has zero head-room for mixing. To not destroy the gain level, the Output Gain should be ~-6 for modern pop music.

I guess you are talking about using "Auto Makeup Gain". So as it says in the description, it tries to adjust volume level to 0dBFS. So if someone wants to decrease it, they definetely can do it via gain knob.

The Make-up switch is a tool to blow your ears/speakers, if you come form manual compensation. Did you consider to reset the gain knob to default while switching?

I think it's a good idea, but I don't know how to introduce some "listener" to the Makeup switch button. Do you know how to do it?

With the effect applied + Auto Make-Up, you are no longer able to mix, because the effect compensates your fade out. Do you have an idea how to fix that? Can we calculate the Make-Up gain form the other knobs alone?

I believe it's because of the same bug as #12451. So if we apply compressor effect to the Deck 1 for example, then it should be applied before fade out (volume control). But in reallity it is applied after volume control and so completely comprensate it.
However, I think it's better to use compressor as an output effect (in main mixer).

The effect introduces crackling. This is sourced form rectangular gain changes. It can be fixed by fade all gains for one buffer size.

I'm not sure, that I understand what you mean. However compressor is a powerful tool, it's always possible to mess everything up with the wrong settings. So it's kind of "by design".

daschuer · 2024-03-15T07:32:27Z

First of all, I am very happy with your work looking forward to integrate it into Mixxx. I am sorry for the delays in review. Sometimes a written review sounds hard and does not transport the precognition your work deserves.

The effect has not meta binding ...

Actually I'm not sure, that it could be useful. Compressor it's not a sound effect, it's more about sound tuning. Usually you find the best settings for you and use it all the time. So it will be strange if someone tries to increase and decrease attack or release knob for example during the performance, because it just mess up the sound.

An issue is that we have pre-fader and postfader effects. Currently the compressor works only for as prefader, which is the Quick effect where only on knob is available. My use case is to use it to adjust 70th tracks or classic tracks with modern pop songs.
Currently the workflow is a bit chunky. Place the compressor into the Effect rack, store find a good setting, safe it and put it into the quick effect. This cannot be done during a set. So we should find OK-ish defaults for the quick effect ans allow to adjust them.

Gain -> Level

Personally I think it should be "Gain", because it's the same in meaning as "Main Output Gain" knob in the main mixer for example.

The "Main Output Gain" is the input Gain of you PA. So we should not judge from that. I have found more instances where it is called level without make-up the context. On the other hand it is usually called "make-up gain" which is quite long, when it controls the "make-up gain" stage which finally controls the "Level". You currently describe the knob as plain Output Gain which is the "Level". So I suggest to use the label "Level" then. But we may adjust this all altogether with the "makeup" button.

The default level is an issue when used as a deck effect, because it is way higher than the default -18 LUFS of replay gain and has zero head-room for mixing. To not destroy the gain level, the Output Gain should be ~-6 for modern pop music.

I guess you are talking about using "Auto Makeup Gain". So as it says in the description, it tries to adjust volume level to 0dBFS. So if someone wants to decrease it, they definitely can do it via gain knob.

make-up is used to turn the output level of the compressed signal up to compensate the compressor effect. It should ideal match the input gain. The default Input gain is NOT 0 dBFs, as deck effect, because you don't have room for Mixing. In the main stage, you should also not compensate for 0 dBFs, because a digital signal of 0dBFs ma have high frequency parts between the samples which will than clip. 0 dBFs is not recommended for broadcast. This means we should not default to a problematic high value. Can we pick a default that matches the default level of Mixxx?

The Make-up switch is a tool to blow your ears/speakers, if you come form manual compensation. Did you consider to reset the gain knob to default while switching?

I think it's a good idea, but I don't know how to introduce some "listener" to the Makeup switch button. Do you know how to do it?

Ah I see, this is an issue, we can do this easy. We may adjust the two gain knob scales that they match each other for the default Mixxx levels. This should lower the level jump we experience with the "Make-up" button.

With the effect applied + Auto Make-Up, you are no longer able to mix, because the effect compensates your fade out. Do you have an idea how to fix that? Can we calculate the Make-Up gain form the other knobs alone?

I believe it's because of the same bug as #12451. So if we apply compressor effect to the Deck 1 for example, then it should be applied before fade out (volume control). But in reallity it is applied after volume control and so completely comprensate it.
However, I think it's better to use compressor as an output effect (in main mixer).

We don't have on other solution to put the effect to the quick effect rack. Maybe the auto-make-up is a bit to fast? Can we slow it down or even make it adjustable? For my understanding it needs a time "learn" the input level.

The effect introduces crackling. This is sourced form rectangular gain changes. It can be fixed by fade all gains for one buffer size.

I'm not sure, that I understand what you mean. However compressor is a powerful tool, it's always possible to mess everything up with the wrong settings. So it's kind of "by design".

You hear it clearly if you put a reverb before the compressor. All gain changes must be ramped to not have the noise from the resulting rectangular wave.

fonsargo · 2024-03-21T21:00:56Z

Thank you for the review!

An issue is that we have pre-fader and postfader effects. Currently the compressor works only for as prefader, which is the Quick effect where only on knob is available. My use case is to use it to adjust 70th tracks or classic tracks with modern pop songs.
Currently the workflow is a bit chunky. Place the compressor into the Effect rack, store find a good setting, safe it and put it into the quick effect. This cannot be done during a set. So we should find OK-ish defaults for the quick effect ans allow to adjust them.

I'm not sure, that I completely understand you. But I agree that we need to find good default values. I thought that I found those, but I will try to do it better.

So I suggest to use the label "Level" then. But we may adjust this all altogether with the "makeup" button.

Ok, I will rename it to "Level".

make-up is used to turn the output level of the compressed signal up to compensate the compressor effect. It should ideal match the input gain. The default Input gain is NOT 0 dBFs, as deck effect, because you don't have room for Mixing. In the main stage, you should also not compensate for 0 dBFs, because a digital signal of 0dBFs ma have high frequency parts between the samples which will than clip. 0 dBFs is not recommended for broadcast. This means we should not default to a problematic high value. Can we pick a default that matches the default level of Mixxx?

Yes, I will decrease the default value. But I agree with you, that strictly speaking it's not a make-up, it's kind of AGC (Auto gain control).

We may adjust the two gain knob scales that they match each other for the default Mixxx levels. This should lower the level jump we experience with the "Make-up" button.

Yes, I will do it too.

We don't have on other solution to put the effect to the quick effect rack. Maybe the auto-make-up is a bit to fast? Can we slow it down or even make it adjustable? For my understanding it needs a time "learn" the input level.

The problem is that if we even don't use the make-up, compressor will compensate fade out until the level becomes lower than compressor threshold. After that the level will drop dramatically, so it will looks like just stopping without fading out. Therefore I think we have to apply fader only after all effects, not before. So may be we need to create an issue for that? Because as I understand most of the other mixxing software do it in such order: firstly effects, secondly fader (or crossfade, microphone ducking and all other stuff).

You hear it clearly if you put a reverb before the compressor. All gain changes must be ramped to not have the noise from the resulting rectangular wave.

Thank you, now I hear it. I think I need some time to investigate into it, maybe we need to use another algorithm for make-up or just delete this part.

fonsargo · 2024-03-27T21:36:07Z

Eventually I decided to change the auto make up algorithm, so for now it tries to match the input loudness level (compensate decreasing gain effect from compressor) - as it usually works. Also I fixed crackling (I believe), and renamed "Gain" knob to "Level".

As I said before, the auto make up algorithm which I used before was a kind of AGC effect, so I decided to split it to a separate effect. But I can't find any existing issue about AGC. Does anyone know if it's ever been requested? Is it okay if I try to create separate PR with AGC effect later?

acolombier · 2024-04-12T19:01:38Z

Does anyone know if it's ever been requested?

AFAIK, the AGC compressor would be very helpful for the NI stem support feature as the DSP compressor setup embedded in the stem metadata. seem to be relying on such a compressor. SO it would be very useful to have this feature as well!

Is it okay if I try to create separate PR with AGC effect later?

I think that makes sense yes

Also, is it worth targeting main (2.5) instead of 2.4?

daschuer · 2024-04-23T05:31:08Z

I just did a brief test, but I was not able to make it work again. Is something broken now?

daschuer · 2024-04-23T05:36:26Z

src/effects/backends/builtin/compressoreffect.cpp

+            continue;
+        }
+
+        double maxSampleDB = ratio2db(maxSample);


ratio2db() is slow. It would be better to do the calculation in ratio and to the db2ratio() conversion of the parameters outside this busy loop.

Do I have to do it? I understand your point, but it's not easy to transform such calculations. I'm sure that it's possible in theory, but the final code will be unreadable. All examples of compressor that I saw before were written in DB values.

No, that was just a suggestion. You may add TODO comment instead, that someone else may pick it up.

fonsargo · 2024-05-04T13:50:16Z

I just did a brief test, but I was not able to make it work again. Is something broken now?

No, it's not broken. It should work. As you can see, all checks have passed in GitHub. Maybe the problem is that it's already too old branch? Do I need to rebase it on new master? I see that the 2.4 version is already released, so maybe I need to rebase on 2.5?

daschuer · 2024-05-04T15:58:30Z

OK, I will test again maybe there was something wrong in my setup.

acolombier · 2024-05-08T18:13:03Z

I see that the 2.4 version is already released, so maybe I need to rebase on 2.5?

Yes, it looks like this PR has been move to the 2.5 milestone, so I think you can rebase to main

fonsargo · 2024-05-09T18:55:13Z

Yes, it looks like this PR has been move to the 2.5 milestone, so I think you can rebase to main

Ok, I've rebased to main

daschuer

Thank you, works good now.

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?

It is good enough for beta I will file an issue for the final release. I would be happy if you could pick it up.

github-actions bot added effects build labels Jan 6, 2024

Swiftb0y reviewed Jan 9, 2024

View reviewed changes

daschuer reviewed Jan 9, 2024

View reviewed changes

daschuer reviewed Jan 24, 2024

View reviewed changes

src/effects/backends/builtin/compressoreffect.cpp Outdated Show resolved Hide resolved

daschuer reviewed Jan 24, 2024

View reviewed changes

src/effects/backends/builtin/compressoreffect.cpp Outdated Show resolved Hide resolved

daschuer requested changes Jan 24, 2024

View reviewed changes

src/util/sample.cpp Outdated Show resolved Hide resolved

daschuer reviewed Jan 24, 2024

View reviewed changes

src/effects/backends/builtin/compressoreffect.cpp Outdated Show resolved Hide resolved

daschuer reviewed Feb 3, 2024

View reviewed changes

ronso0 removed the build label Feb 18, 2024

github-actions bot added the build label Feb 19, 2024

JoergAtGithub added this to the 2.5-beta milestone Feb 19, 2024

fonsargo requested a review from daschuer March 11, 2024 20:48

daschuer requested changes Apr 23, 2024

View reviewed changes

fonsargo added 2 commits May 9, 2024 20:21

Add compressor effect

30af769

Auto make up refactoring

bf0bf06

github-actions bot added cmake search controllers labels May 9, 2024

fonsargo changed the base branch from 2.4 to main May 9, 2024 18:39

daschuer removed skins library engine ui build autodj code quality preferences scanner waveform vinylcontrol soundsource browse sync broadcast Bugs pertaining to streaming radio broadcaster use-case analyzer packaging soundio cmake search controllers labels May 14, 2024

daschuer approved these changes May 14, 2024

View reviewed changes

daschuer merged commit bd8c383 into mixxxdj:main May 14, 2024
1 check passed

daschuer mentioned this pull request May 14, 2024

Compressor effect: Adjust Makeup Time constant #13237

Closed

ronso0 mentioned this pull request Jun 27, 2024

Sound Levels and Silence detector #13412

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compressor effect #12523

Compressor effect #12523

fonsargo commented Jan 6, 2024

JoergAtGithub commented Jan 6, 2024

fonsargo commented Jan 6, 2024

JoergAtGithub commented Jan 6, 2024

Swiftb0y left a comment

daschuer left a comment

daschuer Jan 9, 2024

fonsargo Jan 13, 2024

daschuer Jan 24, 2024

fonsargo Feb 3, 2024

fonsargo Feb 3, 2024

daschuer Feb 3, 2024

fonsargo Feb 4, 2024

daschuer Feb 4, 2024

fonsargo Feb 19, 2024

daschuer Feb 3, 2024

fonsargo Feb 4, 2024

daschuer Feb 4, 2024

fonsargo Feb 19, 2024

daschuer commented Feb 3, 2024

fonsargo commented Feb 19, 2024

daschuer commented Mar 15, 2024

fonsargo commented Mar 21, 2024

fonsargo commented Mar 27, 2024

acolombier commented Apr 12, 2024 •

edited

Loading

daschuer commented Apr 23, 2024

daschuer Apr 23, 2024

fonsargo May 4, 2024

daschuer May 4, 2024

fonsargo commented May 4, 2024

daschuer commented May 4, 2024

acolombier commented May 8, 2024

fonsargo commented May 9, 2024

daschuer left a comment

	"The AutoMakeup button enables automatic makeup gain to 0 db level"));
	"The Auto Makeup button enables automatic gain adjustment so that the perceived volume does not change. "));

Compressor effect #12523

Compressor effect #12523

Conversation

fonsargo commented Jan 6, 2024

JoergAtGithub commented Jan 6, 2024

fonsargo commented Jan 6, 2024

JoergAtGithub commented Jan 6, 2024

Swiftb0y left a comment

Choose a reason for hiding this comment

daschuer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented Feb 3, 2024

fonsargo commented Feb 19, 2024

daschuer commented Mar 15, 2024

fonsargo commented Mar 21, 2024

fonsargo commented Mar 27, 2024

acolombier commented Apr 12, 2024 • edited Loading

daschuer commented Apr 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fonsargo commented May 4, 2024

daschuer commented May 4, 2024

acolombier commented May 8, 2024

fonsargo commented May 9, 2024

daschuer left a comment

Choose a reason for hiding this comment

acolombier commented Apr 12, 2024 •

edited

Loading