Effect unit mixing mode #1676

Be-ing · 2018-05-20T20:14:26Z

Pass the "insertion type" of effect units to effects so the Echo and Reverb effects adjust their output appropriately. I have added a button to the Tango skin, tentatively labelling the two modes "D/W" (dry/wet) and "D+W" (dry+wet). D/W is the default, familiar behavior where Echo and Reverb mix their input with their output. In D+W mode, those effects do not add their input to their output, and the effect unit mix knob adds the wet signal on top of the unmodified dry signal. This allows for chaining effects such that they only change the signals of other effects in the chain without modifying the original track, which allows for frequency-selective effects

A good way to hear the difference is by loading BQ EQ -> Echo. Turn the Low parameter knob of the BQ EQ effect (not the normal deck EQ) all the way down (left) and turn the Send knob for the Echo effect all the way up (right). Enable both effects and turn up the effect unit mix knob. In D/W mode, you will hear that the bass is taken out of the track. In D+W mode, the middle and high frequencies will be echoed and the bass of the track will still be there.

The big questions are what to call these two modes and how to represent them with a button. Please suggest some ideas.

Implementing https://bugs.launchpad.net/mixxx/+bug/1662049

This is implemented on top of the LV2 branch to ensure it doesn't create any merge conflicts with that branch.

Be-ing · 2018-05-21T00:19:20Z

Another fun chain to play with in D+W mode: Bitcrusher -> Moog Filter -> Autopan. Turn up the Resonance on the Moog Filter and play with the LPF.

Be-ing · 2018-05-21T22:39:30Z

It turns out that a dry/wet parameter is needed for the Reverb effect. Before now the effect was always fully wet with no dry, with the amount of the effect control with the Send parameter. However, when the effect unit is in dry + wet mode, the Send parameter is not as useful because it reduces the volume of the whole chain. In this mode, it is more useful to keep Send fully up and adjust Dry/Wet. However, when the effect unit is in D/W mode, Send is better to use to adjust the amount of the effect because lowering it smoothly fades out the effect. Admittedly it is odd to have both parameters, however I think this is the best we can do until @kshitij98 implements parameter hiding and rearrangement. I have updated the parameter tooltips to clarify their use cases.

To test this I recommend playing with the chain BQ EQ -> Echo -> Reverb.

Be-ing · 2018-05-27T21:40:43Z

Anyone care to review? @ronso0, @ferranpujolcamins?

ferranpujolcamins · 2018-05-27T21:57:45Z

I'm short on time, sorry.

ronso0 · 2018-05-28T12:24:14Z

I can't review c++ and I wont find time to test it properly, sry.
I can jump in though when it comes to skin implementations. I think the text label D+W...D/W works well, no need for an icon IMO

That button could also be a flip switch, space is sufficient

Be-ing · 2018-05-28T12:29:36Z

That button could also be a flip switch, space is sufficient

Oh, interesting idea. I think that would help because it's not an on/off switch.

so Echo and Reverb do not add the dry signal to their output in "Send" mode

Using the Send parameter when the effect unit is in D+W mode does not work as desired because it reduces the volume of the whole effect unit.

WaylonR · 2018-05-30T03:29:13Z

"BQ EQ -> Echo. Turn the Low parameter knob of the BQ EQ effect (not the normal deck EQ) all the way down (left) and turn the Send knob for the Echo effect all the way up (right). Enable both effects and turn up the effect unit mix knob. In D/W mode, you will hear that the bass is taken out of the track" - This is referred to as being a high pass echo.

Be-ing · 2018-05-30T05:20:36Z

This is referred to as being a high pass echo.

Yes, but that single use case doesn't help name this new mode.

kazakore · 2018-05-30T18:45:30Z

A good way to hear the difference is by loading BQ EQ -> Echo. Turn the Low parameter knob of the BQ EQ effect (not the normal deck EQ) all the way down (left) and turn the Send knob for the Echo effect all the way up (right). Enable both effects and turn up the effect unit mix knob. In D/W mode, you will hear that the bass is taken out of the track. In D+W mode, the middle and high frequencies will be echoed and the bass of the track will still be there.

