-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reworked AudioBuffer, removed memory allocations in the audio thread,… #32
base: master
Are you sure you want to change the base?
Conversation
… also in process_events(), eliminated Box transmutes where possible etc
…ate and the new API
Ah, the CI builds are failing because they are on stable. |
I would vote to add the feature gate. Aside from that Evrything looks good to me. |
Right now the builds fail on stable because of the |
…an iterator over events that may be values or refs
… also in process_events(), eliminated Box transmutes where possible etc.
This addresses #28 and other things.
The diff might look confusing (especially for buffer.rs) so I'll explain the changes here:
AudioBuffer
allocated in every frame due to thecollect()
s infrom_raw()
. Now,AudioBuffer
doesn't allocate anymore. I modified the tests to use the newAudioBuffer
.When you split an
AudioBuffer
you get zero overhead iterable typesInputs
andOutputs
that behave like slices, you can also zip the buffer directly. As you can see in the changes to the examples, both ways work with minimal changes to existing plugins.process_events()
mentioned in that issue, by implementing a functionapi::Events::events()
for iterating over midi events when receiving and implementing aSendEventBuffer
for sending which reuses the memory and only allocates duringnew()
for a given maximum of midi events. Only plugins that send midi to the host need to use this (and hosts sending midi to plugins). Unfortunatelyimpl Trait
is still not in stable, soapi::Events::events()
which returnsimpl Iterator
only works on nightly (and the type couldn't be expressed by writing it out because it's a Map with an anonymous closure type). But this function could be in anightly
feature (#[cfg(feature = "nightly")]
), then we could makeevents_raw()
public for people on stable so the user code would use that and do the.map(..)
part manually, like:But on nightly it's:
Btw, I tested this in some example plugins, also in the sine_synth example.
midi_note_to_hz()
in the sine_synth example to be more intuitive (and with one less division).ProcessProc
andProcessProcF64
to take the input buffer pointers as immutable. I always make the rust pointers for ffi functions immutable if that's the intended usage, even when the C lib didn't do this, because it's an oversight in the C lib and it makes the Rust code stricter/better because in this case AudioBuffer doesn't need to have mutable references to the inputs.Box::into_raw()
andfrom_raw()
inAEffect::get_plugin()
andlib::main()
because it's more idiomatic (transmutes should be avoided).PluginCache
. There was a huge inefficiency ininterfaces::process_replacing()
andprocess_replacing_f64()
because it calledplugin.get_info()
on EVERY frame to get the number of inputs/outputs and whether it supports f64 precision. First of all, that check for f64 precision isn't necessary. I asked more experienced VST plugin devs, they said that the host won't even call this function if the plugin doesn't support f64 precision processing (which they know from callingget_info()
when loading a plugin and caching that), and even if, that should be a NOP. Secondly and more importantly,Info
contains multipleString
s and each of them allocates every timeget_info()
is called. This is very wasteful. Now theInfo
gets called only during plugin initialization and cached in thePluginCache
which gets stored in theAEffect::object
user data pointer.The goal is to have NO allocations in the audio thread at all.
Overall the API didn't change much at all:
Plugin::process()
now takes a&mut AudioBuffer<f32>
instead of immutable. Same for the f64 variant.Plugin::process_events()
now takes&api::Events
instead ofVec<event::Event>
and uses the zero allocation iterator mentioned above.SendEventBuffer
, I added a usage example to the doc.I made sure the tests succeed and changed the examples in the documentation to match the new API and also tested the audio and midi processing with actual VSTs.