These are my own notes for upstreaming Firewheel into the Bevy game engine. We should consider what features are strictly necessary, and what features could be handled by third party libraries.
- DSP in
firewheel-core::dsp that isn't strictly necessary (some could just be implemented by the nodes themselves):
volume::DbMeterNormalizer - Only used by the PeakMeterNode
interleave/deinterleave - The only time this is used is for FirewheelProcessor::process_interleaved, and we could remove it altogether by using the audioadapter crate (which is already being used by the rubato dependency.)
declick::LowpassDeclicker - I ended up not using this in any of the default nodes, and it doesn't even work that well.
- The buffer types in
dsp::buffer - There is a lot of unsafe here, so we could consider using a more interopable third party audio buffer crate.
algo::max_peak - Only used by the PeakMeterNode.
filter::svf and filter::butterworth - Only used by the SVF node. These also contain quite a bit of extra code in order to be "generic".
atomic-float - This just re-exports portable_atomic::{AtomicF32, AtomicF64}
- There are a few buile in nodes that are a bit questionable if they make sense to be part of bevy itself:
- The white/pink noise generator nodes
- The beep test node
- The stream reader/writer nodes. While these could be useful in some cases, I'm not exactly happy with the API. The main use case I can think of is implementing network voice chat, but that might be better implemented as its own dedicated node/crate anyway. If anything, these nodes could be added later once we figure out a more user friendy API.
- SVF - While SVF filters are useful in audio DSP, I'm not sure how useful they actually are as standalone nodes. And equalizer-like functionality would be better as a dedicated equalizer node.
- The
RtAudio backend would probably make more sense as a 3rd party crate. It will become less relevant once CPAL gets native duplex support anyway.
- Firewheel has many features flags in the
Cargo.toml, which could make bevy's already long feature list even longer. It might be worth consolidating/defaulting some of the features, like individual nodes and such.
- And lastly, it might be worth reconsidering the "node state" API. The
FirewheelContext::node_state() and FirewheelContext::node_state_mut() API doesn't fell very Bevy-y.
Removing unsafe
One big use of unsafe is in firewheel-core::dsp::buffer. Like I mentioned in the list above, it might be better to instead use a dedicated 3rd party audio buffer crate.
The second main use of unsafe is in the schedule module in firewheel-graph. The reason unsafe is used is because the list of audio buffers needs to be built before calling each node's process method, and these buffers could be any arbitrary buffer index (the compiler in theory ensures that buffer indices never alias.)
In retrospect, using runtime-checked borrowing for these buffers probably wouldn't add much overhead. While I'm not sure it's possible to completely avoid unsafe (without resorting to wrapping each buffer in an RefCell<Vec<f32>> which would be a lot less cache efficient than a single contiguous buffer), it could be achieved in an abstraction with only one instance of unsafe.
Crate consolidation
The workspace should be consolidated into as few crates as possible before upstreaming to bevy.
The one crate I know for sure that will be separate is firewheel-cpal. For these other two crates, we need to figure out the best solution:
firewheel-macros - As a proc macro crate, it needs to be separate. However would it make more sense to be part of the already existing bevy_macro_utils crate?
firewheel-symphonium - I'm not sure how bevy's asset loading system will work. Should audio file loading/decoding be part of the firewheel crate, part of the already existing bevy_assets crate, or be its own bevy_symphonium crate?
TODO List:
A rough TODO list so I don't forget anything:
- add a setting to clamp graph inputs to silence if they are below a certain threshold
- add test to enforce clap-compatible threading model
- copy-paste
rtgc dependency code directly into firewheel-core to avoid the orphan rule problem
- update to latest
fft-convolver dependency
- consolidate the workspace crates into as few crates as possible
These are my own notes for upstreaming Firewheel into the Bevy game engine. We should consider what features are strictly necessary, and what features could be handled by third party libraries.
firewheel-core::dspthat isn't strictly necessary (some could just be implemented by the nodes themselves):volume::DbMeterNormalizer- Only used by thePeakMeterNodeinterleave/deinterleave- The only time this is used is forFirewheelProcessor::process_interleaved, and we could remove it altogether by using theaudioadaptercrate (which is already being used by therubatodependency.)declick::LowpassDeclicker- I ended up not using this in any of the default nodes, and it doesn't even work that well.dsp::buffer- There is a lot of unsafe here, so we could consider using a more interopable third party audio buffer crate.algo::max_peak- Only used by thePeakMeterNode.filter::svfandfilter::butterworth- Only used by the SVF node. These also contain quite a bit of extra code in order to be "generic".atomic-float- This just re-exportsportable_atomic::{AtomicF32, AtomicF64}RtAudiobackend would probably make more sense as a 3rd party crate. It will become less relevant once CPAL gets native duplex support anyway.Cargo.toml, which could make bevy's already long feature list even longer. It might be worth consolidating/defaulting some of the features, like individual nodes and such.FirewheelContext::node_state()andFirewheelContext::node_state_mut()API doesn't fell very Bevy-y.Removing unsafe
One big use of unsafe is in
firewheel-core::dsp::buffer. Like I mentioned in the list above, it might be better to instead use a dedicated 3rd party audio buffer crate.The second main use of unsafe is in the
schedulemodule infirewheel-graph. The reason unsafe is used is because the list of audio buffers needs to be built before calling each node's process method, and these buffers could be any arbitrary buffer index (the compiler in theory ensures that buffer indices never alias.)In retrospect, using runtime-checked borrowing for these buffers probably wouldn't add much overhead. While I'm not sure it's possible to completely avoid unsafe (without resorting to wrapping each buffer in an
RefCell<Vec<f32>>which would be a lot less cache efficient than a single contiguous buffer), it could be achieved in an abstraction with only one instance of unsafe.Crate consolidation
The workspace should be consolidated into as few crates as possible before upstreaming to bevy.
The one crate I know for sure that will be separate is
firewheel-cpal. For these other two crates, we need to figure out the best solution:firewheel-macros- As a proc macro crate, it needs to be separate. However would it make more sense to be part of the already existingbevy_macro_utilscrate?firewheel-symphonium- I'm not sure how bevy's asset loading system will work. Should audio file loading/decoding be part of the firewheel crate, part of the already existingbevy_assetscrate, or be its ownbevy_symphoniumcrate?TODO List:
A rough TODO list so I don't forget anything:
rtgcdependency code directly intofirewheel-coreto avoid the orphan rule problemfft-convolverdependency