-
Notifications
You must be signed in to change notification settings - Fork 2
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
Seems there is a regression in parameter/state handling on version 1.2 #2
Comments
After comparing - https://github.com/yamadapc/rnbo-preset-issues/compare/v1.1#diff-844f7832a380add738588b2ea8fcde656fde8b46770b649d010b24a02086bd09L241 I believe the issue is the added "notifying" flag - 75fddac#diff-dc0af4662f43b55e1484928b2ba6f5c8d1c74818e3bd5a26617d9c2c62415d5f Defining I have also confirmed that this bug breaks preset handling (yamadapc/rnbo-preset-issues@21bbbef) Perhaps the fix would be: diff --git a/export/rnbo/adapters/juce/RNBO_JuceAudioProcessor.cpp b/export/rnbo/adapters/juce/RNBO_JuceAudioProcessor.cpp
index 0357fc4..6d110f8 100644
--- a/export/rnbo/adapters/juce/RNBO_JuceAudioProcessor.cpp
+++ b/export/rnbo/adapters/juce/RNBO_JuceAudioProcessor.cpp
@@ -243,6 +243,9 @@ void JuceAudioProcessor::handleParameterEvent(const ParameterEvent& event)
param->setValueNotifyingHost((float)normalizedValue);
param->endChangeGesture();
}
+ else {
+ param->setValue((float)normalizedValue);
+ }
}
}
When I make this change ^, the test-suite on my repository works. |
I'm not sure if preset handling will be totally correct since the parameters would not be notifying the hosts depending on how they are called. `RNBO::JuceAudioProcessor::setCurrentProgram` does not set `_isSettingPresetAsync` as far as I can tell. This fixes the test suite referenced in Cycling74#2. Closes Cycling74#2. - https://github.com/Cycling74/rnbo.adapter.juce/blob/dd233609adc495828fa9efcf756ab2a155a3bd1d/RNBO_JuceAudioProcessor.cpp#L412-L421
I think you found a bug but this "fix" shouldn't be needed, there is a bug in the preset handling within rnbo (I have a unit test in the rnbo repo that shows it, not yet in the released code though). If you look at how parameters work in the JUCE adapter, you'll see that the rnbo parameters get values from the underlying rnbo object, and if presets work correctly, you should be able to query the value of the parameter without doing this round trip update and get the correct value out. I'll link your issue in our ticket and try to update when we have this resolved. Thanks a ton for reporting it! |
Please find a reproduction on v8.6.0 attached. Two new tests in that test-suite are failing on assertions over the nº of presets. |
the main branch adds an |
I have managed to isolate an issue I have with the new changes on https://github.com/yamadapc/rnbo-preset-issues
The test suite on TestProcessorPresets.cpp reproduces the problem and this test-suite passes on v.1.1.2, which I have pushed to a branch.
Everything should be in this repository. You can build/run the test-suite with cmake.
The issue is here:
When restoring state (and in some cases setting presets, but I still have to isolate this issue into this reproduction repository), then the state is properly restored onto:
getStateInformation
But it's not restored into:
rnbo.getParameterValue
This might be breaking preset handling for me.
The text was updated successfully, but these errors were encountered: