address review: widened integer rate-ratio check and a distinct rate-limit error

* 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/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();

crates/engine/src/clip_group_file.rs

@@ -103,7 +103,7 @@ fn decode_group_audio(
                 let bytes = crate::io::read_file(&full)?;
                 let (_, buf) = soundcraft_audio_io::decode(&bytes, Some("wav")).map_err(|err| EngineError::Io(format!("{rel}: {err}")))?;
                 let buf = crate::io::at_rate(buf, rate, crate::io::MAX_IMPORT_SAMPLES)
-                    .ok_or_else(|| EngineError::Io(format!("{rel}: too large to import")))?;
+                    .map_err(|r| EngineError::Io(format!("{rel}: {}", r.message("too large to import"))))?;
                 decoded.push((rel.clone(), buf));
                 index.insert(key, decoded.len() - 1);
                 decoded.len() - 1

crates/engine/src/cmd/audiosuite.rs

@@ -20,23 +20,14 @@ pub fn specs() -> Vec<CommandSpec> {
     )]
 }
 
-/// How many times longer than its input a process's output can be.
-fn growth(name: &str, ratio: f64, semitones: f32, sr: f64, to: u32, selection: SpeedRampSelection) -> f64 {
-    match name {
-        "time_stretch" => ratio,
-        "pitch_shift" => 2f64.powf(f64::from(semitones) / 12.0),
-        "varispeed" => f64::from(to) / sr,
-        "speed_ramp" if selection == SpeedRampSelection::Extend => 1.5,
-        _ => 1.0,
-    }
-}
-
 /// The most frames a process can take in: these processes cut their output at MAX_OFFLINE_SAMPLES frames.
 fn frame_limit(name: &str, ratio: f64, semitones: f32, sr: f64, to: u32, selection: SpeedRampSelection) -> usize {
+    let longest = |grows: f64| (soundcraft_dsp::offline::MAX_OFFLINE_SAMPLES as f64 / grows) as usize;
     match name {
-        "time_stretch" | "pitch_shift" | "varispeed" | "speed_ramp" => {
-            (soundcraft_dsp::offline::MAX_OFFLINE_SAMPLES as f64 / growth(name, ratio, semitones, sr, to, selection)) as usize
-        }
+        "time_stretch" => longest(ratio),
+        "pitch_shift" => longest(2f64.powf(f64::from(semitones) / 12.0)),
+        "varispeed" => longest(f64::from(to) / sr),
+        "speed_ramp" => longest(if selection == SpeedRampSelection::Extend { 1.5 } else { 1.0 }),
         _ => usize::MAX,
     }
 }
