-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Band-Splitter Four Band Option and Tri-State Button #33
Conversation
…ossoverFilter and version hints updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Just one issue with the tri-state button and some testing, and we should be good to go :).
.withOutput ("Band-Split (High)", juce::AudioChannelSet::stereo(), true); | ||
.withOutput ("Band-Split (High)", juce::AudioChannelSet::stereo(), true) | ||
.withOutput ("Band-Split (LowMid", juce::AudioChannelSet::stereo(), true) | ||
.withOutput ("Band-Split (HighMid", juce::AudioChannelSet::stereo(), true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should work. I've been testing to make sure it doesn't break backwards compatibility, by saving a session using the band-splitter with multiple outputs using the previous nightly build, then loading up the new build to make sure everything is the same. I've checking in a couple of DAWs, but any more that you can test in would be great!
chowdsp::BoolParameter::Ptr threeBandOnOff { | ||
juce::ParameterID { "band_split_3band_on", ParameterVersionHints::version1_0_0 }, | ||
"Band Splitter 3-Band", | ||
false | ||
}; | ||
|
||
chowdsp::BoolParameter::Ptr fourBandOnOff { | ||
juce::ParameterID { "band_split_4band_on", ParameterVersionHints::version1_1_0 }, | ||
"Band Splitter 4-Band", | ||
false | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're using two parameters to control this, we might need to be careful in case the user changes the parameters via automation in a way that we're not expecting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Just a couple of little things.
getCutoffParam = [this](int bandIndex) -> const chowdsp::FreqHzParameter::Ptr& { | ||
if (bandIndex < (numBands / 3)) | ||
return bandSplitterParams.cutoff; | ||
else if (bandIndex <= numBands / 2) | ||
return bandSplitterParams.cutoff2; | ||
else | ||
return bandSplitterParams.cutoff3; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably keep the comment that was with this logic previously, just to help avoid confusion in the future.
2b1098d
to
2509dc6
Compare
Screen.Recording.2024-01-10.at.17.41.46.mov