AudioSuite: budget pitch shift at its input length

A clip listed more than once in audiosuite.process was processed again
on its own result: the replacement keeps the clip id, so an expanding
process (time stretch, varispeed, pitch shift) compounded the clip's
length each time, and every intermediate result stayed in the source
pool for the rest of the command.

Deduplicate the clip ids, and before processing anything check each
clip's per-render limit and the projected output of all clips together
against MAX_RENDER_SAMPLES, the same total budget the signal generator
already uses.

crates/clap-host/src/lib.rs

@@ -222,33 +222,10 @@ 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 = fresh_private_dir("soundcraft-clap-garbage");
+        let d = std::env::temp_dir().join(format!("soundcraft-clap-garbage-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&d);
         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

@@ -87,23 +87,16 @@ fn decode_group_audio(
     cmd_id: &str,
 ) -> Result<(Vec<(String, soundcraft_audio_io::AudioBuffer)>, BTreeMap<u64, usize>)> {
     let (mut decoded, mut index, mut file_of) = (Vec::new(), BTreeMap::new(), BTreeMap::new());
-    // The folder as the file system knows it, so a link among the group's files can't lead out.
-    let dir = if dir.as_os_str().is_empty() { Path::new(".") } else { dir };
-    let root = crate::io::canonical_path(dir)?;
     for (old, rel) in audio_entries {
         let full = dir.join(rel);
-        let escapes = |rel: &str| EngineError::BadParams(cmd_id.into(), format!("audio path escapes the group folder: {rel}"));
         if !full.starts_with(dir)
             || Path::new(rel)
                 .components()
                 .any(|c| matches!(c, std::path::Component::ParentDir | std::path::Component::RootDir | std::path::Component::Prefix(_)))
         {
-            return Err(escapes(rel));
-        }
-        let key = crate::io::canonical_path(&full)?;
-        if !key.starts_with(&root) {
-            return Err(escapes(rel));
+            return Err(EngineError::BadParams(cmd_id.into(), format!("audio path escapes the group folder: {rel}")));
         }
+        let key = crate::io::file_key(&full);
         let n = match index.get(&key) {
             Some(n) => *n,
             None => {
@@ -275,59 +268,9 @@ mod tests {
         std::fs::write(&path, file.to_string()).unwrap();
         let r = e.execute("file.import_clip_groups", &json!({"path": path, "at": 1000, "track": "Bass"}));
         assert!(matches!(r, Err(crate::EngineError::BadParams(..))), "absolute paths are refused: {r:?}");
-        // A link inside the folder that leads out of it is refused once resolved.
-        #[cfg(unix)]
-        {
-            let outside = std::env::temp_dir().join(format!("sc-grp-outside-{}.wav", std::process::id()));
-            std::fs::copy(dir.join("Wait... what_0.wav"), &outside).unwrap();
-            std::os::unix::fs::symlink(&outside, dir.join("link.wav")).unwrap();
-            for rel in file["audio"].as_object_mut().unwrap().values_mut() {
-                *rel = json!("link.wav");
-            }
-            std::fs::write(&path, file.to_string()).unwrap();
-            let r = e.execute("file.import_clip_groups", &json!({"path": path, "at": 1000, "track": "Bass"}));
-            assert!(matches!(r, Err(crate::EngineError::BadParams(..))), "links out of the folder are refused: {r:?}");
-            let _ = std::fs::remove_file(&outside);
-        }
         let _ = std::fs::remove_dir_all(&dir);
     }
 
-    /// Under a read root, a link in the group folder to a folder outside the root is refused the same way
-    /// whether or not the file asked for exists there, so the group cannot be used to probe for names.
-    #[cfg(unix)]
-    #[test]
-    fn links_out_of_the_read_root_are_refused_alike_for_existing_and_missing_files() {
-        let base = std::env::temp_dir().join(format!("sc-grp-root-{}", std::process::id()));
-        let _ = std::fs::remove_dir_all(&base);
-        let (dir, outside) = (base.join("group"), base.join("outside"));
-        std::fs::create_dir_all(&dir).unwrap();
-        std::fs::create_dir_all(&outside).unwrap();
-        let path = dir.join("drums.scgrp").to_string_lossy().to_string();
-        let mut e = crate::demo::demo_engine();
-        let kick = e.session().tracks.iter().find(|t| t.name == "Kick").map(|t| t.id).unwrap();
-        let ids: Vec<_> = e.session().track(kick).unwrap().clips().iter().take(1).map(|c| c.id.0).collect();
-        e.execute("file.export_clip_groups", &json!({"path": path, "clips": ids})).unwrap();
-        let mut file: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap();
-        let exported = file["audio"].as_object().unwrap().values().next().unwrap().as_str().unwrap().to_string();
-        std::fs::copy(dir.join(&exported), outside.join("real.wav")).unwrap();
-        std::os::unix::fs::symlink(&outside, dir.join("out")).unwrap();
-        let roots = crate::file_access::AutomationRoots::new(&[&dir], &[]).unwrap().unwrap();
-        let mut refusals = Vec::new();
-        for target in ["out/real.wav", "out/missing.wav"] {
-            for rel in file["audio"].as_object_mut().unwrap().values_mut() {
-                *rel = json!(target);
-            }
-            std::fs::write(&path, file.to_string()).unwrap();
-            let r = crate::file_access::confine(Some(&roots), || {
-                e.execute("file.import_clip_groups", &json!({"path": path, "at": 1000, "track": "Bass"}))
-            });
-            refusals.push(r.unwrap_err().to_string());
-        }
-        assert!(refusals[0].contains("outside the read root"), "{}", refusals[0]);
-        assert_eq!(refusals[0].replace("real.wav", "X"), refusals[1].replace("missing.wav", "X"), "existing and missing names are refused alike");
-        let _ = std::fs::remove_dir_all(&base);
-    }
-
     #[test]
     fn imported_clips_carry_no_auto_rename_scheme() {
         let dir = std::env::temp_dir().join(format!("sc-grp-names-{}", std::process::id()));

crates/engine/src/cmd/audiosuite.rs

@@ -20,14 +20,23 @@ 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" => 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 }),
+        "time_stretch" | "pitch_shift" | "varispeed" | "speed_ramp" => {
+            (soundcraft_dsp::offline::MAX_OFFLINE_SAMPLES as f64 / growth(name, ratio, semitones, sr, to, selection)) as usize
+        }
         _ => usize::MAX,
     }
 }
@@ -76,14 +85,33 @@ 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();
@@ -350,6 +378,40 @@ 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/io.rs

@@ -85,17 +85,6 @@ pub(crate) fn make_dir(dir: impl AsRef<Path>) -> Result<()> {
     std::fs::create_dir_all(dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display())))
 }
 
-/// `path` with its links followed, inside the automation read root when there is one. The root is
-/// checked on the path as named (links followed, a missing tail resolved by its nearest folder)
-/// before anything is resolved, so a name that leads out of the root is refused the same way
-/// whether or not it exists there.
-pub(crate) fn canonical_path(path: impl AsRef<Path>) -> Result<PathBuf> {
-    let path = path.as_ref();
-    let checked = if path.as_os_str().is_empty() { Path::new(".") } else { path };
-    crate::file_access::check_read(checked).map_err(EngineError::Io)?;
-    std::fs::canonicalize(path).map_err(|err| EngineError::Io(format!("{}: {err}", path.display())))
-}
-
 /// Write `bytes` to the file at `path`, inside the automation write root when there is one.
 pub(crate) fn write_path(path: impl AsRef<Path>, bytes: impl AsRef<[u8]>) -> Result<()> {
     let path = path.as_ref();
@@ -340,7 +329,7 @@ impl Out {
         if cfg!(target_arch = "wasm32") {
             return Ok(Out::Memory(std::io::Cursor::new(Vec::new())));
         }
-        Ok(Out::File(std::io::BufWriter::with_capacity(1 << 20, create_new(part)?)))
+        Ok(Out::File(std::io::BufWriter::with_capacity(1 << 20, std::fs::File::create(part)?)))
     }
 
     /// Puts the finished file at `path` and returns its size: the part file is synced and renamed
@@ -473,27 +462,14 @@ pub fn export_clips(e: &Engine, ids: &[soundcraft_model::ClipId], dir: &str) ->
         let stem = sanitize_name(&c.name);
         let mut path = Path::new(dir).join(format!("{stem}.wav"));
         let mut k = 2u32;
-        // The name is taken by creating the file, so whatever entry already has it (a link
-        // to elsewhere included) is left alone rather than written through.
-        let mut file = loop {
-            // An occupied name is taken whatever it is (a link whose target is outside the write
-            // root too): it is only looked at, never resolved, so the root check does not stop the
-            // search for a free name. The folder itself was checked above.
-            if std::fs::symlink_metadata(&path).is_err() {
-                crate::file_access::check_write(&path).map_err(EngineError::Io)?;
-                match std::fs::OpenOptions::new().write(true).create_new(true).open(&path) {
-                    Ok(f) => break f,
-                    Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {}
-                    Err(err) => return Err(EngineError::Io(format!("{}: {err}", path.display()))),
-                }
-            }
+        while path.exists() {
             path = Path::new(dir).join(format!("{stem}-{k}.wav"));
             k = k.saturating_add(1);
             if k > 10_000 {
                 return Err(EngineError::Io(format!("{dir}: too many files named {stem}")));
             }
-        };
-        std::io::Write::write_all(&mut file, &bytes).map_err(|err| EngineError::Io(format!("{}: {err}", path.display())))?;
+        }
+        write_path(&path, bytes)?;
         n += 1;
     }
     Ok(n)
@@ -530,13 +506,13 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess
         {
             src.path = f.join(&src.path).to_string_lossy().into_owned();
         }
-        let exists_rel = same_dir && !Path::new(&src.path).is_absolute() && file_found(dir.join(&src.path));
+        let exists_rel = same_dir && !Path::new(&src.path).is_absolute() && dir.join(&src.path).exists();
         let shared = shared_paths.get(i).copied().unwrap_or(false);
         if exists_rel && !src.unsaved && !shared {
             continue;
         }
         let in_folder = Path::new(&src.path).strip_prefix(&audio_dir).ok().and_then(Path::to_str).map(str::to_string);
-        if let Some(f) = in_folder.filter(|_| !src.unsaved && file_found(&src.path)) {
+        if let Some(f) = in_folder.filter(|_| !src.unsaved && Path::new(&src.path).is_file()) {
             src.path = format!("Audio Files/{f}");
             continue;
         }
@@ -552,7 +528,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess
         if overwrite {
             // One file per source id so autosaves never collide or pile up.
             stem = format!("{stem}_{}", src.id.0);
-            if file_found(audio_dir.join(format!("{stem}.wav"))) {
+            if audio_dir.join(format!("{stem}.wav")).exists() {
                 src.path = format!("Audio Files/{stem}.wav");
                 src.unsaved = false;
                 files.insert(Arc::as_ptr(audio), src.path.clone());
@@ -612,22 +588,11 @@ fn sync_dir(dir: &Path) {
 
 fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
     use std::io::Write;
-    let mut f = create_new(path)?;
+    let mut f = std::fs::File::create(path)?;
     f.write_all(bytes)?;
     f.sync_all()
 }
 
-/// Creates `path` as a new file. A leftover of that name (a part file from an interrupted write)
-/// is removed first, but an existing entry is never opened: a link planted at a predictable name
-/// in the destination folder can't redirect the write elsewhere.
-fn create_new(path: &Path) -> std::io::Result<std::fs::File> {
-    match std::fs::remove_file(path) {
-        Err(err) if err.kind() != std::io::ErrorKind::NotFound => return Err(err),
-        _ => {}
-    }
-    std::fs::OpenOptions::new().write(true).create_new(true).open(path)
-}
-
 /// Copies `from` to `to`, which must not exist yet; an existing entry, even a link, is never followed.
 fn copy_new(from: &Path, to: &Path) -> std::io::Result<()> {
     let mut src = std::fs::File::open(from)?;
@@ -722,12 +687,7 @@ fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> {
                 return Ok((name, true));
             }
             Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
-                // Reused only when it may be read: whether it equals the audio must not be
-                // learnt about a file the roots allow writing beside but not reading.
-                if crate::file_access::check_read(&path).is_ok()
-                    && std::fs::metadata(&path).is_ok_and(|m| m.len() == bytes.len() as u64)
-                    && std::fs::read(&path).is_ok_and(|b| b == bytes)
-                {
+                if std::fs::metadata(&path).is_ok_and(|m| m.len() == bytes.len() as u64) && std::fs::read(&path).is_ok_and(|b| b == bytes) {
                     return Ok((name, false));
                 }
             }
@@ -825,67 +785,6 @@ pub fn plan_takes(e: &Engine, unsaved_dir: &Path, input_channels: usize) -> Vec<
         .collect()
 }
 
-#[cfg(test)]
-mod exclusive_tests {
-    use super::*;
-
-    fn scratch(name: &str) -> PathBuf {
-        let dir = std::env::temp_dir().join(format!("soundcraft-io-{name}-{}", std::process::id()));
-        let _ = std::fs::remove_dir_all(&dir);
-        std::fs::create_dir_all(&dir).unwrap();
-        dir
-    }
-
-    #[cfg(unix)]
-    #[test]
-    fn a_planted_link_at_the_part_name_is_not_written_through() {
-        let dir = scratch("part-link");
-        let (target, part) = (dir.join("elsewhere"), dir.join("session.scraft.tmp"));
-        std::os::unix::fs::symlink(&target, &part).unwrap();
-        write_synced(&part, b"new").unwrap();
-        assert!(!target.exists(), "the link's target was created");
-        assert_eq!(std::fs::read(&part).unwrap(), b"new", "the link itself was replaced by the file");
-        // A leftover regular part file is still replaced.
-        write_synced(&part, b"newer").unwrap();
-        assert_eq!(std::fs::read(&part).unwrap(), b"newer");
-        let _ = std::fs::remove_dir_all(&dir);
-    }
-
-    #[cfg(unix)]
-    #[test]
-    fn export_skips_a_name_held_by_a_dangling_link() {
-        let dir = scratch("export-link");
-        let e = crate::demo::demo_engine();
-        let (t, c) = e.session().tracks.iter().find_map(|t| t.clips().iter().find(|c| c.is_audio()).map(|c| (t.id, c.clone()))).unwrap();
-        let _ = t;
-        let target = dir.join("elsewhere.wav");
-        std::os::unix::fs::symlink(&target, dir.join(format!("{}.wav", sanitize_name(&c.name)))).unwrap();
-        assert_eq!(export_clips(&e, &[c.id], &dir.to_string_lossy()).unwrap(), 1);
-        assert!(!target.exists(), "the export followed the link");
-        assert!(dir.join(format!("{}-2.wav", sanitize_name(&c.name))).is_file(), "the next free name was used");
-        let _ = std::fs::remove_dir_all(&dir);
-    }
-
-    /// With a write root, a link out of it that holds the name is still just an occupied name.
-    #[cfg(unix)]
-    #[test]
-    fn export_skips_a_link_out_of_the_write_root_too() {
-        let dir = scratch("export-root-link");
-        let e = crate::demo::demo_engine();
-        let c = e.session().tracks.iter().find_map(|t| t.clips().iter().find(|c| c.is_audio()).cloned()).unwrap();
-        let elsewhere = scratch("export-root-elsewhere");
-        let target = elsewhere.join("target.wav");
-        std::os::unix::fs::symlink(&target, dir.join(format!("{}.wav", sanitize_name(&c.name)))).unwrap();
-        let roots = crate::file_access::AutomationRoots::new(&[], &[&dir]).unwrap().unwrap();
-        let n = crate::file_access::confine(Some(&roots), || export_clips(&e, &[c.id], &dir.to_string_lossy())).unwrap();
-        assert_eq!(n, 1);
-        assert!(!target.exists(), "the export followed the link out of the root");
-        assert!(dir.join(format!("{}-2.wav", sanitize_name(&c.name))).is_file(), "the next free name was used");
-        let _ = std::fs::remove_dir_all(&dir);
-        let _ = std::fs::remove_dir_all(&elsewhere);
-    }
-}
-
 #[cfg(test)]
 mod stem_tests {
     #[test]

crates/midi/src/sequence.rs

@@ -1,6 +1,4 @@
 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)]
@@ -110,28 +108,14 @@ 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.
-            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);
-                }
+            // 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),
             }
         }
         self.ctrls = kept;

crates/midi/tests/it/ops.rs

@@ -222,72 +222,3 @@ 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}");
-    }
-}