From 1c350101bc494117884d0d3eefd43eb91c937d28 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 21:30:40 -0500 Subject: [PATCH 1/6] clap-host tests: load plugins only from a freshly created private temp directory (#1295) * clap-host tests: scan a freshly created private directory for the garbage-file test The test reused a PID-named directory under the shared temp dir after ignoring a failed cleanup, then loaded every .clap candidate in it. Another local account could pre-create that directory and plant a native library that the test runner would load. Create the directory exclusively (mode 0700 on Unix) and fail instead of reusing one that already exists. * clap-host tests: create the garbage-file test directory beside the test binary, not under TMPDIR --- crates/clap-host/src/lib.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/clap-host/src/lib.rs b/crates/clap-host/src/lib.rs index 7142a9ee..f6b93d13 100644 --- a/crates/clap-host/src/lib.rs +++ b/crates/clap-host/src/lib.rs @@ -222,10 +222,33 @@ mod tests { assert!(plugin_info("reverb").is_none()); } + /// A new directory that this test alone created, readable only by its user on Unix. Creation + /// fails if the path already exists: the test then loads native libraries from it, so it must + /// never reuse a directory someone else could have put a library in. It lives next to the test + /// binary (in the build directory), not under the temp dir: `TMPDIR`/`TEMP` can name a shared + /// directory without a sticky bit or with inheritable write access, where another user could + /// rename or populate it. Anyone who can write to the build directory can already replace the + /// test binary itself, so this parent adds no new trust. + fn fresh_private_dir(prefix: &str) -> std::path::PathBuf { + let exe = std::env::current_exe().expect("test binary path"); + let parent = exe.parent().expect("test binary has a parent directory"); + let nanos = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |t| t.as_nanos()); + let d = parent.join(format!("{prefix}-{}-{nanos}", std::process::id())); + #[cfg(unix)] + let builder = { + let mut b = std::fs::DirBuilder::new(); + std::os::unix::fs::DirBuilderExt::mode(&mut b, 0o700); + b + }; + #[cfg(not(unix))] + let builder = std::fs::DirBuilder::new(); + builder.create(&d).unwrap_or_else(|e| panic!("cannot create a fresh test directory {}: {e}", d.display())); + d + } + #[test] fn garbage_files_are_errors_not_crashes() { - let d = std::env::temp_dir().join(format!("soundcraft-clap-garbage-{}", std::process::id())); - let _ = std::fs::remove_dir_all(&d); + let d = fresh_private_dir("soundcraft-clap-garbage"); std::fs::create_dir_all(d.join("Bundle.clap/Contents/MacOS")).unwrap(); std::fs::write(d.join("garbage.clap"), b"\x7fELF this is not a plugin \x00\x01\x02").unwrap(); std::fs::write(d.join("empty.clap"), b"").unwrap(); From 17d3f00700beb4f50dd018309fbe0bc676d8aa57 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 22:51:02 -0500 Subject: [PATCH 2/6] MIDI: index controller dedup by channel and kind (#1298) * MIDI: index controller dedup by channel and kind Remove Duplicate Notes rescanned every controller already kept at the same tick for each incoming one. A session whose sequence holds one tick with many distinct channel/controller pairs made that quadratic and blocked editing. Track where each channel+kind sits at the current tick in a map instead; the last event per key still wins. * MIDI: tag dedup index entries with their tick instead of clearing per tick Clearing the map on every tick change cost O(capacity) per tick after a dense tick. Store the tick with each entry and treat entries from an earlier tick as absent. Add an oracle test comparing against the previous per-tick rescan on mixed-tick input. --- crates/midi/src/sequence.rs | 28 +++++++++++---- crates/midi/tests/it/ops.rs | 69 +++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/crates/midi/src/sequence.rs b/crates/midi/src/sequence.rs index e50c6614..c5336f06 100644 --- a/crates/midi/src/sequence.rs +++ b/crates/midi/src/sequence.rs @@ -1,4 +1,6 @@ use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::collections::hash_map::Entry; /// A MIDI note. Times are in model ticks ([`crate::PPQ`] per quarter note). #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] @@ -108,14 +110,28 @@ impl Sequence { self.ctrls.sort_by_key(|c| c.tick); let mut kept: Vec = Vec::with_capacity(self.ctrls.len()); + // The latest tick and `kept` index of each channel+kind, so a same-tick repeat is found + // without rescanning the tick's events (a tick can hold many thousands of distinct keys). + // An entry from an earlier tick counts as absent; keeping the tick instead of clearing the + // map per tick keeps the work per event constant after a dense tick. + let mut last_at: HashMap<(u8, (u8, u8)), (i64, usize)> = HashMap::new(); for c in self.ctrls.drain(..) { // Within a tick, replace an earlier event of the same channel+kind. - // Only events at the same tick (the tail of `kept`) can match. - let slot = - kept.iter_mut().rev().take_while(|k| k.tick == c.tick).find(|k| k.channel == c.channel && kind_key(k.kind) == kind_key(c.kind)); - match slot { - Some(slot) => *slot = c, - None => kept.push(c), + match last_at.entry((c.channel, kind_key(c.kind))) { + Entry::Occupied(mut e) => { + let (tick, idx) = *e.get(); + match kept.get_mut(idx) { + Some(slot) if tick == c.tick => *slot = c, + _ => { + e.insert((c.tick, kept.len())); + kept.push(c); + } + } + } + Entry::Vacant(e) => { + e.insert((c.tick, kept.len())); + kept.push(c); + } } } self.ctrls = kept; diff --git a/crates/midi/tests/it/ops.rs b/crates/midi/tests/it/ops.rs index 3a367ee0..431790ff 100644 --- a/crates/midi/tests/it/ops.rs +++ b/crates/midi/tests/it/ops.rs @@ -222,3 +222,72 @@ fn sequence_serde_round_trip() { let json = serde_json::to_string(&seq).unwrap(); assert_eq!(serde_json::from_str::(&json).unwrap(), seq); } + +#[test] +fn remove_duplicates_handles_a_large_same_tick_controller_group() { + // Every channel x controller number at one tick, twice: the second copy of each wins. A + // rescan of the tick's kept events per incoming event would make this quadratic. + let keys: Vec<(u8, u8)> = (0..=255u8).flat_map(|ch| (0..=255u8).map(move |cc| (ch, cc))).collect(); + let ctrls = [0, 1] + .iter() + .flat_map(|&pass| keys.iter().map(move |&(channel, cc)| CtrlEvent { tick: 7, channel, kind: CtrlKind::Cc(cc), value: pass })) + .collect(); + let mut seq = Sequence { notes: Vec::new(), ctrls }; + let start = std::time::Instant::now(); + assert_eq!(seq.remove_duplicates(), keys.len()); + assert!(start.elapsed() < std::time::Duration::from_secs(10), "{:?}", start.elapsed()); + assert_eq!(seq.ctrls.len(), keys.len()); + assert!(seq.ctrls.iter().all(|c| c.value == 1)); +} + +#[test] +fn remove_duplicates_matches_a_per_tick_rescan_on_mixed_ticks() { + // The straightforward per-tick rescan the indexed version replaced, kept as an oracle. + fn reference(mut ctrls: Vec) -> Vec { + ctrls.sort_by_key(|c| c.tick); + let key = |k: CtrlKind| match k { + CtrlKind::Cc(n) => (0, n), + CtrlKind::PitchBend => (1, 0), + CtrlKind::ChannelPressure => (2, 0), + CtrlKind::PolyPressure(p) => (3, p), + CtrlKind::Program => (4, 0), + }; + let mut kept: Vec = Vec::new(); + for c in ctrls { + match kept.iter_mut().rev().take_while(|k| k.tick == c.tick).find(|k| k.channel == c.channel && key(k.kind) == key(c.kind)) { + Some(slot) => *slot = c, + None => kept.push(c), + } + } + kept + } + + // Few ticks, channels and kinds, so the same key recurs both within a tick (interleaved with + // other keys) and across ticks; unsorted input from a fixed xorshift stream. + let mut state = 0x9E37_79B9_7F4A_7C15u64; + let mut next = move || { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + state + }; + let kinds = |r: u64| match r % 5 { + 0 => CtrlKind::Cc((r >> 8) as u8 % 4), + 1 => CtrlKind::PitchBend, + 2 => CtrlKind::ChannelPressure, + 3 => CtrlKind::PolyPressure((r >> 8) as u8 % 4), + _ => CtrlKind::Program, + }; + for round in 0..200 { + let mut ctrls = Vec::new(); + for i in 0..(next() % 64) { + let r = next(); + ctrls.push(CtrlEvent { tick: (r % 6) as i64 - 2, channel: (r >> 16) as u8 % 3, kind: kinds(r >> 24), value: (round * 100 + i) as i32 }); + } + let mut seq = Sequence { notes: Vec::new(), ctrls: ctrls.clone() }; + let removed = seq.remove_duplicates(); + let expected = reference(ctrls.clone()); + assert_eq!(seq.ctrls, expected, "round {round}"); + assert_eq!(removed, ctrls.len() - expected.len(), "round {round}"); + } +} From 0af1ad909ec17edf6cad92d500fef1b38abdb562 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 23:25:34 -0500 Subject: [PATCH 3/6] Mixer: match automation lanes to insert parameters in one merge (#1299) apply_params searched all of the track's automation lanes for every stored insert parameter on every block, so a session with many parameters and many lanes multiplied the two on the audio thread. Collect the slot's lanes once per block, order them by parameter name (keeping the first lane per name, the one the old search found) and walk them alongside the sorted parameter map. The lane list reuses a scratch buffer kept on the plugin slot. --- crates/mix/src/strip.rs | 130 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 6 deletions(-) diff --git a/crates/mix/src/strip.rs b/crates/mix/src/strip.rs index 01f4a11a..1e29653c 100644 --- a/crates/mix/src/strip.rs +++ b/crates/mix/src/strip.rs @@ -24,6 +24,9 @@ pub(crate) struct PluginSlot { pub(crate) plugin: Box, /// Last values pushed to the plugin, in the insert's parameter-map order. applied: Vec, + /// Scratch for `apply_params`: indexes of the track's automation lanes for this slot, kept + /// so the audio thread reuses its allocation. + lanes: Vec, pub(crate) failed: bool, } @@ -191,7 +194,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(), failed: false }) + Some(PluginSlot { id: ins.plugin.clone(), ch, plugin, applied: Vec::new(), lanes: Vec::new(), failed: false }) } } @@ -491,21 +494,40 @@ pub(crate) fn apply_params(slot: &mut PluginSlot, ins: &soundcraft_model::Insert let Ok(slot_u8) = u8::try_from(i) else { return apply_values(slot, ins, |_, v| v); }; - let automated = t.mixer.automation_mode.reads() - && t.automation.iter().any(|l| matches!(&l.param, AutoParam::Plugin { slot, .. } if *slot == slot_u8) && !l.points.is_empty()); + let lanes = &t.automation; + let name = |j: usize| match lanes.get(j).map(|l| &l.param) { + Some(AutoParam::Plugin { param, .. }) => param.as_str(), + _ => "", + }; + // This slot's lanes, ordered by parameter name (the parameter map's order) and keeping only + // the first lane per name (the one a lookup by name finds). Matching them to the parameters + // in one merge costs O(lanes log lanes + params) per block, not O(lanes x params). + let mut order = std::mem::take(&mut slot.lanes); + order.clear(); + if t.mixer.automation_mode.reads() { + order + .extend(lanes.iter().enumerate().filter(|(_, l)| matches!(&l.param, AutoParam::Plugin { slot, .. } if *slot == slot_u8)).map(|(j, _)| j)); + if !order.iter().any(|&j| lanes.get(j).is_some_and(|l| !l.points.is_empty())) { + order.clear(); + } + order.sort_unstable_by(|&a, &b| name(a).cmp(name(b)).then(a.cmp(&b))); + order.dedup_by(|later, kept| name(*later) == name(*kept)); + } + let mut next = order.iter().copied().peekable(); apply_values(slot, ins, |k, val| { - if automated - && let Some(l) = t.automation.iter().find(|l| matches!(&l.param, AutoParam::Plugin { slot, param } if *slot == slot_u8 && param == k)) + while next.next_if(|&j| name(j) < k).is_some() {} + if let Some(l) = next.peek().filter(|&&j| name(j) == k).and_then(|&j| lanes.get(j)) && !l.points.is_empty() { return l.value_at(pos, val); } val }); + slot.lanes = order; } /// Sends each stored param (as mapped by `value`) to the plugin when it changed since last time. -fn apply_values(slot: &mut PluginSlot, ins: &soundcraft_model::Insert, value: impl Fn(&str, f32) -> f32) { +fn apply_values(slot: &mut PluginSlot, ins: &soundcraft_model::Insert, mut value: impl FnMut(&str, f32) -> f32) { if slot.applied.len() != ins.params.len() { slot.applied = vec![f32::NAN; ins.params.len()]; } @@ -517,3 +539,99 @@ fn apply_values(slot: &mut PluginSlot, ins: &soundcraft_model::Insert, value: im } } } + +#[cfg(test)] +mod apply_params_tests { + use super::*; + use soundcraft_model::{AutomationLane, Insert}; + use std::sync::Mutex; + + /// A built-in gain that logs every parameter set. + struct Recorder { + inner: Box, + sets: Arc>>, + } + + impl Plugin for Recorder { + fn info(&self) -> &'static soundcraft_dsp::PluginInfo { + self.inner.info() + } + fn prepare(&mut self, sr: f32, mb: usize, ch: usize) { + self.inner.prepare(sr, mb, ch); + } + fn reset(&mut self) { + self.inner.reset(); + } + fn set_param(&mut self, id: &str, v: f32) -> bool { + self.sets.lock().unwrap().push((id.to_string(), v)); + true + } + fn param(&self, id: &str) -> Option { + self.inner.param(id) + } + fn process(&mut self, io: &mut [Vec], frames: usize) { + self.inner.process(io, frames); + } + } + + fn recorder() -> (PluginSlot, Arc>>) { + let sets = Arc::new(Mutex::new(Vec::new())); + let plugin = Box::new(Recorder { inner: soundcraft_dsp::create("gain").unwrap(), sets: Arc::clone(&sets) }); + (PluginSlot { id: "gain".into(), ch: 2, plugin, applied: Vec::new(), lanes: Vec::new(), failed: false }, sets) + } + + fn lane(slot: u8, param: &str, point: Option) -> AutomationLane { + let mut l = AutomationLane::new(AutoParam::Plugin { slot, param: param.into() }); + if let Some(v) = point { + l.set_point(0, v); + } + l + } + + #[test] + fn first_lane_per_param_of_this_slot_drives_it() { + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Stereo, None); + let tr = s.track_mut(t).unwrap(); + tr.automation = vec![ + lane(0, "c", None), + lane(1, "a", Some(5.0)), + lane(0, "b", Some(9.0)), + lane(0, "c", Some(7.0)), + lane(0, "zz", Some(1.0)), + lane(0, "b", Some(8.0)), + ]; + let mut ins = Insert::new("gain"); + for (k, v) in [("a", 1.0), ("b", 2.0), ("c", 3.0), ("d", 4.0)] { + ins.params.insert(k.into(), v); + } + let (mut slot, sets) = recorder(); + apply_params(&mut slot, &ins, tr, 0, 0); + let got = sets.lock().unwrap().clone(); + let want: Vec<(String, f32)> = [("a", 1.0), ("b", 9.0), ("c", 3.0), ("d", 4.0)].iter().map(|(k, v)| (k.to_string(), *v)).collect(); + assert_eq!(got, want); + // Unchanged values are not sent again. + apply_params(&mut slot, &ins, tr, 0, 0); + assert_eq!(sets.lock().unwrap().len(), 4); + } + + #[test] + fn many_params_and_lanes_apply_without_a_quadratic_search() { + let n = 40_000; + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Stereo, None); + let tr = s.track_mut(t).unwrap(); + tr.automation = (0..n).rev().map(|i| lane(0, &format!("p{i:06}"), Some(i as f32))).collect(); + let mut ins = Insert::new("gain"); + for i in 0..n { + ins.params.insert(format!("p{i:06}"), -1.0); + } + let (mut slot, sets) = recorder(); + let start = std::time::Instant::now(); + apply_params(&mut slot, &ins, tr, 0, 0); + assert!(start.elapsed() < std::time::Duration::from_secs(5), "{:?}", start.elapsed()); + let got = sets.lock().unwrap(); + assert_eq!(got.len(), n); + assert!(got.iter().all(|(k, v)| k == &format!("p{:06}", *v as usize))); + } +} From 64540f109b4e2947ecb832fd15e876a4dac08a94 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 23:27:09 -0500 Subject: [PATCH 4/6] Mixer: compute cyclic aux depths in one pass per hop (#1306) When the aux/folder routing has a cycle, processing_order sorted the auxes by a bounded feeder depth recomputed inside the sort key: each evaluation re-searched every aux's output and sends against a growing list of inputs, for up to eight hops, on every comparison. A session with many auxes sharing buses made that cubic during audio sync. Compute the same capped depth for every track at once: per hop, collect the buses written by auxes reached so far, then mark the tracks reading them. The result matches the old search (tested against it on a mixed graph with cycles) and the sort uses it by lookup. --- crates/mix/src/routing.rs | 132 +++++++++++++++++++++++++++++++------- 1 file changed, 108 insertions(+), 24 deletions(-) diff --git a/crates/mix/src/routing.rs b/crates/mix/src/routing.rs index fcebca85..2f4788fa 100644 --- a/crates/mix/src/routing.rs +++ b/crates/mix/src/routing.rs @@ -46,7 +46,8 @@ pub(crate) fn processing_order(s: &Session) -> Vec { } else { // A cycle keeps the existing bounded ordering for the entire graph, including any // independent acyclic branches. Feedback does not have a finite longest-path depth. - auxes.sort_by_key(|&i| bounded_aux_depth(s, i)); + let depths = bounded_aux_depths(s); + auxes.sort_by_key(|&i| depths.get(i).copied().unwrap_or(0)); } order.extend(auxes); order @@ -115,32 +116,38 @@ fn aux_output_busses(t: &Track) -> impl Iterator + '_ { .filter_map(|route| if let Route::Bus(bus) = route { Some(*bus) } else { None }) } -/// The previous bounded depth calculation, retained unchanged for any cyclic graph. -fn bounded_aux_depth(s: &Session, i: usize) -> usize { - let mut d = 0; - let mut cur = vec![i]; - for _ in 0..8 { - let inputs: Vec<&Route> = cur.iter().filter_map(|&k| s.tracks.get(k)).map(|t| &t.mixer.input).collect(); - let feeders: Vec = auxes_feeding(s, &inputs); - if feeders.is_empty() { +/// Hop cap for the bounded depth used when the aux graph has a cycle. +const MAX_BOUNDED_DEPTH: usize = 8; + +/// For each track, the bounded depth used for a cyclic graph: the length of the longest chain of +/// Aux/Folder feeders into it (through a bus its input reads and the feeder's output or a send +/// writes), capped at [`MAX_BOUNDED_DEPTH`] hops. One pass per hop over the tracks and their bus +/// writes, so the cost is linear in the routing, not a fresh feeder search per track per +/// sort comparison. +fn bounded_aux_depths(s: &Session) -> Vec { + let is_aux = |t: &Track| matches!(t.kind, TrackKind::Aux | TrackKind::Folder); + // Tracks with a feeder chain of the current length ending at them (length 0: every aux). + let mut reached: Vec = s.tracks.iter().map(is_aux).collect(); + let mut depths = vec![0usize; s.tracks.len()]; + let mut fed = std::collections::HashSet::::new(); + for hop in 1..=MAX_BOUNDED_DEPTH { + fed.clear(); + for (t, _) in s.tracks.iter().zip(&reached).filter(|(t, r)| **r && is_aux(t)) { + fed.extend(aux_output_busses(t)); + } + let mut any = false; + for ((t, r), d) in s.tracks.iter().zip(reached.iter_mut()).zip(depths.iter_mut()) { + *r = matches!(&t.mixer.input, Route::Bus(b) if fed.contains(b)); + if *r { + *d = hop; + any = true; + } + } + if !any { break; } - d += 1; - cur = feeders; } - d -} - -fn auxes_feeding(s: &Session, inputs: &[&Route]) -> Vec { - s.tracks - .iter() - .enumerate() - .filter(|(_, t)| matches!(t.kind, TrackKind::Aux | TrackKind::Folder)) - .filter(|(_, t)| { - inputs.iter().any(|r| matches!(r, Route::Bus(_)) && (&&t.mixer.output == r || t.mixer.sends.iter().flatten().any(|sn| &&sn.target == r))) - }) - .map(|(i, _)| i) - .collect() + depths } /// Implicit solo: an aux is not muted when a soloed track feeds its input bus. @@ -204,4 +211,81 @@ mod processing_order_tests { // Both deepest independent returns tie with the cycle at the legacy eight-hop cap. assert_eq!(processing_order(&s), vec![9, 8, 7, 6, 5, 4, 3, 2, 0, 1, 10]); } + + /// The per-track feeder search `bounded_aux_depths` replaced, kept as its reference. + fn reference_depth(s: &Session, i: usize) -> usize { + let mut d = 0; + let mut cur = vec![i]; + for _ in 0..MAX_BOUNDED_DEPTH { + let inputs: Vec<&Route> = cur.iter().filter_map(|&k| s.tracks.get(k)).map(|t| &t.mixer.input).collect(); + let feeders: Vec = s + .tracks + .iter() + .enumerate() + .filter(|(_, t)| matches!(t.kind, TrackKind::Aux | TrackKind::Folder)) + .filter(|(_, t)| { + inputs + .iter() + .any(|r| matches!(r, Route::Bus(_)) && (&&t.mixer.output == r || t.mixer.sends.iter().flatten().any(|sn| &&sn.target == r))) + }) + .map(|(i, _)| i) + .collect(); + if feeders.is_empty() { + break; + } + d += 1; + cur = feeders; + } + d + } + + #[test] + fn bounded_depths_match_the_per_track_feeder_search() { + // A small pseudo-random graph of auxes, folders and audio tracks with cycles, joins, + // sends and unrouted inputs. + let mut s = Session::default(); + let busses: Vec<_> = (0..6).map(|i| s.add_bus(&format!("B{i}"), ChannelFormat::Stereo)).collect(); + let mut x = 12345u32; + let mut next = |m: usize| { + x = x.wrapping_mul(1_103_515_245).wrapping_add(12_345); + (x >> 16) as usize % m + }; + for i in 0..30 { + let kind = [TrackKind::Aux, TrackKind::Folder, TrackKind::Audio][next(3)]; + let id = s.add_track(kind, ChannelFormat::Stereo, None); + let pick = |n: usize| if n < busses.len() { Route::Bus(busses[n]) } else { Route::Main }; + let (input, output, send) = (pick(next(8)), pick(next(8)), pick(next(8))); + let t = s.track_mut(id).unwrap(); + t.mixer.input = input; + t.mixer.output = output; + if i % 2 == 0 { + t.mixer.sends[0] = Some(SendSlot::new(send)); + } + } + let depths = bounded_aux_depths(&s); + for (i, (t, depth)) in s.tracks.iter().zip(&depths).enumerate() { + if matches!(t.kind, TrackKind::Aux | TrackKind::Folder) { + assert_eq!(*depth, reference_depth(&s, i), "track {i}"); + } + } + assert!(depths.contains(&MAX_BOUNDED_DEPTH)); + } + + #[test] + fn a_large_cyclic_graph_orders_quickly() { + // Thousands of auxes all reading and writing one bus: every aux feeds every other. + let mut s = Session::default(); + let bus = s.add_bus("Loop", ChannelFormat::Stereo); + for _ in 0..3000 { + let id = s.add_track(TrackKind::Aux, ChannelFormat::Stereo, None); + let t = s.track_mut(id).unwrap(); + t.mixer.input = Route::Bus(bus); + t.mixer.output = Route::Bus(bus); + t.mixer.sends[0] = Some(SendSlot::new(Route::Bus(bus))); + } + let start = std::time::Instant::now(); + let order = processing_order(&s); + assert!(start.elapsed() < std::time::Duration::from_secs(5), "{:?}", start.elapsed()); + assert_eq!(order, (0..3000).collect::>()); + } } From da02c19fcca8fbcc49d66c957e5efcc67a63befe Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 23:33:19 -0500 Subject: [PATCH 5/6] Bundles: refuse entries that share their data (#1303) Bundle entries are stored, so the files read out of one can only outgrow the bundle when several entries point at the same bytes. A small bundle naming one payload thousands of times was copied out once per name, up to the 1 GiB total limit, in the browser. Check the archive for overlapping entry data before reading any entry. --- crates/engine/src/io.rs | 6 ++++++ crates/engine/src/io/bundle.rs | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 60d9a323..ba456f6c 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -734,6 +734,12 @@ fn read_bundle(bytes: &[u8], limits: BundleLimits) -> Result<(String, std::colle if z.len() > limits.entries { return Err(EngineError::Io("bundle: too many files".into())); } + // Entries are stored, so the files we keep can only outgrow the bundle when several entries + // point at the same bytes: a small bundle naming one payload thousands of times would be + // copied out once per name. + if z.has_overlapping_files().map_err(zerr)? { + return Err(EngineError::Io("bundle: files share data".into())); + } let (mut text, mut files, mut total) = (None, std::collections::HashMap::new(), 0u64); for i in 0..z.len() { let mut f = z.by_index(i).map_err(zerr)?; diff --git a/crates/engine/src/io/bundle.rs b/crates/engine/src/io/bundle.rs index cfe6e0af..b906333d 100644 --- a/crates/engine/src/io/bundle.rs +++ b/crates/engine/src/io/bundle.rs @@ -194,6 +194,23 @@ mod bundle_tests { } } + #[test] + fn entries_sharing_one_payload_are_refused() { + use std::io::Write as _; + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + w.start_file("Song.scraft", opts).unwrap(); + w.write_all(Session::default().to_json().unwrap().as_bytes()).unwrap(); + w.start_file("Audio Files/a.wav", opts).unwrap(); + w.write_all(&[7u8; 4096]).unwrap(); + for i in 0..100 { + w.shallow_copy_file("Audio Files/a.wav", &format!("Audio Files/copy{i}.wav")).unwrap(); + } + let bundle = w.finish().unwrap().into_inner(); + let err = read_bundle(&bundle, BUNDLE_LIMITS).unwrap_err(); + assert!(err.to_string().contains("share data"), "{err}"); + } + #[test] fn a_bundle_is_saved_only_if_it_opens_again() { // #433: the writer keeps to the reader's limits. From 351b9d3a55c08a2f7ca03a7fec317678bb72c2cd Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 06:18:44 -0500 Subject: [PATCH 6/6] Tempo map: binary-search tempo segments in tick/sample conversions (#1297) * Tempo map: index tempo segments so tick/sample conversions are O(log n) Every tick<->sample conversion walked all earlier tempo changes, so code that converts many positions (MIDI import/export, the tempo ruler, score layout, MIDI scheduling and controller chasing on the audio thread) cost positions x tempo changes. A session or MIDI file with many tempo changes and many notes or tracks could stall the engine or the audio callback. TempoMap now keeps the elapsed time at each tempo change (rebuilt on every edit and on deserialize, never serialized) and binary-searches the segment. MIDI scheduling also skips notes whose on and off both fall outside the block before converting them. * Tempo map: index a file's ticks with saturating subtraction --- crates/mix/src/midi.rs | 10 ++- crates/time/src/tempo.rs | 179 +++++++++++++++++++++++++++++++-------- 2 files changed, 152 insertions(+), 37 deletions(-) diff --git a/crates/mix/src/midi.rs b/crates/mix/src/midi.rs index 3b182956..e6efcd51 100644 --- a/crates/mix/src/midi.rs +++ b/crates/mix/src/midi.rs @@ -46,12 +46,20 @@ pub(crate) fn schedule_notes(s: &Session, t: &Track, pos: Samples, frames: usize rtp("delay", 0.0).clamp(-1e7, 1e7) as i64, ) }; + // The block in clip ticks, one tick wider on each side so rounding never drops an event. + let near = s.tempo.samples_to_ticks(pos, sr).saturating_sub(base).saturating_sub(1) + ..=s.tempo.samples_to_ticks(end, sr).saturating_sub(base).saturating_add(1); for n0 in &sequence.notes { let mut n = *n0; n.velocity = (i64::from(n.velocity) + dv).clamp(1, 127) as u8; n.pitch = (i64::from(n.pitch) + dp).clamp(0, 127) as u8; n.length = ((n.length as f64 * dur) as i64).max(1); n.start = n.start.saturating_add(delay).max(0); + // A note whose on and off both fall outside the block sends nothing: skip it before + // converting its positions, so a long sequence costs a comparison per note per block. + if n.start > *near.end() || n.start.saturating_add(n.length) < *near.start() { + continue; + } let on = s.tempo.tick_to_samples(base.saturating_add(n.start), sr); let off = s.tempo.tick_to_samples(base.saturating_add(n.start).saturating_add(n.length), sr).min(clip.end()); if on >= clip.end() { @@ -68,8 +76,6 @@ pub(crate) fn schedule_notes(s: &Session, t: &Track, pos: Samples, frames: usize held.retain(|p| *p != n.pitch); } } - let near = s.tempo.samples_to_ticks(pos, sr).saturating_sub(base).saturating_sub(1) - ..=s.tempo.samples_to_ticks(end, sr).saturating_sub(base).saturating_add(1); // Events before tick 0 are what a start trim hid: they don't play. for c in sequence.ctrls.iter().filter(|c| c.tick >= 0) { let tick = c.tick.saturating_add(delay).max(0); diff --git a/crates/time/src/tempo.rs b/crates/time/src/tempo.rs index 23bb2be0..873034bb 100644 --- a/crates/time/src/tempo.rs +++ b/crates/time/src/tempo.rs @@ -40,14 +40,34 @@ pub struct BarBeat { /// Tempo and meter changes for a session. Always has at least one of each, at tick 0. #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] +#[serde(from = "TempoMapData")] pub struct TempoMap { tempos: Vec, meters: Vec, + /// For each tempo event, the elapsed time at its tick in quarter-note minutes (ticks / bpm, + /// summed over the earlier segments). Lets tick/sample conversions binary-search the segment + /// instead of walking every earlier tempo change, so converting many positions against a large + /// tempo map stays O(log n) each. Rebuilt whenever `tempos` changes. + #[serde(skip)] + elapsed: Vec, +} + +/// The serialized form of [`TempoMap`], without the derived lookup table. +#[derive(serde::Deserialize)] +struct TempoMapData { + tempos: Vec, + meters: Vec, +} + +impl From for TempoMap { + fn from(d: TempoMapData) -> Self { + TempoMap::from_parts(d.tempos, d.meters) + } } impl Default for TempoMap { fn default() -> Self { - TempoMap { tempos: vec![TempoEvent { tick: 0, bpm: 120.0 }], meters: vec![MeterEvent { tick: 0, numerator: 4, denominator: 4 }] } + TempoMap::from_parts(vec![TempoEvent { tick: 0, bpm: 120.0 }], vec![MeterEvent { tick: 0, numerator: 4, denominator: 4 }]) } } @@ -60,6 +80,43 @@ fn valid_meter(numerator: u32, denominator: u32) -> bool { } impl TempoMap { + fn from_parts(tempos: Vec, meters: Vec) -> Self { + let mut m = TempoMap { tempos, meters, elapsed: Vec::new() }; + m.reindex(); + m + } + + /// Rebuild `elapsed` from `tempos`. The first segment runs from tick 0 at the first tempo. + fn reindex(&mut self) { + let mut elapsed = Vec::with_capacity(self.tempos.len()); + let mut acc = 0.0; + let mut seg_tick = 0i64; + let mut bpm = self.tempos.first().map_or(120.0, |e| e.bpm); + for (i, e) in self.tempos.iter().enumerate() { + if i > 0 { + // Saturating: a file's ticks are indexed before `sanitize` orders them. + acc += e.tick.saturating_sub(seg_tick) as f64 / bpm; + seg_tick = e.tick; + bpm = e.bpm; + } + elapsed.push(acc); + } + self.elapsed = elapsed; + } + + /// Samples per quarter-note minute (the unit of `elapsed`) at `sr`. + fn samples_per_elapsed(sr: SampleRate) -> f64 { + sr.as_f64() * 60.0 / TICKS_PER_QUARTER as f64 + } + + /// The tempo segment `i` (an index into `tempos`, 0 when it is empty): its start tick (0 for + /// the first), bpm and elapsed time at its start. + fn segment(&self, i: usize) -> (i64, f64, f64) { + let bpm = self.tempos.get(i).or(self.tempos.first()).map_or(120.0, |e| e.bpm); + let tick = if i == 0 { 0 } else { self.tempos.get(i).map_or(0, |e| e.tick) }; + (tick, bpm, self.elapsed.get(i).copied().unwrap_or(0.0)) + } + pub fn new(bpm: f64, numerator: u32, denominator: u32) -> Result { let mut m = TempoMap::default(); m.set_tempo(0, bpm)?; @@ -91,6 +148,7 @@ impl TempoMap { let (numerator, denominator) = self.meters.first().map_or((4, 4), |m| (m.numerator, m.denominator)); self.meters.insert(0, MeterEvent { tick: 0, numerator, denominator }); } + self.reindex(); } /// Insert or replace the tempo at `tick`. @@ -105,6 +163,7 @@ impl TempoMap { } Err(i) => self.tempos.insert(i, TempoEvent { tick, bpm }), } + self.reindex(); Ok(()) } @@ -115,6 +174,7 @@ impl TempoMap { } let before = self.tempos.len(); self.tempos.retain(|e| e.tick != tick); + self.reindex(); before != self.tempos.len() } @@ -204,36 +264,20 @@ impl TempoMap { } fn tick_to_samples_f(&self, tick: i64, sr: SampleRate) -> f64 { - let mut acc = 0.0; - let mut seg_tick = 0i64; - let mut bpm = self.tempos.first().map_or(120.0, |e| e.bpm); - for e in self.tempos.iter().skip(1) { - if e.tick >= tick { - break; - } - acc += (e.tick - seg_tick) as f64 * Self::samples_per_tick(bpm, sr); - seg_tick = e.tick; - bpm = e.bpm; - } - acc + (tick - seg_tick) as f64 * Self::samples_per_tick(bpm, sr) + // Tempo events after the first that start before `tick` (the segments it has passed). + let passed = self.tempos.get(1..).map_or(0, |rest| rest.partition_point(|e| e.tick < tick)); + let (seg_tick, bpm, elapsed) = self.segment(passed); + elapsed * Self::samples_per_elapsed(sr) + (tick - seg_tick) as f64 * Self::samples_per_tick(bpm, sr) } /// Samples → ticks (fractional). pub fn samples_to_ticks_f(&self, s: Samples, sr: SampleRate) -> f64 { let s = s as f64; - let mut acc = 0.0; - let mut seg_tick = 0i64; - let mut bpm = self.tempos.first().map_or(120.0, |e| e.bpm); - for e in self.tempos.iter().skip(1) { - let seg = (e.tick - seg_tick) as f64 * Self::samples_per_tick(bpm, sr); - if acc + seg > s { - break; - } - acc += seg; - seg_tick = e.tick; - bpm = e.bpm; - } - seg_tick as f64 + (s - acc) / Self::samples_per_tick(bpm, sr) + let scale = Self::samples_per_elapsed(sr); + // Tempo events after the first that start at or before `s`. + let passed = self.elapsed.get(1..).map_or(0, |rest| rest.partition_point(|e| e * scale <= s)); + let (seg_tick, bpm, elapsed) = self.segment(passed); + seg_tick as f64 + (s - elapsed * scale) / Self::samples_per_tick(bpm, sr) } pub fn samples_to_ticks(&self, s: Samples, sr: SampleRate) -> i64 { @@ -319,6 +363,7 @@ impl TempoMap { e.bpm = valid_bpm(e.bpm * factor)?; } self.tempos = out; + self.reindex(); Ok(()) } } @@ -407,24 +452,24 @@ mod tests { fn sanitize_keeps_what_the_setters_accept() { let tempo = |tick, bpm| TempoEvent { tick, bpm }; let meter = |tick, numerator, denominator| MeterEvent { tick, numerator, denominator }; - let mut m = TempoMap { - tempos: vec![tempo(960, 4e9), tempo(3840, 90.0), tempo(-5, 100.0), tempo(1920, 60.0), tempo(1920, 70.0)], - meters: vec![meter(0, 0, 4), meter(3840, 3, 4)], - }; + let mut m = TempoMap::from_parts( + vec![tempo(960, 4e9), tempo(3840, 90.0), tempo(-5, 100.0), tempo(1920, 60.0), tempo(1920, 70.0)], + vec![meter(0, 0, 4), meter(3840, 3, 4)], + ); m.sanitize(); assert_eq!(m.tempos(), [tempo(0, 60.0), tempo(1920, 60.0), tempo(3840, 90.0)]); assert_eq!(m.meters(), [meter(0, 3, 4), meter(3840, 3, 4)]); - let mut empty = TempoMap { tempos: Vec::new(), meters: Vec::new() }; + let mut empty = TempoMap::from_parts(Vec::new(), Vec::new()); empty.sanitize(); assert_eq!(empty, TempoMap::default()); } #[test] fn sanitize_drops_events_beyond_the_position_range() { - let mut m = TempoMap { - tempos: vec![TempoEvent { tick: 0, bpm: 120.0 }, TempoEvent { tick: i64::MAX, bpm: 90.0 }], - meters: vec![MeterEvent { tick: 0, numerator: 4, denominator: 4 }, MeterEvent { tick: i64::MAX, numerator: 3, denominator: 4 }], - }; + let mut m = TempoMap::from_parts( + vec![TempoEvent { tick: 0, bpm: 120.0 }, TempoEvent { tick: i64::MAX, bpm: 90.0 }], + vec![MeterEvent { tick: 0, numerator: 4, denominator: 4 }, MeterEvent { tick: i64::MAX, numerator: 3, denominator: 4 }], + ); m.sanitize(); assert_eq!(m, TempoMap::default()); assert_eq!(m.bar_start_tick(2), TICKS_PER_QUARTER * 4); @@ -446,4 +491,68 @@ mod tests { assert_eq!(m.bar_beat_at_tick(-TICKS_PER_QUARTER * 4), BarBeat { bar: 0, beat: 1, tick: 0 }); assert_eq!(m.bar_beat_at_tick(-TICKS_PER_QUARTER), BarBeat { bar: 0, beat: 4, tick: 0 }); } + + /// The segment walk the indexed conversions replaced, kept as the reference they must match. + fn linear_tick_to_samples(m: &TempoMap, tick: i64, sr: SampleRate) -> i64 { + let mut acc = 0.0; + let mut seg_tick = 0i64; + let mut bpm = m.tempos[0].bpm; + for e in m.tempos.iter().skip(1) { + if e.tick >= tick { + break; + } + acc += (e.tick - seg_tick) as f64 * TempoMap::samples_per_tick(bpm, sr); + seg_tick = e.tick; + bpm = e.bpm; + } + to_samples(acc + (tick - seg_tick) as f64 * TempoMap::samples_per_tick(bpm, sr)) + } + + #[test] + fn indexed_conversions_match_the_segment_walk() { + let sr = SampleRate::HZ_48000; + let mut m = TempoMap::default(); + for i in 1..200i64 { + m.set_tempo(i * 487, 40.0 + ((i * 37) % 300) as f64).unwrap(); + } + assert!(m.remove_tempo(487 * 50)); + for tick in (-1000..200 * 487 + 5000).step_by(113) { + let s = m.tick_to_samples(tick, sr); + assert_eq!(s, linear_tick_to_samples(&m, tick, sr), "tick {tick}"); + assert!((m.samples_to_ticks(s, sr) - tick).abs() <= 1, "tick {tick}"); + } + let json = serde_json::to_string(&m).unwrap(); + assert!(!json.contains("elapsed")); + let back: TempoMap = serde_json::from_str(&json).unwrap(); + assert_eq!(back, m); + assert_eq!(back.tick_to_samples(150 * 487 + 3, sr), m.tick_to_samples(150 * 487 + 3, sr)); + } + + #[test] + fn a_file_with_wild_ticks_is_indexed_without_overflow() { + let json = r#"{"tempos":[{"tick":0,"bpm":120.0},{"tick":-9223372036854775808,"bpm":100.0},{"tick":9223372036854775807,"bpm":90.0}],"meters":[{"tick":0,"numerator":4,"denominator":4}]}"#; + let mut m: TempoMap = serde_json::from_str(json).unwrap(); + m.sanitize(); + assert_eq!(m.tempos().first().map(|e| e.tick), Some(0)); + assert_eq!(m.elapsed.len(), m.tempos().len()); + } + + #[test] + fn conversions_against_a_large_tempo_map_stay_fast() { + // 200k tempo changes, then 200k conversions each way: quadratic if every conversion walks + // the earlier segments, well under a second when it binary-searches them. + let sr = SampleRate::HZ_48000; + let n = 200_000i64; + let tempos = (0..n).map(|i| TempoEvent { tick: i * 10, bpm: 60.0 + (i % 7) as f64 }).collect(); + let mut m = TempoMap::from_parts(tempos, vec![MeterEvent { tick: 0, numerator: 4, denominator: 4 }]); + m.sanitize(); + let start = std::time::Instant::now(); + let mut sum = 0i64; + for i in 0..n { + let s = m.tick_to_samples(i * 10 + 5, sr); + sum = sum.wrapping_add(s).wrapping_add(m.samples_to_ticks(s, sr)); + } + assert!(sum != 0); + assert!(start.elapsed() < std::time::Duration::from_secs(5), "{:?}", start.elapsed()); + } }