This simply isn't true! The amount of frequencies lost from the original is the same on each and is almost definitely due to phase issues of mixing filtered/EQ'd sound back in with the original and is one of the largest issues in digital eq/crossover design. The main difference which is clearly audible is the fact that the effected signal can be much louder without having lost any level from the original as there is not the constant power mixing mode of the wet/dry knob. This is even more evident if you use the Moog Filter instead of the EQ and only put the effect on the very top end of the frequency range.

IMPO this would be better resolved by having a way to make up the gain from the effect chain. Whether this would be a user controlled knob or an Automatic Gain Compensation button (to make effects level to dry level) I'm not sure, although I'd lean towards the AGC method as it would work better for keeping the level consistent when playing with controls such as Feedback or Resonance.

If you don't believe me load in some 20Hz-20kHz ramps, actually record the output and do some real measurements.

Be-ing · 2018-05-30T22:33:53Z

The main difference which is clearly audible is the fact that the effected signal can be much louder without having lost any level from the original

Yes, that is the purpose of this PR. The question is what to call the two different modes.

IMPO this would be better resolved by having a way to make up the gain from the effect chain.

I don't understand how this is relevant. Keeping a consistent level would not be desirable. When I use an Echo effect, I don't want the unechoed signal to get quieter.

kazakore · 2018-05-31T07:34:13Z

The main difference which is clearly audible is the fact that the effected signal can be much louder without having lost any level from the original
Yes, that is the purpose of this PR. The question is what to call the two different modes.

Ahh sorry, from your description I thought you meant something weird was going on as you said you lose more bass from one method than the other, which you don't.

IMPO this would be better resolved by having a way to make up the gain from the effect chain.

I don't understand how this is relevant. Keeping a consistent level would not be desirable. When I use an Echo effect, I don't want the unechoed signal to get quieter.

The unechoed signal would never get affected by this!! You're not doing anything to the dry signal, only the wet signal at the end of the entire effect chain. Wat makes you think the unechoed dry signal would be effected in any way?

Having a gain knob for the effects chain before the chain wet/dry mix wouldn't do that to the wet echoed signal either. Which is what I assume you were actually trying to say....

Even with AGC you would want a release phase of a couple of seconds or more (seen up to around five seconds used) to prevent any abrupt level changes which would sound horrible and almost no echo effect is going to be long enough to adversely affect that with the correct settings. But as always there would be niche cases. Button as it's optional you don't even have to have it turned on in the first place.....

With your method it's still very easy to create effects chains working on only small frequency range which even with D+W mode are barely audible and it would be a larger fix across the board to make it so no matter the output of the effects chain due to filtering etc you can still have it at a level where it can be reasonably mixed in with the original dry signal and clearly heard at output. This is not the achieved by just adding in a different summing more for the wet dry knob!

kazakore · 2018-05-31T07:38:23Z

But for what it is I think your D/W and D+W labelling on the skins is perfect.

In the manuals you could say something along the lines of:
D/W adjust the mix of the Dry and Wet signals while assuming constant power.
D+W controls the Wet (effected) signal strength mixed back into the Dry, uneffected audio.

Be-ing · 2018-05-31T09:35:21Z

With your method it's still very easy to create effects chains working on only small frequency range which even with D+W mode are barely audible

I don't know what you mean. Did you turn the Send parameter of the Echo effect all the way up? The Send knob acts like a volume fader for the whole effect chain in D+W mode. It can only make the output of the effect unit quieter, not louder. I will edit the parameter tooltip to explain this.

for what it is I think your D/W and D+W labelling on the skins is perfect.

Great, I'll go ahead and implement the button for other skins then. In the code, I'll call the modes DrySlashWet and DryPlusWet to match the GUI and rename the EffectChainInsertionType enum to EffectChainMixMode.

Be-ing · 2018-05-31T09:37:05Z

For documentation, I think some language like this could help: "Use D/W mode if you want EQ and filter effects to change the sound of the track. Use D+W mode if you want EQ and filter effects to only change the effected signal."

kazakore · 2018-05-31T09:38:56Z

