From 162aeed6619dc7177f71f1926a045cecc250980a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 19 May 2022 14:43:59 +0200 Subject: [PATCH] Only set up VST3 SHM audio buffers in setActive() This avoids doing the duplicate check (since both `setProcessing()` and `setActive()` would be called), and this also gets rid of the assumption added a couple commits ago that `setupProcessing()` is only ever called once, which is not true. --- docs/architecture.md | 2 +- src/common/logging/vst3.cpp | 18 ++----------- src/common/logging/vst3.h | 2 -- .../vst3/plugin/audio-processor.h | 22 +--------------- .../serialization/vst3/plugin/component.h | 4 +-- .../serialization/vst3/process-data.cpp | 2 +- src/common/serialization/vst3/process-data.h | 2 +- .../bridges/vst3-impls/plugin-proxy.cpp | 25 +++++-------------- src/plugin/bridges/vst3-impls/plugin-proxy.h | 2 +- src/wine-host/bridges/vst3.cpp | 18 ++++--------- src/wine-host/bridges/vst3.h | 2 +- 11 files changed, 21 insertions(+), 78 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 3786bdd0..e8ef7174 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -270,7 +270,7 @@ that gets sent along with the actual audio buffers. This also means that the sockets act as a form of synchronisation, so we do not need any additional inter-process locking. These shared memory audio buffers are defined as part of `AudioShmBuffer`, and they are configured while handling `effMainsChanged` for -VST2 plugins and during `IAudioProcessor::setupProcessing()` for VST3 plugins. +VST2 plugins and during `IAudioProcessor::setActive()` for VST3 plugins. For VST2 plugins this does mean that we will need to keep track of the maximum block size and the sample size reported by the host, since this information is not passed along with `effMainsChanged`. diff --git a/src/common/logging/vst3.cpp b/src/common/logging/vst3.cpp index f863c43e..f9caab4c 100644 --- a/src/common/logging/vst3.cpp +++ b/src/common/logging/vst3.cpp @@ -1007,9 +1007,8 @@ bool Vst3Logger::log_request( // TODO: The channel counts are now capped at what the plugin // supports (based on the audio buffers we set up during - // `IAudioProcessor::setupProcessing()`). Some hosts may send - // more buffers, but we don't reflect that in the output right - // now. + // `IAudioProcessor::setActive()`). Some hosts may send more + // buffers, but we don't reflect that in the output right now. std::ostringstream num_input_channels; num_input_channels << "["; for (bool is_first = true; @@ -1788,19 +1787,6 @@ void Vst3Logger::log_response( }); } -void Vst3Logger::log_response( - bool is_host_vst, - const YaAudioProcessor::SetupProcessingResponse& response) { - log_response_base(is_host_vst, [&](auto& message) { - message << response.result.string(); - if (response.result == Steinberg::kResultOk) { - message << ", "; - } - }); -} - void Vst3Logger::log_response( bool is_host_vst, const YaAudioProcessor::ProcessResponse& response) { diff --git a/src/common/logging/vst3.h b/src/common/logging/vst3.h index 13692d0d..6700a49f 100644 --- a/src/common/logging/vst3.h +++ b/src/common/logging/vst3.h @@ -308,8 +308,6 @@ class Vst3Logger { void log_response(bool is_host_vst, const YaAudioProcessor::GetBusArrangementResponse&); - void log_response(bool is_host_vst, - const YaAudioProcessor::SetupProcessingResponse&); void log_response(bool is_host_vst, const YaAudioProcessor::ProcessResponse&); void log_response(bool is_host_vst, diff --git a/src/common/serialization/vst3/plugin/audio-processor.h b/src/common/serialization/vst3/plugin/audio-processor.h index a8a4e144..a0b6deb4 100644 --- a/src/common/serialization/vst3/plugin/audio-processor.h +++ b/src/common/serialization/vst3/plugin/audio-processor.h @@ -178,32 +178,12 @@ class YaAudioProcessor : public Steinberg::Vst::IAudioProcessor { virtual uint32 PLUGIN_API getLatencySamples() override = 0; - /** - * The response code and written state for a call to - * `IAudioProcessor::setupProcessing(setup)`. - */ - struct SetupProcessingResponse { - UniversalTResult result; - AudioShmBuffer::Config audio_buffers_config; - - template - void serialize(S& s) { - s.object(result); - s.object(audio_buffers_config); - } - }; - /** * Message to pass through a call to * `IAudioProcessor::setupProcessing(setup)` to the Wine plugin host. - * - * Here Wine plugin host will set up the shared memory buffers. - * - * NOTE: This process is repeated as part of `SetActive`. Apparently REAPER - * can change bus arrangements after the processing has been set up. */ struct SetupProcessing { - using Response = SetupProcessingResponse; + using Response = UniversalTResult; native_size_t instance_id; diff --git a/src/common/serialization/vst3/plugin/component.h b/src/common/serialization/vst3/plugin/component.h index 4314cbb0..7ea5b80a 100644 --- a/src/common/serialization/vst3/plugin/component.h +++ b/src/common/serialization/vst3/plugin/component.h @@ -256,7 +256,7 @@ class YaComponent : public Steinberg::Vst::IComponent { /** * The response code and written state for a call to - * `IAudioProcessor::setupProcessing(setup)`. + * `IAudioProcessor::setActive(state)`. */ struct SetActiveResponse { UniversalTResult result; @@ -276,7 +276,7 @@ class YaComponent : public Steinberg::Vst::IComponent { * * NOTE: REAPER may change a plugin's bus arrangements after the processing * has been set up, so we need to check for this on every - * `setActive()` call + * `setActive()` call. */ struct SetActive { using Response = SetActiveResponse; diff --git a/src/common/serialization/vst3/process-data.cpp b/src/common/serialization/vst3/process-data.cpp index d56aec1d..8cada8fd 100644 --- a/src/common/serialization/vst3/process-data.cpp +++ b/src/common/serialization/vst3/process-data.cpp @@ -142,7 +142,7 @@ Steinberg::Vst::ProcessData& YaProcessData::reconstruct( // The actual audio data is contained within a shared memory object, and the // input and output pointers point to regions in that object. These pointers - // are calculated while handling `IAudioProcessor::setupProcessing()`. + // are calculated while handling `IAudioProcessor::setActive()`. // NOTE: The 32-bit and 64-bit audio pointers are a union, and since this is // a raw memory buffer we can set either `channelBuffers32` or // `channelBuffers64` to point at that buffer as long as we do the diff --git a/src/common/serialization/vst3/process-data.h b/src/common/serialization/vst3/process-data.h index 87371e99..e82b294c 100644 --- a/src/common/serialization/vst3/process-data.h +++ b/src/common/serialization/vst3/process-data.h @@ -83,7 +83,7 @@ class YaProcessData { * difficult for us to mess this up, we'll store those bus-channel pointers * in `Vst3Bridge::InstanceInterfaces` and we'll point the pointers in our * `inputs` and `outputs` fields directly to those pointers. They will have - * been set up during `IAudioProcessor::setupProcessing()`. + * been set up during `IAudioProcessor::setActive()`. * * These can be either float or double pointers. Since a pointer is a * pointer and they're stored using a union the actual type doesn't matter, diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp index 4d41314c..ae5e2ec7 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp @@ -186,19 +186,9 @@ uint32 PLUGIN_API Vst3PluginProxyImpl::getLatencySamples() { tresult PLUGIN_API Vst3PluginProxyImpl::setupProcessing(Steinberg::Vst::ProcessSetup& setup) { - const YaAudioProcessor::SetupProcessingResponse response = - bridge_.send_audio_processor_message(YaAudioProcessor::SetupProcessing{ - .instance_id = instance_id(), .setup = setup}); - - // We have now set up the shared audio buffers on the Wine side, and we'll - // be able to able to connect to them by using the same audio configuration - if (!process_buffers_) { - process_buffers_.emplace(response.audio_buffers_config); - } else { - process_buffers_->resize(response.audio_buffers_config); - } - - return response.result; + return bridge_.send_audio_processor_message( + YaAudioProcessor::SetupProcessing{.instance_id = instance_id(), + .setup = setup}); } tresult PLUGIN_API Vst3PluginProxyImpl::setProcessing(TBool state) { @@ -427,13 +417,10 @@ tresult PLUGIN_API Vst3PluginProxyImpl::setActive(TBool state) { YaComponent::SetActive{.instance_id = instance_id(), .state = state}); // NOTE: REAPER may (and will) change a plugin's channel layout after - // calling `setupProcessing()`. Because of that, we need to test - // whether this has happened any time the plugin gets reactivated. - // It's technically legal, so we need to support it. + // calling `IAudioProcessor::setupProcessing()`. Because of that, we + // need to test whether this has happened any time the plugin gets + // reactivated. It's technically legal, so we need to support it. if (response.updated_audio_buffers_config) { - // The host should absolutely not call this function before - // `setupProcessing()` so this should already contain a value, but you - // never know... if (!process_buffers_) { process_buffers_.emplace(*response.updated_audio_buffers_config); } else { diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.h b/src/plugin/bridges/vst3-impls/plugin-proxy.h index bbcb03b6..e1f28204 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.h +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.h @@ -468,7 +468,7 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { * amount of copies required to only once for the input audio, and one more * copy when copying the results back to the host. * - * This will be set up during `IAudioProcessor::setupProcessing()`. + * This will be set up during `IAudioProcessor::setActive()`. */ std::optional process_buffers_; diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index 6efeb856..f78f6f1c 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -1588,23 +1588,15 @@ size_t Vst3Bridge::register_object_instance( const auto& [instance, _] = get_instance(request.instance_id); - const tresult result = - instance.interfaces.audio_processor - ->setupProcessing(request.setup); - // We'll set up the shared audio buffers on the Wine // side after the plugin has finished doing their setup. // This configuration can then be used on the native // plugin side to connect to the same shared audio // buffers. instance.process_setup = request.setup; - const AudioShmBuffer::Config audio_buffers_config = - *setup_shared_audio_buffers(request.instance_id); - return YaAudioProcessor::SetupProcessingResponse{ - .result = result, - .audio_buffers_config = - std::move(audio_buffers_config)}; + return instance.interfaces.audio_processor + ->setupProcessing(request.setup); }, [&](const YaAudioProcessor::SetProcessing& request) -> YaAudioProcessor::SetProcessing::Response { @@ -1773,9 +1765,9 @@ size_t Vst3Bridge::register_object_instance( // NOTE: REAPER may change the bus layout after // calling - // `IAudioProcessor::setupProcessing`. In - // that case we'll need to resize the - // shared memory buffers here. + // `IAudioProcessor::setupProcessing()`, + // so this place is the only safe place to + // setup the buffers const std::optional updated_audio_buffers_config = setup_shared_audio_buffers( diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index ae6be585..bae8cbb9 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -171,7 +171,7 @@ struct Vst3PluginInstance { * A shared memory object we'll write the input audio buffers to on the * native plugin side. We'll then let the plugin write its outputs here on * the Wine side. The buffer will be configured during - * `IAudioProcessor::setupProcessing()`. At that point we'll build the + * `IAudioProcessor::setActive()`. At that point we'll build the * configuration for the object here, on the Wine side, and then we'll * initialize the buffers using that configuration. This same configuration * is then used on the native plugin side to connect to this same shared