Skip to content

Remove adopting Meta knob after effect load#1148

Merged
daschuer merged 1 commit intomixxxdj:masterfrom
daschuer:effect_default
Jan 27, 2017
Merged

Remove adopting Meta knob after effect load#1148
daschuer merged 1 commit intomixxxdj:masterfrom
daschuer:effect_default

Conversation

@daschuer
Copy link
Copy Markdown
Member

As discussed in #1118 with current master you do not get the default values of an effect after load, because the meta knob position is adopted. This is especially annoying if the user has no meta-knob on his controller and in case of the Filer effect which is silent in this case.

This was originally introduces to avoid out of sync situations and the need for "snap in" when turning the meta-knob the first time.

Now we have removed softtakover in when the effect is disabled which allows us the remove the initial meta-knob adoption.

It behaves now like this:

  • Effect loads always with it's own default.
  • If the effect is disabled, which is recommended when chaining effects, A touch at the meta-knob makes the parameter jump to the linked position
  • If the effect is enabled before touching the meta-knob, it plays with the well prepared defaults. When you touch the meta-knob nothing happens until you cross the current position of the mapped knobs.

pParameter->loadEffect(pEffect);
}

slotEffectMetaParameter(m_pControlMetaParameter->get(), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the effect is disabled, which is recommended when chaining effects, A touch at the meta-knob makes the parameter jump to the linked position

This should not require moving the metaknob. That could be easily accomplished by just removing the , true from this line.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove the "true" this line does nothing and can be removed.

Touching the meta-knob for the jump is required to signal that the meta-knob should be adopted and not the effect defaults.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove the "true" this line does nothing and can be removed.

It will do something if the effect is disabled.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK, I have not noticed. But this will not solve the issue where the user has no metaknob.

I wish to disable the effects, select a desired effect, enable the effects and it should play at its default. If I do not like the defaults, I can turn the metaknob to the desired position before I enable the effect and the new value will be immediately adopted.

I concider the current metaknob position during effect load as a perfect position for the previous loaded effect but a random position for the new effect. So the defaults are in any case better than that. If not turning the metaknob is required anyway.

This is almost the same behavior we have in 2.0 with the super knob, precept we have now the bypass that allows to adopt the new value by jump when the effect is disabled.

We may improve this issue later by a default value of the meatknob like 0.5 that matches artificially the default position of the effect. This is done for example at a Pioneer Hardware Mixxx. We have discussed this as well when introducing the Quick effect knob. I still support the idea, but it was rejected because it requires a special effect design or it will lead only to one usable side of the knob, or likewise the same behaviour on both knob sides.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 26, 2017

What is the issue you are trying to solve with this PR? I have played with the metaknobs using my mouse without my controller and I do not think there is an issue when switching effects. On the other hand, introducing a new way that the metaknob can get out of sync with its linked parameters is confusing.

It is a bit odd that some effects do not alter the sound when the metaknob is centered but others do not alter the sound when the metaknob is all the way left. I do not think this PR solves that, and I'm not sure it's really an issue for switching effects. It seems you are trying to set effects so they do not alter the sound when switching to a new effect. If that is desired, the effect can be disabled. I think the behavior you are suggesting is unintuitive. If I turned the metaknob to 2/3 position, I want to be able to quickly hear how different effects sound with their metaknob at that position by loading different effects. That position is not random; I specifically set the metaknob there because I wanted it there.

@daschuer
Copy link
Copy Markdown
Member Author

This PR basically restores the 2.0 behavior during changing effects. For my feeling the current situation is a regression. It is inconsistent that the single parameter knobs are reset to their defaults, but not linked knobs. This destroys a good default.

In case of "Filter", this very annoying, because i use it very often, but now it is silent by default. If I am not careful enough this can be a partystopper.
The super knob out of synced is a learned behavior from 2.0, and we get no complains about it.

It is a bit odd that some effects do not alter the sound when the metaknob is centered but others do not alter the sound when the metaknob is all the way left.

Yes, that is basically the source of the issue. The out of sync state is only a workaround for that.

One other thing is also odd: on my RMX2 controller, I have an endless knob for tweaking effects.
So there is technically no reason for being out of sync. It would be nice if this knob can go never out of sync and the effects are started till with default values. We have currently the "issue" that the Filter jumps to silence when I touch the knob before the effect is enabled. This behavior makes only sense for a fixed scaled knob.

It seems you are trying to set effects so they do not alter the sound when switching to a new effect.

No, I just wish that the effect starts with a good sounding default setting. The Filter at fully dry default is an exception.

If I turned the metaknob to 2/3 position, I want to be able to quickly hear how different effects sound with their metaknob at that position by loading different effects. That position is not random; I specifically set the metaknob there because I wanted it there.

The random issue was not related to the knob position, it is related to the meaning for the effect. For some effects 0.5 means default, for some it is 0.0. So you cannot skim though the effects and listen to their defaults, the resulting effect is random. The user cannot predict the result.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 26, 2017

This PR basically restores the 2.0 behavior during changing effects.

Not quite. In 2.0, switching effects chains applied the current superknob position to the new effect chain. Switching chains was the only way to switch effects in LateNight and the default way in Deere. We got no complaints about this behavior. I just moved it down to the metaknob level.

It is inconsistent that the single parameter knobs are reset to their defaults, but not linked knobs. This destroys a good default.

No it is not inconsistent. The linked parameters are meant to be changed frequently via the metaknob; the unliked parameters are meant to not be manipulated often, so they need good defaults.

In case of "Filter", this very annoying, because i use it very often, but now it is silent by default. If I am not careful enough this can be a partystopper.

Why don't you leave the filter loaded in an effect slot then? When you do not want to use it you can disable that slot.

Yes, that is basically the source of the issue. The out of sync state is only a workaround for that.

I do not think this a good approach to handling that, and as I said above, I don't know if it really needs fixing. It's easy to learn how the two filter effects work and know how they will sound when they are loaded.

One other thing is also odd: on my RMX2 controller, I have an endless knob for tweaking effects.
So there is technically no reason for being out of sync.

Forcing the parameters to adopt different values than where the metaknob is would be moving them out of sync. If you do this, then the metaknob should be updated too. I think to do that would require putting a default metaknob position in each effect manifest. However, that approach would not work well if the user customizes the metaknob linking; perhaps in the future we can let users specify a new metaknob default value, but I do not see a way to easily fit that into the current GUI.

The behavior you are proposing would only make any sense for controllers with encoders for effects, which is a minority of controllers, or using Mixxx without a controller. For most controllers, it would be confusing. You could implement a new preference option "Reset metaknob when switching effects" that is off by default.

@daschuer
Copy link
Copy Markdown
Member Author

This PR basically restores the 2.0 behavior during changing effects.

Not quite. In 2.0, switching effects chains applied the current superknob position to the new effect chain. Switching chains was the only way to switch effects in LateNight and the default way in Deere. We got no complaints about this behavior. I just moved it down to the metaknob level.

This is not the case, I have just checked. Interestingly we had a bug that was complaining about the behavior you have just introduced. This PR fixes the bug again:
https://bugs.launchpad.net/mixxx/+bug/1335355

Why don't you leave the filter loaded in an effect slot then? When you do not want to use it you can disable that slot.

Yes, I can but it is already annoying to have this behavior on the first load.

You could implement a new preference option "Reset metaknob when switching effects" that is off by default.

This will work if each effect has its own mata-knob default.

Other related bug:
https://bugs.launchpad.net/mixxx/+bug/1404741

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 27, 2017

Yes, I can but it is already annoying to have this behavior on the first load.

That will be even less of an issue after #1092 is merged.

This will work if each effect has its own mata-knob default.

Yeah. I'd be okay with merging this if that is implemented and it is made optional. There should also be new tests to ensure correct behavior whether the preference is on or off.

@daschuer
Copy link
Copy Markdown
Member Author

OK, thank you. I will merge this now, because this restores the 2.0 behavior.

A solution for the whole issue requires probably a draft. I would vote for a standard default of meta 0.5 like Pioneer, this is most likely the best predictable solution for the user.

@daschuer daschuer merged commit 9e3c7c9 into mixxxdj:master Jan 27, 2017
@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 27, 2017

I meant I'd be okay with merging this when those conditions are met. Now master is broken for controllers that use knobs for effects.

@daschuer
Copy link
Copy Markdown
Member Author

Why is it broken? It should work like 2.0 changing effect, effect at default, meta knob might be out of sync.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 27, 2017

The metaknob should only be out of sync with parameters if the user adjusts parameters independently of the metaknob. This has created a situation for the metaknob and parameters to be out of sync without the user explicitly setting that, which IMO is a bug.

A solution for the whole issue requires probably a draft. I would vote for a standard default of meta 0.5 like Pioneer, this is most likely the best predictable solution for the user.

Is there something lacking in the solution I outlined? I think 0 would be a better standard default. Effects that require otherwise, which right now is only the filter and Moog filter, should specify otherwise in their manifest.

@daschuer
Copy link
Copy Markdown
Member Author

This has created a situation for the metaknob and parameters to be out of sync without the user explicitly setting that, which IMO is a bug.

We have this "Bug" since 2.0 and it is no issue to have it in 2.1 as well. Fixing it this way will reintroduce this bug: https://bugs.launchpad.net/mixxx/+bug/1335355

Is there something lacking in the solution I outlined? I think 0 would be a better standard default. Effects that require otherwise, which right now is only the filter and Moog filter, should specify otherwise in their manifest.

It is an industrial standard, to have a filter effect with HPF and LPF on each side of the super knob. Braking this would be IMHO surprising fro most users. If we adopt that to all other effects, this would allow us to have two "colors" of the same effect on a single knob. Some time ago I had the chance to play on such pioneer controller and it felt almost natural, especially if you use it for fancy transitions.

By the way: Is the beats knob on your Denon controller endless?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 27, 2017

Going back and forth disputing what is buggy behavior and what isn't will get us nowhere. Please restore the old behavior to default and make this new behavior optional, and make the new behavior update the metaknob accordingly. I intentionally implemented the old behavior when working on #1062 because it was annoying to test metaknobs with different effects without it.

If we adopt that to all other effects, this would allow us to have two "colors" of the same effect on a single knob.

That would be cool, but we only have 2 effects like that currently, so I think 0 is a better standard default. But that's not important; the only difference will be which effects need to add a single line of code to their manifest.

By the way: Is the beats knob on your Denon controller endless?

I don't have any Denon controllers. @timrae has a Denon MC4000. But yeah, controllers designed for Serato have an endless encoder labeled "Beats" in their effects section.

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 28, 2017

It is an industrial standard, to have a filter effect with HPF and LPF on each side of the super knob.

To be fair, A&H (for example) don't do this.

If we adopt that to all other effects, this would allow us to have two "colors" of the same effect on a single knob.

I think this would be weird for all effects that aren't filters. I'm also in favor of using zero as the default and letting filters be the exception.

By the way: Is the beats knob on your Denon controller endless?

Yes. Beats is supposed to be used for the period on periodic effects though, so I'm not sure how it relates to the current discussion.

@rryan
Copy link
Copy Markdown
Member

rryan commented Jan 28, 2017

It's totally fair to propose a change to any behavior in master -- just because something made it through review at some point doesn't mean it gets to stay that way... however -- the behavior in master was the result of two long and recent discussions (#1062 and #1111) so I think this was merged prematurely.

Just because you both consider each other's opinion of how this should work to be a bug, doesn't make this a "bug fix" :). This is solidly in the realm of "how should Mixxx behave". It seems to me there are valid reasons for both behavior choices and they each benefit and harm certain use cases, so let's list the use cases, weigh the pros and cons, and enumerate options.

I'd imagine the center-neutral effects like filters are primarily used for quick effects (which is why we customize the superknob default to 0.5 for the quick effect rack). @daschuer -- I'd like to hear more about your use case. When do you load a filter into an effect unit and for what purpose? Do you chain them with other effects or is it just loading a single effect? Why do you not use the quick effect rack for filters?

@rryan
Copy link
Copy Markdown
Member

rryan commented Jan 28, 2017

Idea: What if effects came with two sets of defaults -- one for loading in a quick effect setting and one for loading in an effect unit?

  • Pro: This would allow us to customize the experience for each case. (e.g. bitcrusher could work better as a quick effect).
  • Pro: the metaknob default of 0 would correspond to the effect default when in effect-unit mode so the effect defaults are not changed if the user has not touched the metaknob, but they would still snap to the right position if the user has changed the metaknob.
  • Con: More defaults argue over :).
  • Con: Potential user confusion?

