move effects post-fader by Be-ing · Pull Request #1254 · mixxxdj/mixxx

Be-ing · 2017-05-09T20:16:41Z

I haven't done very extensive testing nor profiling with this yet. Suggestions for optimization are welcome.

Slamming down the volume fader and hearing an echo tail is quite satisfying. :)

TODO:

fix master equalizer
fix failing PFL signal path tests

daschuer · 2017-05-09T20:31:31Z

 // static
 void SampleUtil::applyRampingGain(CSAMPLE* pBuffer, CSAMPLE_GAIN old_gain,
        CSAMPLE_GAIN new_gain, SINT numSamples) {
+    if (old_gain == new_gain) {


This condition is also checked below. I think the bypass below could be removed.

Be-ing · 2017-05-09T20:42:40Z

Signal flow is:
pregain -> fader and crossfader attenuation -> effect processing -> effect chain dry/wet knob -> mix channels into master mix and crossfader busses

Signal flow was:
pregain -> effect processing -> effect chain dry/wet knob-> fader and crossfader attenuation -> mix channels into master mix and crossfader busses

Is there any use case for pre-fader effects that cannot be satisfied with the new signal flow? If not, I do not think there should be an option for pre-fader effects and they should always be post-fader.

daschuer · 2017-05-09T20:49:41Z

I like this solution! Now the Dry/Wet knob is back to being useful.

I now stumple over the known bug in the echo effect. It remembers the last echo when disabled.
This should not happen so there are no samples in the echo buffer when re-enabled again.
The Moog filter suffers the same issue.

We should clear about this, that we loose the pre-fader effects with this PR, but in favor of a cluttered GUI support this idea. We have still the Quick effect rack as pre-fader.
I have no use-case that we loose or cannot support now.
The only issue is that a synthesizer effect cannot be faded anymore with the cross or line fader.
You need to use the disable button or the D/W knob.

Be-ing · 2017-05-09T21:17:38Z

I now stumple over the known bug in the echo effect. It remembers the last echo when disabled.
This should not happen so there are no samples in the echo buffer when re-enabled again.
The Moog filter suffers the same issue.

Yeah, I think those need to be modified to clear their internal buffers when the effect is disabling.

The only issue is that a synthesizer effect cannot be faded anymore with the cross or line fader.
You need to use the disable button or the D/W knob.

I think this simple UX and signal flow is worth that small trade-off.

Echo tails!

Be-ing · 2017-05-10T07:35:09Z

Moving the effects post-fader means that decks assigned to pre-fader listening do not get effects applied in headphones unless an effect unit is assigned to the headphone channel. It would be possible to apply effects before making the headphone mix in addition to making the master mix, but I do not know if that is worth the extra CPU use. Considering that typically 0 or 1 decks are assigned to PFL at a time, I think it could be okay. Thoughts?

This is redundant with the application of effects for the master mix, but considering that typically 0 or 1 decks are assigned to PFL at a time, I think this is worth the CPU use cost.

Be-ing · 2017-05-10T16:00:36Z

New signal flow:

pregain ---> fader and crossfader attenuation --> channel effect processing  --> mix channels into master mix and crossfader busses --> master effect processing
         \
           --> channel effect processing --> effect chain dry/wet knob --> mix channels into headphone mix --> headphone effect processing

How it is in master:

pregain --->  channel effect processing --> effect chain dry/wet knob -->  fader and crossfader attenuation --> mix channels into master mix and crossfader busses --> master effect processing
                                                                       \
                                                                         --> mix channels into headphone mix --> headphone effect processing

daschuer · 2017-05-10T20:08:35Z

Build fails:

src/test/enginemastertest.cpp:80:56: error: cannot allocate an object of abstract type ‘{anonymous}::EngineChannelMock’

             "[Test1]", EngineChannel::CENTER, m_pMaster);

I also think the headphone issue is now overdone.
The knob is labels PFL = "Pre Fader Listening" - and it should do this.

The headphone has already an effect hock. (used in Shade) this should be sufficient to test certain effects before using them live. This way we have dedicated knob to decide if an effect should be applied or not.

Be-ing · 2017-05-10T20:16:11Z

The headphone has already an effect hock. (used in Shade) this should be sufficient to test certain effects before using them live. This way we have dedicated knob to decide if an effect should be applied or not.

But controllers don't have buttons for this purpose. Also, the headphone channel does not carry the channel metadata, so tempo synced effects sound different.

Be-ing · 2017-05-10T20:16:47Z

Build fails

Yes, I am aware. I will look into it. It builds if you don't build the tests.

daschuer · 2017-05-11T14:40:28Z

The headphone has already an effect hock. (used in Shade) this should be sufficient to test certain effects before using them live. This way we have dedicated knob to decide if an effect should be applied or not. But controllers don't have buttons for this purpose. For me the classic PLF behaviour is somehow mandatory. I understand also how to preview an effect is helpful but it should be not pauschal consume the double of CPU without explicite requested. We want also sick with a simple interface without any additional button. Unfortunately non of the proposed solutions really suits well here. I also do not like the Traktor solution, to configure the effect Rack pre/after fader. This after fader solution really performes nice, so we now need a smart effect preview solution .... :-/ Ideas?

Be-ing · 2017-05-11T15:16:40Z

I think the extra CPU cost is reasonable. It is like enabling the effect unit on one more deck for each deck the headphone button is pressed on. The cost should be documented in the manual though. I am reluctant to add a preference option to disable this, but if it is a requirement for running Mixxx well on slow hardware it may be worth it to put a checkbox in the effects preferences.

I also do not like the Traktor solution, to configure the effect Rack pre/after fader.

I agree. I don't think there's any need for that complication.

Be-ing · 2017-05-11T15:19:02Z

+        for (unsigned int i = 0; i < iBufferSize; ++i) {
+            pOutput[i] = pBuffer0[i];
+        }
+    } else if (totalActive == 2) {


@rryan could you take a look at this? Do you have any suggestions for maximizing efficiency?

memcpy()? Something like this should do:

memcpy(pOutput, pBuffer0, sizeof(pBuffer0[0]) * iBufferSize);

Wouldn't that only work for the special case of 1 channel active?

Honestly I only looked at the for() loop itself - it seems to copy iBufferSize items from pBuffer0 to pOutput. A memcpy call will do the same, but in one "big" transaction, so should provide better performance.

Again; I didn't look at the general code around the loop - so I don't know can the whole lot of it be re-worked for performance.

if only one channel is routed to headphones

ferranlala · 2017-05-11T21:44:07Z

Thank you very much for tackling this!
Doesn't the send parameter of the reverb effect become redundant after this?

ferranlala · 2017-05-11T21:59:54Z

I've noticed that the mix knob does not work the same with echo and phaser than reverb:

With echo and phaser it works as a fader of the effects output, i.e. it only affects the volume of the output of the effect.

With reverb it works as a dry/web know, i.e. it affects both the output of the effect and the output of the deck.

Which is the intended behavior? For me, the former is better.

ferranlala · 2017-05-11T22:23:37Z

About the pfl issue: for me, the best solution is to offer dedicated pfl switches for the effects.

For controllers with no dedicated effect pfl button, we can offer a new CO called "effect_pfl_link".

When "effect_pfl_link" is on, the pfl switch of a channel is bound to the pfl switch of the decks it has assigned. Example:

Deck 1 pfl	Deck 2 pfl	Effect pfl
Off	Off	Off
User switches pfl of deck 1:
On	Off	On
User switches pfl of deck 2:
On	On	On
User switches pfl of deck 1:
Off	On	On
User switches pfl of deck 2:
Off	Off	Off

Truth table for effect pfl switch (assuming 2 decks):

Deck 1 pfl	Deck 2 pfl	Deck 1 assigned to fx	Deck 2 assigned to fx	Effect pfl
0	0	0	0	0
0	0	0	1	0
0	0	1	0	0
0	0	1	1	0
0	1	0	0	0
0	1	0	1	1
0	1	1	0	0
0	1	1	1	1
1	0	0	0	0
1	0	0	1	0
1	0	1	0	1
1	0	1	1	1
1	1	0	0	0
1	1	0	1	1
1	1	1	0	1
1	1	1	1	1

ferranlala · 2017-05-11T22:52:13Z

@Be-ing your branch introduces a crackling distortion (similar to bitcrusher) to the headphone channel. I tried commit ac5791d.

Be-ing · 2017-05-12T03:30:12Z

I've noticed that the mix knob does not work the same with echo and phaser than reverb:
With echo and phaser it works as a fader of the effects output, i.e. it only affects the volume of the output of the effect.
With reverb it works as a dry/web know, i.e. it affects both the output of the effect and the output of the deck.
Which is the intended behavior? For me, the former is better.

I don't understand what you mean. The dry/wet knob is always a dry/wet knob. Some effects like echo generate a new signal that is mixed with the input, others modify the input signal. You can use the send knobs for reverb and echo to control their relative prominence within a chain.

Doesn't the send parameter of the reverb effect become redundant after this?

No, it is still helpful to control how prominent the reverb is within a chain.

For controllers with no dedicated effect pfl button, we can offer a new CO called "effect_pfl_link".

This is way too complicated. There is no space on controllers to introduce new controls to the design. If you press the headphone button on a deck, and that deck has effects on it, you should hear the effects in your headphones. No more complications are needed.

The only use case I have come up with for the headphone channel assignment switch on effect units is to preview how an effect will sound on a deck that is currently playing in the master mix before assigning it to that deck and bringing the effect into the master mix. Does anyone else have other uses for it?

@Be-ing your branch introduces a crackling distortion (similar to bitcrusher) to the headphone channel. I tried commit ac5791d.

I haven't had a chance to test that latest change much yet, but I'll listen for that. Does that occur with the commit prior to that? What effects do you have loaded, which of them are enabled, what deck(s) are they assigned to, and how is the dry/wet knob set? Are you using multiple sound cards?

Thank you for testing.

daschuer · 2017-05-12T05:51:29Z

With reverb it works as a dry/web know, i.e. it affects both the output

of the effect and the output of the deck. In anny case, the dry/wet knob schould work like this: dry = effect bypassed. Wet = insert effect, no bypass. It now depends if the effect, if it has an extra internal bypass like echo. The phaser is a special effect, because of the phase shifter effect signal mixed with the bypass can knock out some extra frequencys. (A 180° phase shift = silence) The same is true for all filter and EQ effects that introduced a phase shift. The bypass and effect signal at incompatible on the time line.

ferranlala · 2017-05-12T06:53:56Z

With the echo is all ok: with the mix knob I control the volume of the effect and the volume of the deck is not affected.

But, with the reverb, I feel that the volume of the deck decrease when I turn the mix knob. Like the deck output and the effect output were crossfaded. You can check this is true by setting the bandwidth parameter full CCW and setting send and mix full CW: You don't listen to the deck output, only the effect output.

Maybe internally the reverb is still configured as an effect that modifies the original signal, rather than an effect that generates a new signal. I've just checked and this also happens on master, so I guess this is not introduced in this PR. Shall I file a bug for this?

Doesn't the send parameter of the reverb effect become redundant after this?

No, it is still helpful to control how prominent the reverb is within a chain.

Yes you are right, send knobs need to be there.

There is no space on controllers to introduce new controls to the design. If you press the headphone button on a deck, and that deck has effects on it, you should hear the effects in your headphones. No more complications are needed.

Do you think a preferences option could be introduced to choose between this?:

Dedicated per effect pfl switches
You always listen to effects on the decks pfl.

Be-ing · 2017-05-12T17:51:34Z

Do you think a preferences option could be introduced to choose between this?:
Dedicated per effect pfl switches
You always listen to effects on the decks pfl.

No, effects should just work on decks assigned to the headphone channel. They do in master, and they should after merging this. The only reason that might not be desirable is because of the CPU usage cost. It may be a decent idea to add a preference option to disable that for slow hardware. On the other hand, is it practical to use effects in the first place on such hardware?

Be-ing · 2017-12-29T00:51:41Z

Actually it was 433e662 that broke it...

Be-ing · 2017-12-29T14:02:23Z

Fixed in #1447

kazakore · 2018-03-05T08:14:49Z

I think you have both overcomplicated things and also gone for a method which differs so much from the industry standard that you are going to confuse a lot of people and get a lot of complaints about the effects not working as expected. Look at the DJM mixer effects path for example (page 14 of the DJM800 service manual can give you an idea) or the physical layout of any physical mixing desk with channel inserts. The effects should return to the channel it is sent from. That is BEFORE the crossfader, not after it. (Although if you assign a fx to a side of the crossfader on the DJM then it is after the crossfader attenuation, rather than applied to everything on the incoming side, but we're talking about effects applied to the channel here.)

This would give you the echos/reverb tails on brining down the channel fader as you all voiced the desire to have at the start of this thread. It would also give you the ability to cut them cleanly using the crossfader, which is often desirable when using reverb to close off a mix (you don't want that muddy tail continuing when you cut into a drop.) And it would save with all this messy confusion of having to have an addition PFL effects channel as long as the channel fader is up.

This is completely ignoring the issue of dynamic effects sounding completely different depending on being pre or post, which Be is now working a hack to fix rather than giving the user the choice per fx chain....

cgrinham · 2018-03-05T09:05:33Z

@kazakore Have you tried it out? I've been using this branch for a while now and itr works how you describe: Fader leaves an effect tail, cross-fader cuts effect cleanly.

kazakore · 2018-03-05T09:19:37Z

Sorry didn't have access to Mixxx when I posted an I didn't notice that GitHub had hidden 344 posts in this thread so there was actually a lot I hadn't read, having only seen the start and the end. If this has already been resolved then I apologise and am happy to hear it :-)

ronso0 · 2018-03-05T12:31:00Z

I've been using this branch for a while now and itr works how you describe: Fader leaves an effect tail, cross-fader cuts effect cleanly.

@christophski Neither in master nor 2.1 branch the crossfader would cut the effect tail. That's why I opened https://bugs.launchpad.net/mixxx/+bug/1744086

kazakore · 2018-03-05T13:21:55Z

@kazakore Have you tried it out? I've been using this branch for a while now and itr works how you describe: Fader leaves an effect tail, cross-fader cuts effect cleanly.

I just tested and it DOES NOT work as I describe. Add and Echo with maximum Send and Feedback for a (near) infinite echo and it will continue no matter the position of the crossfader! Thus my post still stands as a way to fix both this and the messy PFL implementation.

cgrinham · 2018-03-05T13:25:19Z

Ah sorry, I didn't have it in front of me and that's how I remembered it

Be-ing · 2018-03-05T14:32:33Z

There are already 430 comments on this PR and it is a pain to get GitHub to load them all. If you want to continue discussion about effects routing, let's move the discussion to Zulip.

IMO adding a pre/post-fader switch per effect unit would be a bad idea because it would be easy to get confused about which effect unit is in which state, which could mess up a mix if the user expects an echo tail but instead gets silence. Instead I think @ronso0's idea to add an option to configure all effect units pre/post crossfader would make sense. That way the user could chose to move either depending on what they want without having to keep track of more mutable state in their mind. The other way around like the Pioneer DJM mixers (post crossfader and pre channel fader) would not work well for Mixxx because the crossfader buses do not carry the metadata associated with the channel like the tempo (the master tempo could be used, but that wouldn't be as reliable).

daschuer · 2018-03-05T15:13:06Z

I also support the crossfader configuration pre/post. I think we can make it default as pre crossfader or even ditch the config option.
Mixxx has already effect hocks for the xfader busses. The tempo info is currently lost for them, but we can pass it ahead from the channels.

Previously, the autogenerated code allowed for a vectorizing optimization by combining gain application (channel faders + crossfader) with channel mixing. When postfader effects were implemented in Mixxx 2.1 (PR mixxxdj#1254), these two operations had to be decoupled to process effects between gain application and channel mixing. Therefore, the autogenerated code lost its advantage and became an overcomplication. Unused autogenerated functions were removed from SampleUtil as well. Some of the autogenerated SampleUtil functions are still in use, so they were kept.

Previously, the autogenerated code allowed for a vectorizing optimization by combining gain application (channel faders + crossfader) with channel mixing. When postfader effects were implemented in Mixxx 2.1 (PR mixxxdj#1254), these two operations had to be decoupled to process effects between gain application and channel mixing. Therefore, the autogenerated code lost its advantage and became an overcomplication.

Be-ing force-pushed the postfader_effects branch 2 times, most recently from c1cb47b to d5c58c4 Compare May 9, 2017 20:27

daschuer reviewed May 9, 2017

View reviewed changes

Be-ing force-pushed the postfader_effects branch from d5c58c4 to db8cfca Compare May 9, 2017 20:32

Be-ing force-pushed the postfader_effects branch from db8cfca to 89e3a2d Compare May 9, 2017 21:28

move effects post-fader

47de8db

Echo tails!

Be-ing force-pushed the postfader_effects branch from 89e3a2d to 47de8db Compare May 10, 2017 07:20

apply effects to PFL channels before mixing into headphone mix

ee10191

This is redundant with the application of effects for the master mix, but considering that typically 0 or 1 decks are assigned to PFL at a time, I think this is worth the CPU use cost.

ronso0 mentioned this pull request May 10, 2017

sync echo effect to BPM #1256

Merged

fix engine tests build

6824195

Be-ing commented May 11, 2017

View reviewed changes

use channel features for headphone effects processing ...

ac5791d

if only one channel is routed to headphones

Be-ing mentioned this pull request Feb 19, 2018

lp1744550: Fix use-after-free (and more serious issues) in GlobalTrackCache #1492

Merged

Be-ing mentioned this pull request May 2, 2018

Lv2 support2 #1240

Merged

Be-ing mentioned this pull request Dec 27, 2018

remove autogenerated code from ChannelMixer #1966

Merged

Be-ing mentioned this pull request May 7, 2020

effect chain preset saving/loading, custom effect defaults, and parameter hiding/rearrangement #2618

Closed

15 tasks

Be-ing mentioned this pull request Oct 20, 2021

This descibes possible causes for a rare crasher with effects. #4460

Closed

mixxxbot mentioned this pull request Aug 24, 2022

auxilliary send bus #9020

Open

Uh oh!

Conversation

Be-ing commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer May 9, 2017

Choose a reason for hiding this comment

Uh oh!

Be-ing commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented May 9, 2017

Uh oh!

Be-ing commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Be-ing commented May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Be-ing commented May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented May 10, 2017

Uh oh!

Be-ing commented May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Be-ing commented May 10, 2017

Uh oh!

daschuer commented May 11, 2017 via email

Uh oh!

Be-ing commented May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Be-ing May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harryhaaren May 15, 2017

Choose a reason for hiding this comment

Uh oh!

Be-ing May 15, 2017

Choose a reason for hiding this comment

Uh oh!

harryhaaren May 15, 2017

Choose a reason for hiding this comment

Uh oh!

ferranlala commented May 11, 2017

Uh oh!

ferranlala commented May 11, 2017

Uh oh!

ferranlala commented May 11, 2017

Uh oh!

ferranlala commented May 11, 2017

Uh oh!

Be-ing commented May 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented May 12, 2017 via email

Uh oh!

ferranlala commented May 12, 2017

Uh oh!

Be-ing commented May 12, 2017

Uh oh!

Be-ing commented Dec 29, 2017

Uh oh!

Be-ing commented Dec 29, 2017

Uh oh!

kazakore commented Mar 5, 2018

Uh oh!

cgrinham commented Mar 5, 2018

Uh oh!

kazakore commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronso0 commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kazakore commented Mar 5, 2018

Uh oh!

Be-ing commented May 9, 2017 •

edited

Loading

Be-ing commented May 9, 2017 •

edited

Loading

Be-ing commented May 9, 2017 •

edited

Loading

Be-ing commented May 10, 2017 •

edited

Loading

Be-ing commented May 10, 2017 •

edited

Loading

Be-ing commented May 10, 2017 •

edited

Loading

Be-ing commented May 11, 2017 •

edited

Loading

Be-ing May 11, 2017 •

edited

Loading

Be-ing commented May 12, 2017 •

edited

Loading

kazakore commented Mar 5, 2018 •

edited

Loading

ronso0 commented Mar 5, 2018 •

edited

Loading