Safe plugin buffers #26
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 werehanded 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
typeThe new
AudioBuffer<T>
type represents a contiguous buffer of samples of typeT
(usually eitherf32
orf64
).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 ofe.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
implementsget
andput
method (among other things), that allow one to retrieve or set the samplevalue at a given index.
Alternatively, indexing into or iterating over the buffer will yield
&Cell<T>
references to the wanted sample, allowingsimilar 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 beingmutated 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'swhat 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 within-place processing.
If it is not feasible to convert those functions to work with
AudioBuffer
s directly, theAudioBuffer
type providesadditional 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 thefunction does support in-place processing through
Cell
s.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
andOutputPort
types have been merged into a singlePort
type, as well as all thetypes they return:
InputPortsIter
andOutputPortsIter
have been merged intoPortIter
;InputChannels
andOutputChannels
have been merged intoChannels
;InputChannelsIter
andOutputChannelsIter
iter have been merged intoChannelsIter
;For more clarity and consistency, the
PairedChannels
type has been renamed intoChannelsPairs
.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
AudioBuffer
s instead of slices:Channels::channel
now returns anAudioBuffer
reference;ChannelsIter
(Channels
' iterator) now iterates overAudioBuffer
s:ChannelPair::input
andChannelPair::output
returnAudioBuffer
references;ChannelPair
enum itself containsAudioBuffer
references instead of slice references;Audio
now only produces shared referencesBecause
AudioBuffer
s' contents can now be mutated through shared (&
) references, exclusive (&mut
) references donot 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 implementsCopy
andClone
. This is also the case for all the types it produces, namely:Port
andChannels
;PortPair
,ChannelsPairs
, andChannelPair
;SampleType
if its contents are alsoCopy + Clone
(which is the case in all of Clack's APIs);PortsIter
,ChannelsIter
,PortPairsIter
, andChannelsPairsIter
iterator types implementClone
(but notCopy
).IntoIterator
is now implemented onto&Audio
andAudio
directly, instead of requiring&mut audio
.SampleType
's helper methods have been overhauled. SinceSampleType
can now beCopy
and does not deal withmutable references anymore, all of the
as/into[_mut]
methods have been removed, and replaced with simpleto_f32
and
to_f64
methods that produces copies of the pointersSampleType
holds.PortPair::latencies
andPortPair::constant_masks
helper methods have been removed. Their purpose was to allowrecovering 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 referencesNow
Audio
and its produced types (e.g.Port
andPortPair
) all operate through shared references, but the pluginstill needs to write the
constant_mask
field in the underlyingclap_audio_buffer
s to send it back to the host.This means
Audio
,Port
andPortPair
now provide interior mutability. This does not change anything for those whoonly 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
thatAudio
,Port
andPortPair
wrap, will most likely lead toUndefined 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 useptr::read
andptr::write
instead of borrowing it, or useCell
orUnsafeCell
for at least theconstant_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:
Audio::from_raw_buffers
method now takes raw pointers to the buffer slices, instead of references;Audio::raw_buffers
,Audio::to_raw_buffers
,Audio::raw_input_buffers
andAudio::raw_output_buffers
methods;Audio::raw_inputs
andAudio::raw_outputs
have been added instead, which only return raw pointersand do not consume or mutably borrow the
Audio
struct.AudioPortProcessingInfo::from_raw_ptr
method, that reads the metadata part of theclap_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 theclap_process
struct.Audio::input_port
,Audio::output_port
,Audio::input_ports
,Audio::output_ports
;Audio::port_sub_range
;Port::channels
andPort::channel
;PortPair::input
,PortPair::output
, andPortPair::channels
;ChannelsPairs::input_channels
,ChannelsPairs::output_channels
andChannelsPairs::channel_pair
;Channels
now implementsIndex<usize>
, which directly returns theAudioBuffer
of that particular channel.Host-side changes
This also led to a few changes to the host side, in order to avoid possible UB:
InputAudioBuffers
andOutputAudioBuffers
into a singleAudioBuffers
type;&mut
reference;&[Cell<T>]
instead, if the above ends up being too restrictive.AudioBuffers
now implementsCopy
andClone
.StartedPluginAudioProcessor::process
'soutputs
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 withfrom_plugin_audio_inputs
and `from_plugin_audio_outputs;AudioBuffers::as_plugin_audio_with_inputs
, as it's now equivalent toAudioBuffers::as_plugin_audio_with_outputs
, but withAudioBuffers::to_plugin_audio
, since consuming theAudioBuffers
type does not make sense now that it isCopy
.