Add super knob adoption option for quick effect chain presets #11198
Conversation
|
"Effect load behavior" isn't really correct to describe what this new option controls. The new option affects presets, not effects, and only for QuickEffects. Something like "Preset load behavior for QuickEffects" would be more accurate. |
|
You load a group of effects by selecting a new preset, so the choice of the headline is somehow OK. Effects and Quick effects have a Super knob. One is applied when the preset switched and one when the effect is switched. This should be described. Unfortunately only in Shade the Superknob is visible. So in all other skins it will reset to an invisible value which is probably surprising. In addition the Super knob forces its value to the Meta knobs. So that the Metaknob setting is no longer relevant when chaining presets. We need to explain this somehow as well. For my feeling all these settings will be hard to understand for a newbie. I can also imagine I want both behaviours conditionally. On one hand we want to start with a good effect setting after selecting an effect, on the other hand your use case is also very valid. Is there a way to have both? I think this is al around a standard Superknob scaling with the neutral position at center. What do you think? |
|
I have tested a bit and realized that the super knob setting in the chain presets is only applied at the quick effect. That makes chain loading to the effect rack differently to the quick effect rack. When a preset with different meta-knob defaults is created in the effect unit and than loaded to the quick effect, the default is broken. Room for optimization. |
@Be-ing So, move this to a new 'Preset load behavior' section?
Maybe for now this should be changed to a radio button group where you can only choose one of 'Keep super knob position', 'Keep meta knob position', and 'Reset super knob and meta knob positions to effect or preset default'?
Yes, that's true. The original 'reset to the default' behavior was specifically only implemented for quick effects and not all effect chains (for reasons that escape me), so I didn't change that behavior: mixxx/src/effects/chains/quickeffectchain.cpp Lines 53 to 58 in a3ad1cd Should this whole behavior be moved to |
|
Yes that would have the side effect that we can no longer have individual meta knob defaults in the normal Effect rack. So the implementation is good for now. We just need to tweak the GUI strings. Did you consider a future solution with a unified super knob scaling? |
If the option only applies to QuickEffects, that should be indicated in the GUI, for example "QuickEffect preset load behavior". |
What exactly do you mean by unified super knob scaling? I use this as one of the four filter+effect presets with my Traktor Kontrol S3 (#11199):
So at the neutral center position there's no signal feeding into the delay. But yeah the current meta knob assignment is a bit limiting and doesn't work for everything. Being able to assign the meta knob to only part of a knob's range (by dragging an outer ring, for instance) and to have the unipolar/bipolar behavior work more similarly to the modulation curves in Bitwig would be great:
That would probably involve a bit of UI work though. |
|
how does this compare to #4687 ? |
With some effect chains loaded to the Quick Effect knob, we have the neutral position at centre, some are at the left and some do not have a neutral position. I like to a have a "reliable" mode like shown here https://www.youtube.com/watch?v=aL3_keBlCUE where we have always two natures of the same effect on both sides of the Super knob. Of cause it is easy to mess it up, but currently not even our defaults follow this rule. To make this reliable, we need IMHO a filter, that shows only these types of effects on the quick effect selector. Something like an With this in place, the setting "Keep Superknob position" can be default. There is just the corner case for effects not following the rule for being a Quick Effect. They may reset to default that we need to "Soft-Takover" the value. With this in place we probably do not even need a preference option for this. I am afraid this "Soft-Takover" is currently broken, need to test. The second screenshot is interesting. But I am afraid such a fixed enum list is not flexible enough. This probably can be implemented by additional scaling parameters in addition to the current flags. They are: We have current:
and
I think we need two individual parameter for scaling left and right. Since modelling them can't be done during a gig, It would work for me if this requires to open a text editor. A normal user will unlikely tweak them if we have good defaults. |
|
#4687 Covers the issues with soft-takeover. I am afraid we need to incorporate both ideas for a consistent solution. |
@ywwg I tried it but it doesn't seem to work: https://streamable.com/ase2lw Personally I'd be totally okay with a setting that always preserves meta knob values like that if it worked for this use case because the behavior seems less confusing.
Got it. That's the same thing I want and use with the Traktor Kontrol S3 script. Having that be the standard behavior for quick effects would be great and it would indeed match the behavior from DJM mixers and other DJ software (like the Mixer FX in Traktor I'm emulating, which is exactly the same thing).
So those modes are in addition to modulation assignment. You see that 0.415 value in the screenshot? That's the amount the parameter is modulated with when you move the macro knob from 0 to 1 (and it can also be bipolar and go -0.415 in the other direction). You can of course set this value to anything you want, so -0.5 is also an option and then moving the knob from 0 to 1 would cause the target parameter's value to be decreased by 0.5. |
|
I don't think I will have time to continue work on #4687. Since it already underwent a number of reviews maybe it could be a starting point for this fix? @robbert-vdh would you be up to merging them together? And since this is blocking the Traktor S3 change, I would recommend that we try to keep the fixes small and iterative, I am afraid of PR scope creep with all of these edge-cases. |
|
Yeah either way is fine. I think #4687 and this PR address different issues and both fixes should probably be implemented eventually. I won't be able to work on this for the next week or so though. |
|
This PR is marked as stale because it has been open 90 days with no activity. |
Does commenting remove the stale label? (one way to find out) |
|
Sorry for the bogus stale label. We'll try to get to this PR asap. |
|
@robbert-vdh what happens if we merge this without the code fix? Does it break the mapping or is it just harder to use? Can the existing fx mapping work ok, and only the new fx mapping would be broken? |
|
The mapping itself will work just fine. Without the feature enabled it will just lead to slightly more confused users if they're changing preset chains while the knob is not in its neutral position. |
00442dc to
e04bbfb
Compare
Yes, 'meta/super knob' would be correct for the english source. |
| <property name="title"> | ||
| <string>Effect load behavior</string> | ||
| </property> | ||
| <layout class="QHBoxLayout" name="horizontalLayoutMetaKnob"> | ||
| <layout class="QVBoxLayout" name="verticalLayoutEffectKnobOptions"> |
There was a problem hiding this comment.
Ideally, to keep "Keep.." and "Reset.." options aligned vertically, this would be a QGridLayout (i.e. no sub-layouts, just 4 items with explicit row & column attributes.
Tr strings only differ by one character (super vs. meta), so there's no significant misalignment to expect, though the perfectionist part of me prefers the grid layout ;)
@robbert-vdh If you don't have time to convert this let me know.
There was a problem hiding this comment.
I somehow completely missed this comment, sorry!
I rebased the PR on main (only had to add an include) and changed this to a QGridLayout.
|
I would love to get this in but I think it needs to wait for 2.5. It does not seem critical for 2.4. Would people support rebasing this PR to 2.5 or main? |
|
Yes. I don't think rebate is required. |
|
removing 2.4 milestone because of the rebase -- do we want this for 2.4.1 or 2.5? |
|
No objections for 2.4.x |
This is similar to the existing option for meta knobs.
e04bbfb to
a73857c
Compare
|
I rebased this onto |
| @@ -180,6 +180,10 @@ bool EffectsManager::isAdoptMetaknobSettingEnabled() const { | |||
| return m_pConfig->getValue(ConfigKey("[Effects]", "AdoptMetaknobValue"), true); | |||
| } | |||
|
|
|||
| bool EffectsManager::isAdoptSuperknobSettingEnabled() const { | |||
| return m_pConfig->getValue(ConfigKey("[Effects]", "AdoptSuperknobValue"), false); | |||
There was a problem hiding this comment.
We have recently adopted a strict snake_case as the standard for config key
| return m_pConfig->getValue(ConfigKey("[Effects]", "AdoptSuperknobValue"), false); | |
| return m_pConfig->getValue(ConfigKey("[Effects]", "adopt_superknob_value"), false); |
|
oo yeah this is a PR we should try to resurrect |
This is similar to the existing option for meta knobs. This option is needed for DJ controllers that allow the user to apply different quick effect chain presets to tracks, like the Mixer FX section on the Native Instruments Traktor Kontrol line of controllers. The idea with those controllers is that you can pick from a plain filter, or that same filter plus an effect (they have a delay, reverb, white noise, and trance gate), and assign that to a channel. You'd usually select the effect while the filter knob is still in its neutral 12 o' clock position, but if it is not then the filter frequency settings should be preserved and the other effect should merely be added on top of that. I have a Traktor Kontrol S3 script improvement ready that adds this exact feature to Mixxx. That also avoids the potential issue where an effect chain's default super knob position isn't quite at 0.5 and you need to fiddle around with the filter knob to get the soft takeover to work.
This PR doesn't change the default behavior, so if you don't enable the option nothing changes. This change also uncovered another issue with Mixxx's effect handling where changing these chain presets will not immediately switch between presets (when the new one is done loading)i. Instead it removes the old one, and it then takes one or more buffers before the new chain takes effect. Fixing this will require more investigation in the future.
I've added the preference next to the existing meta knob preference using the same style. By the way, shouldn't this be 'meta knob' and 'super knob' instead of 'metaknob' and 'superknob'?
