Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

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

Open
micahrj opened this issue Oct 17, 2021 · 1 comment
Open

Comments

@micahrj
Copy link
Member

micahrj commented Oct 17, 2021

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
Copy link
Contributor

askeksa commented Oct 23, 2021

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:

  • 1 and 2 both conflict with the goal of vst-rs to be a safe, non-allocating wrapper around the VST2 API.
  • 3 is the most flexible solution and the easiest one to port existing plugin code to. We can keep the split method but change its return type, and plugin code can just add get and set in appropriate places. The compiler optimizations that this prevents are exactly the ones that we don't want because they are unsound when the arrays are aliased.
  • 4 can be a convenient solution in some cases, for instance for plugins that have no inputs.

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants