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/autosave.rs

@@ -425,27 +425,6 @@ pub fn create_folder(dir: &Path) -> std::io::Result<()> {
     builder.create(dir)
 }
 
-/// A new folder directly in `parent`, named `{prefix}-` and 32 random hex digits. It is made
-/// exclusively (it is created by this call or the call fails), so a folder or link another account
-/// put there first is never used, and the name cannot be guessed ahead. On Unix it is owner-only
-/// (`0o700`); on other systems it has the mode `parent` gives it.
-pub fn create_unique_folder(parent: &Path, prefix: &str) -> std::io::Result<PathBuf> {
-    use std::hash::{BuildHasher, Hasher};
-    for _ in 0..16 {
-        let random = |_| std::collections::hash_map::RandomState::new().build_hasher().finish();
-        let dir = parent.join(format!("{prefix}-{:016x}{:016x}", random(0), random(1)));
-        let mut builder = std::fs::DirBuilder::new();
-        #[cfg(unix)]
-        std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700);
-        match builder.create(&dir) {
-            Ok(()) => return Ok(dir),
-            Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {}
-            Err(err) => return Err(err),
-        }
-    }
-    Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "no unused folder name"))
-}
-
 fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
     let mut f = std::fs::File::create(path)?;
     f.write_all(bytes)?;
@@ -468,22 +447,6 @@ fn io_error(path: &Path, err: std::io::Error) -> EngineError {
 mod tests {
     use super::*;
     use crate::io::open_session;
-
-    #[test]
-    fn unique_folders_are_new_and_owner_only() {
-        let parent = std::env::temp_dir().join(format!("soundcraft-unique-{}", std::process::id()));
-        let _ = std::fs::remove_dir_all(&parent);
-        std::fs::create_dir_all(&parent).unwrap();
-        let (a, b) = (create_unique_folder(&parent, "T").unwrap(), create_unique_folder(&parent, "T").unwrap());
-        assert_ne!(a, b);
-        assert!(a.is_dir() && a.parent() == Some(parent.as_path()));
-        assert_eq!(a.file_name().unwrap().len(), "T-".len() + 32);
-        #[cfg(unix)]
-        assert_eq!(std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(&a).unwrap().permissions()) & 0o777, 0o700);
-        assert!(create_unique_folder(&parent.join("missing"), "T").is_err(), "no parents are made");
-        let _ = std::fs::remove_dir_all(&parent);
-    }
-
     use serde_json::json;
     use soundcraft_audio_io::AudioBuffer;
     use soundcraft_model::{ChannelFormat, Clip, TrackKind};

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/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}");
-    }
-}

crates/ui-egui/src/app.rs

@@ -46,7 +46,6 @@ impl SoundApp {
             services,
             native_menu_bar: false,
             autosave_dir: None,
-            fallback_take_dir: None,
             autosaver: None,
             autosaved_rev: 0,
             leftovers: Vec::new(),

crates/ui-egui/src/app/playback.rs

@@ -155,21 +155,15 @@ impl SoundApp {
     /// autosave folder. Returns the takes it records.
     fn arm_recorder(&mut self) -> Result<Vec<soundcraft_engine::io::Take>, String> {
         let name = soundcraft_engine::io::sanitize_name(&self.engine.session().name);
-        // Without a settings folder the takes go under the temporary folder, in a folder of this
-        // run's own that nobody else could have made first.
-        let unsaved_dir = match &self.autosave_dir {
-            Some(d) => d.join(name),
-            None => self.fallback_take_dir()?,
-        };
+        let unsaved_dir = self.autosave_dir.as_ref().map_or_else(std::env::temp_dir, |d| d.join(name));
         let Some(r) = &mut self.recorder else { return Err("no audio input device".into()) };
         let plan = soundcraft_engine::io::plan_takes(&self.engine, &unsaved_dir, r.channels);
         if plan.is_empty() {
             return Err("no audio track is record-enabled".into());
         }
-        // The takes are unsaved work: on Unix the folder they go into is owner-only, like the
-        // autosaves, before the recorder makes anything in it (other systems get the folder's
-        // inherited access: no ACLs are set, so the temporary folder's own per-user access applies).
-        if self.engine.path.is_none() {
+        // The takes are unsaved work: the folder they go into under the autosave root is
+        // owner-only, like the autosaves, before the recorder makes anything in it.
+        if self.engine.path.is_none() && self.autosave_dir.is_some() {
             for dir in plan.iter().filter_map(|(_, _, path)| path.parent()) {
                 soundcraft_engine::autosave::create_folder(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
             }
@@ -182,19 +176,6 @@ impl SoundApp {
         Ok(takes)
     }
 
-    /// The folder for the takes of a session never saved when there is no autosave folder, made on
-    /// first use: exclusively and under a name nobody can guess, so another account cannot have
-    /// made it first, with open permissions or as a link.
-    fn fallback_take_dir(&mut self) -> Result<std::path::PathBuf, String> {
-        if let Some(dir) = &self.fallback_take_dir {
-            return Ok(dir.clone());
-        }
-        let dir = soundcraft_engine::autosave::create_unique_folder(&std::env::temp_dir(), "SoundCraft")
-            .map_err(|e| format!("temporary take folder: {e}"))?;
-        self.fallback_take_dir = Some(dir.clone());
-        Ok(dir)
-    }
-
     fn finish_recording(&mut self) {
         self.engine.transport.recording = false;
         if let Some(p) = &self.player {

crates/ui-egui/src/lib.rs

@@ -278,8 +278,6 @@ pub struct SoundApp {
     pub native_menu_bar: bool,
     /// Folder for crash-recovery autosaves (native apps set it).
     pub autosave_dir: Option<std::path::PathBuf>,
-    /// Where the takes of a session never saved go without an autosave folder: made once per run, under the temporary folder.
-    fallback_take_dir: Option<std::path::PathBuf>,
     /// Writes the open session's autosaves in the background, once there is something to write.
     autosaver: Option<soundcraft_engine::autosave::Autosaver>,
     /// The document revision last handed to the autosaver (0 once that autosave is stale).

crates/ui-egui/src/video_track.rs

@@ -90,13 +90,6 @@ impl Worker {
 }
 
 fn open_movie(path: &str) -> Result<Movie, String> {
-    // The last check before the file system: the path came from a session file, so the read
-    // roots apply, and only a regular file is opened (a pipe would hold the video worker
-    // forever).
-    soundcraft_engine::file_access::check_read(path)?;
-    if !std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?.is_file() {
-        return Err(format!("{path}: not a regular file"));
-    }
     #[cfg(not(target_arch = "wasm32"))]
     {
         let f = std::fs::File::open(path).map_err(|e| format!("{path}: {e}"))?;