Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an effects framework. #180

Merged
merged 337 commits into from
Mar 22, 2014
Merged

Add an effects framework. #180

merged 337 commits into from
Mar 22, 2014

Conversation

rryan
Copy link
Member

@rryan rryan commented Feb 18, 2014

This change introduces the basic infrastructure for an effects framework with multiple effects backends.

Design document: http://mixxx.org/wiki/doku.php/effects_framework

This also includes a native effects backend with simple implementations of:

  • Flanger
  • Reverb
  • Echo
  • Bit crusher
  • HP/LP filter

This branch is stable and at the point where the remaining issues can be considered bugs. There are still a few more things I'd like to do before merge but in the interest of releasing a 1.12.0 beta I'd like to merge soon.

To test, use the 1.12.0 Deere skin: https://github.com/mixxxdj/developer_skins/tree/master/DeereSkeleton4Deck1280x800-WXGA

Remaining changes for Effects 1.0:

  • Figure out what to do with load/save to XML -- it's implemented but turned off. Will we load/save by default or only allow rack serialize/deserialize?
  • Make group controls, parameter, and mix properties of the chain slot instead of the chain.
  • Support non-numeric (e.g. enum) parameters.
  • Prevent most mallocs of group-state in the engine by pre-prepping effects with all registered groups upon instantiation.
  • Add Deck feature collectors and provide features to effects.
  • Rack serialize/deserialize buttons.
  • Rack mute.
  • Parameter linking.

rryan and others added 30 commits March 6, 2011 12:59
…ectJ_ParameterK. I feel like it's a lot cleaner. Updated the design doc as well.
…. I should have done this from the beginning, but didn't fully consider what presets would require, so I didn't.
…y, so all the effects framework code is built by default.
… manage their own enabled or mix parameters.
…rying desperately to avoid having to write a Factory class for every effect.
… for managing the iteration order of EffectChains.
… Now the up/down cycle-through-EffectChain buttons work!
…ffects available from backends and instantiating them without having to touch a backend. Move the Flanger creation out of MixxxApp and into EffectsManager::setupDefaultChains()
…lotParameter/EffectParameter. Also convert values that are going into the control system to be scaled by 127. Ugh.
… these changes when the branch merges in favor of Jus's official skins.
…elism of EffectChain/EffectChainSlot Effect/EffectSlot
Conflicts:
	mixxx/build/features.py
	mixxx/src/basetrackplayer.cpp
	mixxx/src/defs.h
	mixxx/src/dlgprefmidibindings.cpp
	mixxx/src/engine/enginedeck.cpp
	mixxx/src/engine/enginedeck.h
	mixxx/src/engine/enginemaster.cpp
	mixxx/src/engine/enginemaster.h
	mixxx/src/main.cpp
	mixxx/src/mixxx.cpp
	mixxx/src/mixxx.h
	mixxx/src/skin/legacyskinparser.cpp
	mixxx/src/skin/legacyskinparser.h
	mixxx/src/skin/skinloader.cpp
	mixxx/src/skin/skinloader.h
@daschuer
Copy link
Member

First of all: Because we are running out of time, we can go with marking them experimental and candidates for removal in 1.13. Or can we go with ControlLinPotmeter in 1.12 only, IMHO that would be better ?

Here my comments:

minimum and maximum will be user-editable

This is a nice feature, but I prefer to set CO members, and not independednt COs. This schould be possible, because Min and Max are not intended to be changable life.
Or do I miss an imprtant use-case?

really need is a CO that can change its behavior on the fly

Yes. This is very required and a good solution for this a great benefit for Mixxx. I have tryed to ouline one below-

If we start with a CO and then set its behavior to be a ControlPotmeterBehavior, skins and MIDI mappings/scripts will not be able to reset the potmeter to its default value or bump it up and down in steps with the PotmeterControls.

I do not understand. Mixxx has currently a lot of parameters, but that was never identified as an issue.
Of cause all inteactive functions like "_step_up" must be COs to be mapp-able. If it reqired, we can add a dedicated "_reset" CO

value_type

I am not sure if we need all them. Ideal we schould stick to the establiched control types. See my proposal below.

value_normalized

We have already a solution for this:

Parameter / MidiParameter

that this leaves MIDI scripters (our most advanced users) with no way to address individual controls other than the normalized value (which takes away the option to touch bool or enum parameters).

I think this is not true with our current COs. So we should try to keep this way doing it.

One hack which I didn't want to do was introduce multiple value controls based on the ControlHint

I Also do not like it.

I think completely changing a control's behavior on the fly (even transforming a control into a string control from a numeric valued control) is a Control 2.0

String control should be handled allways separate.

Controller script:

We should really have a clue how to map buttons and rotaries, in an easy way. Even if move it to scripts, we have not outlined a solution yet. IMHO, my sepearted Buttons and Knob list (namespace) is not that bad. This would also make some parts of my solution below a lot more easy. What will be an alternative? How will a skript solution look like?

Proposal:

Current tree

 * ControlLinPotmeter
  * ControlPotmeter (Min, Max, Range) 
   * ControlObject (key)
    * ControlDoublePrivate (value, default_value, name, description, behaviour) 

  * ControlTTRotary 
   * ControlObject (key)
    * ControlDoublePrivate (value, default_value, name, description, behaviour)  

  * ControlPushButton (ButtonMode, NoStates)  
   * ControlObject (key)
    * ControlDoublePrivate (value, default_value, name, description, behaviour) 

  * ControlIndicator (blinkValue, nextSwitchTime) 
   * ControlObject (key)
    * ControlDoublePrivate (value, default_value, name, description, behaviour) 

New:

  * ControlVariant (type) 
    * one of the trees above 

Features:

  • Fixed Size = sizeof(max(CO size))
  • Fixed ControlObject (key)
  • Re-instatiable in the same memory region
  • Two states: valid: Working; invalid: during type change (must not crash, not blocking, changes are lost)

Requirements:

  • We cannot create and destroy them because of mamory allocation involved
  • We must be able to "recreate" them in place

Replacements:

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

value_normalized

We have already a solution for this:

As I mentioned, it's not currently possible to do a parameterized set from MIDI script.

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

that this leaves MIDI scripters (our most advanced users) with no way to address individual controls other than the normalized value (which takes away the option to touch bool or enum parameters).
I think this is not true with our current COs. So we should try to keep this way doing it.

Not sure what "this" refers to? How would a script address a bool or enum parameter that is represented by a ControlPotmeter? First off, since it can only touch the raw value, it must inspect the max and min value to determine what are valid ranges.

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

If we start with a CO and then set its behavior to be a ControlPotmeterBehavior, skins and MIDI mappings/scripts will not be able to reset the potmeter to its default value or bump it up and down in steps with the PotmeterControls.
I do not understand. Mixxx has currently a lot of parameters, but that was never identified as an issue.
Of cause all inteactive functions like "_step_up" must be COs to be mapp-able. If it reqired, we can add a dedicated "_reset" CO

Sure, that works. Though when the parameter is acting like a potmeter, it is expected by keyboard mappers and script authors that the standard potmeter auxiliary controls are in place.

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

minimum and maximum will be user-editable
This is a nice feature, but I prefer to set CO members, and not independednt COs. This schould be possible, because Min and Max are not intended to be changable life.

Right, but this feature makes them editable live. I do indeed mean that users will be able to change these live.. otherwise how will you know how it sounds? I sure can't hear the difference between 0.99 and 0.91 when I'm writing these effect manifests. I try it out and pick what sounds good.

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

I think completely changing a control's behavior on the fly (even transforming a control into a string control from a numeric valued control) is a Control 2.0

String control should be handled allways separate.

Forget I said anything. I don't want this PR to last another year :P.

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

Features:

Fixed Size = sizeof(max(CO size))
Fixed ControlObject (key)
Re-instatiable in the same memory region
Two states: valid: Working; invalid: during type change (must not crash, not blocking, changes are lost)
Requirements:

We cannot create and destroy them because of mamory allocation involved
We must be able to "recreate" them in place

This is definitely in line with needs to happen for Control 2.0.

I think the behavior approach we have now is the right one. There should be one control class (simply call it "Control") and simply change the behavior. To prevent memory allocation get rid of the polymorphism of ControlBehavior and switch to a TYPE approach. To change the control, simply atomically swap its behavior class.

@rryan
Copy link
Member Author

rryan commented Mar 22, 2014

I appreciate your comments @daschuer but we are getting far beyond the point of diminishing returns here. It's clear the control system as written does not support this (it's not even "intended" behavior to change the range of a potmeter live). While we should definitely improve it so that it can, that's not going to happen for 1.12.0 so this discussion is moot.

I agree this is a janky hack, but if we don't include it then MIDI scripts become somewhat useless in Mixxx 1.12.0 for effects. I'll go ahead and mark these controls as experimental. We could even prefix them with v1_ or legacy_.

I'm also going to merge this now. We can continue the discussion but it's high time we start moving forward with 1.12.0.

rryan added a commit that referenced this pull request Mar 22, 2014
@rryan rryan merged commit b91cdf4 into mixxxdj:master Mar 22, 2014
@daschuer
Copy link
Member

As I mentioned, it's not currently possible to do a parameterized set from MIDI script.

Ah I see, we have only setValue()

Q_INVOKABLE void setValue(QString group, QString name, double newValue);

But if we introduce setParameter() whole Mixxx has the benefit.

@daschuer
Copy link
Member

First off, since it can only touch the raw value, it must inspect the max and min value to determine what are valid ranges.

The same as above, lets introduce a API for this.

@daschuer
Copy link
Member

Right, but this feature makes them editable live. I do indeed mean that users will be able to change these live.. otherwise how will you know how it sounds? I sure can't hear the difference between 0.99 and 0.91 when I'm writing these effect manifests. I try it out and pick what sounds good.

If an effect requires live changes of a Min and Max value, than it is better to introduce a new Effect parameter for this. This way the Min/Max feature has a slick integration like all other parameter.

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.

None yet

6 participants