From f1d7b7bf57569c2804aa6cb407b39942c2116f64 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 7 May 2021 19:24:28 +0200 Subject: [PATCH] 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 | 17 ++- .../vst3/plugin/audio-processor.h | 7 +- .../serialization/vst3/process-data.cpp | 64 ++++++---- src/common/serialization/vst3/process-data.h | 109 +++++++++++------- .../bridges/vst3-impls/plugin-proxy.cpp | 16 ++- src/wine-host/bridges/vst3.cpp | 3 +- 6 files changed, 139 insertions(+), 77 deletions(-) diff --git a/src/common/logging/vst3.cpp b/src/common/logging/vst3.cpp index 9b1458e5..d61e6d12 100644 --- a/src/common/logging/vst3.cpp +++ b/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 << ", "; - 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 << ", 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 << ", num_events() + << (*response.output_data.output_events)->num_events() << " events>"; } else { message << ", host does not support event outputs"; diff --git a/src/common/serialization/vst3/plugin/audio-processor.h b/src/common/serialization/vst3/plugin/audio-processor.h index f5c9f837..75a909d6 100644 --- a/src/common/serialization/vst3/plugin/audio-processor.h +++ b/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 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 - * that we can write back to the `ProcessData` object provided by the host. + * `YaProcessData::create_response()` to create a response object that we + * can write the plugin's changes back to the `ProcessData` object provided + * by the host. */ struct Process { using Response = ProcessResponse; diff --git a/src/common/serialization/vst3/process-data.cpp b/src/common/serialization/vst3/process-data.cpp index e7db26a8..1f634347 100644 --- a/src/common/serialization/vst3/process-data.cpp +++ b/src/common/serialization/vst3/process-data.cpp @@ -151,23 +151,22 @@ void YaAudioBusBuffers::write_back_outputs( buffers); } -void YaProcessDataResponse::write_back_outputs( - Steinberg::Vst::ProcessData& process_data) { - 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); - } -} - -YaProcessData::YaProcessData() {} +YaProcessData::YaProcessData() + // This response object acts as an optimization. It stores pointers to the + // original fields in our objects, so we can both only serialize those + // 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 + // `YaAudioProcessor::ProcessResponse` object with a response object that + // belongs to an actual process data object, because with these changes it's + // no longer possible to deserialize those results into a new ad-hoc created + // object. + : response_object{.outputs = &outputs, + .output_parameter_changes = &output_parameter_changes, + .output_events = &output_events}, + // This needs to be zero initialized so we can safely call + // `create_response()` on the plugin side + reconstructed_process_data() {} 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 - // these objects. + // + // On the plugin side this is not necessary, but it also doesn't hurt for (int i = 0; i < reconstructed_process_data.numOutputs; i++) { outputs[i].silence_flags = reconstructed_process_data.outputs[i].silenceFlags; } - return YaProcessDataResponse{ - .outputs = std::move(outputs), - .output_parameter_changes = std::move(output_parameter_changes), - .output_events = std::move(output_events)}; + // NOTE: We return an object that only contains references to these original + // fields to avoid any copies or moves + return response_object; +} + +void YaProcessData::write_back_outputs( + Steinberg::Vst::ProcessData& process_data) { + assert(static_cast(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); + } } diff --git a/src/common/serialization/vst3/process-data.h b/src/common/serialization/vst3/process-data.h index abf3228c..56fa2b85 100644 --- a/src/common/serialization/vst3/process-data.h +++ b/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 outputs; - std::optional output_parameter_changes; - std::optional 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 - 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 - * object that can be used to write those results back to the host. This - * should of course be done after making a call to the `IAudioProcessor`'s - * `process()` function with the object obtained using `get()`. + * A serializable wrapper around the output fields of `ProcessData`, so we + * only have to copy the information back that's actually important. These + * fields are pointers to the corresponding fields in `YaProcessData`. On + * 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* outputs = nullptr; + std::optional* output_parameter_changes = nullptr; + std::optional* output_events = nullptr; + + template + 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 void serialize(S& s) { @@ -281,9 +301,9 @@ class YaProcessData { std::optional 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 - // `move_outputs_to_response()`. so they can be serialized and written back + // fields back into a `YaProcessData::Response` using + // `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 diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp index 34827dec..2ac44ac3 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp +++ b/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(process_request), process_response); - process_response.output_data.write_back_outputs(data); + process_request.data.write_back_outputs(data); return process_response.result; } diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index 7c58daf5..4a35bcac 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -1268,8 +1268,7 @@ size_t Vst3Bridge::register_object_instance( return YaAudioProcessor::ProcessResponse{ .result = result, - .output_data = - request.data.move_outputs_to_response()}; + .output_data = request.data.create_response()}; }, [&](const YaAudioProcessor::GetTailSamples& request) -> YaAudioProcessor::GetTailSamples::Response {