Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion rustortion-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ anyhow = "1.0"
rustfft = "6.4"
realfft = "3.5"
arc-swap = "1.8"
assert_no_alloc = { version = "1.1", features = ["warn_debug"] }
nam-rs = "0.2.0"

[dev-dependencies]
criterion = { version = "0.8", features = ["html_reports"] }
tempfile = "3.24"
# Test-only: RT-path allocation auditing (tests/no_alloc.rs). `warn_debug`
# makes it count violations instead of aborting the test binary.
assert_no_alloc = { version = "1.1", features = ["warn_debug"] }

[[bench]]
name = "impulse_responses"
Expand Down
2 changes: 1 addition & 1 deletion rustortion-core/benches/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn create_test_cabinet(ir_length: usize, sample_rate: usize) -> IrCabinet {

let mut convolver = Convolver::new_fir(max_ir_samples);
convolver.set_ir(&ir_samples).unwrap();
cabinet.swap_convolver(convolver);
cabinet.set_convolver(convolver);

cabinet
}
Expand Down
46 changes: 39 additions & 7 deletions rustortion-core/src/amp/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,37 @@ struct BypassableStage {
bypassed: bool,
}

/// Stage capacity reserved up front, and the hard cap on chain length.
///
/// `insert_stage` (the RT `AddStage` path) never grows the backing `Vec` past
/// its reserved capacity, so it never reallocates on the RT thread; the UI
/// enforces the same number as the maximum stage count. ~24 bytes per slot, so
/// ~1.5 KB reserved.
pub const DEFAULT_CHAIN_CAPACITY: usize = 64;

// AmplifierChain holds a sequence of processing stages.
#[derive(Default)]
pub struct AmplifierChain {
stages: Vec<BypassableStage>,
}

impl Default for AmplifierChain {
fn default() -> Self {
Self::new()
}
}

