1
0
Fork 0

Redesign the wrapper's interiors for thread safety

There are a lot of locks needed now here, but none of them should be
contended. This is much better than potentially having RefCell's blow up
due to simultaneous mutable borrows, and the Arc is needed for the event
loop.
This commit is contained in:
Robbert van der Helm 2022-01-31 19:40:52 +01:00
parent 2f59adadcc
commit 4495064558
5 changed files with 266 additions and 48 deletions

208
Cargo.lock generated
View file

@ -8,11 +8,98 @@ version = "1.0.53"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "94a45b455c14666b85fc40a019e8ab9eb75e3a124e05494f5397122bc9eb06e0"
[[package]]
name = "autocfg"
version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a"
[[package]]
name = "bitflags"
version = "1.3.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a"
[[package]]
name = "cfg-if"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
[[package]]
name = "crossbeam"
version = "0.8.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4ae5588f6b3c3cb05239e90bd110f257254aecd01e4635400391aeae07497845"
dependencies = [
"cfg-if",
"crossbeam-channel",
"crossbeam-deque",
"crossbeam-epoch",
"crossbeam-queue",
"crossbeam-utils",
]
[[package]]
name = "crossbeam-channel"
version = "0.5.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e54ea8bc3fb1ee042f5aace6e3c6e025d3874866da222930f70ce62aceba0bfa"
dependencies = [
"cfg-if",
"crossbeam-utils",
]
[[package]]
name = "crossbeam-deque"
version = "0.8.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6455c0ca19f0d2fbf751b908d5c55c1f5cbc65e03c4225427254b46890bdde1e"
dependencies = [
"cfg-if",
"crossbeam-epoch",
"crossbeam-utils",
]
[[package]]
name = "crossbeam-epoch"
version = "0.9.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "97242a70df9b89a65d0b6df3c4bf5b9ce03c5b7309019777fbde37e7537f8762"
dependencies = [
"cfg-if",
"crossbeam-utils",
"lazy_static",
"memoffset",
"scopeguard",
]
[[package]]
name = "crossbeam-queue"
version = "0.3.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b979d76c9fcb84dffc80a73f7290da0f83e4c95773494674cb44b76d13a7a110"
dependencies = [
"cfg-if",
"crossbeam-utils",
]
[[package]]
name = "crossbeam-utils"
version = "0.8.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cfcae03edb34f947e64acdb1c33ec169824e20657e9ecb61cef6c8c74dcb8120"
dependencies = [
"cfg-if",
"lazy_static",
]
[[package]]
name = "gain"
version = "0.1.0"
dependencies = [
"nih_plug",
"parking_lot",
]
[[package]]
@ -27,12 +114,38 @@ version = "1.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
[[package]]
name = "libc"
version = "0.2.116"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "565dbd88872dbe4cc8a46e527f26483c1d1f7afa6b884a3bd6cd893d4f98da74"
[[package]]
name = "lock_api"
version = "0.4.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "88943dd7ef4a2e5a4bfa2753aaab3013e34ce2533d1996fb18ef591e315e2b3b"
dependencies = [
"scopeguard",
]
[[package]]
name = "memoffset"
version = "0.6.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5aa361d4faea93603064a027415f07bd8e1d5c88c9fbf68bf56a285428fd79ce"
dependencies = [
"autocfg",
]
[[package]]
name = "nih_plug"
version = "0.1.0"
dependencies = [
"crossbeam",
"lazy_static",
"nih_plug_derive",
"parking_lot",
"serde",
"serde_json",
"vst3-sys",
@ -47,6 +160,29 @@ dependencies = [
"syn",
]
[[package]]
name = "parking_lot"
version = "0.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "87f5ec2493a61ac0506c0f4199f99070cbe83857b0337006a30f3e6719b8ef58"
dependencies = [
"lock_api",
"parking_lot_core",
]
[[package]]
name = "parking_lot_core"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b2f4f894f3865f6c0e02810fc597300f34dc2510f66400da262d8ae10e75767d"
dependencies = [
"cfg-if",
"libc",
"redox_syscall",
"smallvec",
"windows-sys",
]
[[package]]
name = "proc-macro2"
version = "1.0.36"
@ -65,12 +201,27 @@ dependencies = [
"proc-macro2",
]
[[package]]
name = "redox_syscall"
version = "0.2.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8383f39639269cde97d255a32bdb68c047337295414940c68bdd30c2e13203ff"
dependencies = [
"bitflags",
]
[[package]]
name = "ryu"
version = "1.0.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "73b4b750c782965c211b42f022f59af1fbceabdd026623714f104152f1ec149f"
[[package]]
name = "scopeguard"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
[[package]]
name = "serde"
version = "1.0.136"
@ -102,6 +253,12 @@ dependencies = [
"serde",
]
[[package]]
name = "smallvec"
version = "1.8.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f2dd574626839106c320a323308629dcb1acfc96e32a8cba364ddc61ac23ee83"
[[package]]
name = "syn"
version = "1.0.86"
@ -122,7 +279,7 @@ checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3"
[[package]]
name = "vst3-com"
version = "0.1.0"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=feature/pointer-rewrite#5307e7763ed3fb37e67cd5ba9627d33b9bbb9426"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=fix/atomic-reference-count#a8aaaa842322570fa241456a45be2fd11c3597a9"
dependencies = [
"vst3-com-macros",
]
@ -130,7 +287,7 @@ dependencies = [
[[package]]
name = "vst3-com-macros"
version = "0.2.0"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=feature/pointer-rewrite#5307e7763ed3fb37e67cd5ba9627d33b9bbb9426"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=fix/atomic-reference-count#a8aaaa842322570fa241456a45be2fd11c3597a9"
dependencies = [
"proc-macro2",
"quote",
@ -141,7 +298,7 @@ dependencies = [
[[package]]
name = "vst3-com-macros-support"
version = "0.2.0"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=feature/pointer-rewrite#5307e7763ed3fb37e67cd5ba9627d33b9bbb9426"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=fix/atomic-reference-count#a8aaaa842322570fa241456a45be2fd11c3597a9"
dependencies = [
"proc-macro2",
"quote",
@ -151,7 +308,7 @@ dependencies = [
[[package]]
name = "vst3-sys"
version = "0.1.0"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=feature/pointer-rewrite#5307e7763ed3fb37e67cd5ba9627d33b9bbb9426"
source = "git+https://github.com/robbert-vdh/vst3-sys.git?branch=fix/atomic-reference-count#a8aaaa842322570fa241456a45be2fd11c3597a9"
dependencies = [
"vst3-com",
]
@ -162,6 +319,49 @@ version = "1.0.0-beta.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e6f1efe828a707edf85994a4501734ac1c1b9d244cfcf4de235f11c4125ace8f"
[[package]]
name = "windows-sys"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ceb069ac8b2117d36924190469735767f0990833935ab430155e71a44bafe148"
dependencies = [
"windows_aarch64_msvc",
"windows_i686_gnu",
"windows_i686_msvc",
"windows_x86_64_gnu",
"windows_x86_64_msvc",
]
[[package]]
name = "windows_aarch64_msvc"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c3d027175d00b01e0cbeb97d6ab6ebe03b12330a35786cbaca5252b1c4bf5d9b"
[[package]]
name = "windows_i686_gnu"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8793f59f7b8e8b01eda1a652b2697d87b93097198ae85f823b969ca5b89bba58"
[[package]]
name = "windows_i686_msvc"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8602f6c418b67024be2996c512f5f995de3ba417f4c75af68401ab8756796ae4"
[[package]]
name = "windows_x86_64_gnu"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f3d615f419543e0bd7d2b3323af0d86ff19cbc4f816e6453f36a2c2ce889c354"
[[package]]
name = "windows_x86_64_msvc"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "11d95421d9ed3672c280884da53201a5c46b7b2765ca6faf34b0d71cf34a3561"
[[package]]
name = "xtask"
version = "0.1.0"

View file

@ -10,6 +10,8 @@ members = ["nih_plug_derive", "plugins/gain", "xtask"]
[dependencies]
nih_plug_derive = { path = "nih_plug_derive" }
crossbeam = "0.8"
lazy_static = "1.4"
parking_lot = "0.12"
serde = { version = "1.0", features = ["derive"] }

View file

@ -45,7 +45,7 @@ struct GainParams {
impl Default for Gain {
fn default() -> Self {
Self {
params: Pin::new(Box::default()),
params: Box::pin(GainParams::default()),
}
}
}

View file

@ -102,7 +102,7 @@ pub trait Vst3Plugin: Plugin {
}
/// We only support a single main input and output bus at the moment.
#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct BusConfig {
/// The number of input channels for the plugin.
pub num_input_channels: u32,

View file

@ -18,15 +18,18 @@
// complain as soon as a struct has more than 8 fields
#![allow(clippy::too_many_arguments)]
use crossbeam::atomic::AtomicCell;
use lazy_static::lazy_static;
use std::cell::{Cell, RefCell};
use parking_lot::RwLock;
use std::cmp;
use std::collections::HashMap;
use std::ffi::c_void;
use std::marker::PhantomData;
use std::mem;
use std::pin::Pin;
use std::ptr;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use vst3_sys::base::{kInvalidArgument, kNoInterface, kResultFalse, kResultOk, tresult, TBool};
use vst3_sys::base::{IBStream, IPluginBase, IPluginFactory, IPluginFactory2, IPluginFactory3};
use vst3_sys::utils::SharedVstPtr;
@ -77,21 +80,23 @@ macro_rules! check_null_ptr_msg {
#[VST3(implements(IComponent, IEditController, IAudioProcessor))]
pub(crate) struct Wrapper<'a, P: Plugin> {
/// The wrapped plugin instance.
plugin: RefCell<P>,
plugin: Pin<Box<RwLock<P>>>,
/// The current bus configuration, modified through `IAudioProcessor::setBusArrangements()`.
current_bus_config: RefCell<BusConfig>,
current_bus_config: AtomicCell<BusConfig>,
/// Whether the plugin is currently bypassed. This is not yet integrated with the `Plugin`
/// trait.
bypass_state: Cell<bool>,
bypass_state: AtomicBool,
/// The last process status returned by the plugin. This is used for tail handling.
last_process_status: Cell<ProcessStatus>,
last_process_status: AtomicCell<ProcessStatus>,
/// Whether the plugin is currently processing audio. In other words, the last state
/// `IAudioProcessor::setActive()` has been called with.
is_processing: AtomicBool,
/// Contains slices for the plugin's outputs. You can't directly create a nested slice form
/// apointer to pointers, so this needs to be preallocated in the setup call and kept around
/// between process calls.
output_slices: RefCell<Vec<&'a mut [f32]>>,
output_slices: RwLock<Vec<&'a mut [f32]>>,
/// A mapping from parameter ID hashes (obtained from the string parameter IDs) to pointers to
/// parameters belonging to the plugin. As long as `plugin` does not get recreated, these
@ -109,33 +114,42 @@ pub(crate) struct Wrapper<'a, P: Plugin> {
param_hash_to_id: HashMap<&'static str, u32>,
}
// FIXME: This is far, far, far from ideal. The problem is that the `VST` attribute macro adds
// VTable pointers to the struct (that are stored separately by leaking `Box<T>`s), and we
// can't mark those as safe ourselves.
unsafe impl<P: Plugin> Send for Wrapper<'_, P> {}
unsafe impl<P: Plugin> Sync for Wrapper<'_, P> {}
impl<P: Plugin> Wrapper<'_, P> {
pub fn new() -> Box<Self> {
pub fn new() -> Arc<Self> {
let mut wrapper = Self::allocate(
RefCell::new(P::default()),
Box::pin(RwLock::default()), // plugin
// Some hosts, like the current version of Bitwig and Ardour at the time of writing,
// will try using the plugin's default not yet initialized bus arrangement. Because of
// that, we'll always initialize this configuration even before the host requests a
// channel layout.
RefCell::new(BusConfig {
AtomicCell::new(BusConfig {
num_input_channels: P::DEFAULT_NUM_INPUTS,
num_output_channels: P::DEFAULT_NUM_OUTPUTS,
}),
Cell::new(false), // bypass_state
Cell::new(ProcessStatus::Normal), // last_process_status
AtomicBool::new(false), // is_processing
RefCell::new(Vec::new()), // output_slices
HashMap::new(), // param_by_hash
Vec::new(), // param_hashes
Vec::new(), // param_defaults_normalized
HashMap::new(), // param_id_to_hash
HashMap::new(), // param_hash_to_id
AtomicBool::new(false), // bypass_state
AtomicCell::new(ProcessStatus::Normal), // last_process_status
AtomicBool::new(false), // is_processing
RwLock::new(Vec::new()), // output_slices
HashMap::new(), // param_by_hash
Vec::new(), // param_hashes
Vec::new(), // param_defaults_normalized
HashMap::new(), // param_id_to_hash
HashMap::new(), // param_hash_to_id
);
// This is a mapping from the parameter IDs specified by the plugin to pointers to thsoe
// parameters. Since the object returned by `params()` is pinned, these pointers are safe to
// dereference as long as `wrapper.plugin` is alive
let param_map = wrapper.plugin.borrow().params().param_map();
// XXX: This unsafe block is unnecessary. rust-analyzer gets a bit confused and this this
// `read()` function is from `IBStream` which it definitely is not.
#[allow(unused_unsafe)]
let param_map = unsafe { wrapper.plugin.read() }.params().param_map();
nih_debug_assert!(
!param_map.contains_key(BYPASS_PARAM_ID),
"The wrapper alread yadds its own bypass parameter"
@ -161,12 +175,14 @@ impl<P: Plugin> Wrapper<'_, P> {
.map(|(&hash, &id)| (id, hash))
.collect();
let wrapper: Arc<Wrapper<'_, P>> = wrapper.into();
wrapper
}
unsafe fn set_normalized_value_by_hash(&self, hash: u32, normalized_value: f64) -> tresult {
if hash == *BYPASS_PARAM_HASH {
self.bypass_state.set(normalized_value >= 0.5);
self.bypass_state
.store(normalized_value >= 0.5, Ordering::SeqCst);
kResultOk
} else if let Some(param_ptr) = self.param_by_hash.get(&hash) {
@ -234,7 +250,7 @@ impl<P: Plugin> IComponent for Wrapper<'_, P> {
(d, 0) if d == vst3_sys::vst::BusDirections::kInput as i32 => {
info.direction = vst3_sys::vst::BusDirections::kInput as i32;
info.channel_count =
self.current_bus_config.borrow().num_input_channels as i32;
self.current_bus_config.load().num_input_channels as i32;
u16strlcpy(&mut info.name, "Input");
kResultOk
@ -242,7 +258,7 @@ impl<P: Plugin> IComponent for Wrapper<'_, P> {
(d, 0) if d == vst3_sys::vst::BusDirections::kOutput as i32 => {
info.direction = vst3_sys::vst::BusDirections::kOutput as i32;
info.channel_count =
self.current_bus_config.borrow().num_output_channels as i32;
self.current_bus_config.load().num_output_channels as i32;
u16strlcpy(&mut info.name, "Output");
kResultOk
@ -344,7 +360,7 @@ impl<P: Plugin> IComponent for Wrapper<'_, P> {
// Handle the bypass parameter separately
if param_id_str == BYPASS_PARAM_ID {
match param_value {
ParamValue::Bool(b) => self.bypass_state.set(b),
ParamValue::Bool(b) => self.bypass_state.store(b, Ordering::SeqCst),
_ => nih_debug_assert_failure!(
"Invalid serialized value {:?} for parameter {} ({:?})",
param_value,
@ -384,7 +400,7 @@ impl<P: Plugin> IComponent for Wrapper<'_, P> {
// The plugin can also persist arbitrary fields alongside its parameters. This is useful for
// storing things like sample data.
self.plugin
.borrow()
.read()
.params()
.deserialize_fields(&state.fields);
@ -414,12 +430,12 @@ impl<P: Plugin> IComponent for Wrapper<'_, P> {
// Don't forget about the bypass parameter
params.insert(
BYPASS_PARAM_ID.to_string(),
ParamValue::Bool(self.bypass_state.get()),
ParamValue::Bool(self.bypass_state.load(Ordering::SeqCst)),
);
// The plugin can also persist arbitrary fields alongside its parameters. This is useful for
// storing things like sample data.
let fields = self.plugin.borrow().params().serialize_fields();
let fields = self.plugin.read().params().serialize_fields();
let plugin_state = State { params, fields };
match serde_json::to_vec(&plugin_state) {
@ -605,7 +621,7 @@ impl<P: Plugin> IEditController for Wrapper<'_, P> {
unsafe fn get_param_normalized(&self, id: u32) -> f64 {
if id == *BYPASS_PARAM_HASH {
if self.bypass_state.get() {
if self.bypass_state.load(Ordering::SeqCst) {
1.0
} else {
0.0
@ -662,8 +678,8 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
num_input_channels: input_channel_map.count_ones(),
num_output_channels: output_channel_map.count_ones(),
};
if self.plugin.borrow().accepts_bus_config(&proposed_config) {
self.current_bus_config.replace(proposed_config);
if self.plugin.read().accepts_bus_config(&proposed_config) {
self.current_bus_config.store(proposed_config);
kResultOk
} else {
@ -696,7 +712,7 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
}
};
let config = self.current_bus_config.borrow();
let config = self.current_bus_config.load();
let num_channels = match (dir, index) {
(d, 0) if d == vst3_sys::vst::BusDirections::kInput as i32 => config.num_input_channels,
(d, 0) if d == vst3_sys::vst::BusDirections::kOutput as i32 => {
@ -735,21 +751,17 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
vst3_sys::vst::SymbolicSampleSizes::kSample32 as i32
);
let bus_config = self.current_bus_config.borrow();
let bus_config = self.current_bus_config.load();
let buffer_config = BufferConfig {
sample_rate: setup.sample_rate as f32,
max_buffer_size: setup.max_samples_per_block as u32,
};
if self
.plugin
.borrow_mut()
.initialize(&bus_config, &buffer_config)
{
if self.plugin.write().initialize(&bus_config, &buffer_config) {
// Preallocate enough room in the output slices vector so we can convert a `*mut *mut
// f32` to a `&mut [&mut f32]` in the process call
self.output_slices
.borrow_mut()
.write()
.resize_with(bus_config.num_output_channels as usize, || &mut []);
kResultOk
@ -760,7 +772,7 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
unsafe fn set_processing(&self, state: TBool) -> tresult {
// Always reset the processing status when the plugin gets activated or deactivated
self.last_process_status.set(ProcessStatus::Normal);
self.last_process_status.store(ProcessStatus::Normal);
self.is_processing.store(state != 0, Ordering::SeqCst);
// We don't have any special handling for suspending and resuming plugins, yet
@ -822,7 +834,7 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
nih_debug_assert!(data.num_samples >= 0);
// This vector has been reallocated to contain enough slices as there are output channels
let mut output_slices = self.output_slices.borrow_mut();
let mut output_slices = self.output_slices.write();
check_null_ptr_msg!(
"Process output pointer is null",
data.outputs,
@ -862,7 +874,7 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
}
}
match self.plugin.borrow_mut().process(&mut output_slices) {
match self.plugin.write().process(&mut output_slices) {
ProcessStatus::Error(err) => {
nih_debug_assert_failure!("Process error: {}", err);
@ -874,7 +886,7 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
unsafe fn get_tail_samples(&self) -> u32 {
// https://github.com/steinbergmedia/vst3_pluginterfaces/blob/2ad397ade5b51007860bedb3b01b8afd2c5f6fba/vst/ivstaudioprocessor.h#L145-L159
match self.last_process_status.get() {
match self.last_process_status.load() {
ProcessStatus::Tail(samples) => samples,
ProcessStatus::KeepAlive => u32::MAX, // kInfiniteTail
_ => 0, // kNoTail
@ -951,7 +963,11 @@ impl<P: Vst3Plugin> IPluginFactory for Factory<P> {
return kNoInterface;
}
*obj = Box::into_raw(Wrapper::<P>::new()) as *mut vst3_sys::c_void;
// XXX: Right now this `Arc` mostly serves to have clearer, safer interactions between the
// wrapper, the plugin, and the other behind the scene bits. The wrapper will still get
// freed using `FUnknown`'s internal reference count when the host drops it, so we
// actually need to leak this Arc here so it always stays at 1 reference or higher.
*obj = Arc::into_raw(Wrapper::<P>::new()) as *mut vst3_sys::c_void;
kResultOk
}