fix: forward parameter changes to live stages, eliminate rebuild clicks (#217)

Author: OpenSauce

benches/engine.rs

@@ -7,8 +7,8 @@ use rustortion::amp::chain::AmplifierChain;
 use rustortion::amp::stages::level::LevelStage;
 use rustortion::audio::engine::{Engine, EngineHandle};
 use rustortion::audio::peak_meter::PeakMeter;
+use rustortion::audio::rt_drop::RtDropHandle;
 use rustortion::audio::samplers::Samplers;
-use rustortion::ir::load_service::ConvolverDropHandle;
 use rustortion::metronome::Metronome;
 use rustortion::tuner::Tuner;
 
@@ -35,7 +35,7 @@ pub fn build_engine(
         ir_cabinet,
         peak_meter,
         metronome,
-        ConvolverDropHandle::new().0,
+        RtDropHandle::new().0,
     )
     .unwrap();
     (engine, handle)
@@ -119,7 +119,7 @@ fn bench_engine_with_ir_cabinet(c: &mut Criterion) {
                     ir_cabinet,
                     peak_meter,
                     metronome,
-                    ConvolverDropHandle::new().0,
+                    RtDropHandle::new().0,
                 )
                 .unwrap();
 

src/amp/chain.rs

@@ -31,4 +31,130 @@ impl AmplifierChain {
             stage.process_block(input);
         }
     }
+
+    /// Forward a parameter change to a live stage.
+    pub fn set_parameter(
+        &mut self,
+        idx: usize,
+        name: &str,
+        value: f32,
+    ) -> Option<Result<(), &'static str>> {
+        self.stages
+            .get_mut(idx)
+            .map(|stage| stage.set_parameter(name, value))
+    }
+
+    /// Read a parameter from a live stage.
+    pub fn get_parameter(&self, idx: usize, name: &str) -> Option<Result<f32, &'static str>> {
+        self.stages.get(idx).map(|stage| stage.get_parameter(name))
+    }
+
+    /// Insert a stage at the given index.
+    pub fn insert_stage(&mut self, idx: usize, stage: Box<dyn Stage>) {
+        let idx = idx.min(self.stages.len());
+        self.stages.insert(idx, stage);
+    }
+
+    /// Remove and return the stage at the given index.
+    pub fn remove_stage(&mut self, idx: usize) -> Option<Box<dyn Stage>> {
+        if idx < self.stages.len() {
+            Some(self.stages.remove(idx))
+        } else {
+            None
+        }
+    }
+
+    /// Swap two stages by index.
+    pub fn swap_stages(&mut self, a: usize, b: usize) {
+        if a < self.stages.len() && b < self.stages.len() {
+            self.stages.swap(a, b);
+        }
+    }
+
+    /// Replace a stage at the given index, returning the old one.
+    pub fn replace_stage(
+        &mut self,
+        idx: usize,
+        new_stage: Box<dyn Stage>,
+    ) -> Option<Box<dyn Stage>> {
+        if idx < self.stages.len() {
+            let old = std::mem::replace(&mut self.stages[idx], new_stage);
+            Some(old)
+        } else {
+            None
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::amp::stages::level::LevelStage;
+
+    fn make_level(gain: f32) -> Box<dyn Stage> {
+        Box::new(LevelStage::new(gain))
+    }
+
+    #[test]
+    fn set_parameter_updates_live_stage() {
+        let mut chain = AmplifierChain::new();
+        chain.add_stage(make_level(1.0));
+        assert!(chain.set_parameter(0, "gain", 0.5).is_some());
+        let out = chain.process(1.0);
+        assert!((out - 0.5).abs() < 1e-6);
+    }
+
+    #[test]
+    fn set_parameter_out_of_bounds_returns_none() {
+        let mut chain = AmplifierChain::new();
+        assert!(chain.set_parameter(0, "gain", 0.5).is_none());
+    }
+
+    #[test]
+    fn insert_stage_at_position() {
+        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));
+        let out = chain.process(1.0);
+        assert!((out - 0.5).abs() < 1e-6);
+    }
+
+    #[test]
+    fn remove_stage_returns_removed() {
+        let mut chain = AmplifierChain::new();
+        chain.add_stage(make_level(0.5));
+        chain.add_stage(make_level(1.0));
+        let removed = chain.remove_stage(0);
+        assert!(removed.is_some());
+        let out = chain.process(1.0);
+        assert!((out - 1.0).abs() < 1e-6);
+    }
+
+    #[test]
+    fn remove_stage_out_of_bounds() {
+        let mut chain = AmplifierChain::new();
+        assert!(chain.remove_stage(5).is_none());
+    }
+
+    #[test]
+    fn swap_stages_changes_order() {
+        let mut chain = AmplifierChain::new();
+        chain.add_stage(make_level(0.5));
+        chain.add_stage(make_level(2.0));
+        chain.swap_stages(0, 1);
+        assert!((chain.get_parameter(0, "gain").unwrap().unwrap() - 2.0).abs() < 1e-6);
+        assert!((chain.get_parameter(1, "gain").unwrap().unwrap() - 0.5).abs() < 1e-6);
+    }
+
+    #[test]
+    fn replace_stage_preserves_others() {
+        let mut chain = AmplifierChain::new();
+        chain.add_stage(make_level(0.5));
+        chain.add_stage(make_level(1.0));
+        let old = chain.replace_stage(0, make_level(0.25));
+        assert!(old.is_some());
+        let out = chain.process(1.0);
+        assert!((out - 0.25).abs() < 1e-6);
+    }
 }

src/audio/engine.rs

@@ -7,10 +7,10 @@ use crate::amp::stages::Stage;
 use crate::audio::peak_meter::PeakMeter;
 use crate::audio::pitch_shifter::PitchShifter;
 use crate::audio::recorder::Recorder;
+use crate::audio::rt_drop::RtDropHandle;
 use crate::audio::samplers::Samplers;
 use crate::ir::cabinet::IrCabinet;
 use crate::ir::convolver::Convolver;
-use crate::ir::load_service::ConvolverDropHandle;
 use crate::metronome::Metronome;
 use crate::tuner::Tuner;
 
@@ -22,6 +22,11 @@ pub struct PreparedIr {
 pub enum EngineMessage {
     SetAmpChain(Box<AmplifierChain>),
     SetInputFilters(Option<Box<dyn Stage>>, Option<Box<dyn Stage>>),
+    SetParameter(usize, &'static str, f32),
+    ReplaceStage(usize, Box<dyn Stage>),
+    AddStage(usize, Box<dyn Stage>),
+    RemoveStage(usize),
+    SwapStages(usize, usize),
     StartRecording(Recorder),
     StopRecording,
     SwapIrConvolver(Box<PreparedIr>),
@@ -39,8 +44,8 @@ pub struct Engine {
     ir_cabinet: Option<IrCabinet>,
     /// Channel for updating the amplifier chain.
     engine_receiver: Receiver<EngineMessage>,
-    /// Handle for sending old convolvers off the RT thread for deallocation.
-    convolver_drop: ConvolverDropHandle,
+    /// Handle for sending arbitrary objects off the RT thread for deallocation.
+    rt_drop: RtDropHandle,
     samplers: Samplers,
     tuner: Tuner,
     recorder: Option<Recorder>,
@@ -63,16 +68,16 @@ impl Engine {
         ir_cabinet: Option<IrCabinet>,
         peak_meter: PeakMeter,
         metronome: Metronome,
-        convolver_drop: ConvolverDropHandle,
+        rt_drop: RtDropHandle,
     ) -> Result<(Self, EngineHandle)> {
-        let (engine_sender, engine_receiver) = bounded::<EngineMessage>(10);
+        let (engine_sender, engine_receiver) = bounded::<EngineMessage>(32);
 
         Ok((
             Self {
                 chain: Box::new(AmplifierChain::new()),
                 ir_cabinet,
                 engine_receiver,
-                convolver_drop,
+                rt_drop,
                 samplers,
                 tuner,
                 recorder: None,
@@ -188,10 +193,44 @@ impl Engine {
     pub fn handle_messages(&mut self) {
         while let Ok(message) = self.engine_receiver.try_recv() {
             match message {
-                EngineMessage::SetAmpChain(chain) => {
-                    self.chain = chain;
+                EngineMessage::SetAmpChain(new_chain) => {
+                    let old = std::mem::replace(&mut self.chain, new_chain);
+                    self.rt_drop.retire(old);
                     debug!("Received new amplifier chain");
                 }
+                EngineMessage::SetParameter(idx, name, value) => {
+                    if let Some(result) = self.chain.set_parameter(idx, name, value) {
+                        if let Err(e) = result {
+                            error!("Failed to set parameter '{name}' on stage {idx}: {e}");
+                        }
+                    } else {
+                        error!("SetParameter: stage index {idx} out of bounds");
+                    }
+                }
+                EngineMessage::ReplaceStage(idx, new_stage) => {
+                    if let Some(old) = self.chain.replace_stage(idx, new_stage) {
+                        self.rt_drop.retire(old);
+                        debug!("Replaced stage at index {idx}");
+                    } else {
+                        error!("ReplaceStage: stage index {idx} out of bounds");
+                    }
+                }
+                EngineMessage::AddStage(idx, stage) => {
+                    self.chain.insert_stage(idx, stage);
+                    debug!("Added stage at index {idx}");
+                }
+                EngineMessage::RemoveStage(idx) => {
+                    if let Some(old) = self.chain.remove_stage(idx) {
+                        self.rt_drop.retire(old);
+                        debug!("Removed stage at index {idx}");
+                    } else {
+                        error!("RemoveStage: stage index {idx} out of bounds");
+                    }
+                }
+                EngineMessage::SwapStages(a, b) => {
+                    self.chain.swap_stages(a, b);
+                    debug!("Swapped stages {a} and {b}");
+                }
                 EngineMessage::SetInputFilters(hp, lp) => {
                     self.input_highpass = hp;
                     self.input_lowpass = lp;
@@ -201,7 +240,7 @@ impl Engine {
                     if let Some(ref mut cab) = self.ir_cabinet {
                         debug!("IR convolver swapped: {}", prepared.name);
                         let old = cab.swap_convolver(prepared.convolver);
-                        self.convolver_drop.retire(old);
+                        self.rt_drop.retire(Box::new(old));
                     }
                 }
                 EngineMessage::ClearIr => {
@@ -320,6 +359,26 @@ impl EngineHandle {
         self.send(update);
     }
 
+    pub fn set_parameter(&self, stage_idx: usize, name: &'static str, value: f32) {
+        self.send(EngineMessage::SetParameter(stage_idx, name, value));
+    }
+
+    pub fn replace_stage(&self, idx: usize, stage: Box<dyn Stage>) {
+        self.send(EngineMessage::ReplaceStage(idx, stage));
+    }
+
+    pub fn add_stage(&self, idx: usize, stage: Box<dyn Stage>) {
+        self.send(EngineMessage::AddStage(idx, stage));
+    }
+
+    pub fn remove_stage(&self, idx: usize) {
+        self.send(EngineMessage::RemoveStage(idx));
+    }
+
+    pub fn swap_stages(&self, a: usize, b: usize) {
+        self.send(EngineMessage::SwapStages(a, b));
+    }
+
     pub fn set_amp_chain(&self, new_chain: AmplifierChain) {
         let update = EngineMessage::SetAmpChain(Box::new(new_chain));
         self.send(update);

src/audio/manager.rs

@@ -10,9 +10,10 @@ use crate::audio::engine::Engine;
 use crate::audio::engine::EngineHandle;
 use crate::audio::jack::{NotificationHandler, ProcessHandler};
 use crate::audio::peak_meter::{PeakMeter, PeakMeterHandle};
+use crate::audio::rt_drop::RtDropHandle;
 use crate::audio::samplers::Samplers;
 use crate::ir::cabinet::{ConvolverType, DEFAULT_MAX_IR_MS, IrCabinet};
-use crate::ir::load_service::{self, ConvolverDropHandle, IrLoadHandle};
+use crate::ir::load_service::{self, IrLoadHandle};
 use crate::ir::loader::IrLoader;
 use crate::metronome::Metronome;
 use crate::settings::{AudioSettings, Settings};
@@ -66,25 +67,29 @@ impl Manager {
 
         let ir_cabinet = Some(IrCabinet::new(convolver_type, max_ir_samples));
 
-        let (convolver_drop_handle, convolver_drop_rx) = ConvolverDropHandle::new();
+        let (rt_drop_handle, rt_drop_rx) = RtDropHandle::new();
 
         let (engine, engine_handle) = Engine::new(
             tuner,
             samplers,
             ir_cabinet,
             peak_meter,
             metronome,
-            convolver_drop_handle,
+            rt_drop_handle,
         )?;
 
+        let _rt_drop_thread = std::thread::Builder::new()
+            .name("rt-drop-service".into())
+            .spawn(move || rt_drop_rx.run())
+            .expect("Failed to spawn RT drop service thread");
+
         let ir_load_handle = ir_loader.map(|loader| {
             load_service::spawn(
                 loader,
                 engine_handle.clone(),
                 sample_rate,
                 DEFAULT_MAX_IR_MS,
                 convolver_type,
-                convolver_drop_rx,
             )
         });
 

src/audio/mod.rs

@@ -5,4 +5,5 @@ pub mod peak_meter;
 pub mod pitch_shifter;
 pub mod ports;
 pub mod recorder;
+pub mod rt_drop;
 pub mod samplers;