impl AmplifierChain {
#[must_use]
pub const fn new() -> Self {
Self { stages: Vec::new() }
pub fn new() -> Self {
Self::with_capacity(DEFAULT_CHAIN_CAPACITY)
}

/// Create a chain that can hold `capacity` stages before reallocating.
#[must_use]
pub fn with_capacity(capacity: usize) -> Self {
Self {
stages: Vec::with_capacity(capacity),
}
}

pub fn add_stage(&mut self, stage: Box<dyn Stage>) {
Expand Down Expand Up @@ -62,8 +83,18 @@ impl AmplifierChain {
self.stages.get(idx).map(|s| s.inner.get_parameter(name))
}

/// Insert a stage at the given index.
pub fn insert_stage(&mut self, idx: usize, stage: Box<dyn Stage>) {
/// Insert a stage at the given index **without reallocating**.
///
/// This runs on the RT thread (via `AddStage`). If the chain is already at
/// its reserved capacity, the stage is **returned** rather than inserted, so
/// the caller can dispose of it off the RT thread (growing the `Vec` here
/// would allocate, and dropping the rejected box here would free, on the
/// audio thread). Returns `None` when the stage was inserted.
#[must_use]
pub fn insert_stage(&mut self, idx: usize, stage: Box<dyn Stage>) -> Option<Box<dyn Stage>> {
if self.stages.len() == self.stages.capacity() {
return Some(stage);
}
let idx = idx.min(self.stages.len());
self.stages.insert(
idx,
Expand All @@ -72,6 +103,7 @@ impl AmplifierChain {
bypassed: false,
},
);
None
}

/// Remove and return the stage at the given index.
Expand Down Expand Up @@ -144,7 +176,7 @@ mod tests {
let mut chain = AmplifierChain::new();
chain.add_stage(make_level(1.0));
chain.add_stage(make_level(1.0));
chain.insert_stage(1, make_level(0.5));
assert!(chain.insert_stage(1, make_level(0.5)).is_none());
let out = chain.process(1.0);
assert!((out - 0.5).abs() < 1e-6);
}
Expand Down Expand Up @@ -239,7 +271,7 @@ mod tests {
let mut chain = AmplifierChain::new();
chain.add_stage(make_level(1.0));
chain.set_bypassed(0, true);
chain.insert_stage(0, make_level(0.5)); // insert before bypassed
assert!(chain.insert_stage(0, make_level(0.5)).is_none()); // insert before bypassed
// idx 0 = new (active, 0.5x), idx 1 = old (bypassed, 1.0x)
let out = chain.process(1.0);
assert!(
Expand Down
109 changes: 66 additions & 43 deletions rustortion-core/src/audio/engine.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use anyhow::Result;
use assert_no_alloc::permit_alloc;
use crossbeam::channel::{Receiver, Sender, bounded};
use log::{debug, error};

Expand All @@ -17,7 +16,10 @@ use crate::tuner::Tuner;

pub struct PreparedIr {
pub name: String,
pub convolver: Convolver,
/// Boxed so it can be swapped into the cabinet on the RT thread without
/// reallocating, and so the whole `PreparedIr` (old convolver + name) can
/// be retired off the RT thread in one piece.
pub convolver: Box<Convolver>,
}

pub enum EngineMessage {
Expand All @@ -35,7 +37,9 @@ pub enum EngineMessage {
SetIrBypass(bool),
SetIrGain(f32),
SetTunerEnabled(bool),
SetPitchShift(i32),
/// Carries a fully-constructed pitch shifter (built off the RT thread), or
/// `None` to disable pitch shifting (the `0` semitones bypass case).
SetPitchShift(Option<Box<PitchShifter>>),
SetStageBypassed(usize, bool),
SetSamplers(Box<Samplers>),
}
Expand All @@ -49,12 +53,14 @@ pub struct Engine {
engine_receiver: Receiver<EngineMessage>,
/// Handle for sending arbitrary objects off the RT thread for deallocation.
rt_drop: RtDropHandle,
samplers: Samplers,
/// Boxed so swapping samplers (sample-rate / buffer changes) on the RT
/// thread exchanges pointers and retires the old box directly.
samplers: Box<Samplers>,
tuner: Option<Tuner>,
recorder: Option<Recorder>,
peak_meter: Option<PeakMeter>,
metronome: Option<Metronome>,
pitch_shifter: Option<PitchShifter>,
pitch_shifter: Option<Box<PitchShifter>>,
input_highpass: Option<Box<dyn Stage>>,
input_lowpass: Option<Box<dyn Stage>>,
/// When true, skip tuner, peak meter, recorder, and metronome processing.
Expand Down Expand Up @@ -83,7 +89,7 @@ impl Engine {
ir_cabinet,
engine_receiver,
rt_drop,
samplers,
samplers: Box::new(samplers),
tuner: Some(tuner),
recorder: None,
peak_meter: Some(peak_meter),
Expand Down Expand Up @@ -115,7 +121,7 @@ impl Engine {
ir_cabinet,
engine_receiver,
rt_drop: rt_drop_handle,
samplers,
samplers: Box::new(samplers),
tuner: None,
recorder: None,
peak_meter: None,
Expand Down Expand Up @@ -162,27 +168,21 @@ impl Engine {
}

if let Some(ref mut shifter) = self.pitch_shifter {
// FIXME(no_alloc): PitchShifter::process_block uses Vec scratch
// buffers internally — see audio/pitch_shifter.rs.
permit_alloc(|| shifter.process_block(output));
shifter.process_block(output);
}

if let Some(ref mut cab) = self.ir_cabinet {
cab.process_block(output);
}

if let Some(ref mut peak_meter) = self.peak_meter {
// FIXME(no_alloc): PeakMeter::process does Arc::new(PeakMeterInfo)
// every block — see audio/peak_meter.rs:62.
permit_alloc(|| peak_meter.process(output));
peak_meter.process(output);
}

if !self.lightweight
&& let Some(recorder) = self.recorder.as_mut()
{
// FIXME(no_alloc): Recorder::record_block does Vec::with_capacity
// per block — see audio/recorder.rs:47.
permit_alloc(|| recorder.record_block(output))?;
recorder.record_block(output);
}

Ok(())
Expand Down Expand Up @@ -263,8 +263,14 @@ impl Engine {
}
}
EngineMessage::AddStage(idx, stage) => {
self.chain.insert_stage(idx, stage);
debug!("Added stage at index {idx}");
if let Some(rejected) = self.chain.insert_stage(idx, stage) {
// Chain is at its reserved capacity. Retire the rejected
// stage off the RT thread rather than dropping (freeing)
// it here. The UI caps stage count, so this is a backstop.
self.rt_drop.retire(rejected);
} else {
debug!("Added stage at index {idx}");
}
}
EngineMessage::RemoveStage(idx) => {
if let Some(old) = self.chain.remove_stage(idx) {
Expand All @@ -286,16 +292,26 @@ impl Engine {
}
}
EngineMessage::SetInputFilters(hp, lp) => {
self.input_highpass = hp;
self.input_lowpass = lp;
// Retire the previous filters off the RT thread instead of
// dropping them here on direct assignment.
if let Some(old) = std::mem::replace(&mut self.input_highpass, hp) {
self.rt_drop.retire(old);
}
if let Some(old) = std::mem::replace(&mut self.input_lowpass, lp) {
self.rt_drop.retire(old);
}
debug!("Updated input filters");
}
EngineMessage::SwapIrConvolver(prepared) => {
EngineMessage::SwapIrConvolver(mut prepared) => {
if let Some(ref mut cab) = self.ir_cabinet {
debug!("IR convolver swapped: {}", prepared.name);
let old = cab.swap_convolver(prepared.convolver);
permit_alloc(|| self.rt_drop.retire(Box::new(old)));
// Swap the new convolver in; `prepared` is left holding
// the old convolver. Retire the whole `PreparedIr` (old
// convolver + name `String`) off the RT thread so
// nothing deallocates here.
cab.swap_convolver(&mut prepared.convolver);
}
self.rt_drop.retire(prepared);
}
EngineMessage::ClearIr => {
if let Some(ref mut cab) = self.ir_cabinet {
Expand Down Expand Up @@ -326,12 +342,12 @@ impl Engine {
EngineMessage::StopRecording => {
self.handle_stop_recording();
}
EngineMessage::SetPitchShift(semitones) => {
self.handle_pitch_shift(semitones);
EngineMessage::SetPitchShift(shifter) => {
self.handle_pitch_shift(shifter);
}
EngineMessage::SetSamplers(new_samplers) => {
let old = std::mem::replace(&mut self.samplers, *new_samplers);
permit_alloc(|| self.rt_drop.retire(Box::new(old)));
let old = std::mem::replace(&mut self.samplers, new_samplers);
self.rt_drop.retire(old);
debug!("Samplers swapped");
Comment on lines 348 to 351
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 83d0ff7 (and the related dealloc gaps in dafb090).

retire() now leaks the box via mem::forget on a full/disconnected channel instead of dropping it on the RT thread, so no free() runs on the audio thread under channel pressure. The event is surfaced through a new leaked() counter, and the channel capacity was raised 16→64. A unit test (retire_leaks_instead_of_dropping_when_full) asserts the overflow path leaks rather than deallocates.

While here, two more alloc-on-RT paths in the same spirit were closed: the recorder no longer constructs an anyhow error on RT when its writer thread has exited, and the amp chain's AddStage path no longer reallocates its stage Vec past the reserved capacity (it rejects and retires off-RT, with the UI capping chain length).

}
}
Expand Down Expand Up @@ -364,19 +380,15 @@ impl Engine {
self.recorder = None;
}

fn handle_pitch_shift(&mut self, semitones: i32) {
if semitones == 0 {
self.pitch_shifter = None;
debug!("Pitch shift disabled (bypass)");
} else if let Some(ref mut shifter) = self.pitch_shifter {
shifter.set_semitones(semitones as f32);
debug!("Pitch shift set to {semitones} semitones");
} else {
// FIXME(no_alloc): PitchShifter::new allocates FFT scratch buffers
// — see audio/pitch_shifter.rs.
self.pitch_shifter = Some(permit_alloc(|| PitchShifter::new(semitones as f32)));
debug!("Pitch shift set to {semitones} semitones");
fn handle_pitch_shift(&mut self, shifter: Option<Box<PitchShifter>>) {
// The shifter (if any) is constructed off the RT thread in
// `EngineHandle::set_pitch_shift`; here we just swap it in and retire
// the previous one off the RT thread.
let old = std::mem::replace(&mut self.pitch_shifter, shifter);
if let Some(old) = old {
self.rt_drop.retire(old);
}
debug!("Pitch shifter updated");
}
}

Expand Down Expand Up @@ -448,8 +460,14 @@ impl EngineHandle {
}

pub fn set_pitch_shift(&self, semitones: i32) {
let update = EngineMessage::SetPitchShift(semitones);
self.send(update);
// Construct the pitch shifter here (GUI thread) so the RT thread never
// allocates its FFT plans / scratch buffers. `0` semitones == bypass.
let shifter = if semitones == 0 {
None
} else {
Some(Box::new(PitchShifter::new(semitones as f32)))
};
self.send(EngineMessage::SetPitchShift(shifter));
}

pub fn set_stage_bypassed(&self, idx: usize, bypassed: bool) {
Expand All @@ -461,8 +479,13 @@ impl EngineHandle {
self.send(update);
}

pub fn start_recording(&self, sample_rate: usize, output_dir: &str) -> Result<()> {
let recorder = Recorder::new(sample_rate as u32, output_dir)?;
pub fn start_recording(
&self,
sample_rate: usize,
output_dir: &str,
max_block_samples: usize,
) -> Result<()> {
let recorder = Recorder::new(sample_rate as u32, output_dir, max_block_samples)?;

let update = EngineMessage::StartRecording(recorder);
self.send(update);
Expand Down
Loading