Keep all meta knobs when loading presets if the user preference is set to keep the values. by ywwg · Pull Request #4687 · mixxxdj/mixxx

ywwg · 2022-02-26T20:18:32Z

The preference option "keep metaknob value" currently only restores the top-level meta value instead of applying it to the underlying effects.
Because the loaded preset will use the default metaknob values, the preference is essentially inoperative.
This causes the meta knobs in the underlying effects to be out of sync with the user's controller.
If the user wants to overwrite meta values with the saved presets, they can change the preference to restore to default values.

…t to keep the values. The preference option "keep metaknob value" currently only restores the top-level meta value instead of applying it to the underlying effects. Because the loaded preset will use the default metaknob values, the preference is essentially inoperative. This causes the meta knobs in the underlying effects to be out of sync with the user's controller. If the user wants to overwrite meta values with the saved presets, they can change the preference to restore to default values.

daschuer

This looks reasonable to me.
I will delay merge a bit to give others a chance for a review.

ywwg · 2022-03-01T03:21:14Z

@Be-ing any opinion here? I saw the comment that is in the existing code which is why I tried to address in my comment above

Be-ing · 2022-03-02T03:30:17Z

Ehh... I'm not sure about this. On one hand it does reduce soft-takeover situations. On the other hand it reduces chain presets to just being a list of loaded effects and the state of parameters not linked to metaknobs...

ywwg · 2022-03-03T15:29:13Z

Ehh... I'm not sure about this. On one hand it does reduce soft-takeover situations. On the other hand it reduces chain presets to just being a list of loaded effects and the state of parameters not linked to metaknobs...

yeah I understand, that's why I wanted to ask... Is your desired use-case not covered by the existing effect loading preferences? We could add a third option, so we'd have:

Loading a preset keeps all metaknob positions (my desired behavior)
Loading a preset keeps top-level metaknob position (existing "keep" behavior)
Loading a preset resets all metaknob positions (existing other preference)

Be-ing · 2022-03-03T18:08:21Z

The effect loading preference is orthogonal. It determines what happens when loading an effect. The question here is what to do when loading a chain preset.

My motivation for implementing the current behavior was thinking that you should be able to load a preset, turn up the mix knob, and have it sound exactly the same as when the preset was saved.

ywwg · 2022-03-04T19:25:42Z

As per our effect workflow discussion, presets are meant to be loaded into effect units on-demand instead of the old flow, where effect units are mostly static and rerouted to different decks as needed. Therefore presets act as an effect-loading mechanism, and for controllers with a superknob it makes sense to have a mode where all the superknob values are reset. I am not sure what you'd like to do based on your replies, but it feels like it makes sense to add a third preference option so all three situations are covered.

Make the new keep_all behavior a third option in the preferences. Existing user configs will not be touched.

ywwg · 2022-03-07T20:48:20Z

Updated preferences to include all three options. Includes code to migrate preferences from the old bool to the new enum. Existing behavior should not be affected.

Be-ing · 2022-03-09T20:18:52Z

