|
|
20240124_22-21_ardour-dbg-8.2.85_master_warmy_dsp-load.png (19,357 bytes) 20240124_22-35_ardour-dbg-8.2.85_modded_warmy_dsp-load.png (19,072 bytes) audio_device_config.png (32,905 bytes) |
|
|
I will try to make a PR in Github with some code changes for a possible solution but any comments or guidelines that might be helpful for that are welcome. |
|
|
Can you check that when you use automation, rather than manual changes, this is not an issue? |
|
|
That is an interesting find. It'd be great to track down where those duplicate calls originate from. |
|
|
Hi x42, thanks for replying. I just did the test with the automation thing and yes, is an issue, it has the same behavior. Also, I forgot to mention that the issue is not present only when using plugin internal GUI but also when using Ardour inline controls for a plugin parameter, but in this case the methods called between host and plugin have not the exact same sequence. I did a dirty stack trace analysis for each plugin method call from ardour when a parameter is changed, maybe I could do it better so we can understand which core method or something like that is responsible of each plugin method call. First of all, there are plenty calls to "normalizedParamToPlain" for the same value, so maybe a possible solution could be a kind of mapping where to store the plain value if we already have it. If not, we do the call, but only one time. |
|
|
So the issue here is that various UI elements in Ardour request parameter conversion (interface_to_internal, internal_to_interface) ? normalizedParamToPlain is usually cheap, some basic math, at most a few CPU instructions. Except with yabridge, there is a context switch, which can introduce significant overheard. |
|
|
Hey! I was able to make me some time and checked the issue. I focused in the behavior that I mentioned in the description, where the issue is produced when I change a parameter using a VST3 GUI control. The issue shows as audio clicks and high dsp load (measured in the Ardour meter), specifically when using Yabridge under Linux, some Windows VST3 plugin and a very low buffer size configured in Ardour. Regardless of that audio defect, the analysis of the behavior shows multiple unnecessary calls from host to plugin for all VST3 plugins on Ardour. So I tried to identify the root cause of each "extra" call and here are my results. First of all, I'm assuming that if the plugin calls the host notifying some change in a parameter (performEdit), we get that specific value in the notification. So if that value comes as "normalized" format and we need it in "plain" format, we only need to do one call to "normalizedToPlain" and that's it, no more calls are needed. I mean, we don't need to call "normalizedToPlain" again for the same values, neither we need to call "plainToNormalized", because the original call to "performEdit" already gave us the value as "plain", and neither we need to call "setNormalizedParam", because we are reacting to an initial event where a parameter was set from the plugin GUI, so we don't need to set a value that is already setted. I got stacktraces for each call to a Steinberg::VST3PI object when a parameter was modified from a control in the plugin GUI. I identified 7 different calls enumerated in https://pastebin.com/7whetZ6K. You can see that sometimes the same method is called but from a different source, thats why I assigned a number for each call. With this calls identified, I did some research in Ardour code and I could determine 4 reasons to understand the calls and I have some code changes to propose for each. Call 1 is a necessary "normalizedParamToPlain" call. It is directly called from "performEdit" method and it is needed to transform the value from "plain" to "normalized". This shouldn't be changed. So, the extra calls and their reasons: Reason 1 - Produces call 2 and 3 A fix is proposed in commit https://github.com/Ardour/ardour/commit/abb1407611612758b7e6a37f1588ddbd0467b054 of my merge request. Here, when an OnParameterChange singal is emitted with a VST3PI::ValueChange or VST3PI::ParamValueChanged value, there is a call to "Plugin::parameter_changed_externally(param, value)" method. This method definition has its second argument (float value) ignored and it always trigger a "get_parameter(which)" call. I think that this is not needed. We should not ignore the second argument, because it already has the value that we want, and therefore we don't need to call "get_parameter" (method "VSTPlugin::parameter_changed_externally(uint32_t which, float value)" in ardour/libs/ardour/vst_plugin.cc is defined in that way). Reason 2 - Produces call 4 and 5 A fix is proposed in commit https://github.com/Ardour/ardour/commit/afbc775ffa654b5230d15de384fcbb96e3ff20be of my merge request. This calls are originated in method "ProcessorEntry::Control::control_changed()" at ardour/gtk2_ardour/processor_box.cc. That's because the signal Controllable::Changed is emitted without the corresponding control value. This is not the general case for each object that inherits from Controllable but I think that it makes sense to emit the signal with the value because it is part of the context of the event. In my code I modified the signal to accept one more argument and I used boost::none for the cases that I though that there are no relevant value to send with the signal (that's because it is an optional double value). Also I changed "ProcessorEntry::Control::set_tooltip" si it can receive the control value as an argument, and we save another call to Control->get_value Reason 3 - Produces call 6 A fix is proposed in commit https://github.com/Ardour/ardour/commit/87904b2610dfa71a5b28bb3493075b3188ae4b88 of my merge request. Here, when an OnParameterChange singal is emitted with a VST3PI::ValueChange, method "VST3Plugin::parameter_change_handler" adds the modified parameter to a queue. That queue is later consumed at "VST3Plugin::connect_and_run" (in the audio processing thread) and triggers a "Steinberg::VST3PI::set_parameter" call. I think that this is a case where we are setting something that is already setted, so I think that we can ignore the addition of the parameter to the queue, and because is the only line in the switch of "parameter_change_handler" for the value "VST3PI::ValueChange", I replaced the signal emission to use VST3PI::ParamValueChanged. Reason 4 - Produces call 7 A fix is proposed in commit https://github.com/Ardour/ardour/commit/1ef925450dab4bbb2b1518e9c08103bd2940094c of my merge request. Here, when the "performEdit" happends, the modified parameter is flaged in the _update_ctrl vector. Later, when "VST3PI::update_contoller_param" is called from a kind of "super rapid UI Timer" located in method "VST3PluginUI::start_updating" in the file "ardour/gtk2_ardour/vst3_plugin_ui.cc", it set the param again. So I understand that this is not needed (because we are setting something already setted) and it don't have any other effect. I followed the code to understand it, made different tests on Ardour GUI and there is no changes in the expected behavior on the controls of the plugin (plugin GUI, ardour processor box, automation). ------------- So that is my analysis and code to fix the issue. With this changes we only get one call from host to plugin, to the method "normalizedToPlain". I have experience as software developer but no so much with C/C++ neither contributing to open source projects, so feel free to make me any kind of comments please, I will be glad to apply your suggestions or think another ways to address a solution. I did the changes in separated commits trying to do it more easily to review. Each commit has working code. I manually tested the entire merge request and I found everything working fine. Parameters are updated as expected (or at least as I think that is expected :) ). Automation works. Writing automation works. Processor controls get updated when changing from plugin GUI and vice versa. Thank you for developing and mantaining such a magnific and complex piece of software! |
|
|
Link to Merge/Pull Request: https://github.com/Ardour/ardour/pull/920 |
|
|
Nice bit of detective work! Thanks! I've commented on the pull request, reviewing there is easier. |
- Anonymous