@rryan
Copy link
Copy Markdown
Member

rryan commented Jan 28, 2017

Idea: What if the filters were limited to the quick effect rack and we made individual HPF and LPF variants of the filters that scale from neutral at 0 to full at 1 for use in effect chains?

  • Pro: Filter won't accidentally be loaded into an effect unit, potentially unexpectedly cutting the sound out when it snaps to the metaknob position.
  • Con: Limiting the user unnecessarily, which is against the project principles of customizability and flexibility.
  • Con: Maintenance burden of having multiple redundant effects (Filter + HPF + LPF for each type of filter we offer).

@timrae
Copy link
Copy Markdown
Contributor

timrae commented Jan 28, 2017

The effect default should be such that the sound is unaffected. So I agree with @Be-ing that it makes sense to have a default value of zero and customize it for those effects where a different value makes sense, like the combo filter.

@rryan
Copy link
Copy Markdown
Member

rryan commented Jan 28, 2017

The effect default should be such that the sound is unaffected. So I agree with @Be-ing that it makes sense to have a default value of zero and customize it for those effects where a different value makes sense, like the combo filter

I interpret what you're saying as suggesting that we implement @daschuer's proposal to give each effect a default metaknob setting (default 0 for all, except center-neutral effects which default to 0.5). This still hurts @Be-ing's use case (unless it is a preference option that is off by default). Is that what you meant? If not, could you elaborate on the "customize" bit?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 28, 2017

Idea: What if effects came with two sets of defaults -- one for loading in a quick effect setting and one for loading in an effect unit?

This will be unnecessary. The QuickEffect rack can be modified so its superknob is set to the default of the metaknob for the first effect in its chain.

Idea: What if the filters were limited to the quick effect rack and we made individual HPF and LPF variants of the filters that scale from neutral at 0 to full at 1 for use in effect chains?

The metaknob linking can be easily modified to make the effect work as just a HPF or LPF with the metaknob, which will not be too much trouble to set up after #1092 is merged. So that would be an unnecessary maintenance burden and unnecessarily restricting.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 28, 2017

It seems to me there are valid reasons for both behavior choices and they each benefit and harm certain use cases, so let's list the use cases, weigh the pros and cons, and enumerate options.

For the sake of discussion, I'll refer to the behaviors as A & B:
A: sync new effect parameters with current state of metaknob
B: sync metaknob with effect parameter defaults
Presently master has neither of those.

Use cases:

  • Controllers with finite range knobs for metaknobs (most controllers with effects controls): A is necessary
  • Controllers with endless encoders or touchstrips for metaknobs: either A & B could work, matter of personal preference
  • No controller: same as controllers with endless encoders or touchstrips

@daschuer
Copy link
Copy Markdown
Member Author

