From 793432bbfc26ae28ebb7e72bc3462857f78ab6b3 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:45:05 +0100 Subject: [PATCH 1/5] Read plugin state on the UI thread, not the audio thread Saving, bouncing and the two-minute autosave read every third-party plugin's state through the audio callback: a command made the callback call each plugin's save_state between two blocks and reply, while the UI waited for up to 2 s. So: - CLAP's clap.state and VST3's getState ran on the audio thread, although both formats want them on the main thread. - The callback allocated and waited on the plugins; a plugin with a large state (a sampler) could make the audio drop out on every save and autosave. - Without a running callback (a stalled or lost device), every save waited 2 s and then kept stale plugin states. Each instance now hands Side::prepare a state reader before it leaves for the audio thread, the way it hands over its editor handle. CLAP's GuiLink and VST3's EditorLink (which now holds the component as well) read the state under their lock; Audio Units get a StateLink that does the same. Each instance cuts its link off before it is destroyed, so a reader never calls into a dead plugin. Player::capture_states reads them on the calling (UI) thread, and the CaptureStates command is gone. Each host's tests read the state while another thread processes the instance, and get nothing once it is dropped. Playback reads states with no audio thread at all. --- crates/au-host/src/ffi.rs | 86 ++++++++++++++++++--------- crates/au-host/src/plugin.rs | 9 +-- crates/au-host/tests/system_units.rs | 29 +++++++++ crates/clap-host/src/ffi.rs | 55 +++++++++++------ crates/clap-host/src/plugin.rs | 9 +-- crates/clap-host/tests/fixture_e2e.rs | 25 ++++++++ crates/dsp/src/lib.rs | 26 ++++++-- crates/engine/src/cmd/plugin_state.rs | 4 +- crates/engine/tests/clap_e2e.rs | 8 +-- crates/mix/src/lib.rs | 43 +++----------- crates/playback/src/lib.rs | 81 ++++++++++++++----------- crates/ui-egui/src/lib.rs | 4 +- crates/vst3-host/src/ffi.rs | 66 ++++++++++++-------- crates/vst3-host/src/plugin.rs | 9 +-- crates/vst3-host/tests/fixture_e2e.rs | 24 ++++++++ 15 files changed, 315 insertions(+), 163 deletions(-) diff --git a/crates/au-host/src/ffi.rs b/crates/au-host/src/ffi.rs index a1152449..f7b801e3 100644 --- a/crates/au-host/src/ffi.rs +++ b/crates/au-host/src/ffi.rs @@ -369,6 +369,7 @@ pub struct Instance { max_frames: usize, sample_rate: f64, sample_time: f64, + link: std::sync::Arc, } // SAFETY: an Audio Unit instance may be used from any thread as long as calls are not @@ -405,6 +406,7 @@ impl Instance { outputs: Vec::new(), abl: AudioBufferList { number_buffers: 0, buffers: [empty; MAX_AU_CHANNELS] }, has_input, + link: std::sync::Arc::new(StateLink(std::sync::Mutex::new(LinkedUnit(Some(unit))))), initialized: false, channels: 0, max_frames: 0, @@ -678,35 +680,12 @@ impl Instance { out } - /// The unit's `ClassInfo` as a binary property list. - pub fn save_state(&self) -> Option> { - let mut plist: CFTypeRef = ptr::null(); - let mut size = std::mem::size_of::() as u32; - // SAFETY: the property is a `CFPropertyListRef` returned +1; `plist` is a local pointer. - let st = unsafe { AudioUnitGetProperty(self.unit, PROP_CLASS_INFO, SCOPE_GLOBAL, 0, (&mut plist as *mut CFTypeRef).cast(), &mut size) }; - if st != 0 || plist.is_null() { - release(plist); - return None; - } - let mut err: CFTypeRef = ptr::null(); - // SAFETY: `plist` is a live property list; `err` is a local out pointer. - let data = unsafe { CFPropertyListCreateData(ptr::null(), plist, CF_PROPERTY_LIST_BINARY_FORMAT, 0, &mut err) }; - release(err); - release(plist); - if data.is_null() { - return None; - } - // SAFETY: `data` is a live CFData we own; its byte pointer is valid for its length. - let out = unsafe { - let len = CFDataGetLength(data); - let p = CFDataGetBytePtr(data); - if len <= 0 || len as usize > MAX_STATE || p.is_null() { None } else { Some(std::slice::from_raw_parts(p, len as usize).to_vec()) } - }; - release(data); - out + /// The link for state handles. + pub fn state_link(&self) -> std::sync::Arc { + std::sync::Arc::clone(&self.link) } - /// Restores a blob from [`Instance::save_state`] (hostile input: parsed by CoreFoundation, + /// Restores a blob from [`StateLink::save_state`] (hostile input: parsed by CoreFoundation, /// must be a dictionary, and the unit validates it). pub fn load_state(&self, bytes: &[u8]) -> bool { if bytes.is_empty() || bytes.len() > MAX_STATE { @@ -733,8 +712,61 @@ impl Instance { } } +struct LinkedUnit(Option); + +// SAFETY: the pointer is only used under the link's mutex, while the instance keeps the unit alive. +unsafe impl Send for LinkedUnit {} + +/// The link between an instance and its state handles: the unit's state is read on the main +/// thread while the instance renders on the audio thread (hosts make property calls from other +/// threads during rendering). The instance clears the link under its lock before it disposes of +/// the unit, so a handle never reads a dead unit. +pub(crate) struct StateLink(std::sync::Mutex); + +impl StateLink { + fn lock(&self) -> std::sync::MutexGuard<'_, LinkedUnit> { + self.0.lock().unwrap_or_else(std::sync::PoisonError::into_inner) + } + + /// The unit's `ClassInfo` as a binary property list (`None` once the instance is gone). + pub fn save_state(&self) -> Option> { + let g = self.lock(); + let unit = g.0?; + let mut plist: CFTypeRef = ptr::null(); + let mut size = std::mem::size_of::() as u32; + // SAFETY: the unit is live while the link holds it (see the type); the property is a + // `CFPropertyListRef` returned +1; `plist` is a local pointer. + let st = unsafe { AudioUnitGetProperty(unit, PROP_CLASS_INFO, SCOPE_GLOBAL, 0, (&mut plist as *mut CFTypeRef).cast(), &mut size) }; + if st != 0 || plist.is_null() { + release(plist); + return None; + } + let mut err: CFTypeRef = ptr::null(); + // SAFETY: `plist` is a live property list; `err` is a local out pointer. + let data = unsafe { CFPropertyListCreateData(ptr::null(), plist, CF_PROPERTY_LIST_BINARY_FORMAT, 0, &mut err) }; + release(err); + release(plist); + if data.is_null() { + return None; + } + // SAFETY: `data` is a live CFData we own; its byte pointer is valid for its length. + let out = unsafe { + let len = CFDataGetLength(data); + let p = CFDataGetBytePtr(data); + if len <= 0 || len as usize > MAX_STATE || p.is_null() { None } else { Some(std::slice::from_raw_parts(p, len as usize).to_vec()) } + }; + release(data); + out + } + + fn kill(&self) { + self.lock().0 = None; + } +} + impl Drop for Instance { fn drop(&mut self) { + self.link.kill(); // SAFETY: the unit is live (disposed only here); after disposal nothing can call the // input callback, so the leaked input state can be freed. unsafe { diff --git a/crates/au-host/src/plugin.rs b/crates/au-host/src/plugin.rs index c76aab00..eed7fc21 100644 --- a/crates/au-host/src/plugin.rs +++ b/crates/au-host/src/plugin.rs @@ -237,10 +237,6 @@ impl Plugin for AuPlugin { self.push_event(MidiEvent { time: 0, status: 0xB0, data1: 123, data2: 0 }); } - fn save_state(&mut self) -> Option> { - self.inst.save_state() - } - fn load_state(&mut self, data: &[u8]) -> bool { let ok = self.inst.load_state(data); if ok { @@ -248,4 +244,9 @@ impl Plugin for AuPlugin { } ok } + + fn state_reader(&mut self) -> Option> { + let link = self.inst.state_link(); + Some(Box::new(move || link.save_state())) + } } diff --git a/crates/au-host/tests/system_units.rs b/crates/au-host/tests/system_units.rs index 2e246758..85c4f080 100644 --- a/crates/au-host/tests/system_units.rs +++ b/crates/au-host/tests/system_units.rs @@ -100,6 +100,35 @@ fn lowpass_attenuates_highs() { assert!(run_tone(p.as_mut(), 100.0).is_finite()); } +#[test] +fn state_reader_reads_while_another_thread_renders() { + if !available(LOWPASS) { + return; + } + let info = soundcraft_au_host::plugin_info(LOWPASS).unwrap(); + let cutoff = info.params.iter().find(|q| q.unit == soundcraft_dsp::Unit::Hz).unwrap(); + let mut p = soundcraft_au_host::create(LOWPASS).unwrap(); + p.prepare(48_000.0, 512, 2); + assert!(p.set_param(cutoff.id, 500.0)); + let mut reader = p.state_reader().expect("every unit has a ClassInfo"); + let audio = std::thread::spawn(move || { + for _ in 0..4 { + run_tone(p.as_mut(), 100.0); + } + p + }); + while !audio.is_finished() { + assert!(reader.save_state().is_some()); + } + let p = audio.join().unwrap(); + let state = reader.save_state().expect("ClassInfo"); + drop(p); + let mut q = soundcraft_au_host::create(LOWPASS).unwrap(); + assert_eq!(reader.save_state(), None); + assert!(q.load_state(&state)); + assert!((q.param(cutoff.id).unwrap() - 500.0).abs() < 1.0, "{:?}", q.param(cutoff.id)); +} + #[test] fn delay_runs_mono_stereo_and_odd_blocks() { if !available(DELAY) { diff --git a/crates/clap-host/src/ffi.rs b/crates/clap-host/src/ffi.rs index 16aaf33f..69cdeac6 100644 --- a/crates/clap-host/src/ffi.rs +++ b/crates/clap-host/src/ffi.rs @@ -275,7 +275,7 @@ impl Bundle { inst.state_ext = inst.extension::(CLAP_EXT_STATE); let gui = inst.extension::(CLAP_EXT_GUI); inst.gui = Arc::new(GuiLink { - inner: Mutex::new(GuiInner { plugin, gui, alive: !gui.is_null(), created: false, visible: false }), + inner: Mutex::new(GuiInner { plugin, gui, state: inst.state_ext, alive: true, created: false, visible: false }), host: Arc::clone(&inst.state), }); Ok(inst) @@ -696,17 +696,19 @@ fn window_api() -> Option<&'static CStr> { struct GuiInner { plugin: *const clap_plugin, gui: *const clap_plugin_gui, + state: *const clap_plugin_state, /// The plugin instance exists (cleared by `Instance::drop` before it destroys the plugin). alive: bool, created: bool, visible: bool, } -/// The link between an instance and its editor handles. The instance may process on the audio -/// thread while the editor is driven from the main thread: CLAP allows `clap.gui` (main-thread) -/// calls concurrently with audio-thread calls. Every GUI call happens under this lock, and the -/// instance clears `alive` (destroying any GUI) under the same lock before it destroys the -/// plugin, so a handle can never call into a destroyed plugin. +/// The link between an instance and its editor and state handles. The instance may process on +/// the audio thread while the editor is driven and the state saved from the main thread: CLAP +/// allows `clap.gui` and `clap.state` (main-thread) calls concurrently with audio-thread calls. +/// Every such call happens under this lock, and the instance clears `alive` (destroying any GUI) +/// under the same lock before it destroys the plugin, so a handle can never call into a destroyed +/// plugin. pub(crate) struct GuiLink { inner: Mutex, host: Arc, @@ -715,7 +717,14 @@ pub(crate) struct GuiLink { impl Default for GuiLink { fn default() -> Self { GuiLink { - inner: Mutex::new(GuiInner { plugin: std::ptr::null(), gui: std::ptr::null(), alive: false, created: false, visible: false }), + inner: Mutex::new(GuiInner { + plugin: std::ptr::null(), + gui: std::ptr::null(), + state: std::ptr::null(), + alive: false, + created: false, + visible: false, + }), host: Arc::new(HostState::default()), } } @@ -731,7 +740,8 @@ impl GuiLink { } pub fn available(&self) -> bool { - self.lock().alive + let g = self.lock(); + g.alive && !g.gui.is_null() } pub fn host(&self) -> &HostState { @@ -805,6 +815,22 @@ impl GuiLink { g.visible = false; } + /// `clap_plugin_state.save` into memory (`None` without the extension, on failure, or once the + /// plugin is gone). + pub fn save_state(&self) -> Option> { + let g = self.lock(); + if !g.alive { + return None; + } + // SAFETY: `state` is null or the plugin's static extension vtable, valid while `alive`. + let save = unsafe { g.state.as_ref() }?.save?; + let mut out: Vec = Vec::new(); + let stream = clap_ostream { ctx: (&mut out as *mut Vec).cast::(), write: Some(ostream_write) }; + // SAFETY: live plugin (`alive`, under the lock); the stream and its context live on this + // frame for the call. + unsafe { save(g.plugin, &stream) }.then_some(out) + } + /// Closes (destroys) the editor. pub fn close(&self) { if !on_main_thread() { @@ -1012,19 +1038,14 @@ impl Instance { self.gui.available().then(|| Arc::clone(&self.gui)) } - /// `clap_plugin_state.save` into memory (`None` without the extension or on failure). - pub fn save_state(&self) -> Option> { - // SAFETY: the extension pointer, when non-null, points to a static vtable in the plugin. - let save = unsafe { self.state_ext.as_ref() }?.save?; - let mut out: Vec = Vec::new(); - let stream = clap_ostream { ctx: (&mut out as *mut Vec).cast::(), write: Some(ostream_write) }; - // SAFETY: valid instance; the stream and its context live on this frame for the call. - unsafe { save(self.plugin, &stream) }.then_some(out) + /// The same link, for state handles (`None` when the plugin has no `clap.state`). + pub fn state_link(&self) -> Option> { + (!self.state_ext.is_null()).then(|| Arc::clone(&self.gui)) } /// `clap_plugin_state.load` from memory. `false` without the extension or when refused. pub fn load_state(&self, data: &[u8]) -> bool { - // SAFETY: see `save_state`. + // SAFETY: the extension pointer, when non-null, points to a static vtable in the plugin. let Some(load) = (unsafe { self.state_ext.as_ref() }).and_then(|s| s.load) else { return false }; let mut cursor = ReadCursor { data, pos: 0 }; let stream = clap_istream { ctx: (&mut cursor as *mut ReadCursor<'_>).cast::(), read: Some(istream_read) }; diff --git a/crates/clap-host/src/plugin.rs b/crates/clap-host/src/plugin.rs index 6fe1abcf..dff5d429 100644 --- a/crates/clap-host/src/plugin.rs +++ b/crates/clap-host/src/plugin.rs @@ -256,10 +256,6 @@ impl Plugin for ClapPlugin { } } - fn save_state(&mut self) -> Option> { - self.inst.save_state() - } - fn load_state(&mut self, data: &[u8]) -> bool { let ok = self.inst.load_state(data); if ok { @@ -283,6 +279,11 @@ impl Plugin for ClapPlugin { let link = self.inst.gui()?; Some(Box::new(ClapEditor { link, info: self.info, ids: self.ids.clone() })) } + + fn state_reader(&mut self) -> Option> { + let link = self.inst.state_link()?; + Some(Box::new(move || link.save_state())) + } } /// A CLAP plugin's floating editor, driven from the main thread (see [`GuiLink`]). diff --git a/crates/clap-host/tests/fixture_e2e.rs b/crates/clap-host/tests/fixture_e2e.rs index 22164875..10cb04b9 100644 --- a/crates/clap-host/tests/fixture_e2e.rs +++ b/crates/clap-host/tests/fixture_e2e.rs @@ -146,11 +146,36 @@ fn state_saves_and_restores_into_a_new_instance() { // Plugins without clap.state. let mut s = soundcraft_clap_host::create(SYNTH).unwrap(); assert!(s.save_state().is_none()); + assert!(s.state_reader().is_none()); assert!(!s.load_state(&blob)); assert!(s.editor().is_none(), "the synth has no GUI"); assert!(s.open_editor().is_err()); } +#[test] +fn state_reader_reads_while_another_thread_processes() { + fixture_dir(); + let mut p = soundcraft_clap_host::create(GAIN).unwrap(); + p.prepare(48_000.0, 64, 2); + assert!(p.set_param("7", 1.5)); + p.process(&mut vec![vec![0.5f32; 64]; 2], 64); + let mut reader = p.state_reader().expect("the gain fixture has clap.state"); + let blob = p.save_state().unwrap(); + let audio = std::thread::spawn(move || { + for _ in 0..2_000 { + p.process(&mut vec![vec![0.5f32; 64]; 2], 64); + } + p + }); + while !audio.is_finished() { + assert_eq!(reader.save_state().as_ref(), Some(&blob)); + } + let p = audio.join().unwrap(); + assert_eq!(reader.save_state(), Some(blob)); + drop(p); + assert_eq!(reader.save_state(), None); +} + #[test] fn editors_refuse_to_open_off_the_main_thread() { fixture_dir(); diff --git a/crates/dsp/src/lib.rs b/crates/dsp/src/lib.rs index 3d5bd331..f0ca05ad 100644 --- a/crates/dsp/src/lib.rs +++ b/crates/dsp/src/lib.rs @@ -206,11 +206,12 @@ pub trait Plugin: Send { fn set_sample(&mut self, _sample: Option>) -> bool { false } - /// The plugin's complete internal state as an opaque blob (third-party plugins), for saving - /// in the session. `None` when the plugin has no state beyond its parameters (built-ins) or - /// the plugin refused. May allocate: never call it in steady-state audio processing. + /// The plugin's complete internal state as an opaque blob (third-party plugins), as its + /// [`Plugin::state_reader`] reads it. `None` when the plugin has no state beyond its + /// parameters (built-ins) or the plugin refused. May allocate: never call it in steady-state + /// audio processing. fn save_state(&mut self) -> Option> { - None + self.state_reader()?.save_state() } /// Restores a blob from [`Plugin::save_state`]. `false` if unsupported or the plugin rejected /// it (the plugin keeps working with its previous state). The data is hostile input. @@ -229,6 +230,10 @@ pub trait Plugin: Send { fn editor(&mut self) -> Option> { None } + /// Reads this instance's state from the UI thread (`None` when it has no state of its own). + fn state_reader(&mut self) -> Option> { + None + } } /// A plugin's editor (GUI), driven from the main (UI) thread. Obtained from @@ -249,6 +254,19 @@ pub trait PluginEditor: Send { fn set_param(&mut self, _id: &str, _value: f32) {} } +/// Reads a plugin's state on the main (UI) thread while the instance processes on the audio +/// thread, as plugin formats allow. Obtained from [`Plugin::state_reader`]; closures are readers. +pub trait PluginStateReader: Send { + /// What [`Plugin::save_state`] would return; `None` once the instance is gone. + fn save_state(&mut self) -> Option>; +} + +impl Option> + Send> PluginStateReader for F { + fn save_state(&mut self) -> Option> { + self() + } +} + /// Recorded audio a sample-playing instrument reads: planar `f32` at its own sample rate (the /// instrument converts rates as it plays). Implementations must be cheap to read from the audio /// thread: no locks, no I/O. diff --git a/crates/engine/src/cmd/plugin_state.rs b/crates/engine/src/cmd/plugin_state.rs index b3ca0d3d..e999c863 100644 --- a/crates/engine/src/cmd/plugin_state.rs +++ b/crates/engine/src/cmd/plugin_state.rs @@ -1,6 +1,6 @@ //! Third-party plugin state in the session (`mix.insert_state`): the UI reads each live plugin's -//! state from the audio engine right before saving and stores it on its insert (base64), from -//! where the mixer restores it into new instances. +//! state right before saving and stores it on its insert (base64), from where the mixer restores +//! it into new instances. use super::*; use serde_json::json; diff --git a/crates/engine/tests/clap_e2e.rs b/crates/engine/tests/clap_e2e.rs index 9a559f18..404252c0 100644 --- a/crates/engine/tests/clap_e2e.rs +++ b/crates/engine/tests/clap_e2e.rs @@ -92,13 +92,11 @@ fn clap_state_is_captured_stored_and_restored_before_the_first_block() { p.set_param("7", 1.5); let mut io = vec![vec![0.0f32; 64]]; p.process(&mut io, 64); + let mut reader = p.state_reader().unwrap(); eng.adopt(vec![soundcraft_mix::PreparedInstance { track: t, slot: 0, id: gain.into(), ch: 1, plugin: p }]); eng.ensure_synced(e.session()); - let states = eng.capture_states(); - assert_eq!(states.len(), 1); - let (tt, slot, blob) = &states[0]; - assert_eq!((*tt, *slot), (t, 0)); - e.execute("mix.insert_state", &json!({"track": t.0, "slot": slot, "state": soundcraft_model::b64::encode(blob)})).unwrap(); + let blob = reader.save_state().unwrap(); + e.execute("mix.insert_state", &json!({"track": t.0, "slot": 0, "state": soundcraft_model::b64::encode(&blob)})).unwrap(); assert_eq!(eng.take_retired().len(), 0); // Offline render (synchronous creation): the state is restored before the first block. diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index 82690ab3..87a35643 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -39,9 +39,8 @@ //! //! An instance starts from its stored state; the insert's parameter values are then applied on //! top before the first block (the session's values are kept in step with the plugin, including -//! edits made in its own editor, so both agree; the state carries everything else). -//! [`MixEngine::capture_states`] reads every live instance's state back (for saving the session); -//! it calls into the plugins and allocates, so a realtime host runs it only on request. +//! edits made in its own editor, so both agree; the state carries everything else). A host reads +//! states back for saving through each instance's [`Plugin::state_reader`], on its UI thread. //! //! A built-in instrument's parameter values are applied the same way. A sample-playing //! instrument (the Sampler) is handed the decoded audio of its insert's @@ -71,8 +70,8 @@ pub fn main_channels(s: &Session) -> usize { s.main_format().channels().clamp(MAIN_CHANNELS, MAX_CHANNELS) } -/// Slot index used for a track's instrument plugin in [`InstanceSpec`], [`PreparedInstance`] and -/// [`MixEngine::capture_states`] (insert slots use their index). +/// Slot index used for a track's instrument plugin in [`InstanceSpec`] and [`PreparedInstance`] +/// (insert slots use their index). pub const INSTRUMENT_SLOT: usize = usize::MAX; /// Hosted third-party plugin ids (CLAP or VST3): created off the audio thread in realtime use. @@ -431,24 +430,6 @@ impl MixEngine { out } - /// The state of every live plugin that has one (third-party plugins), as - /// `(track, slot or INSTRUMENT_SLOT, state)`. Calls into the plugins and allocates. - pub fn capture_states(&mut self) -> Vec<(TrackId, usize, Vec)> { - let mut out = Vec::new(); - for (tid, st) in self.strips.iter_mut() { - for (i, slot) in st.plugins.iter_mut().enumerate() { - if let Some(data) = slot.as_mut().and_then(|s| s.plugin.save_state()) { - out.push((*tid, i, data)); - } - } - if let Some(data) = st.instrument.as_mut().and_then(|s| s.plugin.save_state()) { - out.push((*tid, INSTRUMENT_SLOT, data)); - } - } - out.sort_by_key(|(t, s, _)| (*t, *s)); - out - } - pub fn max_block(&self) -> usize { self.max_block } @@ -2136,10 +2117,9 @@ mod tests { let _ = render_range(&s, Range::new(0, 4800), 512); } - /// A built-in gain behind a third-party id, with a state blob and a log of parameter sets. + /// A built-in gain behind a third-party id, with a log of parameter sets. struct Stateful { inner: Box, - blob: Vec, sets: Arc, } @@ -2163,9 +2143,6 @@ mod tests { fn process(&mut self, io: &mut [Vec], frames: usize) { self.inner.process(io, frames); } - fn save_state(&mut self) -> Option> { - Some(self.blob.clone()) - } } fn render_block(eng: &mut MixEngine, s: &Session, pos: i64) -> f32 { @@ -2175,7 +2152,7 @@ mod tests { } #[test] - fn external_instances_are_adopted_retired_and_captured() { + fn external_instances_are_adopted_and_retired() { let (mut s, t) = session_with_dc(0.5, 48_000); let mut ins = Insert::new("clap:test.fake"); ins.params.insert("gain".into(), -6.0); @@ -2191,9 +2168,9 @@ mod tests { let sets = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let mut gain = soundcraft_dsp::create("gain").unwrap(); gain.prepare(48_000.0, 512, 1); - let fake = Stateful { inner: gain, blob: b"live state".to_vec(), sets: Arc::clone(&sets) }; + let fake = Stateful { inner: gain, sets: Arc::clone(&sets) }; // An instance for the wrong channel count is retired, not used. - let wrong = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), blob: Vec::new(), sets: Arc::clone(&sets) }; + let wrong = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), sets: Arc::clone(&sets) }; eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "clap:test.fake".into(), ch: 2, plugin: Box::new(wrong) }]); assert!(render_block(&mut eng, &s, 512) > 0.3); assert_eq!(eng.take_retired().len(), 1); @@ -2204,13 +2181,11 @@ mod tests { } assert!((wet / dry - 0.501).abs() < 0.01, "{wet} vs {dry}"); assert!(sets.load(std::sync::atomic::Ordering::Relaxed) > 0, "no stored state: session params are pushed"); - assert_eq!(eng.capture_states(), vec![(t, 0, b"live state".to_vec())]); assert!(eng.take_retired().is_empty()); // Removing the insert retires the instance instead of dropping it here. s.track_mut(t).unwrap().mixer.inserts[0] = None; render_block(&mut eng, &s, 20 * 512); assert_eq!(eng.take_retired().len(), 1); - assert!(eng.capture_states().is_empty()); // Instances nobody claims are retired too; drain_plugins empties the engine. eng.adopt(vec![PreparedInstance { track: t, slot: 3, id: "clap:x".into(), ch: 1, plugin: soundcraft_dsp::create("gain").unwrap() }]); render_block(&mut eng, &s, 21 * 512); @@ -2229,7 +2204,7 @@ mod tests { let mut eng = MixEngine::new(48_000.0, 512); eng.set_external_instances(true); let sets = Arc::new(std::sync::atomic::AtomicUsize::new(0)); - let fake = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), blob: Vec::new(), sets: Arc::clone(&sets) }; + let fake = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), sets: Arc::clone(&sets) }; eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "vst3:00000000000000000000000000000001".into(), ch: 1, plugin: Box::new(fake) }]); render_block(&mut eng, &s, 0); assert_eq!(sets.load(std::sync::atomic::Ordering::Relaxed), 1, "session params apply once"); diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index bd63c382..c079eced 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -16,15 +16,14 @@ //! state and prepared on the thread that calls [`Player::update_session`] (the UI thread), and //! travel to the audio thread together with the session snapshot that needs them; the mix //! engine adopts them (see `soundcraft_mix`'s crate docs). Instances the engine retires come back -//! on a return channel and are dropped on the UI thread ([`Player::idle`]). Plugin states for -//! saving are read on request ([`Player::capture_states`]: a command with a reply; the audio -//! thread answers between two blocks, never blocking). Plugin editors are driven on the UI thread +//! on a return channel and are dropped on the UI thread ([`Player::idle`]). Plugin editors are +//! driven, and plugin states for saving read ([`Player::capture_states`]), on the UI thread //! through handles taken from each instance before it leaves ([`Player::open_editor`]). #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] pub mod record; -use soundcraft_dsp::{Plugin, PluginEditor}; +use soundcraft_dsp::{Plugin, PluginEditor, PluginStateReader}; use soundcraft_mix::{MAX_CHANNELS, MixEngine, PreparedInstance, StripMeter, itu_stereo_fold, main_channels}; use soundcraft_model::{Session, TrackId}; use soundcraft_time::{Range, Samples}; @@ -55,8 +54,6 @@ pub type EditorEdit = (TrackId, usize, String, f32); enum Cmd { /// A new snapshot plus the third-party instances it needs (created on the UI thread). Session(Arc, Vec), - /// Read every plugin state and reply. - CaptureStates(Sender), Input(Arc), Recording(bool), Play { @@ -136,10 +133,6 @@ impl AudioState { match c { Cmd::Input(r) => self.input = Some(r), Cmd::Recording(r) => self.recording = r, - Cmd::CaptureStates(reply) => { - self.mix.ensure_synced(&self.session); - let _ = reply.send(self.mix.capture_states()); - } Cmd::Session(s, instances) => { changed = true; if (s.sample_rate.as_f64() - self.session.sample_rate.as_f64()).abs() > 0.5 { @@ -180,8 +173,8 @@ impl AudioState { } } if changed { - // Place adopted instances now (not only once playback starts), so state captures and - // editors see them; `sync` creates no third-party plugins here. + // Place adopted instances now (not only once playback starts), so editors see them; + // `sync` creates no third-party plugins here. self.mix.ensure_synced(&self.session); } self.send_garbage(); @@ -325,10 +318,16 @@ struct Side { rate: f32, /// Editor handles of the live instances. editors: HashMap<(TrackId, usize), Box>, + /// State readers of the live instances. + states: HashMap<(TrackId, usize), Box>, garbage: Receiver>>, } impl Side { + fn new(rate: f32, garbage: Receiver>>) -> Side { + Side { shadow: HashMap::new(), rate, editors: HashMap::new(), states: HashMap::new(), garbage } + } + /// Drops retired instances (closing their editors first, inside the plugin's drop). fn collect_garbage(&mut self) { while let Ok(g) = self.garbage.try_recv() { @@ -338,6 +337,7 @@ impl Side { fn forget(&mut self, key: (TrackId, usize)) { self.shadow.remove(&key); + self.states.remove(&key); if let Some(mut e) = self.editors.remove(&key) { e.close(); } @@ -376,7 +376,7 @@ impl Player { fn open(session: Arc) -> Player { let (gtx, grx) = channel(); - let side = Mutex::new(Side { shadow: HashMap::new(), rate: session.sample_rate.as_f64() as f32, editors: HashMap::new(), garbage: grx }); + let side = Mutex::new(Side::new(session.sample_rate.as_f64() as f32, grx)); let shared = Arc::new(Shared { position: AtomicI64::new(0), playing: AtomicBool::new(false), @@ -525,22 +525,9 @@ impl Player { let _ = self.tx.send(Cmd::Session(s, instances)); } - /// Reads the state of every live third-party plugin (for saving the session). Waits briefly - /// for the audio thread to answer between two blocks; empty when it does not answer. + /// Reads the state of every live third-party plugin, for saving the session. pub fn capture_states(&self) -> PluginStates { - self.side().collect_garbage(); - #[cfg(not(target_arch = "wasm32"))] - { - let (tx, rx) = channel(); - if self.tx.send(Cmd::CaptureStates(tx)).is_err() { - return Vec::new(); - } - rx.recv_timeout(std::time::Duration::from_secs(2)).unwrap_or_default() - } - #[cfg(target_arch = "wasm32")] - { - Vec::new() - } + self.side().capture_states() } /// Per-UI-frame housekeeping: drops retired instances and services plugin editors. Returns @@ -656,6 +643,12 @@ fn copy_meter_snapshot(source: &MeterSnapshot, out: &mut MeterSnapshot) { } impl Side { + fn capture_states(&mut self) -> PluginStates { + let mut out: PluginStates = self.states.iter_mut().filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))).collect(); + out.sort_by_key(|(t, slot, _)| (*t, *slot)); + out + } + /// Creates the third-party instances `s` needs that the audio thread does not have yet. fn prepare(&mut self, s: &Session) -> Vec { self.collect_garbage(); @@ -686,6 +679,9 @@ impl Side { if let Some(e) = plugin.editor() { self.editors.insert(key, e); } + if let Some(r) = plugin.state_reader() { + self.states.insert(key, r); + } out.push(PreparedInstance { track: spec.track, slot: spec.slot, id: spec.id, ch: spec.ch, plugin }); } } @@ -881,12 +877,8 @@ mod tests { let mut st = AudioState::new(rx, Arc::clone(&shared), Arc::new(soundcraft_model::Session::default()), 48_000.0, gtx); let plugin: Box = Box::new(Stateful(soundcraft_dsp::create("gain").unwrap())); tx.send(Cmd::Session(Arc::clone(&s), vec![PreparedInstance { track: t, slot: 1, id: "clap:test.fake".into(), ch: 2, plugin }])).unwrap(); - let (rtx, rrx) = channel(); - tx.send(Cmd::CaptureStates(rtx)).unwrap(); - // Stopped: the callback only drains commands (adopting the instance) and answers. let mut out = vec![0.0f32; 256]; st.fill(&mut out, 2); - assert_eq!(rrx.try_recv().unwrap(), vec![(t, 1, b"blob".to_vec())]); assert!(grx.try_recv().is_err(), "nothing retired yet"); // A session without the insert retires the instance to the return channel. tx.send(Cmd::Session(Arc::new(soundcraft_model::Session::default()), Vec::new())).unwrap(); @@ -912,11 +904,32 @@ mod tests { assert_eq!(grx.try_recv().unwrap().len(), 1); } + struct Reads(Option<&'static [u8]>); + + impl PluginStateReader for Reads { + fn save_state(&mut self) -> Option> { + self.0.map(<[u8]>::to_vec) + } + } + + #[test] + fn plugin_states_are_read_without_the_audio_thread() { + let mut side = Side::new(48_000.0, channel().1); + let (a, b) = (TrackId(1), TrackId(2)); + side.states.insert((b, 0), Box::new(Reads(Some(b"b0")))); + side.states.insert((a, soundcraft_mix::INSTRUMENT_SLOT), Box::new(Reads(Some(b"synth")))); + side.states.insert((a, 3), Box::new(Reads(Some(b"a3")))); + side.states.insert((a, 4), Box::new(Reads(None))); + let want = vec![(a, 3, b"a3".to_vec()), (a, soundcraft_mix::INSTRUMENT_SLOT, b"synth".to_vec()), (b, 0, b"b0".to_vec())]; + assert_eq!(side.capture_states(), want); + side.forget((a, 3)); + assert_eq!(side.capture_states(), want[1..]); + } + #[test] fn side_creates_each_missing_instance_once() { use soundcraft_model::{ChannelFormat, Insert, TrackKind}; - let (_gtx, grx) = channel(); - let mut side = Side { shadow: HashMap::new(), rate: 48_000.0, editors: HashMap::new(), garbage: grx }; + let mut side = Side::new(48_000.0, channel().1); let mut s = soundcraft_model::Session::default(); let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, None); s.track_mut(t).unwrap().mixer.inserts[0] = Some(Insert::new("clap:does.not.exist")); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 5c4a7514..096f143b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -485,8 +485,8 @@ impl SoundApp { r.arm(plan) } - /// Reads every live third-party plugin's state from the audio engine and stores it on its - /// insert (`mix.insert_state`), so saves and bounces include it. + /// Reads every live third-party plugin's state and stores it on its insert + /// (`mix.insert_state`), so saves and bounces include it. pub fn capture_plugin_states(&mut self) { let Some(p) = &self.player else { return }; for (t, slot, data) in p.capture_states() { diff --git a/crates/vst3-host/src/ffi.rs b/crates/vst3-host/src/ffi.rs index 341ad534..d73d8c9f 100644 --- a/crates/vst3-host/src/ffi.rs +++ b/crates/vst3-host/src/ffi.rs @@ -571,8 +571,10 @@ impl Bundle { // SAFETY: valid controller; the handler (host context) outlives it. unsafe { c.setComponentHandler(h.as_ptr()) }; } - inst.editor = Arc::new(EditorLink { inner: Mutex::new(EditorInner { controller: Some(c.clone()), ..EditorInner::default() }) }); } + inst.editor = Arc::new(EditorLink { + inner: Mutex::new(EditorInner { component: Some(inst.component.clone()), controller: inst.controller.clone(), ..EditorInner::default() }), + }); Ok(inst) } } @@ -1123,6 +1125,11 @@ impl Instance { self.editor.has_controller().then(|| Arc::clone(&self.editor)) } + /// The same link, for state handles. + pub fn state_link(&self) -> Arc { + Arc::clone(&self.editor) + } + /// Writes one state into a fresh memory stream. fn read_state(f: impl FnOnce(*mut sb::IBStream) -> sb::tresult) -> Option> { let stream = MemoryStream::new(Vec::new()); @@ -1130,26 +1137,7 @@ impl Instance { ok(f(s.as_ptr())).then(|| stream.take()) } - /// The component state plus the controller state, in SoundCraft's container (see - /// `STATE_MAGIC`). `None` when the component refuses `getState`. - pub fn save_state(&self) -> Option> { - // SAFETY: valid component; the stream lives in `read_state` for the whole call. - let comp = Instance::read_state(|s| unsafe { self.component.getState(s) })?; - let ctrl = match &self.controller { - // SAFETY: valid controller; as above. - Some(c) => Instance::read_state(|s| unsafe { c.getState(s) }).unwrap_or_default(), - None => Vec::new(), - }; - let mut out = Vec::with_capacity(12 + comp.len() + ctrl.len()); - out.extend_from_slice(STATE_MAGIC); - out.extend_from_slice(&u32::try_from(comp.len()).ok()?.to_le_bytes()); - out.extend_from_slice(&comp); - out.extend_from_slice(&u32::try_from(ctrl.len()).ok()?.to_le_bytes()); - out.extend_from_slice(&ctrl); - Some(out) - } - - /// Restores a [`Instance::save_state`] blob (or a bare component state): component + /// Restores a [`EditorLink::save_state`] blob (or a bare component state): component /// `setState`, then the controller's `setComponentState` and `setState`. `false` when the /// blob is malformed or the component refuses it. pub fn load_state(&self, data: &[u8]) -> bool { @@ -1545,17 +1533,20 @@ fn rect_size(r: &sb::ViewRect) -> (f64, f64) { #[derive(Default)] struct EditorInner { /// `None` once the instance is gone. + component: Option>, + /// `None` once the instance is gone, or without an edit controller. controller: Option>, view: Option>, frame: Option>, window: Option, } -/// The link between an instance and its editor handles. The instance may process on the audio -/// thread while the editor (the edit controller's `IPlugView`) is driven from the main thread, -/// which is VST3's own threading model (controller and views on the UI thread). All view calls -/// happen under this lock, and the instance closes the view and drops the controller reference -/// under the same lock before it terminates, so a handle never calls into a dead plugin. +/// The link between an instance and its editor and state handles. The instance may process on +/// the audio thread while the editor (the edit controller's `IPlugView`) is driven and the state +/// read from the main thread, which is VST3's own threading model (controller, views and +/// `getState` on the UI thread). All such calls happen under this lock, and the instance closes +/// the view and drops the component and controller references under the same lock before it +/// terminates, so a handle never calls into a dead plugin. #[derive(Default)] pub(crate) struct EditorLink { inner: Mutex, @@ -1657,6 +1648,28 @@ impl EditorLink { } } + /// The component state plus the controller state, in SoundCraft's container (see + /// `STATE_MAGIC`). `None` when the component refuses `getState` or the instance is gone. + pub fn save_state(&self) -> Option> { + let g = self.lock(); + let component = g.component.as_ref()?; + // SAFETY: valid component (the instance clears it under this lock before it terminates); + // the stream lives in `read_state` for the whole call. + let comp = Instance::read_state(|s| unsafe { component.getState(s) })?; + let ctrl = match &g.controller { + // SAFETY: valid controller; as above. + Some(c) => Instance::read_state(|s| unsafe { c.getState(s) }).unwrap_or_default(), + None => Vec::new(), + }; + let mut out = Vec::with_capacity(12 + comp.len() + ctrl.len()); + out.extend_from_slice(STATE_MAGIC); + out.extend_from_slice(&u32::try_from(comp.len()).ok()?.to_le_bytes()); + out.extend_from_slice(&comp); + out.extend_from_slice(&u32::try_from(ctrl.len()).ok()?.to_le_bytes()); + out.extend_from_slice(&ctrl); + Some(out) + } + /// Closes the editor. Main thread only (no-op elsewhere). pub fn close(&self) { if window::on_main_thread() { @@ -1739,6 +1752,7 @@ impl EditorLink { } } } + g.component = None; g.controller = None; leaked } diff --git a/crates/vst3-host/src/plugin.rs b/crates/vst3-host/src/plugin.rs index 254671f5..05a3e0ea 100644 --- a/crates/vst3-host/src/plugin.rs +++ b/crates/vst3-host/src/plugin.rs @@ -318,10 +318,6 @@ impl Plugin for Vst3Plugin { } } - fn save_state(&mut self) -> Option> { - self.inst.save_state() - } - fn load_state(&mut self, data: &[u8]) -> bool { let ok = self.inst.load_state(data); if ok { @@ -344,6 +340,11 @@ impl Plugin for Vst3Plugin { let link = self.inst.editor()?; Some(Box::new(Vst3Editor { link, host: self.inst.host_ref(), info: self.info, ids: self.ids.clone(), maps: self.maps.clone() })) } + + fn state_reader(&mut self) -> Option> { + let link = self.inst.state_link(); + Some(Box::new(move || link.save_state())) + } } /// A VST3 plugin's editor (`IPlugView` in a host window), driven from the main thread. diff --git a/crates/vst3-host/tests/fixture_e2e.rs b/crates/vst3-host/tests/fixture_e2e.rs index e3603b7a..f98b37fc 100644 --- a/crates/vst3-host/tests/fixture_e2e.rs +++ b/crates/vst3-host/tests/fixture_e2e.rs @@ -208,6 +208,30 @@ fn state_saves_component_and_controller_and_restores_them() { assert_eq!(q.param("7"), Some(1.0)); } +#[test] +fn state_reader_reads_while_another_thread_processes() { + let id = id_of("Fixture Gain"); + let mut p = soundcraft_vst3_host::create(&id).unwrap(); + p.prepare(48_000.0, 64, 2); + assert!(p.set_param("7", 1.5)); + p.process(&mut vec![vec![0.5f32; 64]; 2], 64); + let mut reader = p.state_reader().expect("every VST3 component has a state"); + let blob = p.save_state().unwrap(); + let audio = std::thread::spawn(move || { + for _ in 0..2_000 { + p.process(&mut vec![vec![0.5f32; 64]; 2], 64); + } + p + }); + while !audio.is_finished() { + assert_eq!(reader.save_state().as_ref(), Some(&blob)); + } + let p = audio.join().unwrap(); + assert_eq!(reader.save_state(), Some(blob)); + drop(p); + assert_eq!(reader.save_state(), None); +} + #[test] fn editor_view_is_probed_and_edits_come_back_as_plain_values() { let id = id_of("Fixture Gain"); From 4c21ff8fe477b76e22b9bfc3098a1227a0dce184 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:47:59 +0100 Subject: [PATCH 2/5] Keep a plugin's saved settings when it refuses to load them When a third-party plugin refused its stored state (a newer plugin version that can't read it, a damaged blob), it ran with its defaults, and the next save or two-minute autosave read its state back into the session: the saved settings were gone, even after updating the plugin. create_instance now says whether the plugin refused the state (NewInstance::refused_state). Playback leaves such an instance out of the states it reads for saving, so the session keeps the stored state, until the plugin's own editor is opened: from then on its state is the user's work. Parameter values are saved apart from the state, so edits made with SoundCraft's own controls survive either way. --- crates/engine/tests/clap_e2e.rs | 13 +++- crates/mix/src/lib.rs | 22 ++++--- crates/playback/src/lib.rs | 110 +++++++++++++++----------------- 3 files changed, 79 insertions(+), 66 deletions(-) diff --git a/crates/engine/tests/clap_e2e.rs b/crates/engine/tests/clap_e2e.rs index 404252c0..cd554a9d 100644 --- a/crates/engine/tests/clap_e2e.rs +++ b/crates/engine/tests/clap_e2e.rs @@ -69,6 +69,17 @@ fn clap_insert_renders_through_the_mix() { assert!(e.execute("mix.insert_param", &json!({"track": t.0, "slot": 0, "param": "Gain", "value": 1.0})).is_err()); } +#[test] +fn a_refused_state_is_reported() { + install_fixture(); + let gain = "clap:org.soundcraft.test.gain"; + let make = |state: Option<&[u8]>| soundcraft_mix::create_instance(gain, state, 48_000.0, 512, 2).unwrap(); + let saved = make(None).plugin.save_state().unwrap(); + assert!(!make(None).refused_state); + assert!(!make(Some(&saved)).refused_state); + assert!(make(Some(&[0xff; 16])).refused_state, "NaN gain"); +} + #[test] fn clap_state_is_captured_stored_and_restored_before_the_first_block() { install_fixture(); @@ -88,7 +99,7 @@ fn clap_state_is_captured_stored_and_restored_before_the_first_block() { eng.set_external_instances(true); let specs = soundcraft_mix::instance_specs(e.session()); assert_eq!(specs.len(), 1); - let mut p = soundcraft_mix::create_instance(&specs[0].id, None, 48_000.0, 512, specs[0].ch).unwrap(); + let mut p = soundcraft_mix::create_instance(&specs[0].id, None, 48_000.0, 512, specs[0].ch).unwrap().plugin; p.set_param("7", 1.5); let mut io = vec![vec![0.0f32; 64]]; p.process(&mut io, 64); diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index 87a35643..a145422c 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -127,17 +127,23 @@ pub fn instance_specs(s: &Session) -> Vec { out } +/// A plugin made by [`create_instance`]. +pub struct NewInstance { + pub plugin: Box, + /// The plugin refused the stored state and starts from its defaults. + pub refused_state: bool, +} + /// Creates a plugin (built-in or hosted), restores `state` (if any) and prepares it. Expensive /// for third-party plugins: in realtime use, call it on the control thread. -pub fn create_instance(id: &str, state: Option<&[u8]>, sample_rate: f32, max_block: usize, ch: usize) -> Option> { - let mut p = create_plugin(id)?; - if let Some(st) = state - && !p.load_state(st) - { +pub fn create_instance(id: &str, state: Option<&[u8]>, sample_rate: f32, max_block: usize, ch: usize) -> Option { + let mut plugin = create_plugin(id)?; + let refused_state = state.is_some_and(|st| !plugin.load_state(st)); + if refused_state { log::warn!("{id}: the plugin did not accept its stored state"); } - p.prepare(sample_rate, max_block, ch); - Some(p) + plugin.prepare(sample_rate, max_block, ch); + Some(NewInstance { plugin, refused_state }) } /// Peak meter values for one strip (linear, per channel, max over the last block). @@ -357,7 +363,7 @@ impl SlotCtx<'_> { } } else { let state = if third { ins.state_bytes() } else { None }; - create_instance(&ins.plugin, state.as_deref(), self.sr, self.mb, ch) + create_instance(&ins.plugin, state.as_deref(), self.sr, self.mb, ch).map(|n| n.plugin) }?; Some(PluginSlot { id: ins.plugin.clone(), ch, plugin, applied: Vec::new() }) } diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index c079eced..6a7df8aa 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -27,7 +27,7 @@ use soundcraft_dsp::{Plugin, PluginEditor, PluginStateReader}; use soundcraft_mix::{MAX_CHANNELS, MixEngine, PreparedInstance, StripMeter, itu_stereo_fold, main_channels}; use soundcraft_model::{Session, TrackId}; use soundcraft_time::{Range, Samples}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::atomic::{AtomicBool, AtomicI64, AtomicU32, Ordering}; use std::sync::mpsc::{Receiver, Sender, channel}; use std::sync::{Arc, Mutex, PoisonError}; @@ -319,13 +319,16 @@ struct Side { /// Editor handles of the live instances. editors: HashMap<(TrackId, usize), Box>, /// State readers of the live instances. - states: HashMap<(TrackId, usize), Box>, + states: BTreeMap<(TrackId, usize), Box>, + /// Instances that refused their stored state: saves keep that state until the plugin's editor + /// is opened. + refused: HashSet<(TrackId, usize)>, garbage: Receiver>>, } impl Side { fn new(rate: f32, garbage: Receiver>>) -> Side { - Side { shadow: HashMap::new(), rate, editors: HashMap::new(), states: HashMap::new(), garbage } + Side { shadow: HashMap::new(), rate, editors: HashMap::new(), states: BTreeMap::new(), refused: HashSet::new(), garbage } } /// Drops retired instances (closing their editors first, inside the plugin's drop). @@ -338,6 +341,7 @@ impl Side { fn forget(&mut self, key: (TrackId, usize)) { self.shadow.remove(&key); self.states.remove(&key); + self.refused.remove(&key); if let Some(mut e) = self.editors.remove(&key) { e.close(); } @@ -551,10 +555,7 @@ impl Player { /// Opens the plugin's own editor window. Main thread only. pub fn open_editor(&self, track: TrackId, slot: usize) -> Result<(), String> { - match self.side().editors.get_mut(&(track, slot)) { - Some(e) => e.open(), - None => Err("this plugin has no editor (or it is not loaded)".into()), - } + self.side().open_editor((track, slot)) } pub fn close_editor(&self, track: TrackId, slot: usize) { @@ -644,9 +645,14 @@ fn copy_meter_snapshot(source: &MeterSnapshot, out: &mut MeterSnapshot) { impl Side { fn capture_states(&mut self) -> PluginStates { - let mut out: PluginStates = self.states.iter_mut().filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))).collect(); - out.sort_by_key(|(t, slot, _)| (*t, *slot)); - out + self.states.iter_mut().filter(|(k, _)| !self.refused.contains(*k)).filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))).collect() + } + + fn open_editor(&mut self, key: (TrackId, usize)) -> Result<(), String> { + let e = self.editors.get_mut(&key).ok_or_else(|| "this plugin has no editor (or it is not loaded)".to_string())?; + e.open()?; + self.refused.remove(&key); + Ok(()) } /// Creates the third-party instances `s` needs that the audio thread does not have yet. @@ -675,13 +681,16 @@ impl Side { self.forget(key); let made = soundcraft_mix::create_instance(&spec.id, spec.state.as_deref(), sr, BLOCK, spec.ch); self.shadow.insert(key, (spec.id.clone(), spec.ch, made.is_some())); - if let Some(mut plugin) = made { + if let Some(soundcraft_mix::NewInstance { mut plugin, refused_state }) = made { if let Some(e) = plugin.editor() { self.editors.insert(key, e); } if let Some(r) = plugin.state_reader() { self.states.insert(key, r); } + if refused_state { + self.refused.insert(key); + } out.push(PreparedInstance { track: spec.track, slot: spec.slot, id: spec.id, ch: spec.ch, plugin }); } } @@ -833,32 +842,6 @@ mod tests { assert_eq!(shared.meters.lock().unwrap().main.peaks.len(), 6); } - struct Stateful(Box); - - impl Plugin for Stateful { - fn info(&self) -> &'static soundcraft_dsp::PluginInfo { - self.0.info() - } - fn prepare(&mut self, sr: f32, mb: usize, ch: usize) { - self.0.prepare(sr, mb, ch); - } - fn reset(&mut self) { - self.0.reset(); - } - fn set_param(&mut self, id: &str, v: f32) -> bool { - self.0.set_param(id, v) - } - fn param(&self, id: &str) -> Option { - self.0.param(id) - } - fn process(&mut self, io: &mut [Vec], frames: usize) { - self.0.process(io, frames); - } - fn save_state(&mut self) -> Option> { - Some(b"blob".to_vec()) - } - } - #[test] fn instances_arrive_with_the_session_and_leave_through_the_return_channel() { use soundcraft_model::{ChannelFormat, Insert, TrackKind}; @@ -875,7 +858,7 @@ mod tests { let (tx, rx) = channel(); let (gtx, grx) = channel(); let mut st = AudioState::new(rx, Arc::clone(&shared), Arc::new(soundcraft_model::Session::default()), 48_000.0, gtx); - let plugin: Box = Box::new(Stateful(soundcraft_dsp::create("gain").unwrap())); + let plugin = soundcraft_dsp::create("gain").unwrap(); tx.send(Cmd::Session(Arc::clone(&s), vec![PreparedInstance { track: t, slot: 1, id: "clap:test.fake".into(), ch: 2, plugin }])).unwrap(); let mut out = vec![0.0f32; 256]; st.fill(&mut out, 2); @@ -887,13 +870,7 @@ mod tests { // A sample-rate change rebuilds the engine without dropping plugins on this thread. tx.send(Cmd::Session( Arc::clone(&s), - vec![PreparedInstance { - track: t, - slot: 1, - id: "clap:test.fake".into(), - ch: 2, - plugin: Box::new(Stateful(soundcraft_dsp::create("gain").unwrap())), - }], + vec![PreparedInstance { track: t, slot: 1, id: "clap:test.fake".into(), ch: 2, plugin: soundcraft_dsp::create("gain").unwrap() }], )) .unwrap(); st.fill(&mut out, 2); @@ -904,28 +881,47 @@ mod tests { assert_eq!(grx.try_recv().unwrap().len(), 1); } - struct Reads(Option<&'static [u8]>); - - impl PluginStateReader for Reads { - fn save_state(&mut self) -> Option> { - self.0.map(<[u8]>::to_vec) - } - } - #[test] fn plugin_states_are_read_without_the_audio_thread() { let mut side = Side::new(48_000.0, channel().1); let (a, b) = (TrackId(1), TrackId(2)); - side.states.insert((b, 0), Box::new(Reads(Some(b"b0")))); - side.states.insert((a, soundcraft_mix::INSTRUMENT_SLOT), Box::new(Reads(Some(b"synth")))); - side.states.insert((a, 3), Box::new(Reads(Some(b"a3")))); - side.states.insert((a, 4), Box::new(Reads(None))); + side.states.insert((b, 0), Box::new(|| Some(b"b0".to_vec()))); + side.states.insert((a, soundcraft_mix::INSTRUMENT_SLOT), Box::new(|| Some(b"synth".to_vec()))); + side.states.insert((a, 3), Box::new(|| Some(b"a3".to_vec()))); + side.states.insert((a, 4), Box::new(|| None)); let want = vec![(a, 3, b"a3".to_vec()), (a, soundcraft_mix::INSTRUMENT_SLOT, b"synth".to_vec()), (b, 0, b"b0".to_vec())]; assert_eq!(side.capture_states(), want); side.forget((a, 3)); assert_eq!(side.capture_states(), want[1..]); } + struct Window; + + impl PluginEditor for Window { + fn open(&mut self) -> Result<(), String> { + Ok(()) + } + fn close(&mut self) {} + fn is_open(&self) -> bool { + false + } + fn idle(&mut self) -> Vec<(String, f32)> { + Vec::new() + } + } + + #[test] + fn a_refused_state_is_kept_until_the_editor_opens() { + let mut side = Side::new(48_000.0, channel().1); + let k = (TrackId(1), 0); + side.states.insert(k, Box::new(|| Some(b"defaults".to_vec()))); + side.editors.insert(k, Box::new(Window)); + side.refused.insert(k); + assert!(side.capture_states().is_empty()); + side.open_editor(k).unwrap(); + assert_eq!(side.capture_states(), vec![(TrackId(1), 0, b"defaults".to_vec())]); + } + #[test] fn side_creates_each_missing_instance_once() { use soundcraft_model::{ChannelFormat, Insert, TrackKind}; From 56ffce6decf2543a135b63a3e043c061d5a7e9e0 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:56:08 +0100 Subject: [PATCH 3/5] Take a plugin that puts out NaN out of the mix instead of silencing it One insert or instrument putting out NaN or infinity silenced the whole mix: the value spread through the busses and the main mix (NaN plus anything is NaN), and the output guard turned all of it into silence. It can also poison the state of filters, reverbs and compressors downstream, which then stay silent after the plugin has calmed down. After each insert (track or master) and each instrument processes, the engine checks its block. If the block holds NaN or infinity, the engine silences that block of the strip and skips the plugin until the next MixEngine::reset (playback starts or stops), which resets it and tries it again. Meanwhile the strip plays on without it, as if bypassed. The check costs about 60 ns per insert for a stereo 512-sample block. StripMeter::plugin_failures counts how often a strip lost a plugin and names the slot of the latest, so the UI names that plugin in the status line each time, also when it fails again after a restart. The AU host no longer zeroes non-finite output itself, so Audio Units are taken out and named like CLAP and VST3 plugins. --- crates/au-host/src/plugin.rs | 5 +- crates/mix/src/lib.rs | 137 +++++++++++++++++++++++++++++++++-- crates/playback/src/lib.rs | 2 +- crates/ui-egui/src/lib.rs | 29 ++++++++ 4 files changed, 163 insertions(+), 10 deletions(-) diff --git a/crates/au-host/src/plugin.rs b/crates/au-host/src/plugin.rs index eed7fc21..89f604e3 100644 --- a/crates/au-host/src/plugin.rs +++ b/crates/au-host/src/plugin.rs @@ -170,13 +170,12 @@ impl Plugin for AuPlugin { if ch == 1 && au_ch >= 2 { if let (Some(l), Some(r)) = (self.inst.output(0, n), self.inst.output(1, n)) { for ((d, a), b) in dst.iter_mut().zip(l).zip(r) { - let v = 0.5 * (a + b); - *d = if v.is_finite() { v } else { 0.0 }; + *d = 0.5 * (a + b); } } } else if let Some(src) = self.inst.output(c.min(au_ch.saturating_sub(1)), n) { for (d, &s) in dst.iter_mut().zip(src) { - *d = if s.is_finite() { s } else { 0.0 }; + *d = s; } } } diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index a145422c..281e5199 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -155,6 +155,24 @@ pub struct StripMeter { pub peaks: Vec, /// Gain reduction of the first dynamics insert, dB. pub gain_reduction_db: f32, + pub plugin_failures: PluginFailures, +} + +/// How often a strip's plugins were taken out of the signal path for putting out NaN or infinity. +/// Each stays out until the engine is next reset (playback starts or stops). +#[derive(Debug, Clone, Copy, Default, PartialEq)] +pub struct PluginFailures { + /// It only grows. + pub count: u32, + /// The slot of the latest: an insert index or [`INSTRUMENT_SLOT`]. + pub last: usize, +} + +impl PluginFailures { + fn record(&mut self, slot: usize) { + self.count = self.count.wrapping_add(1); + self.last = slot; + } } impl StripMeter { @@ -163,6 +181,7 @@ impl StripMeter { self.peak = o.peak; self.peaks.clone_from(&o.peaks); self.gain_reduction_db = o.gain_reduction_db; + self.plugin_failures = o.plugin_failures; } fn measure(&mut self, bufs: &[Vec], frames: usize, gr: f32) { @@ -191,6 +210,7 @@ struct PluginSlot { plugin: Box, /// Last values pushed to the plugin, in the insert's parameter-map order. applied: Vec, + failed: bool, } struct Strip { @@ -217,6 +237,7 @@ struct Strip { align_pre: DelayLine, out_extra: DelayLine, outbuf: Vec>, + failures: PluginFailures, } /// A multichannel delay line (samples), resized only when the delay changes. @@ -287,6 +308,7 @@ impl Strip { align_pre: DelayLine::default(), out_extra: DelayLine::default(), outbuf: Vec::new(), + failures: PluginFailures::default(), } } } @@ -365,7 +387,7 @@ impl SlotCtx<'_> { let state = if third { ins.state_bytes() } else { None }; create_instance(&ins.plugin, state.as_deref(), self.sr, self.mb, ch).map(|n| n.plugin) }?; - Some(PluginSlot { id: ins.plugin.clone(), ch, plugin, applied: Vec::new() }) + Some(PluginSlot { id: ins.plugin.clone(), ch, plugin, applied: Vec::new(), failed: false }) } } @@ -455,10 +477,12 @@ impl MixEngine { for s in self.strips.values_mut() { for p in s.plugins.iter_mut().flatten() { p.plugin.reset(); + p.failed = false; } if let Some(i) = &mut s.instrument { i.plugin.all_notes_off(); i.plugin.reset(); + i.failed = false; } s.held.clear(); } @@ -668,9 +692,13 @@ impl MixEngine { if let (Some(ins), Some(Some(slot))) = (ins, strip.plugins.get_mut(i)) && ins.active && !ins.bypass + && !slot.failed { apply_params(slot, ins, t, i, pos); slot.plugin.process(&mut strip.buf, frames); + if contain_failure(slot, &mut strip.buf, frames) { + strip.failures.record(i); + } } } let v0 = volume_db_at(t, pos); @@ -685,7 +713,9 @@ impl MixEngine { } } } - self.meters.entry(t.id).or_default().measure(&self.main, frames, 0.0); + let m = self.meters.entry(t.id).or_default(); + m.measure(&self.main, frames, 0.0); + m.plugin_failures = strip.failures; } } // Metronome: summed after the master fader, so it reaches the speakers without passing @@ -728,7 +758,9 @@ impl MixEngine { /// Send, meter and pan a processed strip into the busses / main mix. fn route_strip(&mut self, t: &Track, strip: &mut Strip, pos: Samples, frames: usize) { - self.meters.entry(t.id).or_default().measure(&strip.buf, frames, strip.gr); + let m = self.meters.entry(t.id).or_default(); + m.measure(&strip.buf, frames, strip.gr); + m.plugin_failures = strip.failures; let src_fmt = strip_format(t, strip.buf.len()); let law = self.pan_law; for (i, snd) in t.mixer.sends.iter().enumerate() { @@ -872,7 +904,7 @@ fn process_strip( } } TrackKind::Instrument | TrackKind::Midi => { - if let Some(inst) = &mut strip.instrument { + if let Some(inst) = strip.instrument.as_mut().filter(|i| !i.failed) { // #152: push the instrument's stored parameter values into the live plugin // before processing, the same way inserts do below. Without this, the values // loaded from a session (or set via `mix.insert_state`) never reach the plugin @@ -895,6 +927,9 @@ fn process_strip( *x = 0.5 * (*x + *y); } } + if contain_failure(inst, &mut strip.buf, frames) { + strip.failures.record(INSTRUMENT_SLOT); + } } } _ => {} @@ -909,11 +944,15 @@ fn process_strip( let mut latency = 0usize; for (i, ins) in t.mixer.inserts.iter().enumerate() { let (Some(ins), Some(Some(slot))) = (ins, strip.plugins.get_mut(i)) else { continue }; - if !ins.active || ins.bypass { + if !ins.active || ins.bypass || slot.failed { continue; } apply_params(slot, ins, t, i, pos); slot.plugin.process(&mut strip.buf, frames); + if contain_failure(slot, &mut strip.buf, frames) { + strip.failures.record(i); + continue; + } latency = latency.saturating_add(slot.plugin.latency()); if gr == 0.0 { gr = slot.plugin.gain_reduction_db(); @@ -966,6 +1005,20 @@ fn process_strip( } } +/// Silences a block the plugin filled with NaN or infinity and takes the plugin out until the +/// next [`MixEngine::reset`]. True then. +fn contain_failure(slot: &mut PluginSlot, buf: &mut [Vec], frames: usize) -> bool { + // A branch-free fold vectorizes; `all` runs about 8 times slower here. + if buf.iter().all(|c| c.get(..frames).unwrap_or(c).iter().fold(true, |ok, x| ok & x.is_finite())) { + return false; + } + for c in buf.iter_mut() { + c.iter_mut().take(frames).for_each(|x| *x = 0.0); + } + slot.failed = true; + true +} + /// Clip-effect parameters of a clip, or None when it has none or they are bypassed. fn clip_fx_params(s: &Session, clip: u64) -> Option> { if s.edit.values.is_empty() { @@ -2005,6 +2058,11 @@ mod tests { fn session_with_dc(level: f32, frames: usize) -> (Session, TrackId) { let mut s = Session::default(); + let t = add_dc_track(&mut s, level, frames); + (s, t) + } + + fn add_dc_track(s: &mut Session, level: f32, frames: usize) -> TrackId { let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, None); let buf = AudioBuffer { sample_rate: 48_000, channels: vec![vec![level; frames]] }; s.pool.insert(SourceId(900), Arc::new(SourceAudio::new(buf))); @@ -2012,7 +2070,7 @@ mod tests { if let Some(pl) = s.track_mut(t).and_then(|t| t.playlist_mut()) { pl.clips.push(Clip::audio(id, "dc", SourceId(900), 0, 0, frames as i64)); } - (s, t) + t } #[test] @@ -2199,6 +2257,73 @@ mod tests { assert!(eng.drain_plugins().is_empty()); } + struct Nan(Box); + + impl Plugin for Nan { + fn info(&self) -> &'static soundcraft_dsp::PluginInfo { + self.0.info() + } + fn prepare(&mut self, sr: f32, mb: usize, ch: usize) { + self.0.prepare(sr, mb, ch); + } + fn reset(&mut self) { + self.0.reset(); + } + fn set_param(&mut self, id: &str, v: f32) -> bool { + self.0.set_param(id, v) + } + fn param(&self, id: &str) -> Option { + self.0.param(id) + } + fn process(&mut self, io: &mut [Vec], frames: usize) { + io.iter_mut().for_each(|c| c.iter_mut().take(frames).for_each(|x| *x = f32::NAN)); + } + } + + fn nan(track: TrackId, slot: usize, ch: usize) -> Vec { + vec![PreparedInstance { track, slot, id: "clap:test.nan".into(), ch, plugin: Box::new(Nan(soundcraft_dsp::create("gain").unwrap())) }] + } + + #[test] + fn an_insert_that_puts_out_nan_is_taken_out_until_the_next_reset() { + let (mut s, a) = session_with_dc(0.5, 48_000); + let b = add_dc_track(&mut s, 0.5, 48_000); + s.track_mut(a).unwrap().mixer.inserts[2] = Some(Insert::new("clap:test.nan")); + let mut eng = MixEngine::new(48_000.0, 512); + eng.set_external_instances(true); + let both = render_block(&mut eng, &s, 0); + eng.adopt(nan(a, 2, 1)); + let only_b = render_block(&mut eng, &s, 512); + assert!(only_b > 0.3 && (only_b - both / 2.0).abs() < 1e-6, "{only_b} vs {both}"); + assert_eq!(eng.meters[&a].plugin_failures, PluginFailures { count: 1, last: 2 }); + assert_eq!(eng.meters[&b].plugin_failures, PluginFailures::default()); + assert_eq!(render_block(&mut eng, &s, 1024), both); + eng.reset(); + assert_eq!(render_block(&mut eng, &s, 1536), only_b); + assert_eq!(eng.meters[&a].plugin_failures.count, 2); + } + + #[test] + fn nan_from_an_instrument_or_a_master_insert_is_contained() { + let mut s = synth_session(-6.0); + let synth = s.tracks[0].id; + s.track_mut(synth).unwrap().instrument = Some(Insert::new("clap:test.nan")); + let b = add_dc_track(&mut s, 0.5, 48_000); + let mut eng = MixEngine::new(48_000.0, 512); + eng.set_external_instances(true); + eng.adopt(nan(synth, INSTRUMENT_SLOT, 2)); + let only_b = render_block(&mut eng, &s, 0); + assert!(only_b > 0.3, "{only_b}"); + assert_eq!(eng.meters[&synth].plugin_failures, PluginFailures { count: 1, last: INSTRUMENT_SLOT }); + assert_eq!(eng.meters[&b].plugin_failures, PluginFailures::default()); + let m = s.add_track(TrackKind::Master, ChannelFormat::Stereo, None); + s.track_mut(m).unwrap().mixer.inserts[0] = Some(Insert::new("clap:test.nan")); + eng.adopt(nan(m, 0, 2)); + assert_eq!(render_block(&mut eng, &s, 512), 0.0); + assert_eq!(eng.meters[&m].plugin_failures, PluginFailures { count: 1, last: 0 }); + assert_eq!(render_block(&mut eng, &s, 1024), only_b); + } + #[test] fn stored_state_comes_first_then_session_params() { let (mut s, t) = session_with_dc(0.5, 48_000); diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index 6a7df8aa..92019390 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -760,7 +760,7 @@ mod tests { let keep = TrackId(1); let stale = TrackId(2); let mut source = MeterSnapshot::default(); - source.tracks.insert(keep, StripMeter { peak: [0.5, 0.25], peaks: vec![0.5, 0.25], gain_reduction_db: 3.0 }); + source.tracks.insert(keep, StripMeter { peak: [0.5, 0.25], peaks: vec![0.5, 0.25], gain_reduction_db: 3.0, ..StripMeter::default() }); source.main.peaks = vec![0.75, 0.5]; let mut out = MeterSnapshot::default(); out.tracks.insert(stale, StripMeter::default()); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 096f143b..ea09654c 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -199,6 +199,8 @@ pub struct MeterDisplay { pub hold_age: f32, pub clip: bool, pub gr: f32, + /// [`soundcraft_mix::PluginFailures::count`] as last seen. + pub plugin_failures: u32, } /// An in-progress mouse gesture in the Edit window. @@ -755,6 +757,12 @@ impl SoundApp { for (id, m) in &snap.tracks { let d = self.meters.entry(*id).or_default(); feed_meter(d, m.peak, m.gain_reduction_db, dt); + if m.plugin_failures.count != d.plugin_failures { + d.plugin_failures = m.plugin_failures.count; + if let Some(msg) = failed_plugin_status(self.engine.session(), *id, m.plugin_failures.last) { + self.ui.status = msg; + } + } } let main = snap.main.peak; feed_meter(&mut self.main_meter, main, 0.0, dt); @@ -956,6 +964,15 @@ fn feed_meter(d: &mut MeterDisplay, peak: [f32; 2], gr: f32, dt: f32) { d.gr = if gr > d.gr { gr } else { d.gr * fall }; } +/// The status line naming the plugin in `slot` (an insert index or +/// [`soundcraft_mix::INSTRUMENT_SLOT`]), which the engine took out of the signal path. +fn failed_plugin_status(s: &soundcraft_model::Session, track: TrackId, slot: usize) -> Option { + let t = s.track(track)?; + let ins = if slot == soundcraft_mix::INSTRUMENT_SLOT { t.instrument.as_ref() } else { t.mixer.inserts.get(slot)?.as_ref() }?; + let name = crate::mix_window::plugin_info(&ins.plugin).map_or(ins.plugin.as_str(), |p| p.name); + Some(format!("{name} on {} put out invalid audio, so it is bypassed until playback stops or starts again.", t.name)) +} + #[cfg(test)] mod tests { use super::{Services, SoundApp, automation_step_due}; @@ -988,6 +1005,18 @@ mod tests { } } + #[test] + fn the_status_names_a_plugin_the_engine_took_out() { + use soundcraft_model::{ChannelFormat, Insert, TrackKind}; + let mut s = soundcraft_model::Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, Some("Vox")); + s.track_mut(t).unwrap().mixer.inserts[3] = Some(Insert::new("gain")); + let msg = super::failed_plugin_status(&s, t, 3).unwrap(); + assert!(msg.starts_with("Gain on Vox put out invalid audio"), "{msg}"); + assert_eq!(super::failed_plugin_status(&s, t, 0), None); + assert_eq!(super::failed_plugin_status(&s, t, soundcraft_mix::INSTRUMENT_SLOT), None); + } + #[test] fn new_session_cancels_play_queued_before_the_next_frame() { let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); From ca1a6d443e9ff2d1dbca362d2a9b4689bf39c59c Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 13 Oct 2026 05:31:48 -0400 Subject: [PATCH 4/5] Fix build after merging main into #306 Co-Authored-By: Claude Opus 5.5 --- crates/mix/src/lib.rs | 18 ++++++++++++------ crates/playback/src/lib.rs | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index 12784e0f..ee40bda4 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -2414,6 +2414,7 @@ mod tests { /// A built-in gain behind a third-party id, with a log of parameter sets. struct Stateful { inner: Box, + blob: Vec, sets: Arc, } @@ -2437,6 +2438,10 @@ mod tests { fn process(&mut self, io: &mut [Vec], frames: usize) { self.inner.process(io, frames); } + fn state_reader(&mut self) -> Option> { + let blob = self.blob.clone(); + Some(Box::new(move || Some(blob.clone()))) + } } struct Clock { @@ -2614,9 +2619,9 @@ mod tests { let sets = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let mut gain = soundcraft_dsp::create("gain").unwrap(); gain.prepare(48_000.0, 512, 1); - let fake = Stateful { inner: gain, sets: Arc::clone(&sets) }; + let fake = Stateful { inner: gain, blob: Vec::new(), sets: Arc::clone(&sets) }; // An instance for the wrong channel count is retired, not used. - let wrong = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), sets: Arc::clone(&sets) }; + let wrong = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), blob: Vec::new(), sets: Arc::clone(&sets) }; eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "clap:test.fake".into(), ch: 2, plugin: Box::new(wrong) }]); assert!(render_block(&mut eng, &s, 512) > 0.3); assert_eq!(eng.take_retired().len(), 1); @@ -2724,11 +2729,12 @@ mod tests { }; eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "clap:test.fake".into(), ch: 1, plugin: make(b"old") }]); render_block(&mut eng, &s, 0); - assert_eq!(eng.capture_states(), vec![(t, 0, b"old".to_vec())]); + assert!(eng.take_retired().is_empty()); eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "clap:test.fake".into(), ch: 1, plugin: make(b"new") }]); render_block(&mut eng, &s, 512); - assert_eq!(eng.capture_states(), vec![(t, 0, b"new".to_vec())]); - assert_eq!(eng.take_retired().len(), 1, "the old instance is retired"); + let mut retired = eng.take_retired(); + assert_eq!(retired.len(), 1, "the old instance is retired"); + assert_eq!(retired[0].save_state(), Some(b"old".to_vec()), "the new instance stays"); } #[test] @@ -2742,7 +2748,7 @@ mod tests { let mut eng = MixEngine::new(48_000.0, 512); eng.set_external_instances(true); let sets = Arc::new(std::sync::atomic::AtomicUsize::new(0)); - let fake = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), sets: Arc::clone(&sets) }; + let fake = Stateful { inner: soundcraft_dsp::create("gain").unwrap(), blob: Vec::new(), sets: Arc::clone(&sets) }; eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "vst3:00000000000000000000000000000001".into(), ch: 1, plugin: Box::new(fake) }]); render_block(&mut eng, &s, 0); assert_eq!(sets.load(std::sync::atomic::Ordering::Relaxed), 1, "session params apply once"); diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index 73d8394b..f52d3c93 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -1118,7 +1118,7 @@ mod tests { let (tx, rx) = channel(); let (gtx, grx) = channel(); let mut st = AudioState::new(rx, shared, Arc::new(soundcraft_model::Session::default()), 48_000.0, gtx); - let mut side = Side { shadow: HashMap::new(), rate: 48_000.0, editors: HashMap::new(), garbage: grx, old_sessions: None }; + let mut side = Side::new(48_000.0, grx); side.return_old_sessions(&tx); let mut out = vec![0.0f32; 256]; let mut watched = Vec::new(); From baea184bd18edc6688c152407b4ae1155636cdad Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 13 Oct 2026 06:26:54 -0400 Subject: [PATCH 5/5] Keep one VST3 controller-sync path after merging #306's latest The landing branch already syncs host-set values to the controller before saving state (lock-free atomics + EditorLink::sync_param); the PR's later ControllerPending table did the same, so keep one. Keeps the PR's new 'a loaded state replaces earlier edits' assertion, which passes. Co-Authored-By: Claude Opus 5.5 --- crates/mix/src/lib.rs | 5 ++- crates/vst3-host/src/ffi.rs | 56 ---------------------------------- crates/vst3-host/src/plugin.rs | 2 -- 3 files changed, 2 insertions(+), 61 deletions(-) diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index 355c2373..ee40bda4 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -2722,11 +2722,10 @@ mod tests { let mut eng = MixEngine::new(48_000.0, 512); eng.set_external_instances(true); let sets = Arc::new(std::sync::atomic::AtomicUsize::new(0)); - let make = |db: f32| { + let make = |blob: &[u8]| { let mut gain = soundcraft_dsp::create("gain").unwrap(); gain.prepare(48_000.0, 512, 1); - gain.set_param("gain", db); - Box::new(Stateful { inner: gain, sets: Arc::clone(&sets) }) + Box::new(Stateful { inner: gain, blob: blob.to_vec(), sets: Arc::clone(&sets) }) }; eng.adopt(vec![PreparedInstance { track: t, slot: 0, id: "clap:test.fake".into(), ch: 1, plugin: make(b"old") }]); render_block(&mut eng, &s, 0); diff --git a/crates/vst3-host/src/ffi.rs b/crates/vst3-host/src/ffi.rs index 007b4255..ca10647f 100644 --- a/crates/vst3-host/src/ffi.rs +++ b/crates/vst3-host/src/ffi.rs @@ -1587,48 +1587,6 @@ fn rect_size(r: &sb::ViewRect) -> (f64, f64) { (f64::from(w), f64::from(h)) } -/// Normalized values the host set (automation, the session) that the edit controller hasn't been -/// told yet, one slot per parameter. The audio thread fills a slot without locking or allocating; -/// [`EditorLink::save_state`] sends what is waiting before it reads the controller's state, so -/// values the controller keeps in its own state are saved as set. -pub(crate) struct ControllerPending { - ids: Vec, - slots: Vec, -} - -/// An empty slot: a NaN bit pattern, which `set` never stores. -const NO_VALUE: u64 = u64::MAX; - -impl ControllerPending { - pub fn new(ids: &[u32]) -> ControllerPending { - ControllerPending { ids: ids.to_vec(), slots: ids.iter().map(|_| AtomicU64::new(NO_VALUE)).collect() } - } - - /// Notes the value of parameter `i` (index into the plugin's parameter list). - pub fn set(&self, i: usize, n: f64) { - if let Some(s) = self.slots.get(i) - && n.is_finite() - { - s.store(n.clamp(0.0, 1.0).to_bits(), Ordering::Release); - } - } - - /// Forgets every waiting value (a loaded state replaces them). - pub fn clear(&self) { - for s in &self.slots { - s.store(NO_VALUE, Ordering::Release); - } - } - - /// Takes the waiting values as `(ParamID, normalized)`. - fn take(&self) -> impl Iterator + '_ { - self.ids.iter().zip(&self.slots).filter_map(|(&id, s)| { - let b = s.swap(NO_VALUE, Ordering::AcqRel); - (b != NO_VALUE).then(|| (id, f64::from_bits(b))) - }) - } -} - #[derive(Default)] struct EditorInner { /// `None` once the instance is gone. @@ -1638,8 +1596,6 @@ struct EditorInner { view: Option>, frame: Option>, window: Option, - /// Host-set values for the controller, sent before its state is read. - pending: Option>, } /// The link between an instance and its editor and state handles. The instance may process on @@ -1767,13 +1723,6 @@ impl EditorLink { pub fn save_state(&self) -> Option> { let g = self.lock(); let component = g.component.as_ref()?; - if let (Some(c), Some(p)) = (&g.controller, &g.pending) { - for (id, n) in p.take() { - // SAFETY: valid controller (cleared under this lock before the instance - // terminates); UI thread, as `getState` below. - unsafe { c.setParamNormalized(id, n) }; - } - } // SAFETY: valid component (the instance clears it under this lock before it terminates); // the stream lives in `read_state` for the whole call. let comp = Instance::read_state(|s| unsafe { component.getState(s) })?; @@ -1791,11 +1740,6 @@ impl EditorLink { Some(out) } - /// Shares the instance's table of host-set values with the state reader. - pub fn set_controller_pending(&self, p: Arc) { - self.lock().pending = Some(p); - } - /// Closes the editor. Main thread only (no-op elsewhere). pub fn close(&self) { if window::on_main_thread() { diff --git a/crates/vst3-host/src/plugin.rs b/crates/vst3-host/src/plugin.rs index 5cf9018a..2975465f 100644 --- a/crates/vst3-host/src/plugin.rs +++ b/crates/vst3-host/src/plugin.rs @@ -114,7 +114,6 @@ impl Vst3Plugin { }) .collect(); p.values = values; - p.inst.state_link().set_controller_pending(Arc::clone(&p.controller_pending)); p } @@ -235,7 +234,6 @@ impl Plugin for Vst3Plugin { } else if self.pending.len() < self.pending.capacity() { self.pending.push((vid, n)); } - self.controller_pending.set(i, n); true }