From 1deb4cf66490f3c9443ac38f828ff7e08f3f5aba Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 29 Apr 2021 00:32:25 +0200 Subject: [PATCH] Send the VST2 transport info along with processing And cache it during the processing cycle. This greatly reduces the overhead of bridging VST2 plugins. --- CHANGELOG.md | 9 ++++ src/common/serialization/vst2.h | 8 ++++ src/plugin/bridges/vst2.cpp | 31 +++++++++----- src/wine-host/bridges/vst2.cpp | 75 +++++++++++++++++---------------- src/wine-host/bridges/vst2.h | 27 ++++++++---- 5 files changed, 95 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcc29148..40338095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Added +- During VST2 audio processing calls, yabridge will now immediately send the + current transport information along with the audio buffers. This lets us cache + this information during the processing call, which significantly reduce the + overhead of bridging VST2 plugins by avoiding one otherwise mandatory back and + forth function call between yabridge's plugin and the Wine plugin host. This + has an even greater impact on plugins like _SWAM Cello_ that request this + information repeatedly for every sample they process. Previously yabridge had + a `cache_time_info` compatibility option to mitigate the performance hit for + those plugins, but this new caching behaviour supercedes that. - We now always force the CPU's flush-to-zero flag to be set when processing audio. Most plugins will already do this themselves, but plugins like _Kush Audio REDDI_ and _Expressive E Noisy_ that don't will otherwise suffer from diff --git a/src/common/serialization/vst2.h b/src/common/serialization/vst2.h index 0e885763..786f8f50 100644 --- a/src/common/serialization/vst2.h +++ b/src/common/serialization/vst2.h @@ -555,6 +555,13 @@ struct AudioBuffers { */ int sample_frames; + /** + * We'll send the current transport information as part of an audio + * processing call. This lets us a void an unnecessary callback (or in some + * cases, more than one) during every processing cycle. + */ + std::optional current_time_info; + /** * We'll periodically synchronize the realtime priority setting of the * host's audio thread with the Wine plugin host. We'll do this @@ -582,6 +589,7 @@ struct AudioBuffers { }); s.value4b(sample_frames); + s.ext(current_time_info, bitsery::ext::StdOptional{}); s.ext(new_realtime_priority, bitsery::ext::StdOptional{}, [](S& s, int& priority) { s.value4b(priority); }); } diff --git a/src/plugin/bridges/vst2.cpp b/src/plugin/bridges/vst2.cpp index f2e9bd7a..a1583ff0 100644 --- a/src/plugin/bridges/vst2.cpp +++ b/src/plugin/bridges/vst2.cpp @@ -558,6 +558,26 @@ intptr_t Vst2PluginBridge::dispatch(AEffect* /*plugin*/, template void Vst2PluginBridge::do_process(T** inputs, T** outputs, int sample_frames) { + // To prevent unnecessary bridging overhead, we'll send the time information + // together with the buffers because basically every plugin needs this + std::optional current_time_info; + const VstTimeInfo* returned_time_info = + reinterpret_cast(host_callback_function( + &plugin, audioMasterGetTime, 0, 0, nullptr, 0.0)); + if (returned_time_info) { + current_time_info = *returned_time_info; + } + + // We'll synchronize the scheduling priority of the audio thread on the Wine + // plugin host with that of the host's audio thread every once in a while + std::optional new_realtime_priority; + time_t now = std::time(nullptr); + if (now > last_audio_thread_priority_synchronization + + audio_thread_priority_synchronization_interval) { + new_realtime_priority = get_realtime_priority(); + last_audio_thread_priority_synchronization = now; + } + // The inputs and outputs arrays should be `[num_inputs][sample_frames]` and // `[num_outputs][sample_frames]` floats large respectfully. std::vector> input_buffers(plugin.numInputs, @@ -567,18 +587,9 @@ void Vst2PluginBridge::do_process(T** inputs, T** outputs, int sample_frames) { input_buffers[channel].begin()); } - // We'll synchronize the scheduling priority of the audio thread on the Wine - // plugin host with that of the host's audio thread every once in a while - std::optional new_realtime_priority = std::nullopt; - time_t now = std::time(nullptr); - if (now > last_audio_thread_priority_synchronization + - audio_thread_priority_synchronization_interval) { - new_realtime_priority = get_realtime_priority(); - last_audio_thread_priority_synchronization = now; - } - const AudioBuffers request{.buffers = input_buffers, .sample_frames = sample_frames, + .current_time_info = current_time_info, .new_realtime_priority = new_realtime_priority}; sockets.host_vst_process_replacing.send(request, process_buffer); diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 19f2420b..0f1f8aba 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -186,6 +186,16 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context, sockets.host_vst_process_replacing.receive_multi( [&](AudioBuffers request, std::vector& buffer) { + // Since the value cannot change during this processing cycle, + // we'll send the current transport information as part of the + // request so we cache it to avoid unnecessary callbacks from + // the audio thread + std::optional cache_guard = + request.current_time_info + ? std::optional( + time_info_cache.set(*request.current_time_info)) + : std::nullopt; + // As suggested by Jack Winter, we'll synchronize this thread's // audio processing priority with that of the host's audio // thread every once in a while @@ -199,15 +209,6 @@ 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 @@ -264,6 +265,7 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context, AudioBuffers response{ .buffers = output_buffers_single_precision, .sample_frames = request.sample_frames, + .current_time_info = std::nullopt, .new_realtime_priority = std::nullopt}; sockets.host_vst_process_replacing.send(response, buffer); @@ -292,6 +294,7 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context, AudioBuffers response{ .buffers = output_buffers_double_precision, .sample_frames = request.sample_frames, + .current_time_info = std::nullopt, .new_realtime_priority = std::nullopt}; sockets.host_vst_process_replacing.send(response, buffer); @@ -473,9 +476,8 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, class HostCallbackDataConverter : DefaultDataConverter { public: - HostCallbackDataConverter(AEffect* plugin, - std::optional& time_info) - : plugin(plugin), time_info(time_info) {} + HostCallbackDataConverter(AEffect* plugin, VstTimeInfo& last_time_info) + : plugin(plugin), last_time_info(last_time_info) {} EventPayload read(const int opcode, const int index, @@ -548,15 +550,11 @@ class HostCallbackDataConverter : DefaultDataConverter { const EventResult& response) const override { switch (opcode) { case audioMasterGetTime: - // Write the returned `VstTimeInfo` struct into a field and - // make the function return a pointer to it in the function - // below. Depending on whether the host supported the - // requested time information this operations returns either - // a null pointer or a pointer to a `VstTimeInfo` object. - if (std::holds_alternative(response.payload)) { - time_info = std::nullopt; - } else { - time_info = std::get(response.payload); + // If the host returned a valid `VstTimeInfo` object, then we'll + // keep track of it so we can return a pointer to it in the + // function below + if (std::holds_alternative(response.payload)) { + last_time_info = std::get(response.payload); } break; default: @@ -569,14 +567,13 @@ class HostCallbackDataConverter : DefaultDataConverter { const intptr_t original) const override { switch (opcode) { case audioMasterGetTime: { - // Return a pointer to the `VstTimeInfo` object written in - // the function above - VstTimeInfo* time_info_pointer = nullptr; - if (time_info) { - time_info_pointer = &*time_info; + // If the host returned a null pointer, then we'll do the same + // thing here + if (original == 0) { + return 0; + } else { + return reinterpret_cast(&last_time_info); } - - return reinterpret_cast(time_info_pointer); } break; default: return DefaultDataConverter::return_value(opcode, original); @@ -592,7 +589,7 @@ class HostCallbackDataConverter : DefaultDataConverter { private: AEffect* plugin; - std::optional& time_info; + VstTimeInfo& last_time_info; }; intptr_t Vst2Bridge::host_callback(AEffect* effect, @@ -603,18 +600,22 @@ intptr_t Vst2Bridge::host_callback(AEffect* effect, float option) { switch (opcode) { case audioMasterGetTime: { - // 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); + // During a processing call we'll have already sent the current + // transport information from the plugin side to avoid an + // unnecessary callback + const VstTimeInfo* cached_time_info = time_info_cache.get(); + if (cached_time_info) { + // This cached value is temporary, so we'll still use the + // regular time info storing mechanism + // TODO: Log when we hit this cache so it doesn't get hidden + last_time_info = *cached_time_info; + + return reinterpret_cast(&last_time_info); } } break; } - HostCallbackDataConverter converter(effect, time_info); + HostCallbackDataConverter converter(effect, last_time_info); return sockets.vst_host_callback.send_event(converter, std::nullopt, opcode, index, value, data, option); } diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 154ea783..e94c2398 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -96,14 +96,6 @@ class Vst2Bridge : public HostBridge { */ Vst2Logger logger; - /** - * With the `audioMasterGetTime` host callback the plugin expects the return - * value from the calblack to be a pointer to a VstTimeInfo struct. If the - * host did not support a certain time info query, then we'll store the - * returned null pointer as a nullopt. - */ - std::optional time_info; - /** * The IO context used for event handling so that all events and window * message handling can be performed from a single thread, even when hosting @@ -118,6 +110,25 @@ class Vst2Bridge : public HostBridge { */ Configuration config; + /** + * We'll store the last transport information obtained from the host as a + * result of `audioMasterGetTime()` here so we can return a pointer to it if + * the request was successful. To prevent unnecessary back and forth + * communication, we'll send a copy of the current transport information to + * the plugin as part of the audio processing call. + * + * @see cached_time_info + */ + VstTimeInfo last_time_info; + + /** + * This will temporarily cache the current time info during an audio + * processing call to avoid an additional callback every processing cycle. + * Some faulty plugins may even request this information for every sample, + * which would otherwise cause a very noticeable performance hit. + */ + ScopedValueCache time_info_cache; + // FIXME: This emits `-Wignored-attributes` as of Wine 5.22 #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wignored-attributes"