rust-vst2 icon indicating copy to clipboard operation
rust-vst2 copied to clipboard

Reworked AudioBuffer, removed memory allocations in the audio thread,…

Open Boscop opened this issue 8 years ago • 3 comments

… 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, AudioBuffer allocated in every frame due to the collect()s in from_raw(). Now, AudioBuffer doesn't allocate anymore. I modified the tests to use the new AudioBuffer. When you split an AudioBuffer you get zero overhead iterable types Inputs and Outputs 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.
  • I also removed the allocations in process_events() mentioned in that issue, by implementing a function api::Events::events() for iterating over midi events when receiving and implementing a SendEventBuffer for sending which reuses the memory and only allocates during new() 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). Unfortunately impl Trait is still not in stable, so api::Events::events() which returns impl 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 a nightly feature (#[cfg(feature = "nightly")]), then we could make events_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 ProcessProc and ProcessProcF64 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.
  • Changed Box transmutes to Box::into_raw() and from_raw() in AEffect::get_plugin() and lib::main() because it's more idiomatic (transmutes should be avoided).
  • Added a PluginCache. There was a huge inefficiency in interfaces::process_replacing() and process_replacing_f64() because it called plugin.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 calling get_info() when loading a plugin and caching that), and even if, that should be a NOP. Secondly and more importantly, Info contains multiple Strings and each of them allocates every time get_info() is called. This is very wasteful. Now the Info gets called only during plugin initialization and cached in the PluginCache which gets stored in the AEffect::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 of Vec<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.

Boscop avatar Jun 13 '17 22:06 Boscop

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?)

Boscop avatar Jun 13 '17 22:06 Boscop

I would vote to add the feature gate. Aside from that Evrything looks good to me.

zyvitski avatar Jun 13 '17 23:06 zyvitski

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?

Boscop avatar Jun 14 '17 14:06 Boscop