Skip to content

Conversation

prokopyl
Copy link
Owner

@prokopyl prokopyl commented Nov 3, 2024

This PR is a rework of how audio buffers are exposed to plugin code, in order to fix possible unsoundness around
in-place processing.
This is mainly done with a new AudioBuffer type, which replaces the buffer slice types (e.g. &[f32]) that were
handed out up until now.

The main trade-off of this approach is that it is no longer possible to create references to the sample data
(or to slices of it).
This is because any of the samples may be changed by accessing another channel pointer in the case the host is using
in-place processing, i.e. different input and output channel pointers may point to the same buffer in memory.

This means it is not possible to access the buffers through e.g. &f32 / &mut f32, or &[f32] / &mut [f32].
Instead, the AudioBuffer type only allows to read, write, and copy from/to buffers given by the host.

However, this change allows for a slew of usability improvements, detailed below.

New AudioBuffer type

The new AudioBuffer<T> type represents a contiguous buffer of samples of type T (usually either f32 or f64).
It is always valid for both reads and writes, but unlike regular slices it provides interior mutability, i.e. its
contents can be mutated through shared references.

The AudioBuffer type is unsized, much like a regular slice, so you'll always only access it in the form of
e.g. &AudioBuffer<f32>.

References to it can be freely copied, shared, and sub-sliced, and still allow to mutate the buffer. This makes
implementing in-place processing quite trivial, as two AudioBuffer references can in fact point to the same buffer.

The AudioBuffer implements get and put method (among other things), that allow one to retrieve or set the sample
value at a given index.

Alternatively, indexing into or iterating over the buffer will yield &Cell<T> references to the wanted sample, allowing
similar operations.

In return however, this means it is impossible to get an actual reference to the sample data itself, be it through e.g.
&f32 or &[f32]. This is because both kinds of references (& and &mut) would be invalidated by the samples being
mutated from another AudioBuffer reference, leading to UB.

In fact, the AudioBuffer type behaves much like a shared reference to a slice of cells &[Cell<f32>] (because that's
what it essentially is), which makes it completely incompatible with functions that require either exclusive or immutable
access to the sample data (i.e., &[f32] and &mut [f32]), since those functions are essentially incompatible with
in-place processing.

If it is not feasible to convert those functions to work with AudioBuffers directly, the AudioBuffer type provides
additional utility methods to copy the samples to/from a regular slice, or between buffers.

It is also possible to freely convert between &AudioBuffer<T> and &[Cell<T>] back-and-forth at zero cost, if the
function does support in-place processing through Cells.

Renamed and merged Port types

Because both input and output ports now return the same AudioBuffer type to support in-place processing,
there is no longer a need to have them be distinct types.

Therefore, the InputPort and OutputPort types have been merged into a single Port type, as well as all the
types they return:

  • InputPortsIter and OutputPortsIter have been merged into PortIter;
  • InputChannels and OutputChannels have been merged into Channels;
  • InputChannelsIter and OutputChannelsIter iter have been merged into ChannelsIter;

For more clarity and consistency, the PairedChannels type has been renamed into ChannelsPairs.

All further references to these types in the rest of this changelog will use the new names instead of the old ones.

Changes to return AudioBuffer