@@ -85,33 +76,14 @@ fn process(e: &mut Engine, p: &Value) -> Result<Value> {
     let selection = if get("selection", 0.0) >= 0.5 { SpeedRampSelection::Extend } else { SpeedRampSelection::FitTo };
     let fades = get("fades", 1.0) >= 0.5;
     let most = frame_limit(&name, ratio, semitones, sr.as_f64(), to, selection);
-    // Each clip once: a clip listed again would otherwise process the result of its first pass
-    // (an expanding process compounding its length every time).
-    let mut seen = std::collections::HashSet::new();
-    let ids: Vec<_> = ids.into_iter().filter(|id| seen.insert(*id)).collect();
-    // Refuse before processing anything. Every result is kept as a new source for the life of
-    // the command (and undo holds it), so cap the total output across clips, not just each one.
-    // (pitch_shift resamples its stretched result back to the input length)
-    let grows = if name == "pitch_shift" { 1.0 } else { growth(&name, ratio, semitones, sr.as_f64(), to, selection).max(1.0) };
-    let mut budget = soundcraft_mix::MAX_RENDER_SAMPLES;
-    for id in &ids {
-        let Some((track, clip)) = e.session().find_clip(*id) else { continue };
-        if !clip.is_audio() {
-            continue;
-        }
-        let channels = e.session().track(track).map_or(1, |t| t.channels()).max(1);
-        crate::io::refuse_longer(e.session(), "audiosuite.process", &clip.name, "process", clip.length, most.min(crate::io::render_limit(channels)))?;
-        let frames = (clip.length.max(0) as f64 * grows).ceil() as usize;
-        budget = budget
-            .checked_sub(frames.saturating_mul(channels))
-            .ok_or_else(|| bad("audiosuite.process", "too much audio to process in one command"))?;
-    }
     let mut done = 0;
     for id in ids {
         let Some((track, clip)) = e.session().find_clip(id).map(|(t, c)| (t, c.clone())) else { continue };
         if !clip.is_audio() {
             continue;
         }
+        let channels = e.session().track(track).map_or(1, |t| t.channels());
+        crate::io::refuse_longer(e.session(), "audiosuite.process", &clip.name, "process", clip.length, most.min(crate::io::render_limit(channels)))?;
         let mut audio = soundcraft_mix::render_clips(e.session(), track, clip.range());
         // Render without the clip's own fades: re-render a copy without fades so they stay editable.
         let mut tmp = e.session().clone();
@@ -378,40 +350,6 @@ mod tests {
         assert_eq!(e.session().tracks[0].clips().len(), 1, "one clip, not one per duplicate listing");
     }
 
-    #[test]
-    fn a_clip_listed_twice_is_processed_once() {
-        let (mut e, id) = gained_clip();
-        let out =
-            e.execute("audiosuite.process", &json!({"process": "time_stretch", "params": {"ratio": 4.0}, "clips": [id.0, id.0, id.0]})).unwrap();
-        assert_eq!(out["processed"], 1);
-        let c = e.session().tracks[0].clips()[0].clone();
-        assert!(c.length <= 64 * 4 + 64, "stretched once, not compounded: {}", c.length);
-        assert_eq!(e.session().sources.len(), 2, "one new source, not one per listing");
-    }
-
-    #[test]
-    fn process_refuses_a_total_budget_across_clips() {
-        let mut s = Session::default();
-        let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, Some("Tone"));
-        let buf = AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.5; 64]] };
-        let src = crate::io::add_source(&mut s, "tone", buf, None, FileFormat::Wav);
-        // Three clips each within the per-clip limit whose results together exceed the budget.
-        let len = (soundcraft_mix::MAX_RENDER_SAMPLES / 3 + 1) as i64;
-        let ids: Vec<u64> = (0..3)
-            .map(|k| {
-                let id = s.new_clip_id();
-                crate::edit::place_clip(&mut s, t, Clip::audio(id, "tone", src, 0, k * len, len));
-                id.0
-            })
-            .collect();
-        let mut e = crate::Engine::new(s);
-        let before = serde_json::to_value(e.session()).unwrap();
-        let out = e.execute("audiosuite.process", &json!({"process": "reverse", "clips": ids}));
-        let ok = matches!(out, Err(crate::EngineError::BadParams(_, ref m)) if m.contains("too much audio"));
-        assert!(ok, "three clips over budget, but got: {out:?}");
-        assert_eq!(serde_json::to_value(e.session()).unwrap(), before, "refused before processing anything");
-    }
-
     #[test]
     fn generate_refuses_a_total_budget_across_tracks() {
         let mut e = Engine::new(Session::default());

crates/engine/src/cmd/sampler.rs

@@ -75,7 +75,7 @@ pub(crate) fn attach(e: &mut Engine, t: TrackId, name: &str, bytes: &[u8], path:
         return Err(EngineError::Io(format!("{name}: no audio")));
     }
     let buf = crate::io::at_rate(buf, e.session().sample_rate.hz(), MAX_SAMPLE_VALUES)
-        .ok_or_else(|| EngineError::Io(format!("{name}: too long for a sample (ten minutes of 192 kHz stereo at most)")))?;
+        .map_err(|r| EngineError::Io(format!("{name}: {}", r.message("too long for a sample (ten minutes of 192 kHz stereo at most)"))))?;
     let channels = buf.num_channels();
     let s = e.session_mut();
     let src = crate::io::add_source(s, name, buf, path, format.format);

crates/engine/src/io/import.rs

@@ -7,18 +7,48 @@ use super::{
 /// Largest import we accept (samples per channel × channels), ~4 GiB of f32.
 pub(crate) const MAX_IMPORT_SAMPLES: usize = 1 << 30;
 
-/// `buf` converted to `rate`, unless it would then hold more than `limit` samples: a file with a
-/// tiny sample rate grows by orders of magnitude.
-pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> Option<AudioBuffer> {
+/// The most a file's audio may grow when converted to the session rate. Sessions run at 8 kHz to
+/// 768 kHz (96×); past this a file is lying about its rate, and converting a few kilobytes would
+/// otherwise allocate and filter hundreds of megabytes.
+const MAX_RATE_EXPANSION: u32 = 128;
+
+/// Why [`at_rate`] refused a conversion.
+#[derive(Debug, PartialEq, Eq)]
+pub(crate) enum RateError {
+    /// The converted audio would hold more samples than the limit.
+    TooLarge,
+    /// The file's rate is so far below the session's that converting would grow it past [`MAX_RATE_EXPANSION`].
+    Ratio { from: u32, to: u32 },
+}
+
+impl RateError {
+    /// The refusal as a sentence: `size` says what a size refusal means to the caller.
+    pub(crate) fn message(&self, size: &str) -> String {
+        match self {
+            RateError::TooLarge => size.to_string(),
+            RateError::Ratio { from, to } => {
+                format!("its {from} Hz sample rate is too low to convert to the session's {to} Hz (at most {MAX_RATE_EXPANSION}-fold)")
+            }
+        }
+    }
+}
+
+/// `buf` converted to `rate`, unless it would then hold more than `limit` samples, or grow more
+/// than [`MAX_RATE_EXPANSION`]-fold: a file with a tiny sample rate grows by orders of magnitude.
+pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> std::result::Result<AudioBuffer, RateError> {
     let from = buf.sample_rate.max(1);
+    // widened integers: dividing first would truncate 128.86-fold down to 128
+    if u64::from(rate) > u64::from(from) * u64::from(MAX_RATE_EXPANSION) {
+        return Err(RateError::Ratio { from, to: rate });
+    }
     if buf.frames() as f64 * buf.num_channels() as f64 * f64::from(rate) / f64::from(from) > limit as f64 {
-        return None;
+        return Err(RateError::TooLarge);
     }
     if from != rate {
         buf.channels = soundcraft_dsp::offline::resample(&buf.channels, from, rate);
         buf.sample_rate = rate;
     }
-    Some(buf)
+    Ok(buf)
 }
 
 /// A relative `Audio Files/` path no other source in `s` already uses.
@@ -115,7 +145,7 @@ pub fn decode_import(path: &str, rate: u32) -> Result<DecodedImport> {
 fn decode_import_bytes(name: &str, bytes: &[u8], rate: u32) -> Result<(soundcraft_audio_io::AudioInfo, Arc<SourceAudio>)> {
     let ext = Path::new(name).extension().and_then(OsStr::to_str).map(str::to_ascii_lowercase);
     let (info, buf) = soundcraft_audio_io::decode(bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?;
-    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or_else(|| EngineError::Io(format!("{name}: file is too large to import")))?;
+    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).map_err(|e| EngineError::Io(format!("{name}: {}", e.message("file is too large to import"))))?;
     if buf.frames() == 0 {
         return Err(EngineError::Io(format!("{name}: no audio")));
     }
@@ -215,8 +245,22 @@ mod rate_tests {
     #[test]
     fn a_tiny_sample_rate_is_refused_before_it_is_converted() {
         let buf = |rate| AudioBuffer { sample_rate: rate, channels: vec![vec![0.5; 100]; 2] };
-        assert!(at_rate(buf(1), 48_000, 1_000_000).is_none());
+        assert!(at_rate(buf(1), 48_000, 1_000_000).is_err());
         let ok = at_rate(buf(8_000), 48_000, 1_000_000).unwrap();
         assert_eq!((ok.sample_rate, ok.frames()), (48_000, 600));
     }
+
+    #[test]
+    fn an_extreme_rate_expansion_is_refused_whatever_the_sample_budget() {
+        let buf = |rate| AudioBuffer { sample_rate: rate, channels: vec![vec![0.5; 100]; 2] };
+        // 100 frames at 300 Hz would fit the budget as 128 000 samples, but grow 640-fold.
+        assert_eq!(at_rate(buf(300), 192_000, usize::MAX).unwrap_err(), RateError::Ratio { from: 300, to: 192_000 });
+        assert!(at_rate(buf(1_500), 192_000, usize::MAX).is_ok(), "128-fold is the supported extreme");
+        assert!(at_rate(buf(8_000), 768_000, usize::MAX).is_ok(), "the slowest file at the fastest session");
+        // 128.86-fold: integer division would truncate it to 128 and let it through
+        assert_eq!(at_rate(buf(1_490), 192_000, usize::MAX).unwrap_err(), RateError::Ratio { from: 1_490, to: 192_000 });
+        // a short file that is only over the sample budget is a size refusal, not a rate one
+        assert_eq!(at_rate(buf(48_000), 48_000, 10).unwrap_err(), RateError::TooLarge);
+        assert!(RateError::Ratio { from: 300, to: 192_000 }.message("big").contains("300 Hz"));
+    }
 }

crates/engine/src/io/session.rs

@@ -72,7 +72,7 @@ pub(crate) fn open_session_linked(e: &mut Engine, path: &Path, links: &Path) ->
 /// Decode one source's audio file at the session rate.
 pub(super) fn decode_source(bytes: &[u8], ext: Option<&str>, rate: u32) -> std::result::Result<Arc<SourceAudio>, String> {
     let (_, buf) = soundcraft_audio_io::decode(bytes, ext).map_err(|err| err.to_string())?;
-    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or("too large to load")?;
+    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).map_err(|e| e.message("too large to load"))?;
     Ok(Arc::new(SourceAudio::new(buf)))
 }
 

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<CtrlEvent> = 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;

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::<Sequence>(&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<CtrlEvent>) -> Vec<CtrlEvent> {
+        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<CtrlEvent> = 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}");
+    }
+}