From f9bdaffc625cbb5cbf56c14b995c4c9d356dff09 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 20 Oct 2022 16:12:46 +0200 Subject: [PATCH] Don't require Sync for Plugin In hindsight there's really no reason to, because process() requires exclusive access and the other functions won't be called during processing. --- BREAKING_CHANGES.md | 1 + src/plugin.rs | 2 +- src/wrapper/clap/wrapper.rs | 22 +++++++++++----------- src/wrapper/standalone/wrapper.rs | 12 ++++++------ src/wrapper/vst3/inner.rs | 6 +++--- src/wrapper/vst3/wrapper.rs | 14 +++++++------- 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/BREAKING_CHANGES.md b/BREAKING_CHANGES.md index c0331f25..11dd0ffd 100644 --- a/BREAKING_CHANGES.md +++ b/BREAKING_CHANGES.md @@ -8,6 +8,7 @@ code then it will not be listed here. ## [2022-10-20] +- Similar to the below change, `Plugin` also no longer requires `Sync`. - `Editor` and the editor handle returned by `Editor::spawn` now only require `Send` and no longer need `Sync`. This is not a breaking change, but it might be worth being aware of. diff --git a/src/plugin.rs b/src/plugin.rs index eba6c1ce..750853b3 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -24,7 +24,7 @@ use crate::wrapper::state::PluginState; /// supported /// - Audio thread thread pools (with host integration in CLAP) #[allow(unused_variables)] -pub trait Plugin: Default + Send + Sync + 'static { +pub trait Plugin: Default + Send + 'static { const NAME: &'static str; const VENDOR: &'static str; const URL: &'static str; diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index 33c0fc8c..ba7dc434 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -59,7 +59,7 @@ use clap_sys::stream::{clap_istream, clap_ostream}; use crossbeam::atomic::AtomicCell; use crossbeam::channel::{self, SendTimeoutError}; use crossbeam::queue::ArrayQueue; -use parking_lot::{Mutex, RwLock}; +use parking_lot::Mutex; use raw_window_handle::RawWindowHandle; use std::any::Any; use std::cmp; @@ -104,7 +104,7 @@ pub struct Wrapper { this: AtomicRefCell>, /// The wrapped plugin instance. - plugin: RwLock

, + plugin: Mutex

, /// The plugin's parameters. These are fetched once during initialization. That way the /// `ParamPtr`s are guaranteed to live at least as long as this object and we can interact with /// the `Params` object without having to acquire a lock on `plugin`. @@ -541,7 +541,7 @@ impl Wrapper

{ this: AtomicRefCell::new(Weak::new()), - plugin: RwLock::new(plugin), + plugin: Mutex::new(plugin), params, editor, editor_handle: Mutex::new(None), @@ -1607,7 +1607,7 @@ 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() { - let mut plugin = self.plugin.write(); + let mut plugin = self.plugin.lock(); plugin.initialize(&bus_config, &buffer_config, &mut self.make_init_context()); process_wrapper(|| plugin.reset()); } @@ -1705,7 +1705,7 @@ impl Wrapper

{ param.update_smoother(buffer_config.sample_rate, true); } - let mut plugin = wrapper.plugin.write(); + let mut plugin = wrapper.plugin.lock(); if plugin.initialize( &bus_config, &buffer_config, @@ -1779,7 +1779,7 @@ impl Wrapper

{ check_null_ptr!((), plugin); let wrapper = &*(plugin as *const Self); - wrapper.plugin.write().deactivate(); + wrapper.plugin.lock().deactivate(); } unsafe extern "C" fn start_processing(plugin: *const clap_plugin) -> bool { @@ -1794,7 +1794,7 @@ impl Wrapper

{ // To be consistent with the VST3 wrapper, we'll also reset the buffers here in addition to // the dedicated `reset()` function. - process_wrapper(|| wrapper.plugin.write().reset()); + process_wrapper(|| wrapper.plugin.lock().reset()); true } @@ -1810,7 +1810,7 @@ impl Wrapper

{ check_null_ptr!((), plugin); let wrapper = &*(plugin as *const Self); - process_wrapper(|| wrapper.plugin.write().reset()); + process_wrapper(|| wrapper.plugin.lock().reset()); } unsafe extern "C" fn process( @@ -2189,7 +2189,7 @@ impl Wrapper

{ } let result = if buffer_is_valid { - let mut plugin = wrapper.plugin.write(); + let mut plugin = wrapper.plugin.lock(); // SAFETY: Shortening these borrows is safe as even if the plugin overwrites the // slices (which it cannot do without using unsafe code), then they // would still be reset on the next iteration @@ -2251,7 +2251,7 @@ impl Wrapper

{ let bus_config = wrapper.current_bus_config.load(); let buffer_config = wrapper.current_buffer_config.load().unwrap(); - let mut plugin = wrapper.plugin.write(); + let mut plugin = wrapper.plugin.lock(); // FIXME: This is obviously not realtime-safe, but loading presets without doing // this could lead to inconsistencies. It's the plugin's responsibility to // not perform any realtime-unsafe work when the initialize function is @@ -3154,7 +3154,7 @@ impl Wrapper

{ let bus_config = wrapper.current_bus_config.load(); if let Some(buffer_config) = wrapper.current_buffer_config.load() { - let mut plugin = wrapper.plugin.write(); + let mut plugin = wrapper.plugin.lock(); plugin.initialize( &bus_config, &buffer_config, diff --git a/src/wrapper/standalone/wrapper.rs b/src/wrapper/standalone/wrapper.rs index 11552431..0c86356c 100644 --- a/src/wrapper/standalone/wrapper.rs +++ b/src/wrapper/standalone/wrapper.rs @@ -2,7 +2,7 @@ use atomic_refcell::AtomicRefCell; use baseview::{EventStatus, Window, WindowHandler, WindowOpenOptions}; use crossbeam::channel; use crossbeam::queue::ArrayQueue; -use parking_lot::{Mutex, RwLock}; +use parking_lot::Mutex; use raw_window_handle::HasRawWindowHandle; use std::any::Any; use std::collections::{HashMap, HashSet}; @@ -33,7 +33,7 @@ pub struct Wrapper { backend: AtomicRefCell, /// The wrapped plugin instance. - plugin: RwLock

, + plugin: Mutex

, /// The plugin's parameters. These are fetched once during initialization. That way the /// `ParamPtr`s are guaranteed to live at least as long as this object and we can interact with /// the `Params` object without having to acquire a lock on `plugin`. @@ -176,7 +176,7 @@ impl Wrapper { let wrapper = Arc::new(Wrapper { backend: AtomicRefCell::new(backend), - plugin: RwLock::new(plugin), + plugin: Mutex::new(plugin), params, known_parameters: param_map.iter().map(|(_, ptr, _)| *ptr).collect(), param_map: param_map @@ -209,7 +209,7 @@ impl Wrapper { // Right now the IO configuration is fixed in the standalone target, so if the plugin cannot // work with this then we cannot initialize the plugin at all. { - let mut plugin = wrapper.plugin.write(); + let mut plugin = wrapper.plugin.lock(); if !plugin.accepts_bus_config(&wrapper.bus_config) { return Err(WrapperError::IncompatibleConfig { input_channels: wrapper.bus_config.num_input_channels, @@ -309,7 +309,7 @@ impl Wrapper { // Some plugins may use this to clean up resources. Should not be needed for the standalone // application, but it seems like a good idea to stay consistent. - self.plugin.write().deactivate(); + self.plugin.lock().deactivate(); Ok(()) } @@ -386,7 +386,7 @@ impl Wrapper { } let sample_rate = self.buffer_config.sample_rate; - let mut plugin = self.plugin.write(); + let mut plugin = self.plugin.lock(); if let ProcessStatus::Error(err) = plugin.process( buffer, // TODO: Provide extra inputs and outputs in the JACk backend diff --git a/src/wrapper/vst3/inner.rs b/src/wrapper/vst3/inner.rs index 728d9ff6..6d4e6998 100644 --- a/src/wrapper/vst3/inner.rs +++ b/src/wrapper/vst3/inner.rs @@ -29,7 +29,7 @@ use crate::wrapper::util::{hash_param_id, process_wrapper}; /// its own struct. pub(crate) struct WrapperInner { /// The wrapped plugin instance. - pub plugin: RwLock

, + pub plugin: Mutex

, /// The plugin's parameters. These are fetched once during initialization. That way the /// `ParamPtr`s are guaranteed to live at least as long as this object and we can interact with /// the `Params` object without having to acquire a lock on `plugin`. @@ -272,7 +272,7 @@ impl WrapperInner

{ .collect(); let wrapper = Self { - plugin: RwLock::new(plugin), + plugin: Mutex::new(plugin), params, editor, @@ -477,7 +477,7 @@ impl WrapperInner

{ self.notify_param_values_changed(); let bus_config = self.current_bus_config.load(); if let Some(buffer_config) = self.current_buffer_config.load() { - let mut plugin = self.plugin.write(); + let mut plugin = self.plugin.lock(); plugin.initialize(&bus_config, &buffer_config, &mut self.make_init_context()); process_wrapper(|| plugin.reset()); } diff --git a/src/wrapper/vst3/wrapper.rs b/src/wrapper/vst3/wrapper.rs index 7923fa14..be824675 100644 --- a/src/wrapper/vst3/wrapper.rs +++ b/src/wrapper/vst3/wrapper.rs @@ -389,7 +389,7 @@ impl IComponent for Wrapper

{ } let bus_config = self.inner.current_bus_config.load(); - let mut plugin = self.inner.plugin.write(); + let mut plugin = self.inner.plugin.lock(); if plugin.initialize( &bus_config, &buffer_config, @@ -461,7 +461,7 @@ impl IComponent for Wrapper

{ } (true, None) => kResultFalse, (false, _) => { - self.inner.plugin.write().deactivate(); + self.inner.plugin.lock().deactivate(); kResultOk } @@ -519,7 +519,7 @@ impl IComponent for Wrapper

{ let bus_config = self.inner.current_bus_config.load(); if let Some(buffer_config) = self.inner.current_buffer_config.load() { - let mut plugin = self.inner.plugin.write(); + let mut plugin = self.inner.plugin.lock(); plugin.initialize( &bus_config, &buffer_config, @@ -864,7 +864,7 @@ impl IAudioProcessor for Wrapper

{ if self .inner .plugin - .read() + .lock() .accepts_bus_config(&proposed_config) { self.inner.current_bus_config.store(proposed_config); @@ -1001,7 +1001,7 @@ impl IAudioProcessor for Wrapper

{ // This function is also used to reset buffers on the plugin, so we should do the same // thing. We don't call `reset()` in `setup_processing()` for that same reason. if state { - process_wrapper(|| self.inner.plugin.write().reset()); + process_wrapper(|| self.inner.plugin.lock().reset()); } // We don't have any special handling for suspending and resuming plugins, yet @@ -1530,7 +1530,7 @@ impl IAudioProcessor for Wrapper

{ } let result = if buffer_is_valid { - let mut plugin = self.inner.plugin.write(); + let mut plugin = self.inner.plugin.lock(); // SAFETY: Shortening these borrows is safe as even if the plugin overwrites the // slices (which it cannot do without using unsafe code), then they // would still be reset on the next iteration @@ -1784,7 +1784,7 @@ impl IAudioProcessor for Wrapper

{ let bus_config = self.inner.current_bus_config.load(); let buffer_config = self.inner.current_buffer_config.load().unwrap(); - let mut plugin = self.inner.plugin.write(); + let mut plugin = self.inner.plugin.lock(); // FIXME: This is obviously not realtime-safe, but loading presets without doing // this could lead to inconsistencies. It's the plugin's responsibility to // not perform any realtime-unsafe work when the initialize function is