+enum class AdoptMetaknobValue {
+    // When loading effects, never adjust the metaknob positions
+    KEEP_ALL,
+    // When loading effects presets, never adjust the top-most metaknob position.


I have no idea what a "top-most metaknob" is. This is new terminology. Please use existing terminology or explain your new terms, otherwise discussing this is going to be very confusing.

would "Effect Unit Metaknob" be better?

I still don't know what that is. Effects have metaknobs, not effect units.

We are talking about "[EffectRack1_EffectUnit1_Effect2], meta". There is only one, at this layers, so I think we need to adjust the naming.

Currently we have this:
KEEP_ALL -> The current meta knob position is adopted for the connected effect parameters, when loading effects and chain presets.
KEEP_TOP -> When loading a chain preset, the metaknob and the parameters are changed to the saved values, including an out of sync state if a single parameter was adjusted before saving. If a single effect is loaded, the meta knob position is kept like "KEEP_ALL".
LOAD_DEFAULT -> The saved metaknob adjust the parameters into a synced state, a saved individual parameter value is ignored.

How about:
KEEP
ADOPT_FROM_CHAIN_PRESET
ADOPT_FROM_EFFECT

We may also consider to use two preferences option

Effect Load Behavior (Keep/Reset)

Effect Chain Preset Load Behaviour (Keep/Reset)

Do we really need the "KEEP_TOP (ADOPT_FROM_CHAIN_PRESET)" or can we use a common (Keep/Reset) value?

daschuer · 2022-03-15T23:40:18Z

+        slotEffectMetaParameter(m_pControlMetaParameter->get(), true);
+        break;
+    case AdoptMetaknobValue::KEEP_TOP:
+        if (loadingFromPreset) {


Suggested change

if (loadingFromPreset) {

if (loadingFromChainPreset) {

In contrast to the preset (default state) of a single effect.

daschuer · 2022-03-16T07:52:40Z

+       <widget class="QRadioButton" name="radioButtonKeepAllMetaknobPositions">
        <property name="text">
-         <string>Keep metaknob position</string>
+         <string>Keep all metaknob positions</string>


Suggested change

<string>Keep all metaknob positions</string>

<string>Keep meta knob position</string>

Since it is the "Effect load behavior" we have only one in this context.

daschuer · 2022-03-16T07:55:23Z

+      <item>
+       <widget class="QRadioButton" name="radioButtonKeepTopMetaknobPosition">
+        <property name="text">
+         <string>Only keep the top-level Effect Unit metaknob position</string>


Suggested change

<string>Only keep the top-level Effect Unit metaknob position</string>

<string>Restore meta knob position when loading a chain preset and keep it when changing an effect</string>

Below: "Always restore meta knob position"

We may consider to ditch the middle option, because the user may be either used to changing meta knobs or to fixed.
Making this conditional to the effect selector used might be surprising.

daschuer · 2022-03-16T07:57:47Z

+      <item>
+       <widget class="QRadioButton" name="radioButtonKeepTopMetaknobPosition">
+        <property name="text">
+         <string>Only keep the top-level Effect Unit metaknob position</string>


Below: "Always restore meta knob position"

daschuer · 2022-03-16T07:59:12Z

+       <widget class="QRadioButton" name="radioButtonKeepAllMetaknobPositions">
        <property name="text">
-         <string>Keep metaknob position</string>
+         <string>Keep all metaknob positions</string>


Since it is the "Effect load behavior" we have only one in this context.

daschuer · 2022-03-16T08:01:35Z

+      <item>
+       <widget class="QRadioButton" name="radioButtonKeepTopMetaknobPosition">
+        <property name="text">
+         <string>Only keep the top-level Effect Unit metaknob position</string>


We may consider to ditch the middle option, because the user may be either used to changing meta knobs or to fixed.
Making this conditional to the effect selector used might be surprising.

github-actions · 2022-06-15T00:09:18Z

This PR is marked as stale because it has been open 90 days with no activity.

acolombier · 2025-02-05T18:34:26Z

Do you still have interest in completing this @ywwg ? Looks like this was pretty close to completion. I'll mark as draft for now, but feel free to put back as active when you want us to merge this!

ywwg · 2025-02-05T19:45:46Z

I do not have time to work on mixxx code so this will need to be picked up by someone else

daschuer approved these changes Feb 26, 2022

View reviewed changes

Effect Metaknobs:

5466f5d

Make the new keep_all behavior a third option in the preferences. Existing user configs will not be touched.

github-actions Bot added the ui label Mar 7, 2022

Be-ing reviewed Mar 9, 2022

View reviewed changes

daschuer reviewed Mar 15, 2022

View reviewed changes

daschuer reviewed Mar 16, 2022

View reviewed changes

daschuer requested changes Mar 16, 2022

View reviewed changes

github-actions Bot added the stale Stale issues that haven't been updated for a long time. label Jun 15, 2022

ywwg mentioned this pull request Jan 16, 2023

Add super knob adoption option for quick effect chain presets #11198

Draft

robbert-vdh mentioned this pull request Feb 1, 2023

Document the Traktor Kontrol S3 FX changes mixxxdj/manual#533

Merged

github-actions Bot removed the stale Stale issues that haven't been updated for a long time. label Aug 28, 2023

acolombier marked this pull request as draft February 5, 2025 18:34

ywwg mentioned this pull request Jul 8, 2025

Traktor Kontrol S3 "Reset FX chain on startup" config does not work #15057

Open

	<string>Keep all metaknob positions</string>
	<string>Keep meta knob position</string>

	<string>Only keep the top-level Effect Unit metaknob position</string>
	<string>Restore meta knob position when loading a chain preset and keep it when changing an effect</string>

Uh oh!

Conversation

ywwg commented Feb 26, 2022

Uh oh!

daschuer left a comment

Choose a reason for hiding this comment

Uh oh!

ywwg commented Mar 1, 2022

Uh oh!

Be-ing commented Mar 2, 2022

Uh oh!

ywwg commented Mar 3, 2022

Uh oh!

Be-ing commented Mar 3, 2022

Uh oh!

ywwg commented Mar 4, 2022

Uh oh!

ywwg commented Mar 7, 2022

Uh oh!

Be-ing Mar 9, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 15, 2022

Uh oh!

acolombier commented Feb 5, 2025

Uh oh!

ywwg commented Feb 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Be-ing Mar 9, 2022 •

edited

Loading