The following methods and types have been changed to produce AudioBuffers instead of slices:

  • Channels::channel now returns an AudioBuffer reference;
  • ChannelsIter (Channels' iterator) now iterates over AudioBuffers:
  • ChannelPair::input and ChannelPair::output return AudioBuffer references;
  • The ChannelPair enum itself contains AudioBuffer references instead of slice references;

Audio now only produces shared references

Because AudioBuffers' contents can now be mutated through shared (&) references, exclusive (&mut) references do
not bring any useful benefit or guarantee anymore, and have been removed from all plugin-side Audio APIs.

This allows for a bunch of usability improvements and API simplifications:

  • Audio now implements Copy and Clone. This is also the case for all the types it produces, namely:
    • Port and Channels;
    • PortPair, ChannelsPairs, and ChannelPair;
    • SampleType if its contents are also Copy + Clone (which is the case in all of Clack's APIs);
    • Additionally, the PortsIter, ChannelsIter, PortPairsIter, and ChannelsPairsIter iterator types implement Clone (but not Copy).
  • IntoIterator is now implemented onto &Audio and Audio directly, instead of requiring &mut audio.
  • SampleType's helper methods have been overhauled. Since SampleType can now be Copy and does not deal with
    mutable references anymore, all of the as/into[_mut] methods have been removed, and replaced with simple to_f32
    and to_f64 methods that produces copies of the pointers SampleType holds.
  • The PortPair::latencies and PortPair::constant_masks helper methods have been removed. Their purpose was to allow
    recovering that information without mutably borrow the output (which also blocked borrowing the input), but this is
    not needed anymore, now that both the input and output can be borrowed simultaneously.

clap_audio_buffer mutability through shared references

Now Audio and its produced types (e.g. Port and PortPair) all operate through shared references, but the plugin
still needs to write the constant_mask field in the underlying clap_audio_buffers to send it back to the host.

This means Audio, Port and PortPair now provide interior mutability. This does not change anything for those who
only use Clack's safe APIs, but this brings an extra consideration for unsafe code: creating a reference (shared or
mutable) to the same underlying clap_audio_buffer that Audio, Port and PortPair wrap, will most likely lead to
Undefined Behavior, as these types can mutate the clap_audio_buffer.

Users that need to access the clap_audio_buffer must therefore NOT create references to it, and instead use
ptr::read and ptr::write instead of borrowing it, or use Cell or UnsafeCell for at least the constant_mask
field (the other fields are read-only as far as Clack is concerned).

In order to help with this, a few of the raw/unsafe APIs have been changed:

  • The Audio::from_raw_buffers method now takes raw pointers to the buffer slices, instead of references;
  • Removed the Audio::raw_buffers, Audio::to_raw_buffers, Audio::raw_input_buffers and Audio::raw_output_buffers methods;
    • Two new methods, Audio::raw_inputs and Audio::raw_outputs have been added instead, which only return raw pointers
      and do not consume or mutably borrow the Audio struct.
  • Added a new AudioPortProcessingInfo::from_raw_ptr method, that reads the metadata part of the clap_audio_buffer without borrowing it.

Other usability improvements

  • AudioPortProcessingInfo is now a plain-old struct: all its field are now public, and the getter/setter methods have been removed.
  • Audio::from_raw: the returned lifetime can now be arbitrary instead of being tied to the reference to the clap_process struct.
  • All methods that produce types bound by lifetimes are now tied to the lifetimes of the actual buffers, instead of the lifetime to the parent struct. Namely:
    • Audio::input_port, Audio::output_port, Audio::input_ports, Audio::output_ports;
    • Audio::port_sub_range;
    • Port::channels and Port::channel;
    • PortPair::input, PortPair::output, and PortPair::channels;
    • ChannelsPairs::input_channels, ChannelsPairs::output_channels and ChannelsPairs::channel_pair;
    • The iterator types for all the aforementioned port and channel types.
  • Channels now implements Index<usize>, which directly returns the AudioBuffer of that particular channel.

Host-side changes

This also led to a few changes to the host side, in order to avoid possible UB:

  • Merged InputAudioBuffers and OutputAudioBuffers into a single AudioBuffers type;
    • This implies both input and output buffers must be behind a mutable &mut reference;
    • A future release may change those APIs to also allow them to take &[Cell<T>] instead, if the above ends up being too restrictive.
  • AudioBuffers now implements Copy and Clone.
  • StartedPluginAudioProcessor::process's outputs parameter is now a shared (&) reference, instead of a mutable (&mut) one.

Some of the methods that provide interoperability with clack-plugin also have been changed:

  • AudioBuffers::from_plugin_audio[_mut] methods have been replaced with from_plugin_audio_inputs and `from_plugin_audio_outputs;
  • Removed AudioBuffers::as_plugin_audio_with_inputs, as it's now equivalent to AudioBuffers::as_plugin_audio_with_outputs, but with
    • its arguments swapped.
  • Removed AudioBuffers::to_plugin_audio, since consuming the AudioBuffers type does not make sense now that it is Copy.

@prokopyl prokopyl added the enhancement New feature or request label Nov 3, 2024
@prokopyl prokopyl self-assigned this Nov 3, 2024
@prokopyl prokopyl added this to the Clack 0.1 milestone Nov 3, 2024
@prokopyl prokopyl added the bug Something isn't working label Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant