1
0
Fork 0

Make the buffer own the output slices

This gets rid of a lot of lifetime casting and other unsoundness.
This commit is contained in:
Robbert van der Helm 2022-02-02 14:39:21 +01:00
parent fbadfe3e12
commit 048d69213e
4 changed files with 58 additions and 52 deletions

View file

@ -118,11 +118,7 @@ impl Plugin for Gain {
true true
} }
fn process<'samples>( fn process(&mut self, buffer: &mut Buffer, _context: &dyn ProcessContext) -> ProcessStatus {
&mut self,
buffer: &'samples mut Buffer<'_, 'samples>,
_context: &dyn ProcessContext,
) -> ProcessStatus {
// TODO: The wrapper should set FTZ if not yet enabled, mention ths in the process fuctnion // TODO: The wrapper should set FTZ if not yet enabled, mention ths in the process fuctnion
for samples in buffer.iter_mut() { for samples in buffer.iter_mut() {
for sample in samples { for sample in samples {

View file

@ -118,16 +118,12 @@ impl Plugin for Sine {
true true
} }
fn process<'samples>( fn process(&mut self, buffer: &mut Buffer, _context: &dyn ProcessContext) -> ProcessStatus {
&mut self,
buffer: &'samples mut Buffer<'_, 'samples>,
_context: &dyn ProcessContext,
) -> ProcessStatus {
let phase_delta = self.params.frequency.value / self.sample_rate; let phase_delta = self.params.frequency.value / self.sample_rate;
for samples in buffer.iter_mut() { for samples in buffer.iter_mut() {
let sine = (self.phase * consts::TAU).sin(); let sine = (self.phase * consts::TAU).sin();
self.phase = self.phase + phase_delta; self.phase += phase_delta;
if self.phase >= 1.0 { if self.phase >= 1.0 {
self.phase -= 1.0; self.phase -= 1.0;
} }

View file

@ -92,11 +92,7 @@ pub trait Plugin: Default + Send + Sync {
/// assymetric /// assymetric
/// TODO: Handle FTZ stuff on the wrapper side and mention that this has been handled /// TODO: Handle FTZ stuff on the wrapper side and mention that this has been handled
/// TODO: Pass transport and other context information to the plugin /// TODO: Pass transport and other context information to the plugin
fn process<'samples>( fn process(&mut self, buffer: &mut Buffer, context: &dyn ProcessContext) -> ProcessStatus;
&mut self,
buffer: &'samples mut Buffer<'_, 'samples>,
context: &dyn ProcessContext,
) -> ProcessStatus;
} }
/// Provides auxiliary metadata needed for a VST3 plugin. /// Provides auxiliary metadata needed for a VST3 plugin.
@ -149,42 +145,55 @@ pub enum ProcessStatus {
/// inputs already copied to the outputs. You can either use the iterator adapters to conveniently /// inputs already copied to the outputs. You can either use the iterator adapters to conveniently
/// and efficiently iterate over the samples, or you can do your own thing using the raw audio /// and efficiently iterate over the samples, or you can do your own thing using the raw audio
/// buffers. /// buffers.
pub struct Buffer<'outer, 'inner> { pub struct Buffer<'a> {
/// The raw output buffers. /// Contains slices for the plugin's outputs. You can't directly create a nested slice form
raw: &'outer mut [&'inner mut [f32]], /// apointer to pointers, so this needs to be preallocated in the setup call and kept around
/// between process calls. And because storing a reference to this means a) that you need a lot
/// of lifetime annotations everywhere and b) that at some point you need unsound lifetime casts
/// because this `Buffers` either cannot have the same lifetime as the separately stored output
/// buffers, and it also cannot be stored in a field next to it because that would mean
/// containing mutable references to data stored in a mutex.
output_slices: Vec<&'a mut [f32]>,
} }
impl<'outer, 'inner> Buffer<'outer, 'inner> { impl<'a> Buffer<'a> {
/// Construct a new buffer adapter based on a set of audio buffers. /// Construct a new buffer adapter based on a set of audio buffers.
/// pub fn new() -> Self {
/// # Safety Self {
/// output_slices: Vec::new(),
/// The rest of this object assumes all channel lengths are equal. Panics will likely occur if }
/// this is not the case.
pub unsafe fn unchecked_new(buffers: &'outer mut [&'inner mut [f32]]) -> Self {
Self { raw: buffers }
} }
/// Returns true if this buffer does not contain any samples. /// Returns true if this buffer does not contain any samples.
///
/// TODO: Right now we guarantee that empty buffers never make it to the process function,
/// should we keep this?
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.raw.is_empty() || self.raw[0].is_empty() self.output_slices.is_empty() || self.output_slices[0].is_empty()
} }
/// Obtain the raw audio buffers. /// Obtain the raw audio buffers.
pub fn as_raw(&'outer mut self) -> &'outer mut [&'inner mut [f32]] { pub fn as_raw(&mut self) -> &mut [&'a mut [f32]] {
self.raw &mut self.output_slices
} }
/// Iterate over the samples, returning a channel iterator for each sample. /// Iterate over the samples, returning a channel iterator for each sample.
pub fn iter_mut(&'outer mut self) -> Samples<'outer, 'inner> { pub fn iter_mut(&mut self) -> Samples<'_, 'a> {
Samples { Samples {
buffers: &mut self.raw, buffers: &mut self.output_slices,
current_sample: 0, current_sample: 0,
} }
} }
/// Access the raw output slice vector. This neds to be resized to match the number of output
/// channels during the plugin's initialization. Then during audio processing, these slices
/// should be updated to point to the plugin's audio buffers.
///
/// # Safety
///
/// The stored slices must point to live data when this object is passed to the plugins' process
/// function. The rest of this object also assumes all channel lengths are equal. Panics will
/// likely occur if this is not the case.
pub unsafe fn as_raw_vec(&mut self) -> &mut Vec<&'a mut [f32]> {
&mut self.output_slices
}
} }
/// An iterator over all samples in the buffer, yielding iterators over each channel for every /// An iterator over all samples in the buffer, yielding iterators over each channel for every