Yes, correct. This would work for me as preference option and sounds like a good plan.

However, I do not a agree that A is NECESSARY. It is more like a personal preference. But since it will be a preference option it works for me.

We can improve A by far, if we have a common default point on the meta-knob.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 28, 2017

However, I do not a agree that A is NECESSARY. It is more like a personal preference. But since it will be a preference option it works for me.

B could kinda work for controllers with knobs if the controller mapping has soft takeover activated for the metaknob, but it is not optimal because it creates a situation in which soft takeover is needed. Thus, I think A should be the default as it works best for the most use cases.

We can improve A by far, if we have a common default point on the meta-knob.

I don't understand how the metaknob default is relevant to A.

@daschuer
Copy link
Copy Markdown
Member Author

daschuer commented Jan 28, 2017

Did we get mixed up what A and B is?

My understanding:
A: Before: Effect default = 0.3, Meta knob = 0.7 -> After: Effect default = 0.7, Meta knob = 0.7
B: Before: Effect default = 0.3, Meta knob = 0.7 -> After: Effect default = 0.3, Meta knob = 0.3

B: Should work best for GUI only solutions and endless super knobs. The loaded effect simply sets all knobs for its default. mapped paramter knob / Meta Knob / Super knob all at 0.3

A: Works for fixed scale meta knob if out of sync situations are not desired. After effect load: mapped paramter knob / Meta Knob / Super knob all at 0.7. All other knobs still jumps to their default (right?)

