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

@@ -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

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<VstTimeInfo> 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); });
     }

src/plugin/bridges/vst2.cpp

@@ -558,6 +558,26 @@ intptr_t Vst2PluginBridge::dispatch(AEffect* /*plugin*/,
 
 template <typename T, bool replacing>
 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<VstTimeInfo> current_time_info;
+    const VstTimeInfo* returned_time_info =
+        reinterpret_cast<const VstTimeInfo*>(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<int> 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<std::vector<T>> 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<int> 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);
 

src/wine-host/bridges/vst2.cpp

@@ -186,6 +186,16 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context,
 
         sockets.host_vst_process_replacing.receive_multi<AudioBuffers>(
             [&](AudioBuffers request, std::vector<uint8_t>& 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<decltype(time_info_cache)::Guard> 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,
+    HostCallbackDataConverter(AEffect* plugin, VstTimeInfo& last_time_info)
-                              std::optional<VstTimeInfo>& time_info)
+        : plugin(plugin), last_time_info(last_time_info) {}
-        : plugin(plugin), time_info(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
+                // If the host returned a valid `VstTimeInfo` object, then we'll
-                // make the function return a pointer to it in the function
+                // keep track of it so we can return a pointer to it in the
-                // below. Depending on whether the host supported the
+                // function below
-                // requested time information this operations returns either
+                if (std::holds_alternative<VstTimeInfo>(response.payload)) {
-                // a null pointer or a pointer to a `VstTimeInfo` object.
+                    last_time_info = std::get<VstTimeInfo>(response.payload);
-                if (std::holds_alternative<std::nullptr_t>(response.payload)) {
-                    time_info = std::nullopt;
-                } else {
-                    time_info = std::get<VstTimeInfo>(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
+                // If the host returned a null pointer, then we'll do the same
-                // the function above
+                // thing here
-                VstTimeInfo* time_info_pointer = nullptr;
+                if (original == 0) {
-                if (time_info) {
+                    return 0;
-                    time_info_pointer = &*time_info;
+                } else {
+                    return reinterpret_cast<intptr_t>(&last_time_info);
                 }
-
-                return reinterpret_cast<intptr_t>(time_info_pointer);
             } break;
             default:
                 return DefaultDataConverter::return_value(opcode, original);
@@ -592,7 +589,7 @@ class HostCallbackDataConverter : DefaultDataConverter {
 
    private:
     AEffect* plugin;
-    std::optional<VstTimeInfo>& 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
+            // During a processing call we'll have already sent the current
-            //       `audioMasterGetTime()` once for every sample. When this
+            // transport information from the plugin side to avoid an
-            //       option is enabled `time_info` should be reset in the
+            // unnecessary callback
-            //       process function. The `time_info` value is assigned inside
+            const VstTimeInfo* cached_time_info = time_info_cache.get();
-            //       of `HostCallbackDataConverter::write()`.
+            if (cached_time_info) {
-            if (config.cache_time_info && time_info) {
+                // This cached value is temporary, so we'll still use the
-                return reinterpret_cast<intptr_t>(&*time_info);
+                // 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<intptr_t>(&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);
 }

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<VstTimeInfo> 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<VstTimeInfo> time_info_cache;
+
     // FIXME: This emits `-Wignored-attributes` as of Wine 5.22
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wignored-attributes"