View file

@ -114,8 +114,9 @@ struct WrapperInner<'a, P: Plugin> {
current_latency: AtomicU32, current_latency: AtomicU32,
/// Contains slices for the plugin's outputs. You can't directly create a nested slice form /// 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 /// apointer to pointers, so this needs to be preallocated in the setup call and kept around
/// between process calls. /// between process calls. This buffer owns the vector, because otherwise it would need to store
output_slices: RwLock<Vec<&'a mut [f32]>>, /// a mutable reference to the data contained in this mutex.
output_buffer: RwLock<Buffer<'a>>,
/// The keys from `param_map` in a stable order. /// The keys from `param_map` in a stable order.
param_hashes: Vec<u32>, param_hashes: Vec<u32>,
@ -195,7 +196,7 @@ impl<P: Plugin> WrapperInner<'_, P> {
bypass_state: AtomicBool::new(false), bypass_state: AtomicBool::new(false),
last_process_status: AtomicCell::new(ProcessStatus::Normal), last_process_status: AtomicCell::new(ProcessStatus::Normal),
current_latency: AtomicU32::new(0), current_latency: AtomicU32::new(0),
output_slices: RwLock::new(Vec::new()), output_buffer: RwLock::new(Buffer::new()),
param_hashes: Vec::new(), param_hashes: Vec::new(),
param_by_hash: HashMap::new(), param_by_hash: HashMap::new(),
@ -887,8 +888,9 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
// Preallocate enough room in the output slices vector so we can convert a `*mut *mut // 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 // f32` to a `&mut [&mut f32]` in the process call
self.inner self.inner
.output_slices .output_buffer
.write() .write()
.as_raw_vec()
.resize_with(bus_config.num_output_channels as usize, || &mut []); .resize_with(bus_config.num_output_channels as usize, || &mut []);
kResultOk kResultOk
@ -960,21 +962,27 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
); );
nih_debug_assert!(data.num_samples >= 0); nih_debug_assert!(data.num_samples >= 0);
// This vector has been reallocated to contain enough slices as there are output channels let num_output_channels = (*data.outputs).num_channels as usize;
let mut output_slices = self.inner.output_slices.write();
check_null_ptr_msg!( check_null_ptr_msg!(
"Process output pointer is null", "Process output pointer is null",
data.outputs, data.outputs,
(*data.outputs).buffers, (*data.outputs).buffers,
); );
let num_output_channels = (*data.outputs).num_channels as usize; // This vector has been reallocated to contain enough slices as there are output channels
nih_debug_assert_eq!(num_output_channels, output_slices.len()); let mut output_buffer = self.inner.output_buffer.write();
for (output_channel_idx, output_channel_slice) in output_slices.iter_mut().enumerate() { {
*output_channel_slice = std::slice::from_raw_parts_mut( let output_slices = output_buffer.as_raw_vec();
*((*data.outputs).buffers as *mut *mut f32).add(output_channel_idx), nih_debug_assert_eq!(num_output_channels, output_slices.len());
data.num_samples as usize, for (output_channel_idx, output_channel_slice) in output_slices.iter_mut().enumerate() {
); // SAFETY: These pointers may not be valid outside of this function even though
// their lifetime is equal to this structs. This is still safe because they are only
// dereferenced here later as part of this process function.
*output_channel_slice = std::slice::from_raw_parts_mut(
*((*data.outputs).buffers as *mut *mut f32).add(output_channel_idx),
data.num_samples as usize,
);
}
} }
// Most hosts process data in place, in which case we don't need to do any copying // Most hosts process data in place, in which case we don't need to do any copying
@ -1001,15 +1009,12 @@ impl<P: Plugin> IAudioProcessor for Wrapper<'_, P> {
} }
} }
// SAFETY: This borrow never outlives this process function, but this is still super unsafe
// TODO: Try to rewrite this so the buffer owns the slices and is stored in the RwLock
let mut buffer = Buffer::unchecked_new(&mut *(output_slices.as_mut() as *mut _));
match self match self
.inner .inner
.plugin .plugin
.write() .write()
// SAFETY: Same here // SAFETY: Same here
.process(&mut *(&mut buffer as *mut _), self.inner.as_ref()) .process(&mut output_buffer, self.inner.as_ref())
{ {
ProcessStatus::Error(err) => { ProcessStatus::Error(err) => {
nih_debug_assert_failure!("Process error: {}", err); nih_debug_assert_failure!("Process error: {}", err);