From b5dd806b2d537d63601a94d5f536be68a1504947 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 30 Jan 2021 22:24:05 +0100 Subject: [PATCH] Cache VST3 parameter information This is in some cases needed to get decent performance in REAPER, as REAPER seems to query this information (which cannot change without the plugin requesting a restart) four times per second. --- README.md | 5 - src/common/logging/vst3.cpp | 6 +- src/common/logging/vst3.h | 3 +- .../bridges/vst3-impls/plugin-proxy.cpp | 83 +++++++++++--- src/plugin/bridges/vst3-impls/plugin-proxy.h | 107 +++++++++++++++--- src/plugin/bridges/vst3.cpp | 6 +- src/wine-host/editor.cpp | 17 ++- 7 files changed, 181 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index ee214045..96192a8e 100644 --- a/README.md +++ b/README.md @@ -543,11 +543,6 @@ include: section for instructions on how to set this up. This is not necessary for VST3 plugins, as multiple instances of those plugins will always be hosted in a single process by design. -- When using **REAPER**, make sure that REAPER's plugin bridges are disabled. - When this option is enabled REAPER will constantly query all of a plugin's - parameters four times per second. This can amount to multiple tens of - thousands of extra, unnecessary function calls per second, which increases - yabridge's CPU usage considerably. - **Drag-and-drop** from applications running under Wine to X11 does not yet work, so you won't be able to drag samples and MIDI files from a plugin to the host. At least, not directly. Because Windows applications have to create diff --git a/src/common/logging/vst3.cpp b/src/common/logging/vst3.cpp index 0dda71e9..e3d3fcbd 100644 --- a/src/common/logging/vst3.cpp +++ b/src/common/logging/vst3.cpp @@ -1411,13 +1411,17 @@ void Vst3Logger::log_response( void Vst3Logger::log_response( bool is_host_vst, - const YaEditController::GetParameterInfoResponse& response) { + const YaEditController::GetParameterInfoResponse& response, + bool from_cache) { log_response_base(is_host_vst, [&](auto& message) { message << response.result.string(); if (response.result == Steinberg::kResultOk) { std::string param_title = VST3::StringConvert::convert(response.updated_info.title); message << ", "; + if (from_cache) { + message << " (from cache)"; + } } }); } diff --git a/src/common/logging/vst3.h b/src/common/logging/vst3.h index 35b014d8..89db7214 100644 --- a/src/common/logging/vst3.h +++ b/src/common/logging/vst3.h @@ -247,7 +247,8 @@ class Vst3Logger { void log_response(bool is_host_vst, const Vst3PluginProxy::GetStateResponse&); void log_response(bool is_host_vst, - const YaEditController::GetParameterInfoResponse&); + const YaEditController::GetParameterInfoResponse&, + bool from_cache = false); void log_response(bool is_host_vst, const YaEditController::GetParamStringByValueResponse&); void log_response(bool is_host_vst, diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp index 2b09726a..1d41d6c5 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp @@ -63,10 +63,9 @@ bool Vst3PluginProxyImpl::unregister_context_menu(size_t context_menu_id) { return context_menus.erase(context_menu_id); } -void Vst3PluginProxyImpl::clear_bus_cache() { - if (processing_bus_cache) { - processing_bus_cache.emplace(); - } +void Vst3PluginProxyImpl::clear_caches() { + clear_bus_cache(); + clear_parameter_cache(); } tresult PLUGIN_API Vst3PluginProxyImpl::setAudioPresentationLatencySamples( @@ -231,8 +230,8 @@ Vst3PluginProxyImpl::getBusCount(Steinberg::Vst::MediaType type, it != processing_bus_cache->bus_count.end()) { const bool log_response = bridge.logger.log_request(true, request); if (log_response) { - bridge.logger.log_response(true, PrimitiveWrapper(it->second), - true); + bridge.logger.log_response( + true, YaComponent::GetBusCount::Response(it->second), true); } return it->second; @@ -268,11 +267,11 @@ Vst3PluginProxyImpl::getBusInfo(Steinberg::Vst::MediaType type, it != processing_bus_cache->bus_info.end()) { const bool log_response = bridge.logger.log_request(true, request); if (log_response) { - bridge.logger.log_response( - true, - GetBusInfoResponse{.result = Steinberg::kResultOk, - .updated_bus = it->second}, - true); + bridge.logger.log_response(true, + YaComponent::GetBusInfo::Response{ + .result = Steinberg::kResultOk, + .updated_bus = it->second}, + true); } bus = it->second; @@ -436,19 +435,59 @@ Vst3PluginProxyImpl::setComponentState(Steinberg::IBStream* state) { } int32 PLUGIN_API Vst3PluginProxyImpl::getParameterCount() { - return bridge.send_message( - YaEditController::GetParameterCount{.instance_id = instance_id()}); + const auto request = + YaEditController::GetParameterCount{.instance_id = instance_id()}; + + // We'll cache this information to work around an issue in REAPER, see + // `parameter_info_cache` + if (parameter_info_cache.parameter_count) { + const bool log_response = bridge.logger.log_request(true, request); + if (log_response) { + bridge.logger.log_response( + true, + YaEditController::GetParameterCount::Response( + *parameter_info_cache.parameter_count), + true); + } + + return *parameter_info_cache.parameter_count; + } + + const int32 result = bridge.send_message(request); + + parameter_info_cache.parameter_count = result; + + return result; } tresult PLUGIN_API Vst3PluginProxyImpl::getParameterInfo( int32 paramIndex, Steinberg::Vst::ParameterInfo& info /*out*/) { - const GetParameterInfoResponse response = bridge.send_message( - YaEditController::GetParameterInfo{.instance_id = instance_id(), - .param_index = paramIndex, - .info = info}); + const auto request = YaEditController::GetParameterInfo{ + .instance_id = instance_id(), .param_index = paramIndex, .info = info}; + + // We'll cache this information to work around an issue in REAPER, see + // `parameter_info_cache` + if (auto it = parameter_info_cache.parameter_info.find(paramIndex); + it != parameter_info_cache.parameter_info.end()) { + const bool log_response = bridge.logger.log_request(true, request); + if (log_response) { + bridge.logger.log_response( + true, + YaEditController::GetParameterInfo::Response{ + .result = Steinberg::kResultOk, .updated_info = it->second}, + true); + } + + info = it->second; + + return Steinberg::kResultOk; + } + + const GetParameterInfoResponse response = bridge.send_message(request); info = response.updated_info; + parameter_info_cache.parameter_info[paramIndex] = response.updated_info; return response.result; } @@ -1117,3 +1156,13 @@ tresult PLUGIN_API Vst3PluginProxyImpl::getXmlRepresentationStream( return Steinberg::kInvalidArgument; } } + +void Vst3PluginProxyImpl::clear_bus_cache() { + if (processing_bus_cache) { + processing_bus_cache.emplace(); + } +} + +void Vst3PluginProxyImpl::clear_parameter_cache() { + parameter_info_cache = ParameterInfoCache{}; +} diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.h b/src/plugin/bridges/vst3-impls/plugin-proxy.h index 4acd0733..83c61b7e 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.h +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.h @@ -19,6 +19,24 @@ #include "../vst3.h" #include "plug-view-proxy.h" +/** + * Here we pass though all function calls made by the host to the Windows VST3 + * plugin. We sadly had to deviate from yabridge's 'one-to-one passthrough' + * philosphy in two places: + * + * 1. REAPER requests the plugin's input and output bus count and information + * every processing cycle, even though this information cannot change during + * processing. With plugins with many outputs this can lead a lot of extra + * back and forth before the plugin gets to process audio, which thus leads + * to very inflated DSP usage. + * 2. REAPER in some situations (because it doesn't always seem to happen) + * fetches the plugin's parameter information exactly four times per second. + * When plugins have thousands of parameters, this unnecessarily uses up a + * lot of CPU time. + * + * Both of these caches were necessary to get decent performance in REAPER, and + * they should be removed as soon as REAPER no longer needs this. + */ class Vst3PluginProxyImpl : public Vst3PluginProxy { public: Vst3PluginProxyImpl(Vst3PluginBridge& bridge, @@ -55,23 +73,19 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { bool unregister_context_menu(size_t context_menu_id); /** - * Clear the bus count and information cache. We need this cache for REAPER - * as it makes `num_inputs + num_outputs + 2` function calls to retrieve - * this information every single processing cycle. For plugins with a lot of - * outputs this really adds up. According to the VST3 workflow diagrams bus - * information cannot change anymore once `IAudioProcessor::setProcessing()` - * has been called, but REAPER doesn't quite follow the spec here and it - * will set bus arrangements and activate the plugin only after it's called - * `IAudioProcessor::setProcessing()`. Because of that we'll have to - * manually flush this cache when the stores information potentially becomes - * invalid. + * Clear the bus and parameter caches. We'll call this on + * `IComponentHandler::restartComponent`. These caching layers are necessary + * to get decent performance in REAPER as REAPER repeatedly calls these + * functions many times per second, even though their values will never + * change. * * HACK: Once REAPER stops calling these functions, we should remove this * caching layer ASAP as it can only cause issues * - * @see processing_bus_cache + * @see clear_bus_cache + * @see clear_parameter_cache */ - void clear_bus_cache(); + void clear_caches(); // From `IAudioPresentationLatency` tresult PLUGIN_API @@ -383,6 +397,33 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { Steinberg::FUnknownPtr unit_handler_2; private: + /** + * Clear the bus count and information cache. We need this cache for REAPER + * as it makes `num_inputs + num_outputs + 2` function calls to retrieve + * this information every single processing cycle. For plugins with a lot of + * outputs this really adds up. According to the VST3 workflow diagrams bus + * information cannot change anymore once `IAudioProcessor::setProcessing()` + * has been called, but REAPER doesn't quite follow the spec here and it + * will set bus arrangements and activate the plugin only after it's called + * `IAudioProcessor::setProcessing()`. Because of that we'll have to + * manually flush this cache when the stores information potentially becomes + * invalid. + * + * @see processing_bus_cache + */ + void clear_bus_cache(); + + /** + * Clears the parameter information cache. Normally hosts only have to + * request this once, since the information never changes. REAPER however in + * some situations asks for this information four times per second. This + * extra back and forth can really add up once plugins start having + * thousands of parameters. + * + * @see parameter_info_cache + */ + void clear_parameter_cache(); + Vst3PluginBridge& bridge; /** @@ -431,15 +472,45 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { }; /** - * HACK: To work around some behaviour in REAPER where it will repeatedly - * query the same bus information for bus during every processing - * cycle, we'll cache this information during processing. Otherwise - * this will cause `input_busses + output_busses + 2` extra - * unnecessary back and forths for every processing cycle. This can - * really add up for plugins with 16, or even 32 outputs. + * To work around some behaviour in REAPER where it will repeatedly query + * the same bus information for bus during every processing cycle, we'll + * cache this information during processing. Otherwise this will cause + * `input_busses + output_busses + 2` extra unnecessary back and forths for + * every processing cycle. This can really add up for plugins with 16, or + * even 32 outputs. * * Since this information cannot change during processing, this will not * contain a value while the plugin is not processing audio. + * + * HACK: This is only necessary because REAPER requests this information + * once per procession cycle. We can get rid of this once it no longer + * does that. */ std::optional processing_bus_cache; + + /** + * A cache for `IEditController::getParameterCount()` and + * `IEditController::getParameterInfo()` to work around an implementation + * issue in REAPER. In some situations REAPER will query this information + * four times a second, and all of this back and forth communication really + * adds up when a plugin starts having thousands of parameters. + * + * @see parameter_cache + */ + struct ParameterInfoCache { + std::optional parameter_count; + std::map parameter_info; + }; + + /** + * A cache for the parameter count and infos. This is necessary because in + * some situations REAPER queries this information four times per second + * even though it cannot change. This happens when using the plugin bridges, + * but it can also happen in some other cases so I'm not quite sure what the + * trigger is. + * + * HACK: This is only necessary because REAPER sometimes requests this + * information four times per second. + */ + ParameterInfoCache parameter_info_cache; }; diff --git a/src/plugin/bridges/vst3.cpp b/src/plugin/bridges/vst3.cpp index f6e46afd..d67617dd 100644 --- a/src/plugin/bridges/vst3.cpp +++ b/src/plugin/bridges/vst3.cpp @@ -88,9 +88,9 @@ Vst3PluginBridge::Vst3PluginBridge() Vst3PluginProxyImpl& proxy_object = plugin_proxies.at(request.owner_instance_id).get(); - // To err on the safe side, we'll just always clear out bus - // info cache whenever a plugin requests a restart - proxy_object.clear_bus_cache(); + // To err on the safe side, we'll just always clear out all + // of our caches whenever a plugin requests a restart + proxy_object.clear_caches(); return proxy_object.component_handler->restartComponent( request.flags); diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 490627c5..6b7e68f1 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -324,7 +324,9 @@ void Editor::handle_x11_events() const { xcb_generic_event_t* generic_event; while ((generic_event = xcb_poll_for_event(x11_connection.get())) != nullptr) { - switch (generic_event->response_type & event_type_mask) { + const uint8_t event_type = + generic_event->response_type & event_type_mask; + switch (event_type) { // We're listening for `ConfigureNotify` events on the topmost // window before the root window, i.e. the window that's actually // going to get dragged around the by the user. In most cases this @@ -334,6 +336,7 @@ void Editor::handle_x11_events() const { // check is sometimes necessary for using multiple editor windows // within a single plugin group. case XCB_CONFIGURE_NOTIFY: + std::cerr << "XCB_CONFIGURE_NOTIFY" << std::endl; if (!use_xembed) { fix_local_coordinates(); } @@ -342,6 +345,7 @@ void Editor::handle_x11_events() const { // most hosts will only show the window after the plugin has // embedded itself into it. case XCB_VISIBILITY_NOTIFY: + std::cerr << "XCB_VISIBILITY_NOTIFY" << std::endl; if (use_xembed) { do_xembed(); } @@ -357,6 +361,12 @@ void Editor::handle_x11_events() const { // `EnterNotify'. case XCB_ENTER_NOTIFY: case XCB_FOCUS_IN: + if (event_type == XCB_ENTER_NOTIFY) { + std::cerr << "XCB_ENTER_NOTIFY" << std::endl; + } else { + std::cerr << "XCB_FOCUS_IN" << std::endl; + } + if (!use_xembed) { fix_local_coordinates(); } @@ -376,6 +386,7 @@ void Editor::handle_x11_events() const { // to mess with keyboard focus when hovering over the window while // for instance a dialog is open. case XCB_LEAVE_NOTIFY: { + std::cerr << "XCB_LEAVE_NOTIFY" << std::endl; const auto event = reinterpret_cast(generic_event); @@ -393,6 +404,10 @@ void Editor::handle_x11_events() const { set_input_focus(false); } } break; + default: { + std::cerr << "X11 event " << event_type << std::endl; + break; + } } free(generic_event);