Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds block processing capabilities to the audio processing pipeline by introducing a process_block method to the Stage trait. This enables more efficient batch processing of audio samples compared to the existing sample-by-sample approach.
Key changes:
- Added
process_blockmethod to theStagetrait for batch audio processing - Implemented
process_blockfor all stage types (tonestack, preamp, poweramp, noise_gate, level, filter, compressor) - Added
process_blocktoAmplifierChainto process entire buffers through the chain - Created new benchmark suite to compare sample-by-sample vs block processing performance
- Refactored engine benchmarks by removing some tests and exposing
build_engineas public
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sim/stages/mod.rs | Adds process_block method to the Stage trait definition |
| src/sim/stages/tonestack.rs | Implements process_block for ToneStackStage |
| src/sim/stages/preamp.rs | Implements process_block for PreampStage |
| src/sim/stages/poweramp.rs | Implements process_block for PowerAmpStage |
| src/sim/stages/noise_gate.rs | Implements process_block for NoiseGateStage |
| src/sim/stages/level.rs | Implements optimized process_block for LevelStage |
| src/sim/stages/filter.rs | Implements process_block for FilterStage |
| src/sim/stages/compressor.rs | Implements process_block for CompressorStage |
| src/sim/chain.rs | Adds process_block method to AmplifierChain for batch processing |
| benches/engine.rs | Removes redundant benchmarks and exposes build_engine as public |
| benches/chain.rs | Adds new benchmark comparing sample vs block processing performance |
| Cargo.toml | Registers new chain benchmark |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn process(&mut self, input: f32) -> f32; | ||
|
|
||
| // Process a block of samples through this stage | ||
| fn process_block(&mut self, input: &mut [f32]); |
There was a problem hiding this comment.
The process_block method has identical boilerplate implementation across multiple stages (tonestack, preamp, poweramp, noise_gate, filter, compressor). Consider providing a default implementation in the trait that calls self.process() for each sample. This would eliminate code duplication and allow stages to override only when they have a more efficient block processing implementation (like LevelStage does).
| fn process_block(&mut self, input: &mut [f32]); | |
| fn process_block(&mut self, input: &mut [f32]) { | |
| for sample in input.iter_mut() { | |
| *sample = self.process(*sample); | |
| } | |
| } |
|
|
||
| fn process_block(&mut self, input: &mut [f32]) { | ||
| for sample in input.iter_mut() { | ||
| *sample *= self.gain; |
There was a problem hiding this comment.
The process_block implementation here is inconsistent with the process method. The process method returns input * self.gain, but process_block modifies the input in-place using *sample *= self.gain. While both produce the same numerical result, the inconsistency could be confusing. Consider using *sample = self.process(*sample) for consistency, or document why the optimization is used here.
| *sample *= self.gain; | |
| *sample = self.process(*sample); |
No description provided.