Reworked AudioBuffer, removed memory allocations in the audio thread,…#32
Open
Boscop wants to merge 10 commits intooverdrivenpotato:masterfrom
Open
Reworked AudioBuffer, removed memory allocations in the audio thread,…#32Boscop wants to merge 10 commits intooverdrivenpotato:masterfrom
Boscop wants to merge 10 commits intooverdrivenpotato:masterfrom
Conversation
… also in process_events(), eliminated Box transmutes where possible etc
…ate and the new API
Contributor
Author
|
Ah, the CI builds are failing because they are on stable. |
Contributor
|
I would vote to add the feature gate. Aside from that Evrything looks good to me. |
Contributor
Author
|
Right now the builds fail on stable because of the |
…an iterator over events that may be values or refs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
… 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:
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 an
AudioBufferyou 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.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: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).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.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,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.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.