From 069053ca507b2b7055b744656afd70182f78f100 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 28 Dec 2022 01:34:42 +0100 Subject: [PATCH] Also defer latency change for CLAP plugins In case a future host does the same thing Ardour does right now for VST3 plugins. --- src/wrapper/clap/context.rs | 26 ++++++++++++++++++++++++- src/wrapper/clap/wrapper.rs | 39 +++++++++++++++++++------------------ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/wrapper/clap/context.rs b/src/wrapper/clap/context.rs index e6b0a27d..893111bc 100644 --- a/src/wrapper/clap/context.rs +++ b/src/wrapper/clap/context.rs @@ -1,4 +1,5 @@ use atomic_refcell::AtomicRefMut; +use std::cell::Cell; use std::collections::VecDeque; use std::sync::Arc; @@ -13,8 +14,22 @@ use crate::params::internals::ParamPtr; use crate::plugin::ClapPlugin; /// An [`InitContext`] implementation for the wrapper. +/// +/// # Note +/// +/// See the VST3 `WrapperInitContext` for an explanation of why we need this `pending_requests` +/// field. pub(crate) struct WrapperInitContext<'a, P: ClapPlugin> { pub(super) wrapper: &'a Wrapper

, + pub(super) pending_requests: PendingInitContextRequests, +} + +/// Any requests that should be sent out when the [`WrapperInitContext`] is dropped. See that +/// struct's docstring for mroe information. +#[derive(Debug, Default)] +pub(crate) struct PendingInitContextRequests { + /// The value of the last `.set_latency_samples()` call. + latency_changed: Cell>, } /// A [`ProcessContext`] implementation for the wrapper. This is a separate object so it can hold on @@ -34,6 +49,14 @@ pub(crate) struct WrapperGuiContext { pub(super) wrapper: Arc>, } +impl Drop for WrapperInitContext<'_, P> { + fn drop(&mut self) { + if let Some(samples) = self.pending_requests.latency_changed.take() { + self.wrapper.set_latency_samples(samples) + } + } +} + impl InitContext

for WrapperInitContext<'_, P> { fn plugin_api(&self) -> PluginApi { PluginApi::Clap @@ -44,7 +67,8 @@ impl InitContext

for WrapperInitContext<'_, P> { } fn set_latency_samples(&self, samples: u32) { - self.wrapper.set_latency_samples(samples) + // See this struct's docstring + self.pending_requests.latency_changed.set(Some(samples)); } fn set_current_voice_capacity(&self, capacity: u32) { diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index c01b2088..bc431748 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -732,8 +732,15 @@ impl Wrapper

{ Arc::new(WrapperGuiContext { wrapper: self }) } + /// # Note + /// + /// The lock on the plugin must be dropped before this object is dropped to avoid deadlocks + /// caused by reentrant function calls. fn make_init_context(&self) -> WrapperInitContext<'_, P> { - WrapperInitContext { wrapper: self } + WrapperInitContext { + wrapper: self, + pending_requests: Default::default(), + } } fn make_process_context(&self, transport: Transport) -> WrapperProcessContext<'_, P> { @@ -1668,8 +1675,10 @@ impl Wrapper

{ self.notify_param_values_changed(); let bus_config = self.current_bus_config.load(); if let Some(buffer_config) = self.current_buffer_config.load() { + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = self.make_init_context(); let mut plugin = self.plugin.lock(); - plugin.initialize(&bus_config, &buffer_config, &mut self.make_init_context()); + plugin.initialize(&bus_config, &buffer_config, &mut init_context); process_wrapper(|| plugin.reset()); } @@ -1767,12 +1776,10 @@ impl Wrapper

{ param.update_smoother(buffer_config.sample_rate, true); } + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = wrapper.make_init_context(); let mut plugin = wrapper.plugin.lock(); - if plugin.initialize( - &bus_config, - &buffer_config, - &mut wrapper.make_init_context(), - ) { + if plugin.initialize(&bus_config, &buffer_config, &mut init_context) { // NOTE: `Plugin::reset()` is called in `clap_plugin::start_processing()` instead of in // this function @@ -2310,6 +2317,8 @@ impl Wrapper

{ wrapper.notify_param_values_changed(); + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = wrapper.make_init_context(); let bus_config = wrapper.current_bus_config.load(); let buffer_config = wrapper.current_buffer_config.load().unwrap(); let mut plugin = wrapper.plugin.lock(); @@ -2317,13 +2326,7 @@ impl Wrapper

{ // this could lead to inconsistencies. It's the plugin's responsibility to // not perform any realtime-unsafe work when the initialize function is // called a second time if it supports runtime preset loading. - permit_alloc(|| { - plugin.initialize( - &bus_config, - &buffer_config, - &mut wrapper.make_init_context(), - ) - }); + permit_alloc(|| plugin.initialize(&bus_config, &buffer_config, &mut init_context)); plugin.reset(); // We'll pass the state object back to the GUI thread so deallocation can happen @@ -3218,12 +3221,10 @@ impl Wrapper

{ let bus_config = wrapper.current_bus_config.load(); if let Some(buffer_config) = wrapper.current_buffer_config.load() { + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = wrapper.make_init_context(); let mut plugin = wrapper.plugin.lock(); - plugin.initialize( - &bus_config, - &buffer_config, - &mut wrapper.make_init_context(), - ); + plugin.initialize(&bus_config, &buffer_config, &mut init_context); // TODO: This also goes for the VST3 version, but should we call reset here? Won't the // host always restart playback? Check this with a couple of hosts and remove the // duplicate reset if it's not needed.