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

Eq rack2 #364

Merged
merged 122 commits into from
Nov 16, 2014
Merged

Eq rack2 #364

merged 122 commits into from
Nov 16, 2014

Conversation

daschuer
Copy link
Member

This is an updated version on Nicus EQ Rack Branch.
It enables the full power of the effect back-end for the EQ Knobs as well.

You can select which EQ should work on which deck and you can load any other effect to the EQ Slots.

Next steps to merge:

  • Add a second Effect slot, to utilize the Filter knob / fourth band, some Controllers offer.
  • Add an effect chain to preferences for master EQ

… are changed, do ramping in the next process() call
m_deckEffectSelectorsSetup = true; // prevents a recursive call
QList<QPair<QString, QString> > availableEQEffectNames;
if (CheckBoxHideEffects->isChecked()) {
m_pConfig->set(ConfigKey(CONFIG_KEY, "AdvancedView"), QString("no"));
Copy link
Member

Choose a reason for hiding this comment

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

No config changes should be made until Apply is pressed.

Copy link
Member Author

Choose a reason for hiding this comment

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

The original behavior of this preference pane is that we instantly change everything.
I might remember that this was result from a discussion we head. should we change that?

@rryan
Copy link
Member

rryan commented Nov 15, 2014

I am not the opinion, that we need to add a four band EQ or a dedicated filter knob to every skin.
But Mixxx should at least allow this. This will make all owner of a traktor style controller happy:
http://www.digitaldjtips.com/wp-content/uploads/2010/11/traktor-kontrol-s4.png

Hm, if you're referring to the filter knobs I don't think that is served by this PR -- is it? The filter knob use case is more for a "quick effect" concept we've talked about which has a single superknob and an effect selector. That way the users filter sweep knob could change into a bitcrusher if they want to change it on the fly.

@rryan
Copy link
Member

rryan commented Nov 15, 2014

This pull request and the filter_effect pull request do not require to have four knobs lined up in the skin.
It is just an option to allow a specific mixing style. No one will controll those knobs on the skin if a controller is in place and no one will miss those knobs if they use Mixxx without a controller.

Maybe we should think about a DJ Console skin, that has all desired controls, but in a tiny version, intended to give a feedback only (and emergency control) a big library and waveform section.
I think there is such an approach in the forums ...

Any hints or suggestions for this PR?

I realize -- and sorry for turning this PR into a complaint-fest :). The change this PR makes is nice since it enables users to change their EQs in a more general fashion (and in the future maybe use an EQ plugin of their own).

I think we're more discussing the usability implications of per-deck EQ choices and the proposal you put forward of having the number of knobs change dynamically and support for swapping out the EQ with a different type of effect. I see how a super advanced user might benefit -- and there's no reason to not let them in their own skin -- but our default skins need to be easy to use and easy to explain to users.

@daschuer
Copy link
Member Author

All comments issued.

@rryan
Copy link
Member

rryan commented Nov 16, 2014

Ok -- looks good. Cleanups can wait till master :).

rryan added a commit that referenced this pull request Nov 16, 2014
@rryan rryan merged commit b7b7410 into mixxxdj:master Nov 16, 2014
@daschuer daschuer deleted the eq_rack2 branch May 30, 2015 21:55
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.

5 participants