Sorry if I offended @Be-ing, I do agree there is good use case for these modes. But the issues you describe it as fixing don't exist as you have described them. I didn't think I could hear it as explained when initially testing on your request the other day and so when I had free time yesterday I did a null test of the normalised output of the two modes and proved categorically that there is no difference in the frequencies of the output of the two methods with comparable wet mix knob levels (although there is, of course, an obvious level difference.)

As it seems clear that despite the opening statements of the thread that you do fully understand this I apologise. I was worried you were trying to fix something that wasn't faulty in the way you thought and didn't want you wasting your time if that was the case.

Be-ing · 2018-05-31T09:42:45Z

@kazakore what do you think of adding the Dry/Wet parameter to the Reverb effect? Do the parameter tooltips adequately explain the differences between Send and Dry/Wet (you'll need tooltips turned on in the Interface preferences to see them).

kazakore · 2018-05-31T09:43:59Z

For documentation, I think some language like this could work: "Use D/W mode if you want EQ and filter effects to change the sound of the track. Use D+W mode if you want EQ and filter effects to only change the effected signal."

You're right, it does need some clarification that for effecting the track in entirety (IE all wet at output) you need D/W mode and mentioning that this would be normal operation for EQ, Filter and Compression is a good idea. Using the filtered echo as the example for the other mode seems good too.

Be-ing · 2018-05-31T09:46:37Z

Using the filtered echo as the example for the other mode seems good too.

Yes, after @kshitij98 implements effect chain presets, I want to include some default presets in Mixxx. One of those would be BQ EQ -> Echo -> Reverb in D+W mode.

kazakore · 2018-05-31T10:14:06Z

Slightly off topic but as I've noticed them while testing this PR I will comment on it here, hope that's OK:

Flanger and Phaser are both fairly similar effects, with very similar controls. The names of these controls could quite well be standardised across the two effects a bit more.
Secondly the Flanger appears to be missing a Depth control. I guess this is what was meant to be put in place of the Mix control, the tooltip actually matches that for Depth on Phaser rather than Mix on other effects.
Whether they should both have a wet/dry mix control is debatable.....

I also get a little GUI bug (Tango). Any effects loaded after having started Mixxx which have buttons (Quantize/Triplets) the text on the button is not visible unless the button is enabled on loading the effect into the chain. It's fine on the other skins as they don't use two colour text inside the button.

Also changing skins to test that and the skin appears to load at smallest possible size and I have to hit F11 a few times to resize and get the skin actually working. Think I saw this (or similar) reported against another branch on Zulip the other day

Be-ing · 2018-05-31T11:26:06Z

Okay, thank you for the feedback @kazakore. I am going to merge this now so @kshitij98 can start refactoring EffectChain/EffectChainSlot without creating merge conflicts with this branch. I will follow up this PR soon to add the new button to other skins.

daschuer · 2018-05-31T14:24:41Z

I think this branch has not received a code review yet, right?
You have just merged this without even announce an appropriated time, that you will merge this without review.
In this situations, it would be better to create a feature branch.

Be-ing · 2018-05-31T14:33:38Z

Sorry, you are right. It would have been better for @kshitij98 to start working from this branch and wait to merge this until it could have been reviewed more.

daschuer

Ok, I have added some comments. @Be-ing will you issue them post merge?

daschuer · 2018-06-03T20:59:15Z

lib/reverb/Reverb.cc

@@ -434,7 +434,7 @@ Descriptor<PlateX2>::setup()

 // (timrae) we have our left / right samples interleaved in the same array, so use slightly modified version of PlateX2::cycle
 void MixxxPlateX2::processBuffer(const sample_t* in, sample_t* out, const uint frames, const sample_t bandwidthParam,
-								const sample_t decayParam, const sample_t dampingParam, const sample_t blendParam) {
+								const sample_t decayParam, const sample_t dampingParam, const sample_t blendParam, double wet) {


please break long lines.

"double wet" is misleading, because is is not the wet signal, it is the mixParam or the dryWetParam.
all other parameters are reduced to sample_t precision we should do the same for this parameter too.

daschuer · 2018-06-03T21:14:43Z

lib/reverb/Reverb.h

@@ -224,7 +224,7 @@ class PlateX2
 class MixxxPlateX2 : public PlateStub {
 	public:
 		void processBuffer(const sample_t* in, sample_t* out, const uint frames, const sample_t bandwidthParam,
-									const sample_t decayParam, const sample_t dampingParam, const sample_t blendParam);
+									const sample_t decayParam, const sample_t dampingParam, const sample_t blendParam, double wet);


break long lines.

daschuer · 2018-06-03T21:30:57Z

src/effects/builtin/echoeffect.cpp

@@ -82,7 +82,8 @@ EffectManifestPointer EchoEffect::getManifest() {
    send->setName(QObject::tr("Send"));
    send->setShortName(QObject::tr("Send"));
    send->setDescription(QObject::tr(
-        "How much of the signal to send into the delay buffer"));
+        "How much of the signal to send into the delay buffer\n"
+        "When the effect unit is in D+W mode, keep this turned up all the way"));


This comment does not makes sense. You can produce the same effect using the send and D+W knob and we have actually the meta knob mapped as default. So the user can use it as usual independent from the mix mode.

Have you actually tried this? The Send parameter acts as a volume knob for the whole chain in D+W mode.

Yes, that is the essence of a send knob in a return Mixxx.

Right, so the only use case for the Send parameter in this mode is to smoothly fade the effect out. It cannot be used to adjust the level of the Echo relative to the rest of the chain as it behaves in D/W mode. I see now that the wording of this tooltip may imply that the user should never use the Send parameter in D+W mode even in the case of smoothly fading out the effect, so I'll clarify that: "When the effect unit is in D+W mode, keep this turned up all the way unless fading the effect out"

Isn't this the original use case for this?
How about: "Note: In D+W mode the output of the delay buffer is the only input of the following effect."

Maybe it would be better to fully explain how the effect behaves in each mode:
"In D/W mode, the dry signal is passed through.
In D+W mode, only the echoed signal is output, so adjusting this parameter adjusts the volume of the whole chain."

daschuer · 2018-06-03T21:32:47Z

src/effects/defs.h

-    // The number of insertion types. Also used to represent "unknown".
-    Num_Insertion_Types
+enum class EffectChainMixMode {
+    DrySlashWet = 0,


"Crossover" Or "DryWetCrossover"

A crossover is a device that splits a full spectrum audio signal into different frequency bands for different amplifiers or speaker drivers. I do not think it makes sense to use that word here.

Please add a comment what it actually means.

daschuer · 2018-06-03T21:34:30Z

src/effects/defs.h

-    Num_Insertion_Types
+enum class EffectChainMixMode {
+    DrySlashWet = 0,
+    DryPlusWet,


"AddWetToDry"

I think the terms in the code should match the GUI.

OK, But in that case add a comment here what it actually means.

daschuer · 2018-06-03T21:57:07Z

src/effects/builtin/echoeffect.cpp

@@ -221,19 +225,19 @@ void EchoEffect::processChannel(const ChannelHandle& handle, EchoGroupState* pGr
        if (gs.ping_pong < delay_samples / 2) {
            // Left sample plus a fraction of the right sample, normalized
            // by 1 + fraction.
-            pOutput[i] = pInput[i] +
+            pOutput[i] =  (addDry ? pInput[i] : 0) +


these inner conditionals will likely detroy CPU pipelinig. It is better to move it out and duplicate some code for speed here.

daschuer · 2018-06-03T22:02:42Z

src/effects/builtin/reverbeffect.cpp

+        "How much of the signal to send in to the effect\n"
+        "Lowering this fades out the effect smoothly\n"
+        "Use this to adjust the amount of the effect when the effect unit is in D/W mode\n"
+        "When the effect unit is in D+W mode, keep this turned up all the way"));


The same here, the advice does not fit to the metaknob mapping.

There is no mechanism to magically switch the metaknob linking with the mix mode, nor do I think there should be. I think the best way to improve the UX with the Send and Dry/Wet parameters of this effect will be to include default effect chain presets with Mixxx.

Just remove the advise here. The Rest is OK. The meta knob mapping still works as desired even in D+W mode.

daschuer · 2018-06-03T22:09:04Z

lib/reverb/Reverb.cc

@@ -454,7 +454,8 @@ void MixxxPlateX2::processBuffer(const sample_t* in, sample_t* out, const uint f
 		sample_t mono_sample = blend * (in[i] + in[i + 1]) / 2;
 		sample_t xl, xr;
 		PlateStub::process(mono_sample, decay, &xl, &xr);
-		out[i] = xl + in[i];
-		out[i + 1] = xr + in[i + 1];
+		out[i] = (xl * wet) + (in[i] * (1 - wet));


We loose the original reverb sound now. if we set wet to 0.5, we have the original sound but with -6 dB.

daschuer · 2018-06-03T22:12:17Z

src/effects/builtin/reverbeffect.cpp

@@ -73,14 +76,31 @@ EffectManifestPointer ReverbEffect::getManifest() {
    send->setMinimum(0);
    send->setDefault(0);
    send->setMaximum(1);
+
+    EffectManifestParameterPointer dryWet = pManifest->addParameter();
+    dryWet->setId("dry_wet");


What is the use-case of this parameter? Can't we follow the echo implementation?

#1676 (comment)

It turns out that a dry/wet parameter is needed for the Reverb effect. Before now the effect Was always fully wet with no dry, with the amount of the effect control with the Send parameter.

No,It was always "out[i] = xl + in[i];"
The dry_wet knob makes it worse, now and is redundant with the Mix knob.

To test this I recommend playing with the chain BQ EQ -> Echo -> Reverb.

You can argue the same for the revers order, but the Echo has no Dry/Wet Knob.
I think this is a case that musically does not make much sense, compared to the GUI clutter.
We should stick with one concept. If it is really a desired effect, my preferred solution is to put it in a dedicated new effect.

All return effects like echo and reverb, do there effect by taking the dry signal, alter it and put it back, on top of the dry channel. If we chain a echo, and put the reverb into the delay path, it has no access to the dry signal and cannot work as return effect any more. Musically it might may sense to add it as insert effect here. In this case the reverb has to pass through the delayed signal as 100% and adds the reverb onto it. This can be controlled by the send button, and might be additional controlled by a mix knob at the output.

The same is true if you put the reverb first. I don't know if it makes sense to put an echo after but if the echo eats all non delayed signal, the reverb effect is gone.

So it looks like that the effect mode must only be consumed from one effect in the chain to allow the most desired effect without overcomplicate the GUI.

No,It was always "out[i] = xl + in[i];"

Whoops, you are right. I think I got confused because the Echo effect's Send parameter acts as a redundant volume knob for the whole chain in D+W mode, but with Reverb mixed as out[i] = xl + in[i] that is not the same.

So I think we can simply remove the new Dry/Wet parameter for Reverb.

Yes.
We need to decide if we need something else.
Should reverb follow the mix mode like echo? Is there a use case to add a filter to the reverb wet path?
If yes, I think only the first effect in the chain should switch to return mode.

Looking back at the diff of the commit where I introduced the new Dry/Wet parameter, I got confused because I had changed

out[i] = xl + in[i];

to

out[i] = xl + (addDry ? in[i] : 0);

in 2b72491 when I introduced the effect unit mix mode. That made it so the Send parameter of Reverb did act as a redundant volume knob of the whole chain in D+W mode. However, I didn't need to do anything to Reverb in 2b72491. Simply keeping out[i] = xl + in[i]; with the Send parameter controlling the input regardless of the effect unit mix mode as it was before works.

daschuer · 2018-06-03T22:14:45Z

src/effects/builtin/reverbeffect.cpp

+    dryWet->setDescription(QObject::tr(
+        "Mix between the input (dry) and output (wet) of the effect\n"
+        "Lowering this fades out the effect abruptly\n"
+        "Use this to adjust the amount of the effect when the effect unit is in D+W mode"));


Why is the Mix knob not sufficient for this?

The effect unit mix knob affects the whole chain.

Effect unit mixing mode

Be-ing added this to the 2.2.0 milestone May 20, 2018

Be-ing added the effects label May 20, 2018

Be-ing force-pushed the effects_mixing_mode branch from ae6a008 to df34782 Compare May 21, 2018 22:32

Be-ing force-pushed the effects_mixing_mode branch 3 times, most recently from 88d78fe to e4f179e Compare May 27, 2018 16:10

Be-ing added 3 commits May 29, 2018 10:27

pass EffectChainInsertionType to effects

2b72491

so Echo and Reverb do not add the dry signal to their output in "Send" mode

Tango: add button to effect units for insertion type/mix mode

b482b79

Reverb: add Dry/Wet parameter

fd25553

Using the Send parameter when the effect unit is in D+W mode does not work as desired because it reduces the volume of the whole effect unit.

Be-ing force-pushed the effects_mixing_mode branch from e4f179e to fd25553 Compare May 29, 2018 15:28

Be-ing changed the base branch from lv2_support2 to master May 29, 2018 15:28

Be-ing added 2 commits May 31, 2018 06:23

rename effect chain "insertion type" to "mix mode"

f3b9321

add tooltips for effect unit mix modes

2f8cb0e

Be-ing merged commit 4628782 into mixxxdj:master May 31, 2018

daschuer reviewed Jun 3, 2018

View reviewed changes

Be-ing mentioned this pull request Jun 4, 2018

effect unit mixing mode followup #1703

Merged

Be-ing mentioned this pull request Jun 12, 2018

Ramping fixes for Echo and Reverb effects #1709

Closed

ronso0 added a commit to ronso0/mixxx that referenced this pull request Jul 19, 2018

Merge pull request mixxxdj#1676 from Be-ing/effects_mixing_mode

f542eb4

Effect unit mixing mode

mixxxbot mentioned this pull request Aug 23, 2022

option to mix output of effects units without reducing dry signal #8790

Closed

Effect unit mixing mode #1676

Effect unit mixing mode #1676

Conversation

Be-ing commented May 20, 2018 • edited Loading

Be-ing commented May 21, 2018 • edited Loading

Be-ing commented May 21, 2018 • edited Loading

Be-ing commented May 27, 2018

ferranpujolcamins commented May 27, 2018

ronso0 commented May 28, 2018

Be-ing commented May 28, 2018

WaylonR commented May 30, 2018 • edited Loading

Be-ing commented May 30, 2018

kazakore commented May 30, 2018

Be-ing commented May 30, 2018 • edited Loading

kazakore commented May 31, 2018

kazakore commented May 31, 2018 • edited Loading

Be-ing commented May 31, 2018 • edited Loading

Be-ing commented May 31, 2018 • edited Loading

kazakore commented May 31, 2018

Be-ing commented May 31, 2018

kazakore commented May 31, 2018

Be-ing commented May 31, 2018

kazakore commented May 31, 2018 • edited Loading

Be-ing commented May 31, 2018 • edited Loading

daschuer commented May 31, 2018 • edited Loading

Be-ing commented May 31, 2018

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

Be-ing Jun 4, 2018 • edited Loading

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 Jun 4, 2018 • edited Loading

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

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

Be-ing Jun 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Be-ing commented May 20, 2018 •

edited

Loading

Be-ing commented May 21, 2018 •

edited

Loading

Be-ing commented May 21, 2018 •

edited

Loading

WaylonR commented May 30, 2018 •

edited

Loading

Be-ing commented May 30, 2018 •

edited

Loading

kazakore commented May 31, 2018 •

edited

Loading

Be-ing commented May 31, 2018 •

edited

Loading

Be-ing commented May 31, 2018 •

edited

Loading

kazakore commented May 31, 2018 •

edited

Loading

Be-ing commented May 31, 2018 •

edited

Loading

daschuer commented May 31, 2018 •

edited

Loading

Be-ing Jun 4, 2018 •

edited

Loading

daschuer Jun 4, 2018 •

edited

Loading

Be-ing Jun 4, 2018 •

edited

Loading