Reworked AudioBuffer, removed memory allocations in the audio thread,…
… also in process_events(), eliminated Box transmutes where possible etc.
This addresses https://github.com/overdrivenpotato/rust-vst2/issues/28 and other things. The diff might look confusing (especially for buffer.rs) so I'll explain the changes here:
- Before,
AudioBufferallocated in every frame due to thecollect()s infrom_raw(). Now,AudioBufferdoesn't allocate anymore. I modified the tests to use the newAudioBuffer. When you split anAudioBufferyou get zero overhead iterable typesInputsandOutputsthat 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. - I also removed the allocations in
process_events()mentioned in that issue, by implementing a functionapi::Events::events()for iterating over midi events when receiving and implementing aSendEventBufferfor 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 Traitis still not in stable, soapi::Events::events()which returnsimpl Iteratoronly 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 anightlyfeature (#[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:
use event::Event;
use api::Events;
fn process_events(&mut self, events: &Events) {
for &e in events.events_raw() {
let event: Event = Event::from(unsafe { *e });
match event {
// ...
}
}
}
But on nightly it's:
fn process_events(&mut self, events: &Events) {
for event in events.events() {
match event {
// ...
}
}
}
Btw, I tested this in some example plugins, also in the sine_synth example.
- Changed the examples to use the current crate instead of pulling it from github, by using relative paths. Also changed crate-type to cdylib, because that's what it should be. Also makes the resulting dlls smaller and more optimized.
- Changed
midi_note_to_hz()in the sine_synth example to be more intuitive (and with one less division). - Changed
ProcessProcandProcessProcF64to 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. - Changed Box transmutes to
Box::into_raw()andfrom_raw()inAEffect::get_plugin()andlib::main()because it's more idiomatic (transmutes should be avoided). - Added a
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,Infocontains multipleStrings and each of them allocates every timeget_info()is called. This is very wasteful. Now theInfogets called only during plugin initialization and cached in thePluginCachewhich gets stored in theAEffect::objectuser 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::Eventsinstead ofVec<event::Event>and uses the zero allocation iterator mentioned above. - Sending midi events is now done with the
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.
Ah, the CI builds are failing because they are on stable.
Should I do the suggestion above?
(Make api::Events::events_raw() public and make api::Events::events() conditionally available with a nightly feature?)
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 pub(crate) syntax, which isn't on stable yet..
Should I make those symbols completely public?