From 23d5567e72a2984a54ed59229336fdaef874fbb2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 30 Nov 2020 14:49:02 +0100 Subject: [PATCH] Add a time info caching compatibility option #62 This is needed to get good performance out of SWAM Cello until this issue is fixed by the plugin. --- CHANGELOG.md | 10 ++++++++++ README.md | 9 +++++++++ src/common/configuration.cpp | 8 +++++++- src/common/configuration.h | 19 +++++++++++++++++-- src/plugin/plugin-bridge.cpp | 3 +++ src/wine-host/bridges/vst2.cpp | 18 ++++++++++++++++++ 6 files changed, 64 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 788e4f2d..85619682 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- Added an option to cache the time and tempo info returned by the host for the + current processing cycle. This would normally not be needed since plugins + should ask the host for this information only once per audio callback, but a + bug in _SWAM Cello_ causes this to happen repeatedly for every sample, + resutling in very bad performance. See the [compatibility + options](https://github.com/robbert-vdh/yabridge#compatibility-options) + section of the readme for more information on how to enable this. + ### Changed - When `YABRIDGE_DEBUG_LEVEL` is set to 2 or higher and a plugin asks the host diff --git a/README.md b/README.md index 54d511f3..4731ab55 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,7 @@ other. See below for an [example](#example) of how these groups can be set up. | Option | Values | Description | | --------------------- | -------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `cache_time_info` | `{true,false}` | Compatibility option for plugins that call `audioMasterGetTime()` multiple times during a single processing cycle. With this option subsequent calls during a single audio processing cycle will reuse the value returned by the first call to this function. This is a bug in the plugin, and this option serves as a temporary workaround until the plugin fixes the issue. | | `editor_double_embed` | `{true,false}` | Compatibility option for plugins that rely on the absolute screen coordinates of the window they're embedded in. Since the Wine window gets embedded inside of a window provided by your DAW, these coordinates won't match up and the plugin would end up drawing in the wrong location without this option. Currently the only known plugins that require this option are _PSPaudioware_ plugins with expandable GUIs, such as E27. Defaults to `false`. | These options are workarounds for issues mentioned in the [known @@ -283,6 +284,9 @@ group = "toneboosters" ["PSPaudioware"] editor_double_embed = true +["SWAM Cello 64bit.so"] +cache_time_info = true + # Simple glob patterns can be used to avoid unneeded repetition ["iZotope*/Neutron *"] group = "izotope" @@ -437,6 +441,11 @@ include: - **PSPaudioware** plugins with expandable GUIs, such as E27, may have their GUI appear in the wrong location after the GUI has been expanded. You can enable an alternative [editor hosting mode](#compatibility-options) to fix this. +- **SWAM Cello** has a bug where it asks the host for the current buffer's time + and tempo information for every sample it processes instead of doing it only + once per buffer, resulting in very bad performance. You can enable the time + info cache [compatibility option](#compatibility-options) to work around this + until this is fixed on the plugin's side. - Plugins like **FabFilter Pro-Q 3** that can share data between different instances of the same plugin plugins have to be hosted within a single process for that functionality to work. See the [plugin groups](#plugin-groups) diff --git a/src/common/configuration.cpp b/src/common/configuration.cpp index 791df004..8f528a15 100644 --- a/src/common/configuration.cpp +++ b/src/common/configuration.cpp @@ -78,7 +78,13 @@ Configuration::Configuration(const fs::path& config_path, // their defaults. At this point I'd really wish C++ could do pattern // matching. for (const auto& [key, value] : table) { - if (key == "editor_double_embed") { + if (key == "cache_time_info") { + if (const auto parsed_value = value.as_boolean()) { + cache_time_info = parsed_value->get(); + } else { + invalid_options.push_back(key); + } + } else if (key == "editor_double_embed") { if (const auto parsed_value = value.as_boolean()) { editor_double_embed = parsed_value->get(); } else { diff --git a/src/common/configuration.h b/src/common/configuration.h index 275d8410..74981337 100644 --- a/src/common/configuration.h +++ b/src/common/configuration.h @@ -74,8 +74,22 @@ class Configuration { const boost::filesystem::path& yabridge_path); /** - * If this is set to true, then the plugin editor should be embedded in yet - * another window. This would result in an embedding sequence of + * If set to `true`, then after an `audioMasterGetTime()` call all + * subsequent calls to that function during a single processing cycle will + * reuse the results from the first call. In theory it would be a bug on the + * host's side if this did _not_ return the same value every time, but since + * yabridge tries to not have any behaviour of its own and since this + * function should only be called at most once every processing cycle this + * option is not enabled by default. An example of a situation where this + * does become necessary is the SWAM Cello plugin, which calls + * `audioMasterGetTime()` for every sample instead of only once. This would + * tank performance without this caching behaviour. + */ + bool cache_time_info = false; + + /** + * If this is set to `true`, then the plugin editor should be embedded in + * yet another window. This would result in an embedding sequence of * ` <-> <-> * <-> `, where * `` is the new addition. The only plugin I've @@ -119,6 +133,7 @@ class Configuration { template void serialize(S& s) { + s.value1b(cache_time_info); s.value1b(editor_double_embed); s.ext(group, bitsery::ext::StdOptional(), [](S& s, auto& v) { s.text1b(v, 4096); }); diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 3f993b9d..824c5b51 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -653,6 +653,9 @@ void PluginBridge::log_init_message() { init_msg << "other options: "; std::vector other_options; + if (config.cache_time_info) { + other_options.push_back("hack: time info cache"); + } if (config.editor_double_embed) { other_options.push_back("editor: double embed"); } diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index c0ad97dd..7d11aa32 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -169,6 +169,15 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context, // pointers to rather than copies of the events. std::lock_guard lock(next_buffer_midi_events_mutex); + // HACK: Workaround for a bug in SWAM Cello where it would call + // `audioMasterGetTime()` once for every sample. The first + // value returned by this function during an audio + // processing cycle will be reused for the rest of the + // cycle. + if (config.cache_time_info) { + time_info.reset(); + } + // Since the host should only be calling one of `process()`, // processReplacing()` or `processDoubleReplacing()`, we can all // handle them over the same socket. We pick which one to call @@ -505,6 +514,15 @@ intptr_t Vst2Bridge::host_callback(AEffect* effect, intptr_t value, void* data, float option) { + // HACK: Workaround for a bug in SWAM Cello where it would call + // `audioMasterGetTime()` once for every sample. When this option is + // enabled `time_info` should be reset in the process function. The + // `time_info` value is assigned inside of + // `HostCallbackDataConverter::write()`. + if (config.cache_time_info && time_info) { + return reinterpret_cast(&*time_info); + } + HostCallbackDataConverter converter(effect, time_info); return sockets.vst_host_callback.send_event(converter, std::nullopt, opcode, index, value, data, option);