LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8.19k stars 1.01k forks source link

Why is the dry/wet parameter for effects -1 to 1 instead of 0 to 1? #3261

Open PaulBatchelor opened 7 years ago

PaulBatchelor commented 7 years ago

As the the title states, I've noticed that Wet/Dry knob with every plugin effect has a range of -1 to 1. Is there a particular reason for this? Numerically, it makes more sense to have this parameter be 0 to 1 where 0 = all dry and 1 = all wet. It would also save a computational step of scaling. If you don't scale the bipolar range, you end up getting 1 = all wet, 0 = all dry, -1 = all wet, with inverted, -0.5 = 50/50 with inverted phase.

softrabbit commented 7 years ago

I guess mixing the effect return in inverted might be useful in some cases, so why not keep it. Won't probably make much of a difference for delays or something like that, but I know some effects do sound different when turning the knob left.

If you don't scale the bipolar range, you end up getting 1 = all wet, 0 = all dry, -1 = all wet, with inverted, -0.5 = 50/50 with inverted phase.

Off the top of my head, the formulas should look roughly like this: out = in * (1-wetdry) + effect * wetdry; // if wetdry is from 0 to 1 out = in * (1-abs(wetdry)) + effect * wetdry; // wetdry from -1 to 1

Taking the absolute should IIRC be equivalent to nulling the sign bit or something like that, so it's not a huge operation.

EDIT:: ...but of course LMMS does it like this: https://github.com/LMMS/lmms/blob/48cc3bb5ef85bac8cc7c01b11dd3139c547a0677/include/Effect.h#L110 which in my eyes looks like it's the first formula with the second range, so a -1.0 wet level means (1- (-1) )*in + (-1) * effect or twice the input signal mixed with the inverted effect. Could be that I didn't dig deep enough, but right now I can't say anything but WTF?

DeRobyJ commented 7 years ago

That "-1 to 0" range isn't meant to be used with all the effects. It has some uses with some effects, like the Plate reverb. Turn on blending at max, and you won't hear the original sound anymore. But put dry/wet to -1 and you'll hear both the awesome reverb and the original sound.

It mostly has no point, but I think it should stay there :)

tresf commented 7 years ago

I too would like to see justification for this. If it's rarely used, it should be a toggle and the full range should be given back to the user.

PaulBatchelor commented 7 years ago

That "-1 to 0" range isn't meant to be used with all the effects.

Because it is included with every effect plugin, it's probably best practice not to overload the functionality of this knob. Making it unipolar would greatly simply the functionality of this knob. Also, it would bring back more knob real estate for those that want to fine tune the wet-dry balance (however, in the case of #3202, I've converted the bipolar range to unipolar.)

DeRobyJ commented 7 years ago

OK I agree it is strange, but some users have it set to -1. How do we not break backwards compatibility?

tresf commented 7 years ago

How do we not break backwards compatibility?

We'll always break backwards compatibility a little bit with every release. A better question perhaps is "how important is the backwards compatibility" and "how do we mitigate it".

it's rarely used, it should be a toggle and the full range should be given back to the user.

Note, if this was a toggle, there would be no effective compat problem.

PaulBatchelor commented 7 years ago

I say the break is worthwhile in the long run. Having a 0-1 is a more standardized approach to knobs with audio plugin APIs (VST and AU do this for instance.) The current implementation of -1 to 1 for dry/wet is definitely an LMMS quirk.

DeRobyJ commented 7 years ago

Agreed, just realized you can get the same effect as -1 by putting 0.5

What about having two knobs: dry and wet? It feels more intuitive

PaulBatchelor commented 7 years ago

I would argue that it is more mentally taxing to have two knobs instead of one. This task can be achieved with bussing and effects sends.

From a mixing point of view, two also complicates the gain staging. What is the level when they are at 0db? How about when one is at -6db and the other at -3db? Don't forget you can also change levels in the track fader, the master master, the send amount, etc...

Meanwhile, with a dry/wet knob, it's much easier to conceptualize the relationship because it's a balance. It's also more natural to automate. If I have a reverb plugin, and I crank up the wet/dry to be all wet, it gives the effect that the sound is moving away from the listener. Twice as much work to automate with two sliders.

I would recommend that specific plugins that do benefit from separate wet/dry control do so in their plugin UI rather than make it default. This is precisely what I've done for the Reverb plugin I have made for LMMS.

PaulBatchelor commented 7 years ago

Update to this:

It looks like the dryLevel() parameter and the wetLevel() parameter have different ranges.

I got this by dropping printfs in the render loop of my plugin where these are called. The two values will always add up to "1", and that's about the only thing resembling a wet dry knob that I see. Not only are these values messing with phase inversion, but they are also adding ~3dB to the gain staging.

zonkmachine commented 7 years ago

Yeah, I couldn't get my head around that one.

    inline float wetLevel() const
    {
        return m_wetDryModel.value();
    }

    inline float dryLevel() const
    {
        return 1.0f - m_wetDryModel.value();
    }

From Effect.h Shouldn't dryLevel() read: return 1.0f - fabs( m_wetDryModel.value() ); ?

PaulBatchelor commented 7 years ago

The original equation assumes m_wetDryModel.value() is between 0 and 1.

Adding fabs would make the dryLevel() 0 to 1, but it would make the wet level -1 to 1. You just can't have that.

I haven't tested this yet, but the only fix you would have to make is here: https://github.com/LMMS/lmms/blob/master/src/core/Effect.cpp#L49

PaulBatchelor commented 7 years ago

Looking at commit history and git blame, it would appear this range was accidentally changed by someone doing some sweeping changes to the codebase. Prior to that, the range was correct before:

https://github.com/LMMS/lmms/commit/f39c9641abd9f223f16084d41c70a7445b008a22#diff-4707ec0d66434cc27ae2716b4a33c04fL46

JohannesLorenz commented 5 years ago

If it was only introduced by a typing mistake, why not fix it?

Proposal:

  1. Bash the d/w model back to 0..1
  2. Add models "deprecated d/w" and "use deprecated d/w", which are hidden by default (and "use ..." is false by default)
  3. If you load an old file (between the bad commit and "now") where
    • d/w is negative initially or has any automation/controller which makes it negative at any time, set "use ... to true" and connect the "deprecated d/w" knob to the automations/controllers (instead of connecting "d/w" to it)
    • none of the above is true, load the file as usual
  4. show both models in the UI exactly if "use..." was true after loading a file
  5. for the computation, to prevent conflict between d/w and deprecated d/w, "use ..." will decide which knob is used

@pgiblock can you state that it's a typing mistake, or did you add it on purpose?

JohannesLorenz commented 5 years ago

@tresf Unfortunatelly I've made a proposal without looking at yours, as this issue is redundant to the PR #3422 . I think it's almost the same, except:

I need to think more about it...

tresf commented 5 years ago

@JohannesLorenz I see you mentioned me specifically. I'm not sure what proposal (I made?) you're referring to, but we can handle this as either a breaking change to fix it, or as a backwards compatibility issue to fix it and maintain backward compat.

I see little downside to allowing backwards compatibility. We could even be more aggressive and remove the compatibility option on all newly added controls, so that the user never even noticed it and we don't need to code anything into the GUI. If we do that, adding a debug message to the console and some // TODO: items in the source code for permanent removal would be nice.

No matter what we break, the user always has the option to use an older build of LMMS. Most musicians have exploited a bug along the way for a certain sound effect. This is no different.

JohannesLorenz commented 5 years ago

@tresf I meant your proposal in the PR. If you prefer breaking compatibility, that's OK for me, though in the linked PR (#3422), compatibility break was coded and then rejected by most users.

So I'd rather prefer compatibility. If it's possible, why not?