RustAudio / vst-rs

VST 2.4 API implementation in rust. Create plugins or hosts. Previously rust-vst on the RustDSP group.
MIT License
1.04k stars 91 forks source link

`AudioBuffer` API is unsound when input and output buffers are aliased #172

Open glowcoil opened 3 years ago

glowcoil commented 3 years ago

In VST2, it is possible and even common for hosts to pass the same pointer for both an input and output buffer (source). This means that the API of AudioBuffer, which allows mutably borrowing any output buffer at the same time as any input buffer, is unsound.

There are several possible solutions to this, each with downsides:

  1. Have vst-rs keep scratch buffers allocated for copying aliased buffers into. Has the advantage of not changing the public API, but introduces complexity and overhead. vst-rs would have to allocate the worst-case amount of output channels × maximum block size, reallocate when maximum block size changes, and possibly split calls to process() if a host lies about the maximum block size. I don't really like this solution since I think it involves vst-rs doing too much.
  2. Give up on safe bindings and expose the input and output buffers as raw pointers.
  3. Expose input and output buffers as &[Cell<f32>] (like the lv2 crate currently does). This fixes the soundness issue, but it's somewhat cumbersome and limiting as an API (and may end up inhibiting compiler optimizations).
  4. Only allow either inputs or outputs to be borrowed at any given time (i.e. get rid of the split method and replace it with separate inputs and outputs methods). This makes it more cumbersome to write toy examples that process sample-by-sample directly from input buffers into output buffers, but it actually shouldn't cause problems for any plugin that does even a single layer of intermediate buffering/block-based processing between its input and outputs.

The fourth option is my preferred solution. It should also be possible to provide the second and third options as fallback alternatives for when the fourth one is too cumbersome to deal with.

askeksa commented 3 years ago

Great catch, and thanks for the analysis! Indeed a C API with "two arrays that might be aliased" doesn't go well with Rust's way of doing things.

Here's my take on the options:

My suggestion would be to have both 3 and 4. We would need to duplicate the Inputs and Outputs structs into shared and exclusive variants (maybe keep them as they are for the exclusive ones and add a single shared one which can be the same for inputs and outputs). Both split and zip would provide the shared variant, and we can add methods to access the exclusive ones.