From 1617733a98f5b7f58c19de8a6907d704d8ea7cef Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:38:52 +0100 Subject: [PATCH 1/9] Repair a session's sample rate and tempo map when it loads A session file with a sample rate of 0 or a tempo of 4e9 BPM opened fine, but Strip Silence on the first and Extract Chords on the second never finished (one also climbed past 1 GiB). Session::sanitize, which repairs everything else read from a file, skipped both fields. On load, a sample rate outside what sessions accept becomes 48 kHz, and the tempo map keeps only the tempos and meters that set_tempo and set_meter accept, in tick order, with one of each at tick 0. --- crates/model/src/session.rs | 13 ++++++++++++ crates/time/src/tempo.rs | 41 ++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 5956796..9c7e882 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -570,6 +570,10 @@ impl Session { if !self.edit.zoom.samples_per_px.is_finite() || self.edit.zoom.samples_per_px <= 0.0 { self.edit.zoom.samples_per_px = 1024.0; } + if SampleRate::new(self.sample_rate.hz()).is_err() { + self.sample_rate = SampleRate::default(); + } + self.tempo.sanitize(); } } @@ -641,6 +645,15 @@ mod tests { assert_eq!(c.source_offset(), 580); } + #[test] + fn a_bad_sample_rate_or_tempo_is_repaired_on_load() { + let mut v = serde_json::to_value(Session::default()).unwrap(); + v["sample_rate"] = serde_json::json!(0); + v["tempo"]["tempos"] = serde_json::json!([{"tick": 0, "bpm": 4e9}]); + let s = Session::from_json(&v.to_string()).unwrap(); + assert_eq!((s.sample_rate, s.tempo), (SampleRate::HZ_48000, TempoMap::default())); + } + #[test] fn rejects_garbage_and_future_versions() { assert!(Session::from_json("not json").is_err()); diff --git a/crates/time/src/tempo.rs b/crates/time/src/tempo.rs index 5b87a3a..72936d8 100644 --- a/crates/time/src/tempo.rs +++ b/crates/time/src/tempo.rs @@ -55,6 +55,10 @@ fn valid_bpm(bpm: f64) -> Result { if bpm.is_finite() && (5.0..=1000.0).contains(&bpm) { Ok(bpm) } else { Err(TimeError::Tempo(bpm)) } } +fn valid_meter(numerator: u32, denominator: u32) -> bool { + (1..=64).contains(&numerator) && [1, 2, 4, 8, 16, 32, 64].contains(&denominator) +} + impl TempoMap { pub fn new(bpm: f64, numerator: u32, denominator: u32) -> Result { let mut m = TempoMap::default(); @@ -70,6 +74,25 @@ impl TempoMap { &self.meters } + /// Repair a map read from a file: only valid tempos and meters, in tick order, with one of each + /// at tick 0. + pub fn sanitize(&mut self) { + self.tempos.retain(|e| e.tick >= 0 && valid_bpm(e.bpm).is_ok()); + self.tempos.sort_by_key(|e| e.tick); + self.tempos.dedup_by_key(|e| e.tick); + if self.tempos.first().is_none_or(|e| e.tick > 0) { + let bpm = self.tempos.first().map_or(120.0, |e| e.bpm); + self.tempos.insert(0, TempoEvent { tick: 0, bpm }); + } + self.meters.retain(|m| m.tick >= 0 && valid_meter(m.numerator, m.denominator)); + self.meters.sort_by_key(|m| m.tick); + self.meters.dedup_by_key(|m| m.tick); + if self.meters.first().is_none_or(|m| m.tick > 0) { + let (numerator, denominator) = self.meters.first().map_or((4, 4), |m| (m.numerator, m.denominator)); + self.meters.insert(0, MeterEvent { tick: 0, numerator, denominator }); + } + } + /// Insert or replace the tempo at `tick`. pub fn set_tempo(&mut self, tick: i64, bpm: f64) -> Result<(), TimeError> { let bpm = valid_bpm(bpm)?; @@ -97,7 +120,7 @@ impl TempoMap { /// Insert or replace the meter at `tick` (snapped to the bar line at or before it). pub fn set_meter(&mut self, tick: i64, numerator: u32, denominator: u32) -> Result<(), TimeError> { - if !(1..=64).contains(&numerator) || ![1, 2, 4, 8, 16, 32, 64].contains(&denominator) { + if !valid_meter(numerator, denominator) { return Err(TimeError::Meter(numerator, denominator)); } let tick = if tick <= 0 { 0 } else { self.bar_start_tick(self.bar_beat_at_tick(tick).bar) }; @@ -317,6 +340,22 @@ mod tests { assert_eq!(m.bar_beat_at_tick(TICKS_PER_QUARTER / 2), BarBeat { bar: 1, beat: 2, tick: 0 }); } + #[test] + 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)], + }; + 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() }; + empty.sanitize(); + assert_eq!(empty, TempoMap::default()); + } + #[test] fn rejects_bad_values() { let mut m = TempoMap::default(); From 7f4ab5c76bccbe6788bbe3a0184f9f1669db09a0 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:42:09 +0100 Subject: [PATCH 2/9] Refuse devices and pipes where a file is expected A session whose audio file path pointed at /dev/zero filled 18 GB of memory while opening, and one pointing at a named pipe waited forever. The same went for a path given to the import commands, the sampler, Import Session Data, clip group and track preset files, and the session file itself, all of which read whatever the path named to its end. Those reads now go through io::read_file / read_text, which refuse anything but a regular file. A session's missing audio is reported as missing, as before; an import fails with "not a regular file". --- crates/engine/src/clip_group_file.rs | 4 +- crates/engine/src/cmd/file.rs | 6 +-- crates/engine/src/cmd/sampler.rs | 2 +- crates/engine/src/cmd/setup_more.rs | 4 +- crates/engine/src/cmd/track_more.rs | 2 +- crates/engine/src/io.rs | 58 +++++++++++++++++++++++++--- 6 files changed, 62 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index c889c99..8f657fd 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -82,7 +82,7 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { /// are made when the session runs out). Returns the ids of the placed clips. pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) -> Result> { let id = "file.import_clip_groups"; - let text = std::fs::read_to_string(path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text = crate::io::read_text(path)?; let file: GroupFile = serde_json::from_str(&text).map_err(|err| EngineError::BadParams(id.into(), format!("not a clip group file: {err}")))?; if file.format != FORMAT { return Err(EngineError::BadParams(id.into(), "not a SoundCraft clip group file".into())); @@ -98,7 +98,7 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - if !full.starts_with(&dir) || rel.contains("..") { return Err(EngineError::BadParams(id.into(), format!("audio path escapes the group folder: {rel}"))); } - let bytes = std::fs::read(&full).map_err(|err| EngineError::Io(format!("{}: {err}", full.display())))?; + let bytes = crate::io::read_file(&full).map_err(|err| EngineError::Io(format!("{}: {err}", full.display())))?; let (_, buf) = soundcraft_audio_io::decode(&bytes, Some("wav")).map_err(|err| EngineError::Io(format!("{rel}: {err}")))?; decoded.insert(*old, (rel.clone(), buf)); } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index ddbabf8..4fa6353 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -66,7 +66,7 @@ pub fn specs() -> Vec { cmd!("file.import_midi", "MIDI...", ["File", "Import"], Some("Cmd+Alt+I"), "{path, at?}", always, |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("file.import_midi", "`path` required"))?.to_string(); let at = position_param(e, "file.import_midi", p, "at")?.unwrap_or(0); - let bytes = std::fs::read(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(&path)?; let tracks = crate::io::import_midi_bytes(e, &bytes, at, bool_or(p, "tempo_map", true))?; Ok(json!({"tracks": tracks})) }), @@ -80,7 +80,7 @@ pub fn specs() -> Vec { |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("file.import_video", "`path` required"))?.to_string(); let at = position_param(e, "file.import_video", p, "at")?.unwrap_or(0).max(0); - let bytes = std::fs::read(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(&path)?; crate::io::import_video_bytes(e, &path, bytes, at) } ), @@ -231,7 +231,7 @@ fn import_audio(e: &mut Engine, p: &Value) -> Result { let target = import_audio_target(e, p)?; let mut out = Vec::new(); for path in &paths { - let bytes = std::fs::read(path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(path)?; let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or("Audio").to_string(); let r = crate::io::import_audio_bytes(e, &name, &bytes, Some(path.as_str()), target, at)?; out.push(r); diff --git a/crates/engine/src/cmd/sampler.rs b/crates/engine/src/cmd/sampler.rs index ea659a8..7bac7e8 100644 --- a/crates/engine/src/cmd/sampler.rs +++ b/crates/engine/src/cmd/sampler.rs @@ -55,7 +55,7 @@ fn load(e: &mut Engine, p: &Value) -> Result { values.push((k.clone(), pi.clamp(x as f32))); } } - let bytes = std::fs::read(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(&path)?; let name = Path::new(&path).file_name().and_then(|n| n.to_str()).unwrap_or("Sample").to_string(); let ext = Path::new(&name).extension().and_then(|x| x.to_str()).map(str::to_ascii_lowercase); let (format, buf) = soundcraft_audio_io::decode(&bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?; diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index 1d21e99..8fd8809 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -404,7 +404,7 @@ fn midi_inputs(e: &mut Engine, p: &Value) -> Result { /// Decode a source file of another session (paths relative to that session's folder). fn load_source(dir: &Path, src: &Source, sr: u32) -> Option { let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; - let bytes = std::fs::read(&p).ok()?; + let bytes = crate::io::read_file(&p).ok()?; let (_, mut buf) = soundcraft_audio_io::decode(&bytes, p.extension().and_then(|x| x.to_str())).ok()?; if buf.sample_rate != sr && buf.sample_rate > 0 { buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, sr); @@ -416,7 +416,7 @@ fn load_source(dir: &Path, src: &Source, sr: u32) -> Option Result { let id = "file.import_session_data"; let path = str_param(p, "path").filter(|x| !x.trim().is_empty()).ok_or_else(|| bad(id, "`path` required"))?.to_string(); - let text_v = std::fs::read_to_string(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text_v = crate::io::read_text(&path)?; let other = Session::from_json(&text_v).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; let dir = Path::new(&path).parent().map(Path::to_path_buf).unwrap_or_default(); let names: Option> = diff --git a/crates/engine/src/cmd/track_more.rs b/crates/engine/src/cmd/track_more.rs index 8818342..30b1b32 100644 --- a/crates/engine/src/cmd/track_more.rs +++ b/crates/engine/src/cmd/track_more.rs @@ -271,7 +271,7 @@ fn load_preset(e: &mut Engine, p: &Value) -> Result { let id = "track.load_preset"; let path = str_param(p, "path").ok_or_else(|| bad(id, "`path` required"))?.to_string(); let tracks = tracks_required(e, id, p)?; - let text = std::fs::read_to_string(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text = crate::io::read_text(&path)?; let v: Value = serde_json::from_str(&text).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; if v.get("format").and_then(Value::as_str) != Some(PRESET_FORMAT) { return Err(EngineError::Io(format!("{path}: not a SoundCraft track preset"))); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 108330e..7b770a0 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -598,7 +598,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess std::fs::create_dir_all(&dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display())))?; // Atomic-ish write: temp file then rename. let tmp = p.with_extension("scraft.tmp"); - if !overwrite && std::fs::read(&p).is_ok_and(|old| old != text.as_bytes()) { + if !overwrite && read_file(&p).is_ok_and(|old| old != text.as_bytes()) { back_up(&p); } replace_file(&tmp, &p, text.as_bytes())?; @@ -706,15 +706,32 @@ fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> { Err(EngineError::Io(format!("{}: no free file name for {stem}", dir.display()))) } +/// Read a whole file. Only a regular file qualifies: a device or a pipe named by a session or a +/// command would never end or never answer. Errors name the file. +pub fn read_file(path: impl AsRef) -> Result> { + let path = path.as_ref(); + let err = |err: std::io::Error| EngineError::Io(format!("{}: {err}", path.display())); + if !std::fs::metadata(path).map_err(err)?.is_file() { + return Err(EngineError::Io(format!("{}: not a regular file", path.display()))); + } + std::fs::read(path).map_err(err) +} + +/// [`read_file`] as UTF-8 text. +pub fn read_text(path: impl AsRef) -> Result { + let path = path.as_ref(); + String::from_utf8(read_file(path)?).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) +} + /// Open a `.scraft` session and decode its audio files (missing files are reported, not fatal). pub fn open_session(e: &mut Engine, path: &str) -> Result> { - let text = std::fs::read_to_string(path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text = read_text(path)?; let mut s = Session::from_json(&text).map_err(|err| EngineError::Io(err.to_string()))?; let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); let mut missing = Vec::new(); for src in s.sources.clone() { let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; - match std::fs::read(&p) { + match read_file(&p) { Ok(bytes) => { if let Err(err) = load_source_audio(&mut s, src.id, &bytes, p.extension().and_then(|x| x.to_str())) { missing.push(format!("{}: {err}", p.display())); @@ -932,8 +949,9 @@ pub fn add_recording(e: &mut Engine, start: Samples, takes: Vec<(TrackId, PathBu let before = e.session_arc(); let mut out = Vec::new(); for (tid, path) in takes { - let read = std::fs::read(&path).map_err(|x| x.to_string()).and_then(|b| soundcraft_audio_io::decode(&b, None).map_err(|x| x.to_string())); - let Ok((_, buf)) = read.inspect_err(|err| e.message(format!("{}: {err}", path.display()))) else { continue }; + let read = + read_file(&path).and_then(|b| soundcraft_audio_io::decode(&b, None).map_err(|err| EngineError::Io(format!("{}: {err}", path.display())))); + let Ok((_, buf)) = read.inspect_err(|err| e.message(err.to_string())) else { continue }; let s = e.session_mut(); if s.track(tid).is_none() { continue; @@ -1214,6 +1232,36 @@ mod save_tests { } } +#[cfg(all(test, unix))] +mod pipe_tests { + use super::*; + + #[test] + fn a_pipe_named_as_media_is_refused_not_waited_on() { + let dir = std::env::temp_dir().join(format!("soundcraft-pipe-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let fifo = dir.join("pipe.wav"); + assert!(std::process::Command::new("mkfifo").arg(&fifo).status().unwrap().success()); + let mut s = Session::default(); + let audio = AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.0; 10]] }; + add_source(&mut s, "pipe", audio, Some(&fifo.to_string_lossy()), FileFormat::Wav); + let path = dir.join("Song.scraft").to_string_lossy().into_owned(); + std::fs::write(&path, s.to_json().unwrap()).unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + let pipe = fifo.to_string_lossy().into_owned(); + std::thread::spawn(move || { + let mut e = Engine::default(); + let imported = e.execute("file.import_audio", &json!({"path": pipe})).is_ok(); + let _ = tx.send((imported, open_session(&mut e, &path).map_err(|err| err.to_string()))); + }); + let (imported, opened) = rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap(); + assert!(!imported); + assert_eq!(opened.unwrap(), [fifo.display().to_string()]); + let _ = std::fs::remove_dir_all(&dir); + } +} + #[cfg(test)] mod stem_tests { #[test] From d81d5756d421b104443ae17f4738657ef31aa590 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:44:35 +0100 Subject: [PATCH 3/9] Check an import's size at the session's sample rate An 80 KB WAV with a sample rate of 1 Hz froze the app on import: the size limit was checked before conversion to the session rate, which then made it 48,000 times longer, past 2 GiB of memory and more than a minute of work (4.5 GiB and 75 s in an earlier measurement). Opening a session, Import Session Data, the sampler and clip group files converted without any limit. io::at_rate checks the size the audio will have at the session's rate before converting it. Imports keep their limit (2^30 samples, about 4 GiB) and their message; the sampler keeps its own limit; a session's source that is too large is reported like other unreadable media. --- crates/engine/src/clip_group_file.rs | 3 ++ crates/engine/src/cmd/sampler.rs | 5 ++-- crates/engine/src/cmd/setup_more.rs | 8 ++---- crates/engine/src/io.rs | 41 ++++++++++++++++++++++------ 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index 8f657fd..6ffd7fb 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -93,6 +93,7 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); // Decode audio first so a bad file changes nothing. let mut decoded = BTreeMap::new(); + let rate = e.session().sample_rate.hz(); for (old, rel) in &file.audio { let full = dir.join(rel); if !full.starts_with(&dir) || rel.contains("..") { @@ -100,6 +101,8 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - } let bytes = crate::io::read_file(&full).map_err(|err| EngineError::Io(format!("{}: {err}", full.display())))?; 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")))?; decoded.insert(*old, (rel.clone(), buf)); } let src_rate = f64::from(file.sample_rate.max(1)); diff --git a/crates/engine/src/cmd/sampler.rs b/crates/engine/src/cmd/sampler.rs index 7bac7e8..e467234 100644 --- a/crates/engine/src/cmd/sampler.rs +++ b/crates/engine/src/cmd/sampler.rs @@ -62,9 +62,8 @@ fn load(e: &mut Engine, p: &Value) -> Result { if buf.frames() == 0 || buf.num_channels() == 0 { return Err(EngineError::Io(format!("{name}: no audio"))); } - if buf.frames().saturating_mul(buf.num_channels()) > MAX_SAMPLE_VALUES { - return Err(EngineError::Io(format!("{name}: too long for a sample (ten minutes of 192 kHz stereo at most)"))); - } + 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)")))?; let channels = buf.num_channels(); let s = e.session_mut(); let src = crate::io::add_source(s, &name, buf, Some(&path), format.format); diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index 8fd8809..e896394 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -405,12 +405,8 @@ fn midi_inputs(e: &mut Engine, p: &Value) -> Result { fn load_source(dir: &Path, src: &Source, sr: u32) -> Option { let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; let bytes = crate::io::read_file(&p).ok()?; - let (_, mut buf) = soundcraft_audio_io::decode(&bytes, p.extension().and_then(|x| x.to_str())).ok()?; - if buf.sample_rate != sr && buf.sample_rate > 0 { - buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, sr); - buf.sample_rate = sr; - } - Some(buf) + let (_, buf) = soundcraft_audio_io::decode(&bytes, p.extension().and_then(|x| x.to_str())).ok()?; + crate::io::at_rate(buf, sr, crate::io::MAX_IMPORT_SAMPLES) } fn import_session_data(e: &mut Engine, p: &Value) -> Result { diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b770a0..f20dd07 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -9,7 +9,21 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; /// Largest import we accept (samples per channel × channels), ~4 GiB of f32. -const MAX_IMPORT_SAMPLES: usize = 1 << 30; +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 { + let from = buf.sample_rate.max(1); + if buf.frames() as f64 * buf.num_channels() as f64 * f64::from(rate) / f64::from(from) > limit as f64 { + return None; + } + if from != rate { + buf.channels = soundcraft_dsp::offline::resample(&buf.channels, from, rate); + buf.sample_rate = rate; + } + Some(buf) +} /// A relative `Audio Files/` path no other source in `s` already uses. fn unique_audio_rel(s: &Session, stem: &str) -> String { @@ -58,9 +72,8 @@ pub fn add_source(s: &mut Session, name: &str, mut buf: AudioBuffer, path: Optio pub fn import_audio_bytes(e: &mut Engine, name: &str, bytes: &[u8], path: Option<&str>, target: Option, at: Samples) -> Result { let ext = Path::new(name).extension().and_then(|x| x.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}")))?; - if buf.frames().saturating_mul(buf.num_channels().max(1)) > MAX_IMPORT_SAMPLES { - return Err(EngineError::Io(format!("{name}: file is too large to import"))); - } + let buf = at_rate(buf, e.session().sample_rate.hz(), MAX_IMPORT_SAMPLES) + .ok_or_else(|| EngineError::Io(format!("{name}: file is too large to import")))?; if buf.frames() == 0 { return Err(EngineError::Io(format!("{name}: no audio"))); } @@ -752,11 +765,8 @@ pub fn open_session(e: &mut Engine, path: &str) -> Result> { /// Decode one source's audio file into the session pool, at the session rate. fn load_source_audio(s: &mut Session, id: SourceId, bytes: &[u8], ext: Option<&str>) -> std::result::Result<(), String> { - let (_, mut buf) = soundcraft_audio_io::decode(bytes, ext).map_err(|err| err.to_string())?; - if buf.sample_rate != s.sample_rate.hz() && buf.sample_rate > 0 { - buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, s.sample_rate.hz()); - buf.sample_rate = s.sample_rate.hz(); - } + let (_, buf) = soundcraft_audio_io::decode(bytes, ext).map_err(|err| err.to_string())?; + let buf = at_rate(buf, s.sample_rate.hz(), MAX_IMPORT_SAMPLES).ok_or("too large to load")?; s.pool.insert(id, Arc::new(SourceAudio::new(buf))); Ok(()) } @@ -1232,6 +1242,19 @@ mod save_tests { } } +#[cfg(test)] +mod rate_tests { + use super::*; + + #[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()); + let ok = at_rate(buf(8_000), 48_000, 1_000_000).unwrap(); + assert_eq!((ok.sample_rate, ok.frames()), (48_000, 600)); + } +} + #[cfg(all(test, unix))] mod pipe_tests { use super::*; From a8a36540d68ce64c8cb43d88203f7462f01093e3 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:29:37 +0100 Subject: [PATCH 4/9] Name thousands of same-named tracks and files in linear time Finding a free name tried "Name 1", "Name 2" and so on, rescanning every track (or every source path) for each try, so each new name cost time proportional to the square of the session and a bulk import the cube. Importing a 119 KB MIDI file with 4,095 tracks all named "Piano" took 28 seconds; a clip group file with 8,000 empty lanes or 8,000 audio files of one name ran for over a minute, and 100,000 lanes never finished. Import Session Data from a session with 2,000 same-named tracks took 7 seconds. unique_track_name and the new audio file name now collect the numbers already in use once and take the first free one, which gives the same names. A clip group file also counts each empty lane and each audio file against its limit of 10,000. The same imports now take 1 second (MIDI), 2 seconds (8,000 lanes) and a third of a second (Import Session Data); 100,000 lanes are refused. --- crates/engine/src/clip_group_file.rs | 4 +-- crates/engine/src/io.rs | 41 ++++++++++++++++++++++------ crates/model/src/session.rs | 28 +++++++++++++------ 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index 6ffd7fb..66735f9 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; pub const EXTENSION: &str = "scgrp"; const FORMAT: &str = "soundcraft.clipgroup"; -/// Refuse files that would place absurd numbers of clips. +/// Refuse files that would place absurd numbers of clips, tracks or audio files. const MAX_CLIPS: usize = 10_000; #[derive(Debug, Serialize, Deserialize)] @@ -87,7 +87,7 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - if file.format != FORMAT { return Err(EngineError::BadParams(id.into(), "not a SoundCraft clip group file".into())); } - if file.lanes.iter().map(Vec::len).sum::() > MAX_CLIPS { + if file.lanes.iter().map(|l| l.len().max(1)).sum::() > MAX_CLIPS || file.audio.len() > MAX_CLIPS { return Err(EngineError::BadParams(id.into(), "clip group file has too many clips".into())); } let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index f20dd07..f1bf6a4 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -28,16 +28,22 @@ pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> Option String { let stem = sanitize_name(stem); - let mut path = format!("Audio Files/{stem}.wav"); - let mut n = 2u32; - while s.sources.iter().any(|src| src.path == path) { - path = format!("Audio Files/{stem}-{n}.wav"); - n = n.saturating_add(1); - if n > 10_000 { - break; - } + let (plain, prefix) = (format!("Audio Files/{stem}.wav"), format!("Audio Files/{stem}-")); + let used: std::collections::HashSet = s + .sources + .iter() + .filter_map(|src| { + if src.path == plain { + return Some(1); + } + let n = src.path.strip_prefix(&prefix)?.strip_suffix(".wav")?; + n.parse().ok().filter(|i: &u64| *i > 1 && i.to_string() == n) + }) + .collect(); + match (1..).find(|i| !used.contains(i)) { + Some(n) if n > 1 => format!("{prefix}{n}.wav"), + _ => plain, } - path } /// Register decoded audio as a new source. Resamples to the session rate if needed. @@ -1255,6 +1261,23 @@ mod rate_tests { } } +#[cfg(test)] +mod name_tests { + use super::*; + + #[test] + fn new_sources_take_the_first_free_file_name() { + let mut s = Session::default(); + let buf = || AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.0; 4]] }; + add_source(&mut s, "k", buf(), Some("Audio Files/k-02.wav"), FileFormat::Wav); + let mut path = || { + let id = add_source(&mut s, "k", buf(), None, FileFormat::Wav); + s.source(id).map(|x| x.path.clone()).unwrap_or_default() + }; + assert_eq!([path(), path(), path()], ["Audio Files/k.wav", "Audio Files/k-2.wav", "Audio Files/k-3.wav"]); + } +} + #[cfg(all(test, unix))] mod pipe_tests { use super::*; diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 9c7e882..9d384f5 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -432,14 +432,15 @@ impl Session { if !base.is_empty() && !taken(base) && !matches!(base, "Audio" | "Aux" | "MIDI" | "Inst" | "VCA" | "Folder" | "Master") { return base.to_string(); } - let mut i = 1; - loop { - let cand = format!("{base} {i}"); - if !taken(&cand) { - return cand; - } - i += 1; - } + let prefix = format!("{base} "); + let used: std::collections::HashSet = self + .tracks + .iter() + .filter_map(|t| t.name.strip_prefix(&prefix)) + .filter_map(|n| n.parse().ok().filter(|i: &u64| i.to_string() == n)) + .collect(); + let i = (1..).find(|i| !used.contains(i)).unwrap_or(1); + format!("{base} {i}") } /// Add a track with defaults; returns its id. @@ -645,6 +646,17 @@ mod tests { assert_eq!(c.source_offset(), 580); } + #[test] + fn unique_track_names_take_the_first_free_number() { + let mut s = Session::default(); + for name in ["Piano", "Piano 1", "Piano 3", "Piano 02", "Pianos 2"] { + s.add_track(TrackKind::Audio, ChannelFormat::Mono, Some(name)); + } + assert_eq!(s.unique_track_name("Piano"), "Piano 2"); + assert_eq!(s.unique_track_name("Organ"), "Organ"); + assert_eq!(s.unique_track_name("Audio"), "Audio 1"); + } + #[test] fn a_bad_sample_rate_or_tempo_is_repaired_on_load() { let mut v = serde_json::to_value(Session::default()).unwrap(); From 7ca978fdb95cfc27cae47e7eadbad5db3b2a1c70 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:36:00 +0100 Subject: [PATCH 5/9] Cut names from files to a length that can be drawn A MIDI file whose track name was a million characters long imported fine, and the next frame crashed the app: drawing that name needed a 320 MB vertex buffer, past wgpu's 256 MB limit, and the panic comes from painting, outside anything that catches it. Saved, the session crashed again on every open. A 4 MB status line in ui.json did the same at every launch. Names read from files are now cut to 256 characters and comments to 4096: everything Session::sanitize repairs on load (session, track, playlist, clip, marker, bus, output, group, source, video and key names), MIDI track names on import and clip names from clip group files. The status bar draws at most 1024 characters. --- crates/engine/src/clip_group_file.rs | 1 + crates/engine/src/io.rs | 14 ++++++++- crates/model/src/lib.rs | 5 ++- crates/model/src/session.rs | 46 ++++++++++++++++++++++++++++ crates/ui-egui/src/edit_window.rs | 3 +- 5 files changed, 66 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index 66735f9..e60beed 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -144,6 +144,7 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - continue; } c.id = s.new_clip_id(); + soundcraft_model::truncate_chars(&mut c.name, soundcraft_model::MAX_NAME_CHARS); c.start = at.saturating_add(scale(c.start.max(0))); c.length = scale(c.length).max(1); c.group = Some(group_id); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index f1bf6a4..a99f7cc 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -203,7 +203,8 @@ pub fn import_midi_bytes(e: &mut Engine, bytes: &[u8], at: Samples, tempo_map: b if tr.sequence.notes.is_empty() { continue; } - let name = if tr.name.trim().is_empty() { format!("MIDI {}", i + 1) } else { tr.name.clone() }; + let mut name = if tr.name.trim().is_empty() { format!("MIDI {}", i + 1) } else { tr.name.clone() }; + soundcraft_model::truncate_chars(&mut name, soundcraft_model::MAX_NAME_CHARS); let id = s.add_track(TrackKind::Instrument, ChannelFormat::Stereo, Some(&name)); let drums = tr.sequence.notes.iter().all(|n| n.channel == 9); if let Some(t) = s.track_mut(id) { @@ -1265,6 +1266,17 @@ mod rate_tests { mod name_tests { use super::*; + #[test] + fn a_midi_track_name_is_cut_to_a_length_that_can_be_drawn() { + let note = soundcraft_midi::Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start: 0, length: 960 }; + let sequence = soundcraft_midi::Sequence { notes: vec![note], ..Default::default() }; + let smf = soundcraft_midi::Smf { tracks: vec![soundcraft_midi::SmfTrack { name: "n".repeat(1_000_000), sequence }], ..Default::default() }; + let mut e = Engine::default(); + let ids = import_midi_bytes(&mut e, &soundcraft_midi::write_smf(&smf).unwrap(), 0, false).unwrap(); + let track = e.session().track(ids[0]).unwrap(); + assert_eq!((track.name.len(), track.clips()[0].name.len()), (soundcraft_model::MAX_NAME_CHARS, soundcraft_model::MAX_NAME_CHARS)); + } + #[test] fn new_sources_take_the_first_free_file_name() { let mut s = Session::default(); diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index 55d605d..ec8aaa2 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -22,7 +22,10 @@ pub use markers::{Group, MarkerKind, MemoryLocation}; pub use mixer::{ Bus, ChannelFormat, INSERT_SLOTS, Insert, Mixer, OutputPath, Route, SEND_SLOTS, SendSlot, Speaker, SurroundPan, fader_db_to_pos, fader_pos_to_db, }; -pub use session::{BitDepthSetting, EditMode, EditState, SESSION_EXTENSION, Session, SessionError, Tool, TrackView, ZoomState}; +pub use session::{ + BitDepthSetting, EditMode, EditState, MAX_COMMENT_CHARS, MAX_NAME_CHARS, SESSION_EXTENSION, Session, SessionError, Tool, TrackView, ZoomState, + truncate_chars, +}; pub use source::{Source, SourceAudio, SourcePool, VideoSource}; pub use track::{Playlist, TRACK_COLORS, Track, TrackHeight, TrackKind}; diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 9d384f5..a3f906c 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -575,6 +575,39 @@ impl Session { self.sample_rate = SampleRate::default(); } self.tempo.sanitize(); + truncate_chars(&mut self.name, MAX_NAME_CHARS); + truncate_chars(&mut self.comments, MAX_COMMENT_CHARS); + for t in &mut self.tracks { + truncate_chars(&mut t.name, MAX_NAME_CHARS); + truncate_chars(&mut t.comments, MAX_COMMENT_CHARS); + for p in &mut t.playlists { + truncate_chars(&mut p.name, MAX_NAME_CHARS); + p.clips.iter_mut().for_each(|c| truncate_chars(&mut c.name, MAX_NAME_CHARS)); + } + } + for m in &mut self.markers { + truncate_chars(&mut m.name, MAX_NAME_CHARS); + truncate_chars(&mut m.comments, MAX_COMMENT_CHARS); + } + self.key_signatures.iter_mut().for_each(|(_, k)| truncate_chars(k, MAX_NAME_CHARS)); + self.busses.iter_mut().for_each(|b| truncate_chars(&mut b.name, MAX_NAME_CHARS)); + self.outputs.iter_mut().for_each(|o| truncate_chars(&mut o.name, MAX_NAME_CHARS)); + self.groups.iter_mut().for_each(|g| truncate_chars(&mut g.name, MAX_NAME_CHARS)); + self.sources.iter_mut().for_each(|s| truncate_chars(&mut s.name, MAX_NAME_CHARS)); + self.videos.iter_mut().for_each(|v| truncate_chars(&mut v.name, MAX_NAME_CHARS)); + } +} + +/// The most characters kept of a name read from a file: longer ones can't be shown, and drawing +/// a name of a million characters overflows the renderer's buffers. +pub const MAX_NAME_CHARS: usize = 256; +/// The most characters kept of a comment read from a file. +pub const MAX_COMMENT_CHARS: usize = 4096; + +/// Cut `s` to at most `max` characters. +pub fn truncate_chars(s: &mut String, max: usize) { + if let Some((end, _)) = s.char_indices().nth(max) { + s.truncate(end); } } @@ -657,6 +690,19 @@ mod tests { assert_eq!(s.unique_track_name("Audio"), "Audio 1"); } + #[test] + fn names_from_a_file_are_cut_to_a_length_that_can_be_drawn() { + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, None); + if let Some(tr) = s.track_mut(t) { + tr.name = "é".repeat(1_000); + } + s.comments = "c".repeat(10_000); + let back = Session::from_json(&s.to_json().unwrap()).unwrap(); + assert_eq!(back.track(t).map(|tr| tr.name.chars().count()), Some(MAX_NAME_CHARS)); + assert_eq!(back.comments.len(), MAX_COMMENT_CHARS); + } + #[test] fn a_bad_sample_rate_or_tempo_is_repaired_on_load() { let mut v = serde_json::to_value(Session::default()).unwrap(); diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index f13bc78..dc2453b 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -157,7 +157,7 @@ fn status_bar(app: &mut SoundApp, ui: &mut Ui) { let t = Tokens::current(); let r = ui.max_rect(); let s = app.engine.session(); - let left = format!( + let mut left = format!( "{} · {} Hz · {} · {} tracks · {}", s.name, s.sample_rate.hz(), @@ -169,6 +169,7 @@ fn status_bar(app: &mut SoundApp, ui: &mut Ui) { s.tracks.len(), app.ui.status ); + soundcraft_model::truncate_chars(&mut left, 4 * soundcraft_model::MAX_NAME_CHARS); ui.painter().text(pos2(r.min.x + 10.0, r.center().y), Align2::LEFT_CENTER, left, regular(11.0), t.text_dim); let undo = app.engine.undo_label().map_or(String::new(), |l| format!("Undo {l}")); ui.painter().text( From 1b8baa10afbac50d4710ba05cb4c8fb5e4676518 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:40:08 +0100 Subject: [PATCH 6/9] Keep Extract Chords to a bounded walk over the selection Extract Chords walked the selection bar by bar, looking at every note of the selected tracks in each bar, until it had 2000 chords or reached the end. After Select All over a session file with a clip at the far end of the timeline (2^50 samples) it ran for ever over empty bars, and a 317-byte MIDI file with one note held for 8.5 years kept it busy for millions of bars that never form a chord. It now takes the notes in time order, keeps only those sounding in the bar it looks at, jumps over bars where nothing sounds, and stops after 100,000 bars that hold notes (27 hours at 120 BPM) or 10 million sounding notes. The chords it finds are the same. --- crates/engine/src/cmd/event_more.rs | 40 ++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/event_more.rs b/crates/engine/src/cmd/event_more.rs index 3dc634c..c0780a5 100644 --- a/crates/engine/src/cmd/event_more.rs +++ b/crates/engine/src/cmd/event_more.rs @@ -555,6 +555,10 @@ pub fn chord_name(weights: &[f64; 12], bass: usize) -> Option { Some(name) } +/// The most bars Extract Chords looks at (55 hours of 4/4 at 120 BPM), and notes sounding in them. +const MAX_CHORD_BARS: usize = 100_000; +const MAX_CHORD_NOTES: usize = 10_000_000; + fn extract_chords(e: &mut Engine, p: &Value) -> Result { let id = "event.extract_chords"; let (_, t0, t1) = selection_ticks(e, p, id)?; @@ -580,20 +584,36 @@ fn extract_chords(e: &mut Engine, p: &Value) -> Result { } } } - let first_bar = s.tempo.bar_beat_at_tick(t0).bar; - let mut bar = first_bar; + notes.sort_unstable_by_key(|n| n.0); + let mut bar = s.tempo.bar_beat_at_tick(t0).bar; + let mut sounding: Vec<(i64, i64, u8)> = Vec::new(); + let (mut next, mut bars, mut heard) = (0, 0, 0); let mut chords: Vec = Vec::new(); let mut marks: Vec<(Samples, String)> = Vec::new(); let mut last = String::new(); - while chords.len() < 2000 { + while chords.len() < 2000 && bars < MAX_CHORD_BARS && heard < MAX_CHORD_NOTES { let a = s.tempo.bar_start_tick(bar).max(t0); let b = s.tempo.bar_start_tick(bar + 1).min(t1); if a >= t1 { break; } + while let Some(&n) = notes.get(next).filter(|n| n.0 < b) { + sounding.push(n); + next += 1; + } + sounding.retain(|n| n.1 > a); + if sounding.is_empty() { + match notes.get(next) { + Some(n) => bar = (bar + 1).max(s.tempo.bar_beat_at_tick(n.0).bar), + None => break, + } + continue; + } + bars += 1; + heard += sounding.len(); let mut w = [0.0f64; 12]; let mut bass: Option = None; - for (na, nb, pitch) in ¬es { + for (na, nb, pitch) in &sounding { let ov = (*nb).min(b) - (*na).max(a); if ov > 0 { if let Some(x) = w.get_mut(usize::from(*pitch) % 12) { @@ -901,6 +921,18 @@ mod tests { assert!(e.session().markers.iter().any(|m| m.ruler == 2)); } + #[test] + fn chords_far_down_the_timeline_are_found_without_walking_every_bar() { + let mut e = crate::demo::demo_engine(); + let keys = e.session().track_by_name("Keys").unwrap().id; + let far = 1i64 << 39; + if let Some(p) = e.session_mut().track_mut(keys).and_then(soundcraft_model::Track::playlist_mut) { + p.clips.iter_mut().for_each(|c| c.start += far); + } + let r = e.execute("event.extract_chords", &json!({"tracks": [keys.0], "start": 0, "end": far + 48_000 * 600})).unwrap(); + assert!(!r["chords"].as_array().unwrap().is_empty(), "{r}"); + } + #[test] fn beat_detective_finds_demo_tempo() { let mut e = crate::demo::demo_engine(); From fe1d7e4c589cbcd5a5014a03131a959f4ac6574a Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:40:08 +0100 Subject: [PATCH 7/9] Refuse MIDI files that run longer than a day A MIDI file of 317 bytes, with one note whose end lies 41 maximal deltas after its start, imported as an 8.5-year clip. Everything that then rendered or walked that clip (Track Bounce, Commit, Separate at Transients, Extract Chords) worked for seconds to forever and used up to 3 GB. Importing now refuses a MIDI track that ends more than 24 hours after the import point; its end is computed with saturation, so a huge one can't wrap around and slip past the check. A file with more than 10,000 meter changes is refused too: placing each one walks those before it, so 200,000 of them (1.8 MB) took 57 s and fit easily within a day. --- crates/engine/src/io.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index a99f7cc..1cbe7a7 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -185,11 +185,18 @@ pub fn relink_videos(s: &mut Session, session_dir: &Path) -> Vec { missing } +/// Most meter changes a MIDI file may bring into the tempo map; scores have a handful. +const MAX_MIDI_METERS: usize = 10_000; + /// Import a Standard MIDI File as MIDI tracks (one per SMF track). pub fn import_midi_bytes(e: &mut Engine, bytes: &[u8], at: Samples, tempo_map: bool) -> Result> { let smf = soundcraft_midi::read_smf(bytes).map_err(|err| EngineError::Io(err.to_string()))?; let s = e.session_mut(); if tempo_map { + // Each meter is placed by walking the ones before it. + if smf.meters.len() > MAX_MIDI_METERS { + return Err(EngineError::Io(format!("MIDI file: more than {MAX_MIDI_METERS} meter changes"))); + } for (tick, bpm) in &smf.tempos { let _ = s.tempo.set_tempo(*tick, *bpm); } @@ -210,8 +217,11 @@ pub fn import_midi_bytes(e: &mut Engine, bytes: &[u8], at: Samples, tempo_map: b if let Some(t) = s.track_mut(id) { t.instrument = Some(soundcraft_model::Insert::new(if drums { "drum_synth" } else { "subtractive_synth" })); } - let end_tick = base_tick + tr.sequence.end_tick().max(1); - let len = s.tempo.tick_to_samples(end_tick, s.sample_rate) - at; + let end_tick = base_tick.saturating_add(tr.sequence.end_tick().max(1)); + let len = s.tempo.tick_to_samples(end_tick, s.sample_rate).saturating_sub(at); + if len > s.sample_rate.samples(86_400.0) { + return Err(EngineError::Io(format!("MIDI track {}: longer than 24 hours", i + 1))); + } let cid = s.new_clip_id(); crate::edit::place_clip(s, id, Clip::midi(cid, name, at, len.max(1), tr.sequence.clone())); out.push(id); @@ -1277,6 +1287,25 @@ mod name_tests { assert_eq!((track.name.len(), track.clips()[0].name.len()), (soundcraft_model::MAX_NAME_CHARS, soundcraft_model::MAX_NAME_CHARS)); } + #[test] + fn midi_longer_than_a_day_is_refused() { + let held = soundcraft_midi::Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start: 0, length: (1 << 28) - 1 }; + let sequence = soundcraft_midi::Sequence { notes: vec![held], ..Default::default() }; + let smf = soundcraft_midi::Smf { tracks: vec![soundcraft_midi::SmfTrack { name: String::new(), sequence }], ..Default::default() }; + let err = import_midi_bytes(&mut Engine::default(), &soundcraft_midi::write_smf(&smf).unwrap(), 0, false).unwrap_err(); + assert!(err.to_string().contains("longer than 24 hours"), "{err}"); + } + + #[test] + fn midi_with_more_meter_changes_than_scores_have_is_refused() { + let note = soundcraft_midi::Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start: 0, length: 960 }; + let sequence = soundcraft_midi::Sequence { notes: vec![note], ..Default::default() }; + let meters = (0..=MAX_MIDI_METERS as i64).map(|bar| (bar * 3840, 4, 4)).collect(); + let smf = soundcraft_midi::Smf { tracks: vec![soundcraft_midi::SmfTrack { name: String::new(), sequence }], meters, ..Default::default() }; + let err = import_midi_bytes(&mut Engine::default(), &soundcraft_midi::write_smf(&smf).unwrap(), 0, true).unwrap_err(); + assert!(err.to_string().contains("meter changes"), "{err}"); + } + #[test] fn new_sources_take_the_first_free_file_name() { let mut s = Session::default(); From 328760fd703d5b150c38242c10548fa18c2d2e38 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:11:03 +0100 Subject: [PATCH 8/9] Keep positions from files within the range commands use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commands clamp every position they take to ±2^40 samples so the arithmetic after them can't overflow, but session and clip group files skipped that. A clip, MIDI note, marker, automation point or selection near i64::MAX made Duplicate, Insert Silence, Insert Time, Heal, Trim to File Boundaries, Quantize to Grid, Separate on Grid, Thin Automation, Copy, Move Selection, Extract Chords and Track Commit overflow: a panic in debug builds, wrapped positions in release. MAX_POSITION moves to soundcraft-time so the model can use it. Loading a session clamps clip starts, lengths, offsets, sync points, gain envelopes and MIDI notes, automation points, markers, key changes, the selection, playhead, pre/post-roll, scroll and timecode start. A clip's repairs are now Clip::sanitize, which clip group import uses too, and Session::sanitize visits each collection once. The Edit window adds a clip's offset to its file's time stamp without overflowing. --- crates/engine/src/clip_group_file.rs | 27 ++++++++-- crates/engine/src/cmd/mod.rs | 4 +- crates/engine/tests/hostile_sessions.rs | 37 ++++++++++++++ crates/model/src/clip.rs | 37 +++++++++++++- crates/model/src/session.rs | 68 ++++++++++++++----------- crates/time/src/lib.rs | 4 ++ crates/ui-egui/src/edit_window.rs | 11 +++- 7 files changed, 149 insertions(+), 39 deletions(-) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index e60beed..37f95d9 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -144,11 +144,10 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - continue; } c.id = s.new_clip_id(); - soundcraft_model::truncate_chars(&mut c.name, soundcraft_model::MAX_NAME_CHARS); c.start = at.saturating_add(scale(c.start.max(0))); - c.length = scale(c.length).max(1); + c.length = scale(c.length); c.group = Some(group_id); - c.clamp_fades(); + c.sanitize(); placed.push(c.id); crate::edit::place_clip(s, target, c); } @@ -194,6 +193,28 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn positions_in_a_group_file_are_kept_within_reach() { + let dir = std::env::temp_dir().join(format!("sc-grp-far-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + let p = dir.join("far.scgrp"); + let note = json!({"pitch": 60, "velocity": 100, "release_velocity": 64, "channel": 0, "start": i64::MAX, "length": i64::MAX}); + let clip = + json!({"id": 1, "name": "far", "start": i64::MAX, "length": i64::MAX, "content": {"Midi": {"sequence": {"notes": [note], "ctrls": []}}}}); + let file = json!({"format": "soundcraft.clipgroup", "version": 1, "sample_rate": 48000, "length": 1, "lanes": [[clip]], "audio": {}}); + std::fs::write(&p, file.to_string()).unwrap(); + let mut e = crate::demo::demo_engine(); + let r = e.execute("file.import_clip_groups", &json!({"path": p.to_string_lossy()})).unwrap(); + let id = soundcraft_model::ClipId(r["clips"][0].as_u64().unwrap()); + let c = e.session().find_clip(id).unwrap().1; + assert!(c.start <= crate::cmd::MAX_POSITION && c.length <= crate::cmd::MAX_POSITION); + e.execute("edit.select_all", &json!({})).unwrap(); + for cmd in ["edit.duplicate", "event.extract_chords"] { + assert!(!matches!(e.execute(cmd, &json!({})), Err(crate::EngineError::Internal(..))), "{cmd}"); + } + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn rejects_foreign_files_and_escaping_paths() { let dir = std::env::temp_dir().join(format!("sc-grp-bad-{}", std::process::id())); diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index d0a453c..6e3e362 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -238,9 +238,7 @@ pub fn tracks_required(e: &Engine, cmd: &str, p: &Value) -> Result> if t.is_empty() { Err(bad(cmd, "no tracks: pass `track`/`tracks` or select tracks")) } else { Ok(t) } } -/// Largest position accepted from parameters: 2^40 samples (about 265 days at 48 kHz). Clamping -/// here keeps every later addition and subtraction on positions far away from overflow. -pub const MAX_POSITION: Samples = 1 << 40; +pub use soundcraft_time::MAX_POSITION; /// Position from `key`: a number of samples, or `{"seconds": x}`, or a string in the main counter format. /// Values are clamped to ±[`MAX_POSITION`]. diff --git a/crates/engine/tests/hostile_sessions.rs b/crates/engine/tests/hostile_sessions.rs index 5a98b02..06b785e 100644 --- a/crates/engine/tests/hostile_sessions.rs +++ b/crates/engine/tests/hostile_sessions.rs @@ -26,6 +26,43 @@ fn demo_json() -> Value { serde_json::to_value(e.session()).unwrap() } +#[test] +fn positions_at_the_ends_of_the_range_do_not_overflow_commands() { + let (max, min) = (serde_json::json!(i64::MAX), serde_json::json!(i64::MIN)); + let kick = "/tracks/0/playlists/0/clips/0"; + let cases = [ + (format!("{kick}/start"), &max, "edit.duplicate"), + (format!("{kick}/start"), &max, "edit.insert_silence"), + (format!("{kick}/length"), &max, "edit.heal"), + (format!("{kick}/length"), &max, "edit.trim_to_file_boundaries"), + (format!("{kick}/content/Audio/offset"), &min, "edit.trim_to_file_boundaries"), + (format!("{kick}/sync_point"), &max, "clip.quantize_to_grid"), + (format!("{kick}/gain_env"), &serde_json::json!([[i64::MIN, 0.0]]), "edit.separate_on_grid"), + ("/tracks/4/automation/0/points/0/at".into(), &min, "automation.thin_all"), + ("/tracks/5/playlists/0/clips/0/content/Midi/sequence/notes/0/start".into(), &max, "event.extract_chords"), + ("/markers/0/start".into(), &max, "event.insert_time"), + ("/markers/1/end".into(), &min, "edit.copy"), + ("/edit/selection/start".into(), &min, "edit.move_selection_left"), + ]; + let (demo, base) = (soundcraft_engine::demo::demo_engine(), demo_json()); + let failed: Vec = cases + .into_iter() + .filter_map(|(path, value, cmd)| { + let mut v = base.clone(); + *v.pointer_mut(&path).unwrap() = value.clone(); + let mut s = soundcraft_model::Session::from_json(&v.to_string()).unwrap(); + s.pool = demo.session().pool.clone(); + let mut e = soundcraft_engine::Engine::new(s); + if !path.starts_with("/edit") { + e.execute("edit.select_all", &serde_json::json!({})).unwrap(); + } + let r = e.execute(cmd, &serde_json::json!({})); + matches!(r, Err(soundcraft_engine::EngineError::Internal(..))).then(|| format!("{path} = {value}, {cmd}: {r:?}")) + }) + .collect(); + assert!(failed.is_empty(), "{failed:#?}"); +} + proptest! { #![proptest_config(ProptestConfig { cases: 96, .. ProptestConfig::default() })] #[test] diff --git a/crates/model/src/clip.rs b/crates/model/src/clip.rs index 4d30bea..71853ab 100644 --- a/crates/model/src/clip.rs +++ b/crates/model/src/clip.rs @@ -1,8 +1,8 @@ //! Clips: windows onto audio sources or MIDI sequences, placed on a playlist. -use crate::{ClipId, SourceId}; +use crate::{ClipId, MAX_NAME_CHARS, SourceId, truncate_chars}; use soundcraft_midi::Sequence; -use soundcraft_time::{Range, Samples}; +use soundcraft_time::{MAX_POSITION, Range, Samples}; /// Fade curve shapes (Pro Tools offers standard, equal-power, S-curve and custom presets). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, serde::Serialize, serde::Deserialize)] @@ -230,6 +230,39 @@ impl Clip { self.fade_in.len = self.fade_in.len.clamp(0, self.length); self.fade_out.len = self.fade_out.len.clamp(0, self.length); } + + /// Repair a clip read from a file: positions and lengths within ±[`MAX_POSITION`], as commands + /// keep theirs, fades within the clip, a name that can be drawn, and a usable gain and stretch. + pub fn sanitize(&mut self) { + bound(&mut self.start); + self.length = self.length.clamp(1, MAX_POSITION); + bound(&mut self.sync_point); + self.gain_env.iter_mut().for_each(|(at, _)| bound(at)); + match &mut self.content { + ClipContent::Audio { offset, .. } | ClipContent::Video { offset, .. } => bound(offset), + ClipContent::Midi { sequence } => bound_sequence(sequence), + } + self.clamp_fades(); + truncate_chars(&mut self.name, MAX_NAME_CHARS); + if !self.gain_db.is_finite() { + self.gain_db = 0.0; + } + if !self.stretch.is_finite() || self.stretch <= 0.0 { + self.stretch = 1.0; + } + } +} + +pub(crate) fn bound(at: &mut Samples) { + *at = (*at).clamp(-MAX_POSITION, MAX_POSITION); +} + +pub(crate) fn bound_sequence(s: &mut Sequence) { + for n in &mut s.notes { + bound(&mut n.start); + n.length = n.length.clamp(1, MAX_POSITION); + } + s.ctrls.iter_mut().for_each(|c| bound(&mut c.tick)); } fn env_value(env: &[(Samples, f32)], at: Samples) -> f32 { diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index a3f906c..653e369 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -1,5 +1,6 @@ //! The session: the whole document. +use crate::clip::{bound, bound_sequence}; use crate::{ Bus, BusId, ChannelFormat, Clip, ClipId, Group, GroupId, MarkerId, MarkerKind, MemoryLocation, OutputPath, Source, SourceId, SourcePool, TRACK_COLORS, Track, TrackId, TrackKind, VideoSource, @@ -529,8 +530,12 @@ impl Session { .max() .unwrap_or(0); self.next_id = self.next_id.max(max_id.saturating_add(1)); + truncate_chars(&mut self.name, MAX_NAME_CHARS); + truncate_chars(&mut self.comments, MAX_COMMENT_CHARS); for t in &mut self.tracks { t.mixer.sanitize(); + truncate_chars(&mut t.name, MAX_NAME_CHARS); + truncate_chars(&mut t.comments, MAX_COMMENT_CHARS); if t.playlists.is_empty() { t.playlists.push(crate::Playlist::new(t.name.clone())); } @@ -538,24 +543,42 @@ impl Session { t.active_playlist = 0; } for p in &mut t.playlists { + truncate_chars(&mut p.name, MAX_NAME_CHARS); p.clips.retain(|c| c.length > 0); - for c in &mut p.clips { - c.fade_in.len = c.fade_in.len.clamp(0, c.length); - c.fade_out.len = c.fade_out.len.clamp(0, c.length); - if !c.gain_db.is_finite() { - c.gain_db = 0.0; - } - if !c.stretch.is_finite() || c.stretch <= 0.0 { - c.stretch = 1.0; - } - } + p.clips.iter_mut().for_each(Clip::sanitize); p.sort(); } for l in &mut t.automation { l.points.retain(|p| p.value.is_finite()); + l.points.iter_mut().for_each(|p| bound(&mut p.at)); l.points.sort_by_key(|p| p.at); } } + for m in &mut self.markers { + bound(&mut m.start); + bound(&mut m.end); + truncate_chars(&mut m.name, MAX_NAME_CHARS); + truncate_chars(&mut m.comments, MAX_COMMENT_CHARS); + } + for (at, key) in &mut self.key_signatures { + bound(at); + truncate_chars(key, MAX_NAME_CHARS); + } + self.midi_originals.values_mut().for_each(bound_sequence); + let e = &mut self.edit; + for at in [ + &mut e.selection.start, + &mut e.selection.end, + &mut e.timeline_selection.start, + &mut e.timeline_selection.end, + &mut e.playhead, + &mut e.pre_roll, + &mut e.post_roll, + &mut e.zoom.scroll, + &mut self.timecode_start, + ] { + bound(at); + } for v in &mut self.videos { if !v.frame_rate.is_finite() || v.frame_rate < 0.0 { v.frame_rate = 0.0; @@ -563,7 +586,12 @@ impl Session { if !v.duration.is_finite() || v.duration < 0.0 { v.duration = 0.0; } + truncate_chars(&mut v.name, MAX_NAME_CHARS); } + self.busses.iter_mut().for_each(|b| truncate_chars(&mut b.name, MAX_NAME_CHARS)); + self.outputs.iter_mut().for_each(|o| truncate_chars(&mut o.name, MAX_NAME_CHARS)); + self.groups.iter_mut().for_each(|g| truncate_chars(&mut g.name, MAX_NAME_CHARS)); + self.sources.iter_mut().for_each(|s| truncate_chars(&mut s.name, MAX_NAME_CHARS)); // Drop performances whose clip no longer exists anywhere (any playlist). let clip_ids: std::collections::BTreeSet = self.tracks.iter().flat_map(|t| t.playlists.iter().flat_map(|p| p.clips.iter().map(|c| c.id.0))).collect(); @@ -575,26 +603,6 @@ impl Session { self.sample_rate = SampleRate::default(); } self.tempo.sanitize(); - truncate_chars(&mut self.name, MAX_NAME_CHARS); - truncate_chars(&mut self.comments, MAX_COMMENT_CHARS); - for t in &mut self.tracks { - truncate_chars(&mut t.name, MAX_NAME_CHARS); - truncate_chars(&mut t.comments, MAX_COMMENT_CHARS); - for p in &mut t.playlists { - truncate_chars(&mut p.name, MAX_NAME_CHARS); - p.clips.iter_mut().for_each(|c| truncate_chars(&mut c.name, MAX_NAME_CHARS)); - } - } - for m in &mut self.markers { - truncate_chars(&mut m.name, MAX_NAME_CHARS); - truncate_chars(&mut m.comments, MAX_COMMENT_CHARS); - } - self.key_signatures.iter_mut().for_each(|(_, k)| truncate_chars(k, MAX_NAME_CHARS)); - self.busses.iter_mut().for_each(|b| truncate_chars(&mut b.name, MAX_NAME_CHARS)); - self.outputs.iter_mut().for_each(|o| truncate_chars(&mut o.name, MAX_NAME_CHARS)); - self.groups.iter_mut().for_each(|g| truncate_chars(&mut g.name, MAX_NAME_CHARS)); - self.sources.iter_mut().for_each(|s| truncate_chars(&mut s.name, MAX_NAME_CHARS)); - self.videos.iter_mut().for_each(|v| truncate_chars(&mut v.name, MAX_NAME_CHARS)); } } diff --git a/crates/time/src/lib.rs b/crates/time/src/lib.rs index 4b68934..7e9d4b9 100644 --- a/crates/time/src/lib.rs +++ b/crates/time/src/lib.rs @@ -19,6 +19,10 @@ pub use timecode::{FrameRate, Timecode}; /// A position or length in samples. pub type Samples = i64; +/// Largest position accepted from parameters and files: 2^40 samples (about 265 days at 48 kHz). +/// Clamping there keeps every later addition and subtraction on positions far away from overflow. +pub const MAX_POSITION: Samples = 1 << 40; + /// Errors from parsing or converting time values. #[derive(Debug, Clone, PartialEq, thiserror::Error)] pub enum TimeError { diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index dc2453b..11a502d 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -1330,7 +1330,7 @@ fn draw_clip(app: &SoundApp, painter: &egui::Painter, s: &Session, track: &Track } else if s.edit.flag("view.clip.time.original") || s.edit.flag("view.clip.time.user") { clip.source().and_then(|src| s.source(src)).map(|src| { format_position( - i64::try_from(src.time_reference).unwrap_or(0) + clip.source_offset(), + i64::try_from(src.time_reference).unwrap_or(0).saturating_add(clip.source_offset()), TimeFormat::Timecode, s.sample_rate, &s.tempo, @@ -2069,6 +2069,15 @@ mod tests { h } + #[test] + fn a_file_time_stamp_at_the_end_of_the_range_draws() { + let mut engine = soundcraft_engine::demo::demo_engine(); + let s = engine.session_mut(); + s.edit.flags.insert("view.clip.time.original".into()); + s.sources.iter_mut().for_each(|src| src.time_reference = i64::MAX as u64); + harness_with(engine).run_steps(2); + } + fn smart_demo() -> Harness<'static, SoundApp> { let mut engine = soundcraft_engine::demo::demo_engine(); engine.execute("edit.tool", &json!({"tool": "smart"})).unwrap(); From 6b3c73b7ff871a7a1c929cff620a47c15cfc8d48 Mon Sep 17 00:00:00 2001 From: grovebit <337789029+grovebit@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:15:15 +0100 Subject: [PATCH 9/9] Decode each audio file once when a session names it many times Opening a session decoded every source entry on its own, so a 400 KB session listing one 11 MB WAV 2,000 times took 16 s and 22 GB, and 20,000 entries never finished. Import Session Data and clip group files did the same. All three now decode a file the first time it is named, by its canonical path, and share the decoded audio with later sources that name it; the pool's audio is never changed in place, so sharing it is safe. Saving keeps sources that play one file on that file, and an autosave or a copy writes shared audio once instead of once per source. That session now opens in 0.3 s at 110 MB, and a normal session bounces to the same bytes as before. --- crates/engine/src/clip_group_file.rs | 54 +++++++++--- crates/engine/src/cmd/setup_more.rs | 31 +++---- crates/engine/src/io.rs | 119 ++++++++++++++++++++++----- 3 files changed, 149 insertions(+), 55 deletions(-) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index 37f95d9..d520342 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -91,29 +91,41 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - return Err(EngineError::BadParams(id.into(), "clip group file has too many clips".into())); } let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); - // Decode audio first so a bad file changes nothing. - let mut decoded = BTreeMap::new(); + // Decode audio first so a bad file changes nothing, and each file once however many ids name it. + let (mut decoded, mut index, mut file_of) = (Vec::new(), BTreeMap::new(), BTreeMap::new()); let rate = e.session().sample_rate.hz(); for (old, rel) in &file.audio { let full = dir.join(rel); if !full.starts_with(&dir) || rel.contains("..") { return Err(EngineError::BadParams(id.into(), format!("audio path escapes the group folder: {rel}"))); } - let bytes = crate::io::read_file(&full).map_err(|err| EngineError::Io(format!("{}: {err}", full.display())))?; - 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")))?; - decoded.insert(*old, (rel.clone(), buf)); + let key = std::fs::canonicalize(&full).unwrap_or_else(|_| full.clone()); + let n = match index.get(&key) { + Some(n) => *n, + None => { + 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")))?; + decoded.push((rel.clone(), buf)); + index.insert(key, decoded.len() - 1); + decoded.len() - 1 + } + }; + file_of.insert(*old, n); } let src_rate = f64::from(file.sample_rate.max(1)); let s = e.session_mut(); let ratio = f64::from(s.sample_rate.hz()) / src_rate; let scale = |v: Samples| -> Samples { (v as f64 * ratio).round() as Samples }; - let mut map: BTreeMap = BTreeMap::new(); - for (old, (rel, buf)) in decoded { - let name = Path::new(&rel).file_name().and_then(|x| x.to_str()).unwrap_or("clip group").to_string(); - map.insert(old, crate::io::add_source(s, &name, buf, None, FileFormat::Wav)); - } + let sources: Vec = decoded + .into_iter() + .map(|(rel, buf)| { + let name = Path::new(&rel).file_name().and_then(|x| x.to_str()).unwrap_or("clip group").to_string(); + crate::io::add_source(s, &name, buf, None, FileFormat::Wav) + }) + .collect(); + let map: BTreeMap = file_of.into_iter().filter_map(|(old, n)| Some((old, *sources.get(n)?))).collect(); let start_idx = first.and_then(|t| s.tracks.iter().position(|x| x.id == t)); let mut placed = Vec::new(); let group_id = s.alloc(); @@ -215,6 +227,24 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn a_file_named_by_many_ids_becomes_one_source() { + let dir = std::env::temp_dir().join(format!("sc-grp-shared-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + let audio = soundcraft_audio_io::AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.25; 480]] }; + std::fs::write(dir.join("k.wav"), soundcraft_audio_io::encode(&audio, &soundcraft_audio_io::EncodeOptions::default()).unwrap()).unwrap(); + let clip = |id: u64| json!({"id": id, "name": "k", "start": id * 1000, "length": 480, "content": {"Audio": {"source": id, "offset": 0}}}); + let audio_paths = json!({"1": "k.wav", "2": "./k.wav", "3": "k.wav"}); + let file = json!({"format": "soundcraft.clipgroup", "version": 1, "sample_rate": 48000, "length": 4000, "lanes": [[clip(1), clip(2), clip(3)]], "audio": audio_paths}); + let p = dir.join("k.scgrp"); + std::fs::write(&p, file.to_string()).unwrap(); + let mut e = crate::demo::demo_engine(); + let before = e.session().sources.len(); + let r = e.execute("file.import_clip_groups", &json!({"path": p.to_string_lossy()})).unwrap(); + assert_eq!((r["clips"].as_array().unwrap().len(), e.session().sources.len()), (3, before + 1)); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn rejects_foreign_files_and_escaping_paths() { let dir = std::env::temp_dir().join(format!("sc-grp-bad-{}", std::process::id())); diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index e896394..ba5c3fc 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -5,10 +5,9 @@ use super::more_util::{set_text, store_value, text, toggle_flag, values_with_pre use super::*; use crate::cmd; use serde_json::json; -use soundcraft_model::{BusId, ChannelFormat, ClipContent, OutputPath, Route, Session, Source, SourceAudio, SourceId, TrackId, TrackKind}; +use soundcraft_model::{BusId, ChannelFormat, ClipContent, OutputPath, Route, Session, SourceId, TrackId, TrackKind}; use soundcraft_time::Timecode; use std::path::{Path, PathBuf}; -use std::sync::Arc; macro_rules! option { ($id:literal, $label:literal, $flag:literal) => { @@ -401,14 +400,6 @@ fn midi_inputs(e: &mut Engine, p: &Value) -> Result { Ok(json!({"enabled": enabled})) } -/// Decode a source file of another session (paths relative to that session's folder). -fn load_source(dir: &Path, src: &Source, sr: u32) -> Option { - let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; - let bytes = crate::io::read_file(&p).ok()?; - let (_, buf) = soundcraft_audio_io::decode(&bytes, p.extension().and_then(|x| x.to_str())).ok()?; - crate::io::at_rate(buf, sr, crate::io::MAX_IMPORT_SAMPLES) -} - fn import_session_data(e: &mut Engine, p: &Value) -> Result { let id = "file.import_session_data"; let path = str_param(p, "path").filter(|x| !x.trim().is_empty()).ok_or_else(|| bad(id, "`path` required"))?.to_string(); @@ -437,31 +428,29 @@ fn import_session_data(e: &mut Engine, p: &Value) -> Result { needed.sort_by_key(|x| x.0); needed.dedup(); let sr = e.session().sample_rate.hz(); - let decoded: Vec<(Source, Option)> = - needed.iter().filter_map(|sid| other.source(*sid).cloned()).map(|src| (src.clone(), load_source(&dir, &src, sr))).collect(); let s = e.session_mut(); + let mut files = crate::io::DecodedFiles::new(); let mut map: std::collections::BTreeMap = Default::default(); let mut missing = Vec::new(); - for (src, buf) in decoded { - let nid = SourceId(s.alloc()); + for src in needed.iter().filter_map(|sid| other.source(*sid)) { let abs = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; let mut ns = src.clone(); - ns.id = nid; + ns.id = SourceId(s.alloc()); ns.path = abs.to_string_lossy().into_owned(); - match buf { - Some(b) => { - ns.frames = b.frames() as u64; + match crate::io::decode_file(&mut files, &abs, sr) { + Ok(audio) => { + ns.frames = audio.buffer.frames() as u64; ns.sample_rate = sr; ns.unsaved = true; - s.pool.insert(nid, Arc::new(SourceAudio::new(b))); + s.pool.insert(ns.id, audio); } - None => { + Err(_) => { ns.unsaved = false; missing.push(ns.path.clone()); } } + map.insert(src.id.0, ns.id); s.sources.push(ns); - map.insert(src.id.0, nid); } let mut out = Vec::new(); for mut tr in picked { diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 1cbe7a7..a1d371e 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -570,6 +570,7 @@ pub fn save_session_copy(e: &Engine, path: &str) -> Result { /// Write `session` to `path`; returns the session with updated media paths, the number of audio /// files written and the final path. fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Session, usize, PathBuf)> { + use std::collections::{HashMap, HashSet}; let p = session_file(path); let dir = p.parent().map(Path::to_path_buf).unwrap_or_default(); let audio_dir = dir.join("Audio Files"); @@ -577,8 +578,16 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess let from = s.folder.clone(); let same_dir = from.as_deref().is_some_and(|f| same_path(f, &dir)); let mut written = 0; - let shared_paths: Vec = - s.sources.iter().enumerate().map(|(i, src)| s.sources.iter().enumerate().any(|(j, other)| j != i && other.path == src.path)).collect(); + // A file can stay put for all the sources that name it, unless their audio differs. + let shared_paths: Vec = { + let mut audio_by_path: HashMap<&str, HashSet>> = HashMap::new(); + for src in &s.sources { + audio_by_path.entry(&src.path).or_default().insert(s.pool.get(src.id).map(Arc::as_ptr)); + } + s.sources.iter().map(|src| audio_by_path.get(src.path.as_str()).is_some_and(|a| a.len() > 1)).collect() + }; + // Files written for audio during this save: sources playing the same audio share one. + let mut files: HashMap<*const SourceAudio, String> = HashMap::new(); for (i, src) in s.sources.iter_mut().enumerate() { if !same_dir && s.pool.get(src.id).is_none() @@ -598,6 +607,12 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess continue; } let Some(audio) = s.pool.get(src.id) else { continue }; + if let Some(path) = files.get(&Arc::as_ptr(audio)) { + src.path.clone_from(path); + src.unsaved = false; + src.format = FileFormat::Wav; + continue; + } std::fs::create_dir_all(&audio_dir).map_err(|err| EngineError::Io(format!("{}: {err}", audio_dir.display())))?; let mut stem = sanitize_name(&src.name); if overwrite { @@ -606,6 +621,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess 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()); continue; } } @@ -615,6 +631,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess src.path = format!("Audio Files/{name}"); src.unsaved = false; src.format = FileFormat::Wav; + files.insert(Arc::as_ptr(audio), src.path.clone()); written += usize::from(new); } if written > 0 { @@ -759,15 +776,12 @@ pub fn open_session(e: &mut Engine, path: &str) -> Result> { let mut s = Session::from_json(&text).map_err(|err| EngineError::Io(err.to_string()))?; let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); let mut missing = Vec::new(); - for src in s.sources.clone() { + let mut files = DecodedFiles::new(); + for src in &s.sources { let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; - match read_file(&p) { - Ok(bytes) => { - if let Err(err) = load_source_audio(&mut s, src.id, &bytes, p.extension().and_then(|x| x.to_str())) { - missing.push(format!("{}: {err}", p.display())); - } - } - Err(_) => missing.push(p.display().to_string()), + match decode_file(&mut files, &p, s.sample_rate.hz()) { + Ok(audio) => s.pool.insert(src.id, audio), + Err(err) => missing.push(err), } } missing.extend(relink_videos(&mut s, &dir)); @@ -780,12 +794,26 @@ pub fn open_session(e: &mut Engine, path: &str) -> Result> { Ok(missing) } -/// Decode one source's audio file into the session pool, at the session rate. -fn load_source_audio(s: &mut Session, id: SourceId, bytes: &[u8], ext: Option<&str>) -> std::result::Result<(), String> { +/// Decode one source's audio file at the session rate. +fn decode_source(bytes: &[u8], ext: Option<&str>, rate: u32) -> std::result::Result, String> { let (_, buf) = soundcraft_audio_io::decode(bytes, ext).map_err(|err| err.to_string())?; - let buf = at_rate(buf, s.sample_rate.hz(), MAX_IMPORT_SAMPLES).ok_or("too large to load")?; - s.pool.insert(id, Arc::new(SourceAudio::new(buf))); - Ok(()) + let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or("too large to load")?; + Ok(Arc::new(SourceAudio::new(buf))) +} + +/// Audio files decoded so far, by path: a session may name one file thousands of times. +pub(crate) type DecodedFiles = std::collections::HashMap, String>>; + +/// The audio of the file at `p` at `rate`, decoded the first time it is asked for. The error +/// names the file. +pub(crate) fn decode_file(files: &mut DecodedFiles, p: &Path, rate: u32) -> std::result::Result, String> { + files + .entry(std::fs::canonicalize(p).unwrap_or_else(|_| p.to_path_buf())) + .or_insert_with(|| { + let bytes = read_file(p).map_err(|_| p.display().to_string())?; + decode_source(&bytes, p.extension().and_then(|x| x.to_str()), rate).map_err(|err| format!("{}: {err}", p.display())) + }) + .clone() } // ─── Hosts without a filesystem (the browser) ───────────────────────────────────────────────── @@ -888,14 +916,12 @@ pub fn open_session_bytes(e: &mut Engine, name: &str, bytes: &[u8]) -> Result { - let ext = Path::new(&src.path).extension().and_then(|x| x.to_str()).map(str::to_string); - if let Err(err) = load_source_audio(&mut s, src.id, &audio, ext.as_deref()) { - missing.push(format!("{}: {err}", src.path)); - } - } + Some(audio) => match decode_source(&audio, Path::new(&src.path).extension().and_then(|x| x.to_str()), s.sample_rate.hz()) { + Ok(audio) => s.pool.insert(src.id, audio), + Err(err) => missing.push(format!("{}: {err}", src.path)), + }, None => missing.push(src.path.clone()), } } @@ -1272,6 +1298,55 @@ mod rate_tests { } } +#[cfg(test)] +mod open_tests { + use super::*; + + #[test] + fn a_file_named_by_many_sources_is_decoded_once() { + let dir = std::env::temp_dir().join(format!("soundcraft-shared-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let audio = AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.25; 480]] }; + std::fs::write(dir.join("k.wav"), soundcraft_audio_io::encode(&audio, &EncodeOptions::default()).unwrap()).unwrap(); + let mut s = Session::default(); + for _ in 0..3 { + add_source(&mut s, "k", audio.clone(), Some("k.wav"), FileFormat::Wav); + } + let path = dir.join("Song.scraft").to_string_lossy().into_owned(); + std::fs::write(&path, s.to_json().unwrap()).unwrap(); + let mut e = Engine::default(); + assert!(open_session(&mut e, &path).unwrap().is_empty()); + let (sources, pool) = (&e.session().sources, &e.session().pool); + let first = pool.get(sources[0].id).unwrap(); + assert!(sources.iter().all(|src| pool.get(src.id).is_some_and(|a| Arc::ptr_eq(a, first)))); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn sources_playing_one_file_share_it_when_saved() { + let dir = std::env::temp_dir().join(format!("soundcraft-shared-save-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Audio Files")).unwrap(); + let audio = AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.25; 480]] }; + std::fs::write(dir.join("Audio Files/k.wav"), soundcraft_audio_io::encode(&audio, &EncodeOptions::default()).unwrap()).unwrap(); + let mut s = Session::default(); + for _ in 0..3 { + add_source(&mut s, "k", audio.clone(), Some("Audio Files/k.wav"), FileFormat::Wav); + } + s.sources.iter_mut().for_each(|src| src.unsaved = false); + let path = dir.join("Song.scraft").to_string_lossy().into_owned(); + std::fs::write(&path, s.to_json().unwrap()).unwrap(); + let mut e = Engine::default(); + open_session(&mut e, &path).unwrap(); + assert_eq!(save_session(&mut e, &path).unwrap(), 0); + assert!(e.session().sources.iter().all(|src| src.path == "Audio Files/k.wav")); + assert_eq!(save_copy(&e, &dir.join("copy/Song.scraft").to_string_lossy()).unwrap(), 1); + assert_eq!(std::fs::read_dir(dir.join("copy/Audio Files")).unwrap().count(), 1); + let _ = std::fs::remove_dir_all(&dir); + } +} + #[cfg(test)] mod name_tests { use super::*;