Avoid allocations in VST3 process response

This is very ugly so hopefully I can think of a neater way, but now the
response object is just a set of pointers, so we can avoid all copies
and moves on the Wine side.

src/common/logging/vst3.cpp

@@ -1774,11 +1774,11 @@ void Vst3Logger::log_response(
 
         // This is incredibly verbose, but if you're really a plugin that
         // handles processing in a weird way you're going to need all of this
-
         std::ostringstream num_output_channels;
         num_output_channels << "[";
+        assert(response.output_data.outputs);
         for (bool is_first = true;
-             const auto& buffers : response.output_data.outputs) {
+             const auto& buffers : *response.output_data.outputs) {
             num_output_channels << (is_first ? "" : ", ")
                                 << buffers.num_channels();
             is_first = false;
@@ -1788,18 +1788,23 @@ void Vst3Logger::log_response(
         message << ", <AudioBusBuffers array with " << num_output_channels.str()
                 << " channels>";
 
-        if (response.output_data.output_parameter_changes) {
+        // Our optimization strategy sadly meant that this had to become a
+        // pointer to an `std::optional<>`, so this becomes a bit ugly.
+        // TODO: Can we make this prettier again?
+        assert(response.output_data.output_parameter_changes);
+        if (*response.output_data.output_parameter_changes) {
             message << ", <IParameterChanges* for "
-                    << response.output_data.output_parameter_changes
+                    << (*response.output_data.output_parameter_changes)
                            ->num_parameters()
                     << " parameters>";
         } else {
             message << ", host does not support parameter outputs";
         }
 
-        if (response.output_data.output_events) {
+        assert(response.output_data.output_events);
+        if (*response.output_data.output_events) {
             message << ", <IEventList* with "
-                    << response.output_data.output_events->num_events()
+                    << (*response.output_data.output_events)->num_events()
                     << " events>";
         } else {
             message << ", host does not support event outputs";

src/common/serialization/vst3/plugin/audio-processor.h

@@ -222,7 +222,7 @@ class YaAudioProcessor : public Steinberg::Vst::IAudioProcessor {
      */
     struct ProcessResponse {
         UniversalTResult result;
-        YaProcessDataResponse output_data;
+        YaProcessData::Response output_data;
 
         template <typename S>
         void serialize(S& s) {
@@ -238,8 +238,9 @@ class YaAudioProcessor : public Steinberg::Vst::IAudioProcessor {
      * provided by the host so we can send it to the Wine plugin host. We can
      * then use `YaProcessData::reconstruct()` on the Wine plugin host side to
      * reconstruct the original `ProcessData` object, and we then finally use
-     * `YaProcessData::move_outputs_to_response()` to create a response object
+     * `YaProcessData::create_response()` to create a response object that we
-     * that we can write back to the `ProcessData` object provided by the host.
+     * can write the plugin's changes back to the `ProcessData` object provided
+     * by the host.
      */
     struct Process {
         using Response = ProcessResponse;

src/common/serialization/vst3/process-data.cpp

@@ -151,23 +151,22 @@ void YaAudioBusBuffers::write_back_outputs(
         buffers);
 }
 
-void YaProcessDataResponse::write_back_outputs(
+YaProcessData::YaProcessData()
-    Steinberg::Vst::ProcessData& process_data) {
+    // This response object acts as an optimization. It stores pointers to the
-    for (int i = 0; i < process_data.numOutputs; i++) {
+    // original fields in our objects, so we can both only serialize those
-        outputs[i].write_back_outputs(process_data.outputs[i]);
+    // fields when sending the response from the Wine side. This lets us avoid
-    }
+    // allocations by not having to copy or move the data. On the plugin side we
-
+    // need to be careful to deserialize into an existing
-    if (output_parameter_changes && process_data.outputParameterChanges) {
+    // `YaAudioProcessor::ProcessResponse` object with a response object that
-        output_parameter_changes->write_back_outputs(
+    // belongs to an actual process data object, because with these changes it's
-            *process_data.outputParameterChanges);
+    // no longer possible to deserialize those results into a new ad-hoc created
-    }
+    // object.
-
+    : response_object{.outputs = &outputs,
-    if (output_events && process_data.outputEvents) {
+                      .output_parameter_changes = &output_parameter_changes,
-        output_events->write_back_outputs(*process_data.outputEvents);
+                      .output_events = &output_events},
-    }
+      // This needs to be zero initialized so we can safely call
-}
+      // `create_response()` on the plugin side
-
+      reconstructed_process_data() {}
-YaProcessData::YaProcessData() {}
 
 void YaProcessData::repopulate(
     const Steinberg::Vst::ProcessData& process_data) {
@@ -291,21 +290,36 @@ Steinberg::Vst::ProcessData& YaProcessData::reconstruct() {
     return reconstructed_process_data;
 }
 
-YaProcessDataResponse YaProcessData::move_outputs_to_response() {
+YaProcessData::Response& YaProcessData::create_response() {
     // NOTE: We _have_ to manually copy over the silence flags from the
     //       `ProcessData` object generated in `get()` here sicne these of
     //       course are not references or pointers like all other fields, so
     //       they're not implicitly copied like all of our other fields
-    // FIXME: Instead of moving, the `YaProcessDataResponse` should be an
+    //
-    //        (optional) field. Moving defeats the point of us trying to reuse
+    //       On the plugin side this is not necessary, but it also doesn't hurt
-    //        these objects.
     for (int i = 0; i < reconstructed_process_data.numOutputs; i++) {
         outputs[i].silence_flags =
             reconstructed_process_data.outputs[i].silenceFlags;
     }
 
-    return YaProcessDataResponse{
+    // NOTE: We return an object that only contains references to these original
-        .outputs = std::move(outputs),
+    //       fields to avoid any copies or moves
-        .output_parameter_changes = std::move(output_parameter_changes),
+    return response_object;
-        .output_events = std::move(output_events)};
+}
+
+void YaProcessData::write_back_outputs(
+    Steinberg::Vst::ProcessData& process_data) {
+    assert(static_cast<int32>(outputs.size()) == process_data.numOutputs);
+    for (int i = 0; i < process_data.numOutputs; i++) {
+        outputs[i].write_back_outputs(process_data.outputs[i]);
+    }
+
+    if (output_parameter_changes && process_data.outputParameterChanges) {
+        output_parameter_changes->write_back_outputs(
+            *process_data.outputParameterChanges);
+    }
+
+    if (output_events && process_data.outputEvents) {
+        output_events->write_back_outputs(*process_data.outputEvents);
+    }
 }

src/common/serialization/vst3/process-data.h

@@ -131,41 +131,15 @@ class YaAudioBusBuffers {
         buffers;
 };
 
-/**
- * A serializable wrapper around the output fields of `ProcessData`. We send
- * this back as a response to a process call so we can write those fields back
- * to the host. It would be possible to just send `YaProcessData` back and have
- * everything be in a single structure, but that would involve a lot of
- * unnecessary copying (since, at least in theory, all the input audio buffers,
- * events and context data shouldn't have been changed by the plugin).
- *
- * @see YaProcessData
- */
-struct YaProcessDataResponse {
-    // These fields are directly moved from a `YaProcessData` object. See the
-    // docstrings there for more information
-    std::vector<YaAudioBusBuffers> outputs;
-    std::optional<YaParameterChanges> output_parameter_changes;
-    std::optional<YaEventList> output_events;
-
-    /**
-     * Write all of this output data back to the host's `ProcessData` object.
-     */
-    void write_back_outputs(Steinberg::Vst::ProcessData& process_data);
-
-    template <typename S>
-    void serialize(S& s) {
-        s.container(outputs, max_num_speakers);
-        s.ext(output_parameter_changes, bitsery::ext::StdOptional{});
-        s.ext(output_events, bitsery::ext::StdOptional{});
-    }
-};
-
 /**
  * A serializable wrapper around `ProcessData`. We'll read all information from
  * the host so we can serialize it and provide an equivalent `ProcessData`
- * struct to the plugin. Then we can create a `YaProcessDataResponse` object
+ * struct to the plugin. Then we can create a `YaProcessData::Response` object
  * that contains all output values so we can write those back to the host.
+ *
+ * Be sure to double check how `YaProcessData::Response` is used. We do some
+ * pointer tricks there to avoid copies and moves when serializing the results
+ * of our audio processing.
  */
 class YaProcessData {
    public:
@@ -194,12 +168,58 @@ class YaProcessData {
     Steinberg::Vst::ProcessData& reconstruct();
 
     /**
-     * **Move** all output written by the Windows VST3 plugin to a response
+     * A serializable wrapper around the output fields of `ProcessData`, so we
-     * object that can be used to write those results back to the host. This
+     * only have to copy the information back that's actually important. These
-     * should of course be done after making a call to the `IAudioProcessor`'s
+     * fields are pointers to the corresponding fields in `YaProcessData`. On
-     * `process()` function with the object obtained using `get()`.
+     * the plugin side this information can then be written back to the host.
+     *
+     * HACK: All of this is an optimization to avoid unnecessarily copying or
+     *       moving and reallocating. Directly serializing and deserializing
+     *       from and to pointers does make all of this very error prone, hence
+     *       all of the assertions.
+     *
+     * @see YaProcessData
      */
-    YaProcessDataResponse move_outputs_to_response();
+    struct Response {
+        // We store raw pointers instead of references so we can default
+        // initialize this object during deserialization
+        std::vector<YaAudioBusBuffers>* outputs = nullptr;
+        std::optional<YaParameterChanges>* output_parameter_changes = nullptr;
+        std::optional<YaEventList>* output_events = nullptr;
+
+        template <typename S>
+        void serialize(S& s) {
+            assert(outputs && output_parameter_changes && output_events);
+            // Since these fields are references to the corresponding fields on
+            // the surrounding object, we're actually serializing those fields.
+            // This means that on the plugin side we can _only_ deserialize into
+            // an existing object, since our serializing code doesn't touch the
+            // actual pointers.
+            s.container(*outputs, max_num_speakers);
+            s.ext(*output_parameter_changes, bitsery::ext::StdOptional{});
+            s.ext(*output_events, bitsery::ext::StdOptional{});
+        }
+    };
+
+    /**
+     * Create a `YaProcessData::Response` object that refers to the output
+     * fields in this object. The object doesn't store any actual data, and may
+     * not outlive this object. We use this so we only have to copy the relevant
+     * fields back to the host. On the Wine side this function should only be
+     * called after we call the plugin's `IAudioProcessor::process()` function
+     * with the reconstructed process data obtained from
+     * `YaProcessData::reconstruct()`.
+     *
+     * On the plugin side this should be used to create a response object that
+     * **must** be received into, since we're deserializing directly into some
+     * pointers.
+     */
+    Response& create_response();
+
+    /**
+     * Write all of this output data back to the host's `ProcessData` object.
+     */
+    void write_back_outputs(Steinberg::Vst::ProcessData& process_data);
 
     template <typename S>
     void serialize(S& s) {
@@ -281,9 +301,9 @@ class YaProcessData {
     std::optional<Steinberg::Vst::ProcessContext> process_context;
 
    private:
-    // These are the same fields as in `YaProcessDataResponse`. We'll generate
+    // These are the same fields as in `YaProcessData::Response`. We'll generate
     // these as part of creating `reconstructed_process_data`, and they will be
-    // moved into a response object during `move_outputs_to_response()`.
+    // referred to in the response object created in `create_response()`
 
     /**
      * The outputs. Will be created based on `outputs_num_channels` (which
@@ -308,10 +328,19 @@ class YaProcessData {
     // reconstruct the original `ProcessData` object. Here we also initialize
     // these `output*` fields so the Windows VST3 plugin can write to them
     // though a regular `ProcessData` object. Finally we can wrap these output
-    // fields back into a `YaProcessDataResponse` using
+    // fields back into a `YaProcessData::Response` using
-    // `move_outputs_to_response()`. so they can be serialized and written back
+    // `create_response()`. so they can be serialized and written back
     // to the host's `ProcessData` object.
 
+    /**
+     * This is a `Response` object that refers to the fields below.
+     *
+     * NOTE: We use this on the plugin side as an optimization to be able to
+     *       directly receive data into this object, avoiding the need for any
+     *       allocations.
+     */
+    Response response_object;
+
     /**
      * Obtained by calling `.get()` on every `YaAudioBusBuffers` object in
      * `intputs`. These objects contain pointers to the data in `inputs` and may

src/plugin/bridges/vst3-impls/plugin-proxy.cpp

@@ -220,13 +220,27 @@ Vst3PluginProxyImpl::process(Steinberg::Vst::ProcessData& data) {
     process_request.data.repopulate(data);
     process_request.new_realtime_priority = new_realtime_priority;
 
+    // HACK: This is a bit ugly. This `YaProcessData::Response` object actually
+    //       contains pointers to the corresponding `YaProcessData` fields in
+    //       this object, so we can only send back the fields that are actually
+    //       relevant. This is necessary to avoid allocating copies or moves on
+    //       the Wine side. This `create_response()` function creates a response
+    //       object that points to the fields in `process_request.data`, so when
+    //       we deserialize into `process_response` we end up actually writing
+    //       to the actual `process_request.data` object. Thus we can also call
+    //       `process_request.data.write_back_outputs()` later.
+    //
+    //       `YaProcessData::Response::serialize()` should make this a lot
+    //       clearer.
+    process_response.output_data = process_request.data.create_response();
+
     // We'll also receive the response into an existing object so we can also
     // avoid heap allocations there
     bridge.receive_audio_processor_message_into(
         MessageReference<YaAudioProcessor::Process>(process_request),
         process_response);
 
-    process_response.output_data.write_back_outputs(data);
+    process_request.data.write_back_outputs(data);
 
     return process_response.result;
 }

src/wine-host/bridges/vst3.cpp

@@ -1268,8 +1268,7 @@ size_t Vst3Bridge::register_object_instance(
 
                         return YaAudioProcessor::ProcessResponse{
                             .result = result,
-                            .output_data =
+                            .output_data = request.data.create_response()};
-                                request.data.move_outputs_to_response()};
                     },
                     [&](const YaAudioProcessor::GetTailSamples& request)
                         -> YaAudioProcessor::GetTailSamples::Response {