Only if we have a common value for the meta-knob default, A is usable for me. In this case I can turn the knob to the middle and all effects are at their default.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 30, 2017

A: Before: Effect default = 0.3, Meta knob = 0.7 -> After: Effect default = 0.7, Meta knob = 0.7
B: Before: Effect default = 0.3, Meta knob = 0.7 -> After: Effect default = 0.3, Meta knob = 0.3

If by "After: Effect default" you are referring to the parameter linked to the metaknob by default, yes, that is correct.

B: Should work best for GUI only solutions and endless super knobs

Whether A or B is best for those use cases is a matter of personal preference. However, B creates a confusing situation for most controllers. Users with controllers should not have to change this preference option to get loading effects to work well.

A: Works for fixed scale meta knob if out of sync situations are not desired.

I don't think anyone desires out of sync situations.

All other knobs still jumps to their default (right?)

Yes, parameters that are not linked to the metaknob are set to their default values with behavior A.

@daschuer
Copy link
Copy Markdown
Member Author

That's why IMHO B is also desired on Controller s with fixed scale knobs. Loading the effects will put the single parameter knobs out of sync, but not the metaknob. That is not consistent. If you have only one metaknob. Only this out of sync feature allows to load am effect with the default or later the stored knob positions. If we made A default this would be no option.

I do not distinguish how the an effect is mapped to the metaknob. Loading an effect, or moving the superknob focus schould behave the he same and not alter the previous set sound.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 30, 2017

Loading the effects will put the single parameter knobs out of sync, but not the metaknob. That is not consistent.

You're correct, but in the 2 months I have been working with and testing metaknobs I have not noticed this as an issue. On the other hand, before behavior A was implemented, it was annoying me. When I am loading different effects, I typically don't have an effect focused so my controller is controlling the metaknobs.

I don't understand what you're saying in the rest of your comment.

I think we should put aside discussion of the default value until both options are implemented and can be tested.

@daschuer
Copy link
Copy Markdown
Member Author

I don't understand what you're saying in the rest of your comment.

OK here two possible use-cases:

Lets look on a controller with on only one effect knob. This is the super knob in effect chain mode an becomes a metaknob if you switch to single effect mode.

The supeknob mode is kind of useless, because it will control all effects at once. but you can focus the effect, and set the desired sound. A focus change will lead inevitably to an out of sync situation, which is a good idea to not destroy the previous tweaked sound.

If you now load new chain, in A the metaknob settings of the old chain are adopted, which is a kind of random result and might not sound well. The default of each effect will be much nicer. The out of sync situation is no issue in this case, because the superknob is anyway out of sync when switching though the effects

The other use-case where A suites better if we allow to save chains including their parameter settings.
B will allow to store the same chain twice with different setting. A won't work well, because the stored setting are overridden when adopting the current meta knob value.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Jan 30, 2017

If you now load new chain, in A the metaknob settings of the old chain are adopted, which is a kind of random result and might not sound well.

Ah, I had not thought about this situation. Yes, it would be random for a controller with only one knob, but it would not be random for controllers with more knobs. We may need more detailed options than A & B to handle switching chains, like a separate option to reset metaknobs when switching chains. However, there is no use for switching chains in 2.1, so we can leave this issue for 2.2.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Feb 6, 2017

Could the metaknob default be calculated from the effect parameters' neutralPointOnScale()?

@daschuer
Copy link
Copy Markdown
Member Author

daschuer commented Feb 6, 2017

The neutral position on scale is not equal to the default of the mapped parameter. Why not adopt the value form the first first mapped knob respecting the mapping options like inverse?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Feb 6, 2017

To clarify, do you mean adopting the default position of the first linked knob? That's worth a try. It may work fairly well for automatically determining the default metaknob position for custom linkings too.

@daschuer
Copy link
Copy Markdown
Member Author

daschuer commented Feb 6, 2017

Yes, exactly.

@daschuer daschuer mentioned this pull request Feb 7, 2017
@Be-ing Be-ing mentioned this pull request Mar 13, 2017
@Be-ing Be-ing mentioned this pull request Sep 26, 2017
2 tasks
Be-ing added a commit to Be-ing/mixxx that referenced this pull request Feb 22, 2018
@daschuer daschuer deleted the effect_default branch September 7, 2021 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants