From e10b24565212e0279dbe2f37d427d724c851c1a0 Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:56:29 +0000 Subject: [PATCH 01/55] Keep mute changes in place when thinning automation --- crates/model/src/automation.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/model/src/automation.rs b/crates/model/src/automation.rs index 3a68b041..a6d3963b 100644 --- a/crates/model/src/automation.rs +++ b/crates/model/src/automation.rs @@ -204,7 +204,8 @@ impl AutomationLane { /// Thin: remove points whose removal changes the curve by less than `tolerance`. pub fn thin(&mut self, tolerance: f32) -> usize { - if self.points.len() < 3 || tolerance.is_nan() { + // Linear thinning would move the transitions of step lanes. + if matches!(self.param, AutoParam::Mute | AutoParam::SendMute(_)) || self.points.len() < 3 || tolerance.is_nan() { return 0; } let tolerance = f64::from(tolerance); @@ -303,6 +304,19 @@ mod tests { } } + #[test] + fn thin_keeps_mute_changes_in_place() { + for param in [AutoParam::Mute, AutoParam::SendMute(0)] { + let mut lane = AutomationLane::new(param); + for (at, value) in [(0, 0.0), (9000, 1.0), (10000, 1.0)] { + lane.set_point(at, value); + } + lane.thin(0.1); + assert_eq!(lane.value_at(8999, 0.0), 0.0); + assert_eq!(lane.value_at(9000, 0.0), 1.0); + } + } + #[test] fn param_ids() { assert_eq!(AutoParam::parse("send_b_level"), Some(AutoParam::SendLevel(1))); From 606ddac16970584b486ed523371f76d19d710176 Mon Sep 17 00:00:00 2001 From: Justin John <34035011+justinjohn0306@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:44:34 +0530 Subject: [PATCH 02/55] =?UTF-8?q?Track=20=E2=80=BA=20Instrument:=20choose?= =?UTF-8?q?=20the=20selected=20track's=20instrument=20from=20the=20menu=20?= =?UTF-8?q?bar=20(#374)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The submenu under Track › New... opens the same picker as the Mix window's instrument slot for the first selected instrument track, and is greyed out with a hint when none is selected. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/menus.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9eef0b22..44428206 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -243,6 +243,9 @@ pub fn menu_bar(app: &mut SoundApp, ui: &mut egui::Ui) { if c.path == "File > Open" { crate::recent::menu(app, ui); } + if c.path == "Track > New..." { + instrument_menu(app, ui); + } } if root.label == "AudioSuite" { ui.separator(); @@ -276,6 +279,23 @@ pub fn menu_bar(app: &mut SoundApp, ui: &mut egui::Ui) { ); } +/// Track › Instrument: the instrument picker for the selected instrument track (#374). +fn instrument_menu(app: &mut SoundApp, ui: &mut egui::Ui) { + match instrument_track(app.engine.session()) { + Some(id) => { + ui.menu_button("Instrument", |ui| crate::mix_window::instrument_menu(app, ui, id)); + } + None => { + ui.add_enabled(false, egui::Button::new("Instrument")).on_disabled_hover_text("Select an instrument track"); + } + } +} + +/// The first selected instrument track, in track order. +fn instrument_track(s: &soundcraft_model::Session) -> Option { + s.tracks.iter().find(|t| t.kind == soundcraft_model::TrackKind::Instrument && s.edit.selected_tracks.contains(&t.id)).map(|t| t.id) +} + fn menu_node(app: &mut SoundApp, ui: &mut egui::Ui, n: &MenuNode, extra: &[(&str, &str)]) { if !n.children.is_empty() { ui.menu_button(&n.label, |ui| { @@ -720,4 +740,16 @@ mod tests { assert!(!app.engine.is_dirty()); let _ = std::fs::remove_dir_all(&dir); } + + #[test] + fn track_instrument_menu_targets_the_selected_instrument_track() { + let mut e = soundcraft_engine::Engine::default(); + let audio = e.execute("track.new", &json!({"kind": "audio"})).unwrap()["tracks"][0].as_u64().unwrap(); + assert_eq!(instrument_track(e.session()), None); + let inst = e.execute("track.new", &json!({"kind": "instrument"})).unwrap()["tracks"][0].as_u64().unwrap(); + e.execute("edit.select", &json!({"tracks": [audio]})).unwrap(); + assert_eq!(instrument_track(e.session()), None, "an audio track has no instrument"); + e.execute("edit.select", &json!({"tracks": [audio, inst]})).unwrap(); + assert_eq!(instrument_track(e.session()).map(|t| t.0), Some(inst)); + } } From 194cf8c83db96c81436a2be3859e65fc6435621e Mon Sep 17 00:00:00 2001 From: Justin John <34035011+justinjohn0306@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:04:26 +0530 Subject: [PATCH 03/55] Portable mode: portable.txt beside the executable keeps settings in SoundCraftData (#268) With the marker present, ui.json, Autosave, Presets and logs go to /SoundCraftData instead of the user profile. SOUNDCRAFT_CONFIG_DIR still wins, and a data folder that can't be written falls back to the profile with a warning in the log. Co-Authored-By: Claude Opus 5.5 --- apps/soundcraft/src/config.rs | 88 +++++++++++++++++++++++++++++------ docs/control-protocol.md | 2 +- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/apps/soundcraft/src/config.rs b/apps/soundcraft/src/config.rs index 1ade4ee0..5efab9a7 100644 --- a/apps/soundcraft/src/config.rs +++ b/apps/soundcraft/src/config.rs @@ -1,7 +1,12 @@ //! Per-user settings and automation profile directories. use std::ffi::OsString; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; + +/// Portable mode (#268): this file beside the executable keeps settings, autosaves, presets and +/// logs in [`PORTABLE_DATA_DIR`] next to it instead of the user profile. +const PORTABLE_MARKER: &str = "portable.txt"; +const PORTABLE_DATA_DIR: &str = "SoundCraftData"; #[derive(Clone, Copy)] enum Platform { @@ -13,7 +18,18 @@ enum Platform { /// SoundCraft's settings folder. A nonempty `$SOUNDCRAFT_CONFIG_DIR` overrides the platform default. /// The override is the complete directory; no application-name suffix is appended. An explicitly /// empty value disables the settings directory instead of falling back to a personal profile. +/// Otherwise `portable.txt` beside the executable selects `/SoundCraftData`, when that +/// folder can be written (a read-only medium falls back to the platform default). pub fn config_dir() -> Option { + static DIR: std::sync::OnceLock> = std::sync::OnceLock::new(); + DIR.get_or_init(|| { + let exe = std::env::current_exe().ok(); + resolve(exe.as_deref().and_then(Path::parent)) + }) + .clone() +} + +fn resolve(exe_dir: Option<&Path>) -> Option { let platform = if cfg!(target_os = "macos") { Platform::Mac } else if cfg!(windows) { @@ -21,13 +37,20 @@ pub fn config_dir() -> Option { } else { Platform::Other }; - config_dir_with(platform, |name| std::env::var_os(name)) + config_dir_with(platform, |name| std::env::var_os(name), exe_dir) } -fn config_dir_with(platform: Platform, env: impl Fn(&str) -> Option) -> Option { +fn config_dir_with(platform: Platform, env: impl Fn(&str) -> Option, exe_dir: Option<&Path>) -> Option { if let Some(dir) = env("SOUNDCRAFT_CONFIG_DIR") { return (!dir.is_empty()).then(|| PathBuf::from(dir)); } + if let Some(exe_dir) = exe_dir.filter(|d| d.join(PORTABLE_MARKER).is_file()) { + let data = exe_dir.join(PORTABLE_DATA_DIR); + match writable(&data) { + Ok(()) => return Some(data), + Err(e) => log::warn!("portable mode: {} can't be written ({e}); using the user profile instead", data.display()), + } + } match platform { Platform::Mac => env("HOME").map(|h| PathBuf::from(h).join("Library/Application Support/SoundCraft")), Platform::Windows => env("APPDATA").map(|a| PathBuf::from(a).join("SoundCraft")), @@ -37,6 +60,14 @@ fn config_dir_with(platform: Platform, env: impl Fn(&str) -> Option) - } } +/// Create `dir` if needed and prove a file can be written in it. +fn writable(dir: &Path) -> std::io::Result<()> { + std::fs::create_dir_all(dir)?; + let probe = dir.join(format!(".write-test-{}", std::process::id())); + std::fs::write(&probe, b"")?; + std::fs::remove_file(probe) +} + /// Read bounded settings, recovering unreadable docking metadata independently of other settings. pub fn load_ui(path: &std::path::Path) -> Result, String> { read_ui(path, true) @@ -195,16 +226,20 @@ mod config_tests { } fn resolve(platform: Platform, vars: &[(&str, &str)]) -> Option { - config_dir_with(platform, |name| vars.iter().find_map(|(key, value)| (*key == name).then(|| OsString::from(value)))) + config_dir_with(platform, |name| vars.iter().find_map(|(key, value)| (*key == name).then(|| OsString::from(value))), None) } #[test] fn config_override_wins_without_reading_user_profile() { for platform in [Platform::Mac, Platform::Windows, Platform::Other] { - let actual = config_dir_with(platform, |name| { - assert_eq!(name, "SOUNDCRAFT_CONFIG_DIR"); - Some(OsString::from("isolated/settings")) - }); + let actual = config_dir_with( + platform, + |name| { + assert_eq!(name, "SOUNDCRAFT_CONFIG_DIR"); + Some(OsString::from("isolated/settings")) + }, + None, + ); assert_eq!(actual, Some(PathBuf::from("isolated/settings"))); } } @@ -213,10 +248,14 @@ mod config_tests { fn config_empty_override_disables_profile_without_reading_defaults() { for platform in [Platform::Mac, Platform::Windows, Platform::Other] { assert_eq!( - config_dir_with(platform, |name| { - assert_eq!(name, "SOUNDCRAFT_CONFIG_DIR"); - Some(OsString::new()) - }), + config_dir_with( + platform, + |name| { + assert_eq!(name, "SOUNDCRAFT_CONFIG_DIR"); + Some(OsString::new()) + }, + None + ), None ); } @@ -234,11 +273,34 @@ mod config_tests { } } + #[test] + fn portable_txt_beside_the_exe_keeps_settings_next_to_it() { + let exe = std::env::temp_dir().join(format!("soundcraft-portable-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&exe); + std::fs::create_dir_all(&exe).unwrap(); + let vars = [("APPDATA", "/roaming")]; + assert_eq!(resolve_with_exe(&vars, &exe), Some(PathBuf::from("/roaming/SoundCraft")), "no marker: the profile"); + std::fs::write(exe.join("portable.txt"), b"").unwrap(); + assert_eq!(resolve_with_exe(&vars, &exe), Some(exe.join("SoundCraftData"))); + assert!(exe.join("SoundCraftData").is_dir()); + // The explicit override still wins. + assert_eq!(resolve_with_exe(&[("SOUNDCRAFT_CONFIG_DIR", "/custom")], &exe), Some(PathBuf::from("/custom"))); + // A data folder that can't be made (a file is in the way) falls back to the profile. + std::fs::remove_dir_all(exe.join("SoundCraftData")).unwrap(); + std::fs::write(exe.join("SoundCraftData"), b"").unwrap(); + assert_eq!(resolve_with_exe(&vars, &exe), Some(PathBuf::from("/roaming/SoundCraft"))); + std::fs::remove_dir_all(exe).unwrap(); + } + + fn resolve_with_exe(vars: &[(&str, &str)], exe: &Path) -> Option { + config_dir_with(Platform::Windows, |name| vars.iter().find_map(|(key, value)| (*key == name).then(|| OsString::from(value))), Some(exe)) + } + #[cfg(unix)] #[test] fn config_override_preserves_non_unicode_paths() { use std::os::unix::ffi::OsStringExt as _; let value = OsString::from_vec(b"isolated/\xff".to_vec()); - assert_eq!(config_dir_with(Platform::Mac, |_| Some(value.clone())), Some(PathBuf::from(value))); + assert_eq!(config_dir_with(Platform::Mac, |_| Some(value.clone()), None), Some(PathBuf::from(value))); } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 95a5bbf6..9efe3789 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -15,7 +15,7 @@ input), and every command is reachable from: ## Isolated automation profiles Set `SOUNDCRAFT_CONFIG_DIR` to a fresh directory to isolate UI preferences, `Autosave`, `Presets`, and `logs`. -The nonempty value is used as the complete settings directory. Unset retains the normal platform paths; +The nonempty value is used as the complete settings directory, even in portable mode (`portable.txt` beside the executable, which otherwise uses `SoundCraftData` next to it). Unset retains the normal platform paths; an explicit empty value disables the settings directory instead of falling back to a personal profile. Save synthetic sessions to explicit scratch paths. Do not use `SOUNDCRAFT_NO_PREFS` when testing preference persistence across restarts. From 1bebc52bf4633e76247b3b203423539aa3c716e9 Mon Sep 17 00:00:00 2001 From: Justin John <34035011+justinjohn0306@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:13:40 +0530 Subject: [PATCH 04/55] MIDI writer: clamp out-of-range pitches and controller numbers instead of wrapping (#472) Pitch, release velocity, CC number and poly-pressure pitch were masked with 0x7F, so 128 became 0 and 255 became 127. They now clamp to 127, as the write_smf doc promises and note-on velocity already did. Co-Authored-By: Claude Opus 5.5 --- crates/midi/src/smf.rs | 8 ++++---- crates/midi/tests/smf.rs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/midi/src/smf.rs b/crates/midi/src/smf.rs index 36c18a0b..7747a061 100644 --- a/crates/midi/src/smf.rs +++ b/crates/midi/src/smf.rs @@ -485,20 +485,20 @@ pub fn write_smf(smf: &Smf) -> Result, MidiError> { } for n in &track.sequence.notes { let ch = n.channel & 0x0F; - let pitch = n.pitch & 0x7F; + let pitch = n.pitch.min(127); let start = clamp_tick(n.start); let end = clamp_tick(n.end()).max(start.saturating_add(1)); ev.push(WEvent { tick: start, order: 3, bytes: vec![0x90 | ch, pitch, n.velocity.clamp(1, 127)] }); - ev.push(WEvent { tick: end, order: 1, bytes: vec![0x80 | ch, pitch, n.release_velocity & 0x7F] }); + ev.push(WEvent { tick: end, order: 1, bytes: vec![0x80 | ch, pitch, n.release_velocity.min(127)] }); } for c in &track.sequence.ctrls { let ch = c.channel & 0x0F; let v7 = c.value.clamp(0, 127) as u8; let bytes = match c.kind { - CtrlKind::Cc(num) => vec![0xB0 | ch, num & 0x7F, v7], + CtrlKind::Cc(num) => vec![0xB0 | ch, num.min(127), v7], CtrlKind::Program => vec![0xC0 | ch, v7], CtrlKind::ChannelPressure => vec![0xD0 | ch, v7], - CtrlKind::PolyPressure(p) => vec![0xA0 | ch, p & 0x7F, v7], + CtrlKind::PolyPressure(p) => vec![0xA0 | ch, p.min(127), v7], CtrlKind::PitchBend => { let v = (c.value.saturating_add(8192)).clamp(0, 16383); vec![0xE0 | ch, (v & 0x7F) as u8, ((v >> 7) & 0x7F) as u8] diff --git a/crates/midi/tests/smf.rs b/crates/midi/tests/smf.rs index f1861d83..f069be39 100644 --- a/crates/midi/tests/smf.rs +++ b/crates/midi/tests/smf.rs @@ -332,3 +332,21 @@ proptest! { prop_assert_eq!(a, b); } } + +/// #472: out-of-range pitches, release velocities and controller numbers clamp to 127 instead of +/// wrapping into other notes and controllers (128 & 0x7F would be 0). +#[test] +fn writer_clamps_out_of_range_data_bytes() { + for raw in [127u8, 128, 255] { + let mut seq = Sequence::default(); + seq.notes.push(Note { pitch: raw, velocity: 100, release_velocity: raw, channel: 0, start: 0, length: 960 }); + seq.ctrls.push(CtrlEvent { tick: 0, channel: 0, kind: CtrlKind::Cc(raw), value: 64 }); + seq.ctrls.push(CtrlEvent { tick: 0, channel: 0, kind: CtrlKind::PolyPressure(raw), value: 64 }); + let smf = Smf { tracks: vec![SmfTrack { name: String::new(), sequence: seq }], ..Smf::default() }; + let back = read_smf(&write_smf(&smf).unwrap()).unwrap(); + let s = &back.tracks.last().unwrap().sequence; + assert_eq!((s.notes[0].pitch, s.notes[0].release_velocity), (127, 127), "input {raw}"); + let kinds: Vec = s.ctrls.iter().map(|c| c.kind).collect(); + assert!(kinds.contains(&CtrlKind::Cc(127)) && kinds.contains(&CtrlKind::PolyPressure(127)), "input {raw}: {kinds:?}"); + } +} From bf3cd7ac09be0f09162e8f944b8f601dd047567f Mon Sep 17 00:00:00 2001 From: Jafz2001 Date: Tue, 13 Oct 2026 11:40:34 -0300 Subject: [PATCH 05/55] fix(routing): remove folder bus on delete --- crates/engine/src/cmd/track.rs | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/engine/src/cmd/track.rs b/crates/engine/src/cmd/track.rs index 87912389..5e19182c 100644 --- a/crates/engine/src/cmd/track.rs +++ b/crates/engine/src/cmd/track.rs @@ -493,6 +493,15 @@ fn folder_bus(s: &Session, folder: TrackId) -> Option { s.track(folder).map(|folder| folder.mixer.input.clone()).filter(|route| matches!(route, Route::Bus(_))) } +/// The summing bus owned by a routing folder. +fn owned_folder_bus(s: &Session, folder: TrackId) -> Option { + let folder = s.track(folder).filter(|track| track.kind == TrackKind::Folder)?; + match folder.mixer.input { + Route::Bus(id) => Some(id), + _ => None, + } +} + /// Keep only the selected roots so selecting a folder and one of its children preserves the subtree. fn root_tracks(s: &Session, tracks: Vec) -> Vec { tracks.iter().copied().filter(|track| !tracks.iter().any(|root| root != track && in_folder(s, *track, *root))).collect() @@ -577,6 +586,20 @@ fn change_width(e: &mut Engine, p: &Value) -> Result { fn delete(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "track.delete", p)?; let s = e.session_mut(); + let removed_busses: Vec<_> = tracks + .iter() + .filter_map(|folder| { + let bus = owned_folder_bus(s, *folder)?; + let mut parent = s.track(*folder).and_then(|folder| folder.folder); + for _ in 0..64 { + match parent { + Some(candidate) if tracks.contains(&candidate) => parent = s.track(candidate).and_then(|folder| folder.folder), + _ => break, + } + } + Some((bus, parent.and_then(|parent| folder_bus(s, parent)).unwrap_or(Route::Main))) + }) + .collect(); // Promote children of deleted folders and repair only routes that used the deleted // folder's own bus. Custom outputs remain untouched. let mut promoted = Vec::new(); @@ -607,6 +630,22 @@ fn delete(e: &mut Engine, p: &Value) -> Result { for g in &mut s.groups { g.members.retain(|t| !tracks.contains(t)); } + for (bus, replacement) in removed_busses { + s.busses.retain(|candidate| candidate.id != bus); + for track in &mut s.tracks { + if track.mixer.input == Route::Bus(bus) { + track.mixer.input = Route::None; + } + if track.mixer.output == Route::Bus(bus) { + track.mixer.output = replacement.clone(); + } + for send in &mut track.mixer.sends { + if send.as_ref().is_some_and(|send| send.target == Route::Bus(bus)) { + *send = None; + } + } + } + } Ok(json!({"deleted": tracks.len()})) } @@ -920,6 +959,14 @@ mod tests { e.execute("track.move_to_folder", &json!({"tracks": ["Kick"], "folder": inner.0})).unwrap(); e.execute("track.move_to_folder", &json!({"tracks": [inner.0], "folder": outer.0})).unwrap(); + let inner_bus = match e.session().track(inner).unwrap().mixer.input { + Route::Bus(bus) => bus, + _ => panic!("folder did not receive a bus"), + }; + let outer_bus = match e.session().track(outer).unwrap().mixer.input { + Route::Bus(bus) => bus, + _ => panic!("folder did not receive a bus"), + }; assert!(e.execute("track.move_to_folder", &json!({"tracks": [outer.0], "folder": inner.0})).is_err()); let kick = e.session().track_by_name("Kick").unwrap(); @@ -946,6 +993,8 @@ mod tests { let kick = e.session().track_by_name("Kick").unwrap(); assert_eq!(kick.folder, None); assert_eq!(kick.mixer.output, Route::Main); + assert!(e.session().bus(inner_bus).is_none(), "the deleted folder's owned bus must be removed"); + assert!(e.session().bus(outer_bus).is_some(), "the surviving parent folder's bus must remain"); } #[test] From 0c19777c8a90a86fffc4146323fc0597307c644d Mon Sep 17 00:00:00 2001 From: Royce Date: Tue, 13 Oct 2026 22:50:49 +0800 Subject: [PATCH 06/55] fix(ui): add Fedora font paths so DejaVu and Liberation load on Fedora (#45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Linux font search list in `crates/ui-egui/src/fonts.rs` covered Debian/Ubuntu (`/usr/share/fonts/truetype/dejavu`, `/usr/share/fonts/TTF`, `/usr/share/fonts/dejavu`) and Arch (`/usr/local/share/fonts/dejavu`) for DejaVu, and a single path for Liberation Sans. It did not cover Fedora's package layout: - `/usr/share/fonts/dejavu-sans-fonts/DejaVuSans.ttf` (and -Bold) - `/usr/share/fonts/liberation-sans-fonts/LiberationSans-Regular.ttf` On Fedora, neither path matched, the `load_from_fontconfig` path returned None (e.g. the user only has Liberation Sans installed, not DejaVu), and the UI fell back to egui's built-in fonts. Those don't cover the menu glyphs (`→ ▸ ▾ ♯ ✕`), so the symbols showed as empty boxes. Add the two Fedora paths to the REGULAR and BOLD candidates, in the same order as the existing Debian and Arch paths. `load` still tries fontconfig first; the direct paths are the fallback. cargo test -p soundcraft-ui-egui: 105 passed, 0 failed. --- crates/ui-egui/src/fonts.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ui-egui/src/fonts.rs b/crates/ui-egui/src/fonts.rs index 46d50bae..ae185834 100644 --- a/crates/ui-egui/src/fonts.rs +++ b/crates/ui-egui/src/fonts.rs @@ -12,8 +12,10 @@ const REGULAR: &[(&str, u32)] = &[ ("/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", 0), ("/usr/share/fonts/TTF/DejaVuSans.ttf", 0), ("/usr/share/fonts/dejavu/DejaVuSans.ttf", 0), + ("/usr/share/fonts/dejavu-sans-fonts/DejaVuSans.ttf", 0), ("/usr/local/share/fonts/dejavu/DejaVuSans.ttf", 0), ("/usr/share/fonts/truetype/liberation/LiberationSans-Regular.ttf", 0), + ("/usr/share/fonts/liberation-sans-fonts/LiberationSans-Regular.ttf", 0), ]; const BOLD: &[(&str, u32)] = &[ ("/System/Library/Fonts/HelveticaNeue.ttc", 1), @@ -22,8 +24,10 @@ const BOLD: &[(&str, u32)] = &[ ("/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf", 0), ("/usr/share/fonts/TTF/DejaVuSans-Bold.ttf", 0), ("/usr/share/fonts/dejavu/DejaVuSans-Bold.ttf", 0), + ("/usr/share/fonts/dejavu-sans-fonts/DejaVuSans-Bold.ttf", 0), ("/usr/local/share/fonts/dejavu/DejaVuSans-Bold.ttf", 0), ("/usr/share/fonts/truetype/liberation/LiberationSans-Bold.ttf", 0), + ("/usr/share/fonts/liberation-sans-fonts/LiberationSans-Bold.ttf", 0), ]; /// Hangul fallback, appended after every family so Korean session, track and clip names draw /// whatever the interface language. The Noto Sans CJK collection's face 1 is its Korean face. From 8d252c8b2eef4bb4469d2b1857281b5c964fb72f Mon Sep 17 00:00:00 2001 From: euisuh <55032583+euisuh@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:15:26 +0900 Subject: [PATCH 07/55] fix(mcp): M8 reject oversized batches before execution Reject oversized batches before any side effects instead of reporting success after truncating calls. Closes #460 Co-Authored-By: OpenAI Codex --- ROADMAP.md | 4 ++- crates/automation/src/mcp.rs | 10 +++++--- crates/automation/tests/agent_tasks.rs | 34 ++++++++++++++++++++++++++ docs/mcp.md | 2 +- docs/roadmap.md | 6 ++++- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 73197650..c44cdcf6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: pre-alpha** · next: alpha, ~5% and ~110–180 h away (the alpha gate's three failing workflows) -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-10 · **Change:** minor (readiness by audience with hours to ~95 %) · **Target:** Avid Pro Tools Ultimate 2026.4.1 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** trivial (MCP batch limit validation) · **Target:** Avid Pro Tools Ultimate 2026.4.1 SoundCraft is a clean-room, pure-Rust digital audio workstation that aims at full parity with Avid Pro Tools, and then beyond it: faster, open, scriptable and agent-controllable. This page is @@ -122,6 +122,7 @@ mutated and truncated movies. Field reports from the first public week are track ## Progress log +- **2026-10-14:** MCP `batch` rejects more than 1000 calls before executing any command, instead of silently running only the first 1000 (#460). No readiness percentage change. - **2026-10-13:** `mix.instrument` takes the built-in instrument's parameters in `params` (validated, clamped) and refuses any other field, so a synth setting written beside `plugin` is an error instead of being accepted and ignored (#265) - **2026-10-13:** A bounce of a bus that an aux returns onto lines up with the timeline: the aux insert's lookahead is compensated like the main mix's (it left the bus late and cut the end off), and tracks routed to that bus wait for it too (#246) - **2026-10-13:** Delay compensation covers the inserts of the master faders: a bounce of the main mix with a Maximizer on the master lines up with the same mix without it (it came out 71 samples late) (#351) @@ -155,6 +156,7 @@ mutated and truncated movies. Field reports from the first public week are track | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | Progress log: oversized MCP batches rejected before execution (#460) | | 2026-10-13 | trivial | Progress log: mix.instrument params, unknown fields refused (#265) | | 2026-10-13 | trivial | Progress log: bus bounce compensates aux returns (#246) | | 2026-10-13 | trivial | Progress log: master insert latency compensated (#351) | diff --git a/crates/automation/src/mcp.rs b/crates/automation/src/mcp.rs index cacd9a03..886b25d7 100644 --- a/crates/automation/src/mcp.rs +++ b/crates/automation/src/mcp.rs @@ -6,6 +6,7 @@ use std::io::{BufRead, Write}; pub const PROTOCOL_VERSION: &str = "2025-06-18"; const SUPPORTED: &[&str] = &["2025-06-18", "2025-03-26", "2024-11-05"]; +const MAX_BATCH_CALLS: usize = 1000; const INSTRUCTIONS: &str = "SoundCraft is a digital audio workstation (Pro Tools-class). Everything a user can do is a \ command: call `list_commands` to discover ids and their params, then `execute` (or `batch`). Positions are samples \ @@ -37,8 +38,8 @@ pub fn tool_definitions() -> Vec { ), tool( "batch", - "Run several commands in order; stops at the first error unless keep_going.", - json!({"calls": {"type": "array", "items": {"type": "object", "properties": {"command": {"type": "string"}, "params": {"type": "object"}}}}, "keep_going": {"type": "boolean"}}), + "Run up to 1000 commands in order; stops at the first error unless keep_going. Larger batches are rejected before any command runs.", + json!({"calls": {"type": "array", "maxItems": MAX_BATCH_CALLS, "items": {"type": "object", "properties": {"command": {"type": "string"}, "params": {"type": "object"}}}}, "keep_going": {"type": "boolean"}}), &["calls"], ), tool( @@ -135,9 +136,12 @@ impl Server { let Some(calls) = args.get("calls").and_then(Value::as_array) else { return error_result("`calls` must be an array"); }; + if calls.len() > MAX_BATCH_CALLS { + return error_result(&format!("`calls` must contain at most {MAX_BATCH_CALLS} commands; no commands were executed")); + } let mut out = Vec::new(); let mut failed = false; - for c in calls.iter().take(1000) { + for c in calls { let Some(cmd) = c.get("command").and_then(Value::as_str).map(str::trim).filter(|s| !s.is_empty()) else { out.push(json!({"command": "", "ok": false, "error": "`command` required"})); failed = true; diff --git a/crates/automation/tests/agent_tasks.rs b/crates/automation/tests/agent_tasks.rs index 3636cfe3..b94178b1 100644 --- a/crates/automation/tests/agent_tasks.rs +++ b/crates/automation/tests/agent_tasks.rs @@ -31,6 +31,8 @@ fn handshake_and_tool_list() { assert_eq!(r["result"]["serverInfo"]["name"], "soundcraft"); let r = rpc(&mut s, 2, "tools/list", json!({})); assert!(r["result"]["tools"].as_array().unwrap().len() >= 10); + let batch = r["result"]["tools"].as_array().unwrap().iter().find(|tool| tool["name"] == "batch").unwrap(); + assert_eq!(batch["inputSchema"]["properties"]["calls"]["maxItems"], 1000); assert!(s.handle(&json!({"jsonrpc": "2.0", "method": "notifications/initialized"})).is_none()); let r = rpc(&mut s, 3, "nope", json!({})); assert_eq!(r["error"]["code"], -32601); @@ -171,3 +173,35 @@ fn batch_rejects_non_array_calls() { assert!(text.contains("`calls` must be an array"), "{text}"); assert_eq!(session(&mut s)["tracks"].as_array().unwrap().len(), 0); } + +#[test] +fn batch_rejects_oversize_calls_before_any_mutation() { + for keep_going in [false, true] { + let mut s = Server::new(Headless::new(Engine::default())); + let before = session(&mut s); + let mut calls = vec![json!({"command": "engine.history"}); 1001]; + calls[0] = json!({"command": "track.new", "params": {"name": "Must not be created"}}); + let r = rpc(&mut s, 1, "tools/call", json!({"name": "batch", "arguments": {"calls": calls, "keep_going": keep_going}})); + assert_eq!(session(&mut s), before, "oversize batch mutated the session (keep_going={keep_going})"); + assert!(s.backend.engine.journal.is_empty()); + assert!(!s.backend.engine.can_undo()); + assert_eq!(r["result"]["isError"], true); + let text = r["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("1000"), "{text}"); + } +} + +#[test] +fn batch_accepts_empty_and_limit_sized_calls_in_order() { + for count in [0, 1000] { + let mut s = Server::new(Headless::new(Engine::default())); + let calls: Vec<_> = (0..count).map(|at| json!({"command": "session.format_time", "params": {"at": at, "format": "samples"}})).collect(); + let results = tool(&mut s, "batch", json!({"calls": calls})); + let results = results.as_array().unwrap(); + assert_eq!(results.len(), count); + for (at, result) in results.iter().enumerate() { + assert_eq!(result["ok"], true); + assert_eq!(result["result"]["text"], at.to_string()); + } + } +} diff --git a/docs/mcp.md b/docs/mcp.md index d31712bb..558852c1 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -90,7 +90,7 @@ files it was given rather than guarding against other software on the computer. |---|---| | `list_commands` | Discover every command (id, label, menu path, shortcut, params doc, enabled). | | `execute` | Run one command with JSON params. | -| `batch` | Run several commands in order (`keep_going` optional). Any failed call makes the tool result `isError: true`; with `keep_going` the remaining calls still run and every result is listed. | +| `batch` | Run up to 1000 commands in order (`keep_going` optional). Larger batches return `isError: true` before executing any command. Any failed call makes the tool result `isError: true`; with `keep_going` the remaining calls still run and every result is listed. | | `inspect_session` | Verify work: tracks, clips, mixer, plugins, sends, automation, markers, selection; each `sources[]` entry includes `loaded` (decoded audio in the pool). | | `new_session`, `open_session`, `save_session` | Session files (`.scraft`, audio in `Audio Files/`). `open_session` returns `missing`: the media files that could not be loaded (empty when all are found). `save_session` won't save over another existing file unless `replace` is true. | | `import_audio` | WAV, AIFF, FLAC, MP3, OGG, AAC/M4A, ALAC, CAF onto a new or existing track. | diff --git a/docs/roadmap.md b/docs/roadmap.md index d8ea3f8c..c24f4255 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # SoundCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-10 · **Change:** minor (alpha gate added; stage re-normalized to pre-alpha) · **Target:** Avid Pro Tools Ultimate 2026.4.1 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** trivial (M8 batch limit validation) · **Target:** Avid Pro Tools Ultimate 2026.4.1 Forward-looking plan. Status and numbers: [`../ROADMAP.md`](../ROADMAP.md) and [`target-app-parity.md`](target-app-parity.md); the ranked work list: [`gaps.md`](gaps.md). @@ -54,6 +54,9 @@ Status: ✅ done · 🟡 in progress · ⬜ not started. | M16 | Built-in plugin depth, sidechain, ARA | ⬜ | 110–170 | | M17 | Immersive delivery (ADM BWF, object renderer, binaural) and video depth | ⬜ | 80–130 | +M8 follow-up (2026-10-14): MCP batches above 1000 calls are rejected before execution (#460). +The limit is advertised in the tool schema and description; stage and readiness estimates are unchanged. + ## To beta (~350–570 h, about 60–70 % parallelizable) Beta needs ~75 % ready for real work and no blocking gap in session interchange. @@ -86,5 +89,6 @@ There is no `cargo xtask scorecard` in this repo; the measured number is `cargo | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | M8: reject oversized MCP batches before execution (#460) | | 2026-10-10 | minor | Added the alpha gate (3 of 6 core workflows fail → pre-alpha) | | 2026-10-10 | major | Created from ROADMAP.md's milestones and alpha list; added M12–M17 and the beta plan with estimates | From 8ac95607b718d16e21d057270f974b4728c494f2 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:14 +0200 Subject: [PATCH 08/55] Return TooLarge when the WAV header size overflows u64 (#467) --- crates/audio-io/src/wav.rs | 5 ++++- crates/audio-io/tests/roundtrip.rs | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/audio-io/src/wav.rs b/crates/audio-io/src/wav.rs index 1aa847d4..7dcd36b8 100644 --- a/crates/audio-io/src/wav.rs +++ b/crates/audio-io/src/wav.rs @@ -442,7 +442,10 @@ pub(crate) fn header( if is_float { push_chunk(&mut meta, b"fact", &u32::try_from(frames).unwrap_or(u32::MAX).to_le_bytes()); } - let total = (4 + (8 + rf64_body_len) + meta.len() + 8) as u64 + data_len + (data_len & 1); + let total = ((4 + (8 + rf64_body_len) + meta.len() + 8) as u64) + .checked_add(data_len) + .and_then(|t| t.checked_add(data_len & 1)) + .ok_or_else(|| AudioError::TooLarge("WAV size overflow".into()))?; let rf64 = force_rf64 || u32::try_from(total).is_err(); if rf64 { diff --git a/crates/audio-io/tests/roundtrip.rs b/crates/audio-io/tests/roundtrip.rs index a53c272a..10ba2ecf 100644 --- a/crates/audio-io/tests/roundtrip.rs +++ b/crates/audio-io/tests/roundtrip.rs @@ -284,3 +284,10 @@ fn float_wav_header_is_rewritten_in_place_as_the_file_grows() { assert_eq!((&big[..4], big.len()), (&b"RF64"[..], len)); assert!(wav_float_header(0, 48_000, 0).is_err()); } + +#[test] +fn float_wav_header_rejects_a_frame_count_whose_total_size_overflows() { + // 4 bytes per frame: the data length fits in u64, but adding the header to it does not. + let frames = u64::MAX / 4; + assert!(matches!(wav_float_header(1, 48_000, frames), Err(soundcraft_audio_io::AudioError::TooLarge(_)))); +} From edd2b0ef73b6e9472db6e106ef6993814679a591 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:22 +0200 Subject: [PATCH 09/55] Keep audio-io stream test limits as u64 so it compiles on 32-bit targets (#462) --- crates/audio-io/tests/stream.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/audio-io/tests/stream.rs b/crates/audio-io/tests/stream.rs index 61a28946..7b149ca6 100644 --- a/crates/audio-io/tests/stream.rs +++ b/crates/audio-io/tests/stream.rs @@ -52,13 +52,15 @@ fn streamed_files_match_whole_buffer_encodes() { #[test] fn blocks_past_a_format_limit_are_refused_before_anything_is_written() { let buf = tone(2, 100); - for (format, limit) in [(FileFormat::Aiff, 1usize << 30), (FileFormat::Flac, 1 << 36)] { + for (format, limit) in [(FileFormat::Aiff, 1u64 << 30), (FileFormat::Flac, 1u64 << 36)] { let opts = EncodeOptions { format, bit_depth: BitDepth::Int16, dither: false, bwf: None }; let mut enc = FileEncoder::new(Cursor::new(Vec::new()), &opts, 2, buf.sample_rate, None).unwrap(); enc.write(&buf.channels, 100).unwrap(); - assert!(enc.check_room(limit as u64 / 2).is_ok(), "{format:?}"); - assert!(matches!(enc.check_room(limit as u64), Err(AudioError::TooLarge(_))), "{format:?}"); - assert!(matches!(enc.write(&[], limit), Err(AudioError::TooLarge(_))), "{format:?}"); + assert!(enc.check_room(limit / 2).is_ok(), "{format:?}"); + assert!(matches!(enc.check_room(limit), Err(AudioError::TooLarge(_))), "{format:?}"); + if let Ok(frames) = usize::try_from(limit) { + assert!(matches!(enc.write(&[], frames), Err(AudioError::TooLarge(_))), "{format:?}"); + } assert_eq!(enc.finish().unwrap().into_inner(), encode(&buf, &opts).unwrap()); } } From 9209909ff78da930fa8d934315f3c13460379bbb Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:15 +0200 Subject: [PATCH 10/55] Count the partial quartet in the base64 decoded-size cap (#468) --- crates/model/src/b64.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/model/src/b64.rs b/crates/model/src/b64.rs index 8288c466..1435d313 100644 --- a/crates/model/src/b64.rs +++ b/crates/model/src/b64.rs @@ -37,15 +37,19 @@ fn value(c: u8) -> Option { /// Decodes padded (or unpadded) standard base64. `None` for malformed input or a result larger /// than [`MAX_DECODED_BYTES`]. pub fn decode(text: &str) -> Option> { + if text.bytes().filter(|c| !c.is_ascii_whitespace()).count() > MAX_DECODED_BYTES.div_ceil(3).saturating_mul(4) { + return None; + } let syms: Vec = text.bytes().filter(|c| !c.is_ascii_whitespace()).collect(); let body = syms.strip_suffix(b"==").or_else(|| syms.strip_suffix(b"=")).unwrap_or(&syms); if body.contains(&b'=') || (syms.len() != body.len() && !syms.len().is_multiple_of(4)) || body.len() % 4 == 1 { return None; } - if body.len() / 4 * 3 > MAX_DECODED_BYTES { + let decoded_len = body.len() / 4 * 3 + body.len() % 4 * 3 / 4; + if decoded_len > MAX_DECODED_BYTES { return None; } - let mut out = Vec::with_capacity(body.len() / 4 * 3 + 2); + let mut out = Vec::with_capacity(decoded_len); for chunk in body.chunks(4) { let mut n = 0u32; for (i, &c) in chunk.iter().enumerate() { @@ -81,4 +85,17 @@ mod tests { assert!(decode(bad).is_none(), "{bad}"); } } + + #[test] + fn size_cap_counts_the_partial_quartet() { + for (extra, ok) in [(-1, true), (0, true), (1, false), (2, false)] { + let data = vec![7u8; MAX_DECODED_BYTES.saturating_add_signed(extra)]; + let padded = encode(&data); + let unpadded = padded.trim_end_matches('='); + assert_eq!(decode(&padded).is_some(), ok, "padded, {extra}"); + assert_eq!(decode(unpadded).is_some(), ok, "unpadded, {extra}"); + let spaced = format!("{}\n {}", &padded[..8], &padded[8..]); + assert_eq!(decode(&spaced).is_some(), ok, "whitespace, {extra}"); + } + } } From 7bcb4e48026e0aa3a41101b89233cdf7417c4404 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:27 +0200 Subject: [PATCH 11/55] Do not retry remote-control commands that may already have run (#458) --- crates/automation/src/lib.rs | 74 ++++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 12 deletions(-) diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 17c3a5bc..1172f7be 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -106,37 +106,48 @@ impl Remote { Ok(()) } - fn roundtrip(&mut self, method: &str, params: &Value) -> Result { - self.connect()?; + /// One request/reply exchange. On failure the flag says whether the request line was already + /// written to the socket (so the app may have executed it). + fn roundtrip(&mut self, method: &str, params: &Value) -> Result { + self.connect().map_err(|e| (e, false))?; let id = self.next_id; self.next_id += 1; let line = json!({"id": id, "method": method, "params": params}).to_string(); - let (w, r) = self.conn.as_mut().ok_or("not connected")?; - writeln!(w, "{line}").map_err(|e| e.to_string())?; + let (w, r) = self.conn.as_mut().ok_or_else(|| ("not connected".to_string(), false))?; + writeln!(w, "{line}").map_err(|e| (e.to_string(), false))?; let mut buf = String::new(); - r.read_line(&mut buf).map_err(|e| e.to_string())?; + r.read_line(&mut buf).map_err(|e| (e.to_string(), true))?; if buf.is_empty() { - return Err("connection closed".into()); + return Err(("connection closed".into(), true)); } - let v: Value = serde_json::from_str(&buf).map_err(|e| e.to_string())?; + let v: Value = serde_json::from_str(&buf).map_err(|e| (e.to_string(), true))?; if v.get("ok").and_then(Value::as_bool).unwrap_or(false) { Ok(v.get("result").cloned().unwrap_or(Value::Null)) } else { - Err(v.get("error").and_then(Value::as_str).unwrap_or("error").to_string()) + Err((v.get("error").and_then(Value::as_str).unwrap_or("error").to_string(), true)) } } } +/// Methods that only read state, so repeating them after a lost reply is harmless. +fn is_read_only(method: &str) -> bool { + matches!(method, "session.inspect" | "document.inspect" | "engine.commands" | "engine.parity" | "ui.inspect" | "ui.menu.list") +} + impl Backend for Remote { fn call(&mut self, method: &str, params: Value) -> Result { match self.roundtrip(method, ¶ms) { Ok(v) => Ok(v), - Err(e) if e.contains("closed") || e.contains("Broken pipe") || e.contains("reset") => { - // Retry once on a fresh connection. + Err((e, sent)) if e.contains("closed") || e.contains("Broken pipe") || e.contains("reset") => { self.conn = None; - self.roundtrip(method, ¶ms) + if sent && !is_read_only(method) { + // The app may already have executed the request; repeating it could apply it twice. + return Err(format!("{e}; `{method}` may or may not have been executed, check the session before repeating it")); + } + // Retry once on a fresh connection. + self.roundtrip(method, ¶ms).map_err(|(e, _)| e) } - Err(e) => Err(e), + Err((e, _)) => Err(e), } } fn has_ui(&self) -> bool { @@ -167,4 +178,43 @@ mod tests { let mut r = Remote::new("127.0.0.1:1"); assert!(r.call("session.inspect", json!({})).is_err()); } + + /// Fake app: accepts one connection per entry in `replies`, counts the request line it reads, + /// then either drops the connection (`false`) or answers ok (`true`). + fn fake_app(replies: Vec) -> (String, Arc) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let seen = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let counter = seen.clone(); + std::thread::spawn(move || { + for answer in replies { + let Ok((mut sock, _)) = listener.accept() else { return }; + let mut line = String::new(); + if BufReader::new(sock.try_clone().unwrap()).read_line(&mut line).unwrap_or(0) > 0 { + counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + if answer { + let _ = writeln!(sock, "{}", json!({"id": 1, "ok": true, "result": {"done": true}})); + } + } + } + }); + (addr, seen) + } + + #[test] + fn remote_does_not_repeat_a_mutation_after_a_lost_reply() { + let (addr, seen) = fake_app(vec![false, true]); + let mut r = Remote::new(&addr); + let err = r.call("engine.execute", json!({"command": "track.new"})).unwrap_err(); + assert!(err.contains("may or may not have been executed"), "{err}"); + assert_eq!(seen.load(std::sync::atomic::Ordering::SeqCst), 1); + } + + #[test] + fn remote_retries_a_read_after_a_lost_reply() { + let (addr, seen) = fake_app(vec![false, true]); + let mut r = Remote::new(&addr); + assert!(r.call("session.inspect", json!({})).is_ok()); + assert_eq!(seen.load(std::sync::atomic::Ordering::SeqCst), 2); + } } From 807230167d1fa218590f98e93356f6ac0cf88881 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:26 +0200 Subject: [PATCH 12/55] Reset clip stretch after AudioSuite bakes it into the new source (#451) --- crates/engine/src/cmd/audiosuite.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/engine/src/cmd/audiosuite.rs b/crates/engine/src/cmd/audiosuite.rs index 585f2bec..911ccca5 100644 --- a/crates/engine/src/cmd/audiosuite.rs +++ b/crates/engine/src/cmd/audiosuite.rs @@ -117,6 +117,8 @@ fn process(e: &mut Engine, p: &Value) -> Result { nc.name = new_name; nc.content = ClipContent::Audio { source: src, offset: 0 }; nc.length = i64::try_from(frames).unwrap_or(nc.length); + // The render already has the clip's stretch applied; keeping it would stretch the new file again. + nc.stretch = 1.0; // Invert and Duplicate are phase/copy operations. The clip's gain stays a clip gain, // the same way its fades stay editable. Other processes bake a new file at unity gain. if !matches!(name.as_str(), "invert" | "duplicate") { @@ -216,6 +218,21 @@ mod tests { assert!((sample - 0.5).abs() < 1e-3, "duplicate keeps the file level and the clip gain, got {sample}"); } + #[test] + fn processed_clip_does_not_keep_the_stretch_it_baked() { + for stretch in [0.5, 2.0] { + for process in ["invert", "duplicate"] { + let (mut e, id) = gained_clip(); + e.session_mut().find_clip_mut(id).unwrap().stretch = stretch; + e.execute("audiosuite.process", &json!({"process": process, "clips": [id.0]})).unwrap(); + let c = e.session().tracks[0].clips()[0].clone(); + let frames = e.session().pool.get(c.source().unwrap()).unwrap().buffer.channels[0].len(); + assert_eq!(c.stretch, 1.0, "{process} at {stretch}"); + assert_eq!(c.length, i64::try_from(frames).unwrap(), "{process} at {stretch}"); + } + } + } + #[test] fn signal_generator_fills_the_selection_with_a_clip() { let (mut e, _) = gained_clip(); From 6c84416c3cffb72d8d350ddfefc4fc9ee954ca1a Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:23 +0200 Subject: [PATCH 13/55] Reject invalid numeric prereleases and restore Cargo.toml when version update fails (#471) --- xtask/src/version.rs | 75 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 13 deletions(-) diff --git a/xtask/src/version.rs b/xtask/src/version.rs index d9bb7aa5..d469c2e9 100644 --- a/xtask/src/version.rs +++ b/xtask/src/version.rs @@ -46,11 +46,20 @@ pub fn validate(v: &str) -> Result<(), String> { None => (v, None), }; let nums: Vec<&str> = core.split('.').collect(); - if nums.len() != 3 || nums.iter().any(|n| n.is_empty() || !n.bytes().all(|b| b.is_ascii_digit()) || (n.len() > 1 && n.starts_with('0'))) { + // Numeric parts must fit in u64, like the semver crate Cargo uses. + if nums.len() != 3 + || nums.iter().any(|n| !n.bytes().all(|b| b.is_ascii_digit()) || (n.len() > 1 && n.starts_with('0')) || n.parse::().is_err()) + { return bad(); } + // A numeric prerelease identifier has no leading zero (any length is fine); alphanumeric ones may start with `0`. + let bad_id = |id: &str| { + id.is_empty() + || !id.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') + || (id.len() > 1 && id.starts_with('0') && id.bytes().all(|b| b.is_ascii_digit())) + }; if let Some(p) = pre - && (p.is_empty() || p.split('.').any(|id| id.is_empty() || !id.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-'))) + && (p.is_empty() || p.split('.').any(bad_id)) { return bad(); } @@ -91,6 +100,31 @@ fn cargo_update(root: &Path, offline: bool) -> Result<(), String> { if st.success() { Ok(()) } else { Err(format!("cargo update --workspace{} failed", if offline { " --offline" } else { "" })) } } +/// Write atomically: a half-written root manifest breaks every build in the workspace. +fn write_manifest(root: &Path, path: &Path, content: &str) -> Result<(), String> { + let dir = root.join("target"); + std::fs::create_dir_all(&dir).map_err(|e| format!("create {}: {e}", dir.display()))?; + let tmp = dir.join("Cargo.toml.xtask-version"); + std::fs::write(&tmp, content).map_err(|e| format!("write {}: {e}", tmp.display()))?; + std::fs::rename(&tmp, path).map_err(|e| format!("replace {}: {e}", path.display())) +} + +/// Writes version `new` into the manifest at `path` (current text `text`), then runs `update`. +/// If `update` fails the old manifest is put back. Returns the old version. +fn set(root: &Path, path: &Path, text: &str, new: &str, update: impl FnOnce() -> Result<(), String>) -> Result { + let old = read(text)?; + let updated = replace(text, new)?; + write_manifest(root, path, &updated)?; + if let Err(e) = update() { + // Put the old manifest back so a failed update does not leave the workspace changed. + return Err(match write_manifest(root, path, text) { + Ok(()) => format!("{e}; Cargo.toml restored to {old}"), + Err(r) => format!("{e}; could not restore Cargo.toml: {r}"), + }); + } + Ok(old) +} + /// `cargo xtask version [set X.Y.Z[-pre]]`; `args` are the words after `version`. pub fn run(args: &[String]) -> Result<(), String> { let root = root(); @@ -103,18 +137,8 @@ pub fn run(args: &[String]) -> Result<(), String> { Ok(()) } ["set", new] => { - let old = read(&text)?; - let updated = replace(&text, new)?; - // Write atomically: a half-written root manifest breaks every build in the workspace. - let dir = root.join("target"); - std::fs::create_dir_all(&dir).map_err(|e| format!("create {}: {e}", dir.display()))?; - let tmp = dir.join("Cargo.toml.xtask-version"); - std::fs::write(&tmp, updated).map_err(|e| format!("write {}: {e}", tmp.display()))?; - std::fs::rename(&tmp, &path).map_err(|e| format!("replace {}: {e}", path.display()))?; // Refresh the workspace members' entries in Cargo.lock (no dependency upgrades). - if cargo_update(&root, true).is_err() { - cargo_update(&root, false)?; - } + let old = set(&root, &path, &text, new, || cargo_update(&root, true).or_else(|_| cargo_update(&root, false)))?; println!("version: {old} -> {new}"); Ok(()) } @@ -160,6 +184,31 @@ mod tests { assert!(replace(MANIFEST, "nope").is_err()); } + #[test] + fn rejects_what_cargo_rejects() { + for bad in ["1.0.0-01", "1.0.0-rc.01", "1.0.0-0.00", "1.0.0-rc.1.007", "99999999999999999999.0.0", "1.18446744073709551616.0"] { + assert!(validate(bad).is_err(), "{bad}"); + assert!(replace(MANIFEST, bad).is_err(), "{bad}"); + } + for ok in ["1.0.0-0", "1.0.0-rc.0", "1.0.0-0a", "1.0.0-01a", "1.0.0-99999999999999999999", "18446744073709551615.0.0"] { + assert!(validate(ok).is_ok(), "{ok}"); + } + } + + #[test] + fn a_failed_update_restores_the_manifest() { + let root = std::env::temp_dir().join(format!("xtask-version-restore-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let path = root.join("Cargo.toml"); + std::fs::write(&path, MANIFEST).unwrap(); + let err = set(&root, &path, MANIFEST, "2.0.0", || Err("cargo update failed".into())).unwrap_err(); + assert!(err.contains("restored to 0.1.0"), "{err}"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), MANIFEST); + assert_eq!(set(&root, &path, MANIFEST, "2.0.0", || Ok(())).unwrap(), "0.1.0"); + assert_eq!(read(&std::fs::read_to_string(&path).unwrap()).unwrap(), "2.0.0"); + std::fs::remove_dir_all(&root).unwrap(); + } + #[test] fn the_real_manifest_has_a_valid_version() { let text = std::fs::read_to_string(root().join("Cargo.toml")).unwrap(); From 2a43710e526570c6e459dbe2554c90f78ac8349d Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:51:48 +0200 Subject: [PATCH 14/55] Scale fades, sync point and gain envelope when importing a clip group at another sample rate (#453) --- crates/engine/src/clip_group_file.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index e20caf0c..ee1f3764 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -160,6 +160,10 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - c.id = s.new_clip_id(); c.start = at.saturating_add(scale(c.start.max(0))); c.length = scale(c.length); + c.fade_in.len = scale(c.fade_in.len); + c.fade_out.len = scale(c.fade_out.len); + c.sync_point = scale(c.sync_point); + c.gain_env.iter_mut().for_each(|(at, _)| *at = scale(*at)); c.group = Some(group_id); c.sanitize(); placed.push(c.id); @@ -229,6 +233,28 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn fades_sync_point_and_envelope_follow_the_sample_rate() { + let dir = std::env::temp_dir().join(format!("sc-grp-rate-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + let p = dir.join("rate.scgrp"); + let mut e = crate::demo::demo_engine(); + // The file was saved at half the session rate, so every position doubles. + let src_rate = e.session().sample_rate.hz() / 2; + let fade = |len: i64| json!({"len": len, "shape": "Linear"}); + let clip = json!({ + "id": 1, "name": "m", "start": 0, "length": 10_000, "content": {"Midi": {"sequence": {"notes": [], "ctrls": []}}}, + "fade_in": fade(1000), "fade_out": fade(2000), "sync_point": 500, "gain_env": [[100, -6.0], [4000, 0.0]] + }); + let file = json!({"format": "soundcraft.clipgroup", "version": 1, "sample_rate": src_rate, "length": 10_000, "lanes": [[clip]], "audio": {}}); + std::fs::write(&p, file.to_string()).unwrap(); + let r = e.execute("file.import_clip_groups", &json!({"path": p.to_string_lossy()})).unwrap(); + let c = e.session().find_clip(soundcraft_model::ClipId(r["clips"][0].as_u64().unwrap())).unwrap().1; + assert_eq!((c.length, c.fade_in.len, c.fade_out.len, c.sync_point), (20_000, 2000, 4000, 1000)); + assert_eq!(c.gain_env.iter().map(|&(at, _)| at).collect::>(), vec![200, 8000]); + 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())); From 75eb152f03ddbd03249ca81cacc23b06f742a760 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:37 +0200 Subject: [PATCH 15/55] Create session backups exclusively and stop when backup numbers run out (#426) --- crates/engine/src/io.rs | 62 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..aee56ad1 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1010,6 +1010,18 @@ fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> { f.sync_all() } +/// Copies `from` to `to`, which must not exist yet; an existing entry, even a link, is never followed. +fn copy_new(from: &Path, to: &Path) -> std::io::Result<()> { + let mut src = std::fs::File::open(from)?; + let mut dst = std::fs::OpenOptions::new().write(true).create_new(true).open(to)?; + if let Err(err) = std::io::copy(&mut src, &mut dst) { + drop(dst); + let _ = std::fs::remove_file(to); + return Err(err); + } + Ok(()) +} + /// Copy the session file about to be replaced to `Backups/ backup .`, keeping the /// newest ten. fn back_up(file: &Path) { @@ -1030,9 +1042,16 @@ fn back_up(file: &Path) { .flatten() .filter_map(|f| f.file_name().to_str()?.strip_prefix(&prefix)?.strip_suffix(&suffix)?.parse().ok()) .collect(); - let next = numbers.iter().max().map_or(1, |n| n.saturating_add(1)); - let copied = std::fs::create_dir_all(&backups).and_then(|()| std::fs::copy(file, backups.join(format!("{prefix}{next}{suffix}")))); - if let Err(err) = copied { + let Some(next) = numbers.iter().max().map_or(Some(1), |n| n.checked_add(1)) else { + log::warn!("no backup of {}: backup numbers are exhausted", file.display()); + return; + }; + let dest = backups.join(format!("{prefix}{next}{suffix}")); + if let Err(err) = crate::file_access::check_write(&dest) { + log::warn!("no backup of {}: {err}", file.display()); + return; + } + if let Err(err) = std::fs::create_dir_all(&backups).and_then(|()| copy_new(file, &dest)) { log::warn!("no backup of {}: {err}", file.display()); return; } @@ -1648,6 +1667,43 @@ mod save_tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn backup_never_overwrites_an_existing_entry_when_numbers_run_out() { + let dir = std::env::temp_dir().join(format!("soundcraft-backups-full-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Backups")).unwrap(); + let path = dir.join("Song.scraft"); + std::fs::write(&path, b"session").unwrap(); + let last = dir.join(format!("Backups/Song backup {}.scraft", u32::MAX)); + std::fs::write(&last, b"sentinel").unwrap(); + back_up(&path); + assert_eq!(std::fs::read(&last).unwrap(), b"sentinel"); + let dest = dir.join("Backups/Song backup 1.scraft"); + std::fs::write(&dest, b"taken").unwrap(); + assert!(copy_new(&path, &dest).is_err()); + assert_eq!(std::fs::read(&dest).unwrap(), b"taken"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn backup_never_writes_through_a_link_to_an_outside_file() { + let dir = std::env::temp_dir().join(format!("soundcraft-backups-link-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Project/Backups")).unwrap(); + let outside = dir.join("outside.txt"); + std::fs::write(&outside, b"sentinel").unwrap(); + let path = dir.join("Project/Song.scraft"); + std::fs::write(&path, b"session").unwrap(); + std::os::unix::fs::symlink(&outside, dir.join(format!("Project/Backups/Song backup {}.scraft", u32::MAX))).unwrap(); + back_up(&path); + let link = dir.join("Project/Backups/Song backup 1.scraft"); + std::os::unix::fs::symlink(&outside, &link).unwrap(); + assert!(copy_new(&path, &link).is_err()); + assert_eq!(std::fs::read(&outside).unwrap(), b"sentinel"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn save_gives_sources_that_share_a_path_their_own_files() { let mut s = Session::default(); From 9afb266fcee3505a931194ba5539a249f545b95f Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:42 +0200 Subject: [PATCH 16/55] Fix bundle opening losing audio for sources that share one file (#446) --- crates/engine/src/io.rs | 55 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..29198daa 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1248,14 +1248,32 @@ pub fn open_session_bytes(e: &mut Engine, name: &str, bytes: &[u8]) -> Result>> = std::collections::HashMap::new(); for src in &s.sources { - match files.remove(&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()), + if let Some(audio) = decoded.get(&src.path) { + if let Some(audio) = audio { + s.pool.insert(src.id, Arc::clone(audio)); + } + continue; } + let audio = match files.remove(&src.path) { + Some(bytes) => match decode_source(&bytes, Path::new(&src.path).extension().and_then(|x| x.to_str()), s.sample_rate.hz()) { + Ok(audio) => Some(audio), + Err(err) => { + missing.push(format!("{}: {err}", src.path)); + None + } + }, + None => { + missing.push(src.path.clone()); + None + } + }; + if let Some(audio) = &audio { + s.pool.insert(src.id, Arc::clone(audio)); + } + decoded.insert(src.path.clone(), audio); } // Movies are referenced by path, never copied into a session: none can be found here. missing.extend(s.videos.iter().map(|v| v.path.clone())); @@ -1886,6 +1904,31 @@ mod bundle_tests { assert_eq!(r.path.as_deref(), Some("Midnight Groove.scraftz")); } + #[test] + fn sources_sharing_one_file_all_load_from_a_bundle() { + use std::io::Write as _; + let e = crate::demo::demo_engine(); + let (bytes, _) = session_bundle(e.session()).unwrap(); + let (text, files) = read_bundle(&bytes).unwrap(); + let mut s = Session::from_json(&text).unwrap(); + s.sources[1].path = s.sources[0].path.clone(); + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + for path in s.sources.iter().map(|x| &x.path).collect::>() { + w.start_file(path.as_str(), opts).unwrap(); + w.write_all(&files[path]).unwrap(); + } + w.start_file("Song.scraft", opts).unwrap(); + w.write_all(s.to_json().unwrap().as_bytes()).unwrap(); + let bundle = w.finish().unwrap().into_inner(); + let mut r = Engine::default(); + let missing = open_session_bytes(&mut r, "Song.scraftz", &bundle).unwrap(); + assert!(missing.is_empty(), "{missing:?}"); + for src in &r.session().sources { + assert!(r.session().pool.get(src.id).is_some(), "{} has no audio", src.name); + } + } + #[test] fn same_named_sources_get_distinct_files() { let mut e = crate::demo::demo_engine(); From 33d59b26a644a33b7fbe21afb24fd14b99d7913b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:51:12 +0200 Subject: [PATCH 17/55] Keep controller-only tracks when importing MIDI (#454) --- crates/engine/src/io.rs | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..d07f3eea 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -278,17 +278,19 @@ pub fn import_midi_bytes(e: &mut Engine, bytes: &[u8], at: Samples, tempo_map: b let base_tick = s.tempo.samples_to_ticks(at, s.sample_rate); let mut out = Vec::new(); for (i, tr) in smf.tracks.iter().enumerate() { - if tr.sequence.notes.is_empty() { + if tr.sequence.notes.is_empty() && tr.sequence.ctrls.is_empty() { continue; } 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); + let drums = !tr.sequence.notes.is_empty() && tr.sequence.notes.iter().all(|n| n.channel == 9); 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.saturating_add(tr.sequence.end_tick().max(1)); + // A controller on the clip's last tick would not play, so the clip runs one tick past the last one. + let past_ctrls = tr.sequence.ctrls.iter().map(|c| c.tick.saturating_add(1)).max().unwrap_or(0); + let end_tick = base_tick.saturating_add(tr.sequence.end_tick().max(past_ctrls).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))); @@ -1748,6 +1750,28 @@ 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 a_controller_only_midi_track_is_imported_with_its_events() { + let ctrls = vec![ + soundcraft_midi::CtrlEvent { tick: 0, channel: 0, kind: soundcraft_midi::CtrlKind::Cc(7), value: 100 }, + soundcraft_midi::CtrlEvent { tick: 480, channel: 0, kind: soundcraft_midi::CtrlKind::Cc(7), value: 64 }, + soundcraft_midi::CtrlEvent { tick: 960, channel: 0, kind: soundcraft_midi::CtrlKind::Cc(7), value: 20 }, + ]; + let sequence = soundcraft_midi::Sequence { ctrls: ctrls.clone(), ..Default::default() }; + let smf = soundcraft_midi::Smf { tracks: vec![soundcraft_midi::SmfTrack { name: "Vol".into(), sequence }], ..Default::default() }; + let mut e = Engine::default(); + let ids = import_midi_bytes(&mut e, &soundcraft_midi::write_smf(&smf).unwrap(), 0, false).unwrap(); + assert_eq!(ids.len(), 1); + let track = e.session().track(ids[0]).unwrap(); + assert_eq!(track.instrument.as_ref().map(|i| i.plugin.as_str()), Some("subtractive_synth")); + // The clip must reach past the last event, or playback skips it. + assert!(track.clips()[0].end() > e.session().tempo.tick_to_samples(960, e.session().sample_rate)); + match &track.clips()[0].content { + ClipContent::Midi { sequence } => assert_eq!(sequence.ctrls, ctrls), + _ => panic!("expected a MIDI clip"), + } + } + #[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 }; From 52dc4f9822775b1f34e5e4ad23d3813120d6c20c Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:50 +0200 Subject: [PATCH 18/55] Reset clip effect EQ/compressor state on stop and seek (#442) --- crates/mix/src/lib.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index e8c4fbad..3d78bde5 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -515,6 +515,9 @@ impl MixEngine { s.align.clear(); s.align_pre.clear(); s.out_extra.clear(); + for fx in s.clip_fx.values_mut() { + fx.reset(); + } } self.meters.clear(); self.main_meter = StripMeter::default(); @@ -1149,6 +1152,12 @@ impl ClipFx { fn new(sr: f32, ch: usize) -> ClipFx { ClipFx { eq: None, comp: None, gain_db: 0.0, sr, ch } } + /// Clear filter, detector and lookahead history (transport stop / locate). + fn reset(&mut self) { + for p in self.eq.iter_mut().chain(self.comp.iter_mut()) { + p.reset(); + } + } fn make(&self, id: &str) -> Option> { soundcraft_dsp::create(id).map(|mut p| { p.prepare(self.sr, 16_384, self.ch.max(1)); @@ -2902,6 +2911,29 @@ mod tests { assert!(out.iter().flatten().all(|x| *x == 0.0), "old aligned audio after a reset"); } + /// A reset (stop, locate) must clear the cached clip EQ/compressor history too. + #[test] + fn reset_clears_clip_effect_history() { + // Compressor lookahead delay, then EQ high-pass filter state, each primed with DC. + for (param, value) in [("comp.lookahead", 5.0), ("eq.hpf_on", 1.0)] { + let (mut s, t) = session_with_dc(0.5, 9600); + s.edit.click = false; + let cid = s.track(t).unwrap().clips()[0].id.0; + s.edit.values.insert(format!("clip_fx.{cid}.{param}"), value); + let mut out = vec![vec![0.0f32; 512]; 2]; + let mut eng = MixEngine::new(48_000.0, 512); + for b in 0..4 { + eng.render(&s, b * 512, 512, &mut out); + } + eng.reset(); + eng.render(&s, 4096, 512, &mut out); + let mut fresh = MixEngine::new(48_000.0, 512); + let mut expect = vec![vec![0.0f32; 512]; 2]; + fresh.render(&s, 4096, 512, &mut expect); + assert_eq!(out, expect, "{param}: clip effect state survived the reset"); + } + } + fn impulse_at(ch: &[f32]) -> Vec { ch.iter().enumerate().filter(|(_, v)| v.abs() > 1e-4).map(|(i, _)| i).collect() } From 95b14c8df065d494435576941f9fed855adee091 Mon Sep 17 00:00:00 2001 From: Mikkel konyher <146730338+mikkelkonyher@users.noreply.github.com> Date: Tue, 13 Oct 2026 20:05:37 +0200 Subject: [PATCH 19/55] Open a plugin when it's inserted (#486) Picking a plugin for an insert slot added it but opened nothing, so every new plugin took an extra click on its slot. As in other DAWs, the pick now opens it: a hosted plugin's own editor, else its plugin window (built-in plugins, no audio engine, or an editor that can't open here, such as CLAP plugins with only an embedded editor on macOS). The insert menu's picks go through pick_insert, which remembers the slot when mix.insert succeeds; SoundApp::open_inserted opens it on the next logic pass, once the audio engine has the instance. Only the menu opens it, so mix.insert from the CLI, MCP or control channel still opens nothing. It covers empty and filled slots, in the Mix window and the Edit window's Inserts column. Tests: tests/open_on_insert.rs (empty slot, search and Enter, replacing, Edit window column). menu_search closes the windows the picks now open before it looks for the menu's Bypass. --- crates/ui-egui/src/lib.rs | 25 ++++++ crates/ui-egui/src/mix_window.rs | 16 +++- crates/ui-egui/tests/menu_search.rs | 3 + crates/ui-egui/tests/open_on_insert.rs | 109 +++++++++++++++++++++++++ docs/ui-parity.md | 4 +- 5 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 crates/ui-egui/tests/open_on_insert.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ddd54275..b093ec9b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -301,6 +301,8 @@ pub struct SoundApp { synthetic_grace: u32, control_rx: Option>, pending_shots: Vec, + /// An insert just picked from its menu, opened once the audio engine has its instance. + pub(crate) pending_open: Option<(TrackId, usize)>, last_rev: u64, fonts_ready: bool, fonts_installed: bool, @@ -384,6 +386,7 @@ impl SoundApp { synthetic_grace: 0, control_rx: None, pending_shots: Vec::new(), + pending_open: None, last_rev: 0, fonts_ready: false, fonts_installed: false, @@ -617,6 +620,27 @@ impl SoundApp { } } + /// Opens the insert just picked from its menu: a hosted plugin's own editor, else (a built-in + /// plugin, no audio engine, or an editor that can't open here) its plugin window. + fn open_inserted(&mut self) { + let Some((track, slot)) = self.pending_open.take() else { return }; + let Some(plugin) = self.engine.session().track(track).and_then(|t| t.mixer.inserts.get(slot).cloned().flatten()).map(|i| i.plugin) else { + return; + }; + if soundcraft_mix::is_third_party(&plugin) + && let Some(p) = &self.player + && p.has_editor(track, slot) + { + match p.open_editor(track, slot) { + Ok(()) => return, + Err(e) => self.ui.status = format!("{}: {e}", mix_window::plugin_info(&plugin).map_or(plugin.as_str(), |i| i.name)), + } + } + if !self.ui.plugin_windows.contains(&(track, slot)) { + self.ui.plugin_windows.push((track, slot)); + } + } + /// Parameter edits made in plugin editors go into the session (one undo step per gesture). fn apply_editor_edits(&mut self) { let Some(p) = &self.player else { return }; @@ -1153,6 +1177,7 @@ impl SoundApp { p.update_session(self.engine.session_arc()); } } + self.open_inserted(); self.autosave_changes(); self.write_automation(ctx); self.autosave(); diff --git a/crates/ui-egui/src/mix_window.rs b/crates/ui-egui/src/mix_window.rs index 7cc98c75..22ade9d0 100644 --- a/crates/ui-egui/src/mix_window.rs +++ b/crates/ui-egui/src/mix_window.rs @@ -615,7 +615,7 @@ pub fn plugin_menu(app: &mut SoundApp, ui: &mut Ui, id: TrackId, slot: usize, oc let search = MenuSearch::show(ui, egui::Id::new(("plugin-search", id.0, slot)), "Search plugins…"); if search.typing() { if let Some(plugin) = plugin_search(ui, &search, false) { - run_pick(app, ui, "mix.insert", json!({"track": id.0, "slot": slot, "plugin": plugin})); + pick_insert(app, ui, id, slot, &plugin); } return; } @@ -638,13 +638,13 @@ pub fn plugin_menu(app: &mut SoundApp, ui: &mut Ui, id: TrackId, slot: usize, oc ui.menu_button(format!("{c:?}"), |ui| { for p in soundcraft_dsp::plugins().iter().filter(|p| p.category == c && !p.is_instrument) { if ui.button(p.name).clicked() { - run_pick(app, ui, "mix.insert", json!({"track": id.0, "slot": slot, "plugin": p.id})); + pick_insert(app, ui, id, slot, p.id); } } }); } if let Some(plugin) = hosted_menus(ui, false) { - run_pick(app, ui, "mix.insert", json!({"track": id.0, "slot": slot, "plugin": plugin})); + pick_insert(app, ui, id, slot, &plugin); } } @@ -702,6 +702,16 @@ pub fn instrument_menu(app: &mut SoundApp, ui: &mut Ui, id: TrackId) { } } +/// Inserts the plugin picked in an insert's menu and, as DAWs do, opens it once the audio engine has +/// it (`SoundApp::open_inserted`). +fn pick_insert(app: &mut SoundApp, ui: &Ui, id: TrackId, slot: usize, plugin: &str) { + match app.run("mix.insert", json!({"track": id.0, "slot": slot, "plugin": plugin})) { + Ok(_) => app.pending_open = Some((id, slot)), + Err(e) => app.ui.status = e, + } + ui.close(); +} + /// Runs a menu's pick, shows its error in the status line and closes the menu. fn run_pick(app: &mut SoundApp, ui: &Ui, cmd: &str, params: serde_json::Value) { if let Err(e) = app.run(cmd, params) { diff --git a/crates/ui-egui/tests/menu_search.rs b/crates/ui-egui/tests/menu_search.rs index 0f2c7f53..973e6f5d 100644 --- a/crates/ui-egui/tests/menu_search.rs +++ b/crates/ui-egui/tests/menu_search.rs @@ -107,6 +107,9 @@ fn picks_without_the_search_still_close_the_menu() { assert_eq!(h.state().engine.session().tracks[0].mixer.inserts[2].as_ref().unwrap().plugin, plugin.id); assert!(h.query_by_label(&category).is_none(), "a plugin from a category closes the menu"); } + // The picks opened the plugin's window (#486), whose Bypass button would match the menu's. + h.state_mut().ui.plugin_windows.clear(); + h.run_steps(2); for (item, bypassed) in [("Bypass", Some(true)), ("no insert", None)] { h.get_by_label("Insert C").click_secondary(); diff --git a/crates/ui-egui/tests/open_on_insert.rs b/crates/ui-egui/tests/open_on_insert.rs new file mode 100644 index 00000000..c21b75cf --- /dev/null +++ b/crates/ui-egui/tests/open_on_insert.rs @@ -0,0 +1,109 @@ +//! #486: picking a plugin for an insert slot opens it straight away, as DAWs do, from the Mix +//! window's slots and the Edit window's Inserts column alike. + +use egui::{Key, vec2}; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use serde_json::json; +use soundcraft_model::TrackId; +use soundcraft_ui_egui::{MainWindow, Services, SoundApp}; + +fn harness(engine: soundcraft_engine::Engine, window: MainWindow) -> Harness<'static, SoundApp> { + let mut app = SoundApp::new(engine, None, Services::default()); + app.ui.window = window; + app.run("view.edit_inserts_ae", json!({})).unwrap(); + app.run("track.height", json!({"tracks": ["Vox"], "height": "medium"})).unwrap(); + let mut h = Harness::builder().with_size(vec2(1400.0, 1000.0)).build_ui_state( + |ui, app: &mut SoundApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + app, + ); + h.run_steps(4); + h +} + +fn vox() -> soundcraft_engine::Engine { + let mut engine = soundcraft_engine::Engine::default(); + engine.execute("track.new", &json!({"name": "Vox"})).unwrap(); + engine +} + +fn vox_id(h: &Harness) -> TrackId { + h.state().engine.session().tracks[0].id +} + +/// Two built-in effects from one category, and the category's menu label. +fn two_effects() -> (String, &'static soundcraft_dsp::PluginInfo, &'static soundcraft_dsp::PluginInfo) { + let first = soundcraft_dsp::plugins().iter().find(|p| !p.is_instrument).unwrap(); + let other = soundcraft_dsp::plugins().iter().find(|p| p.category == first.category && p.id != first.id).unwrap(); + (format!("{:?} ⏵", first.category), first, other) +} + +/// Opens `slot`'s menu (a click on an empty slot, a right-click on a filled one) and picks +/// `plugin` from its category. +fn pick(h: &mut Harness, slot: &str, filled: bool, category: &str, plugin: &str) { + let node = h.get_by_label(slot); + if filled { + node.click_secondary(); + } else { + node.click(); + } + h.run_steps(3); + h.get_by_label(category).hover(); + h.run_steps(3); + h.get_by_label(plugin).click(); + h.run_steps(3); +} + +fn inserted(h: &Harness) -> Option { + h.state().engine.session().tracks[0].mixer.inserts[0].as_ref().map(|i| i.plugin.clone()) +} + +#[test] +fn picking_a_plugin_for_an_empty_insert_opens_it() { + let (category, plugin, _) = two_effects(); + let mut h = harness(vox(), MainWindow::Mix); + pick(&mut h, "Insert A", false, &category, plugin.name); + assert_eq!(inserted(&h).as_deref(), Some(plugin.id)); + let id = vox_id(&h); + assert_eq!(h.state().ui.plugin_windows, vec![(id, 0)], "the new insert's window opened"); +} + +#[test] +fn picking_from_the_search_opens_it_too() { + let mut h = harness(vox(), MainWindow::Mix); + h.get_by_label("Insert A").click(); + h.run_steps(3); + h.event(egui::Event::Text("reverb".into())); + h.run_steps(2); + h.key_press(Key::Enter); + h.run_steps(3); + assert!(inserted(&h).is_some(), "Enter inserted a reverb"); + let id = vox_id(&h); + assert_eq!(h.state().ui.plugin_windows, vec![(id, 0)]); +} + +#[test] +fn replacing_a_filled_insert_opens_the_new_plugin() { + let (category, first, second) = two_effects(); + let mut engine = vox(); + let track = engine.session().tracks[0].id; + engine.execute("mix.insert", &json!({"track": track.0, "slot": 0, "plugin": first.id})).unwrap(); + let mut h = harness(engine, MainWindow::Mix); + assert!(h.state().ui.plugin_windows.is_empty(), "nothing opens before a pick"); + pick(&mut h, "Insert A", true, &category, second.name); + assert_eq!(inserted(&h).as_deref(), Some(second.id)); + assert_eq!(h.state().ui.plugin_windows, vec![(track, 0)]); +} + +#[test] +fn the_edit_windows_inserts_column_opens_it_too() { + let (category, plugin, _) = two_effects(); + let mut h = harness(vox(), MainWindow::Edit); + pick(&mut h, "Insert A", false, &category, plugin.name); + assert_eq!(inserted(&h).as_deref(), Some(plugin.id)); + let id = vox_id(&h); + assert_eq!(h.state().ui.plugin_windows, vec![(id, 0)]); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 7f42803c..cd40ace0 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-13 · **Change:** minor (drag across checkboxes, #399; waveforms preview fades during drag) · **Target:** Avid Pro Tools Ultimate 2026.4.1 +> **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-13 · **Change:** minor (drag across checkboxes, #399; waveforms preview fades during drag; a plugin opens when inserted, #486) · **Target:** Avid Pro Tools Ultimate 2026.4.1 How SoundCraft feels in the hands of a Pro Tools user: tools, modes, modifiers, shortcuts, windows, meters. Measured from `crates/model/src/session.rs` (tools, modes), the command @@ -51,6 +51,7 @@ Legend: ✅ matches · 🟡 partial · ❌ missing. | Preferences | 🟡 | Many settings stored, fewer honoured | | Window configurations, Tile/Cascade | 🟡 | Configurations ✅; Tile/Cascade in PR #151 | | Plugin windows | 🟡 | Generic parameter sliders; Pro Tools plugins have bespoke GUIs (#138 asks for skeuomorphic design); multi-page plugin views (#6) | +| A plugin opens when it's inserted | ✅ | #486; a hosted plugin's own editor, else its plugin window | | Command palette (Search) | ✅ | Over every command | ## Look and meters @@ -68,6 +69,7 @@ Legend: ✅ matches · 🟡 partial · ❌ missing. | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | A plugin opens when it's inserted (#486) | | 2026-10-13 | minor | Drag across checkboxes (#399) | | 2026-10-13 | minor | Waveforms and fade curves share temporary envelopes during drag, without rebuilding media or editing the document before release. Readiness estimate unchanged | | 2026-10-13 | minor | Fade curves use bounded physical-pixel sampling; midpoint handles stay visible and inside the clip, with repeat-drag coverage after extreme bends. Readiness estimate unchanged | From 90318fd6d6ac9e329c11d6dfc27191bda882d6a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:28:26 +0000 Subject: [PATCH 20/55] playback: retry the next stream config when a built stream won't start --- crates/playback/src/lib.rs | 47 +++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index b945d4b6..c16713d4 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -526,7 +526,6 @@ impl Player { let (gtx, grx) = channel(); let state = AudioState::new(rx, Arc::clone(&shared), Arc::clone(&session), f64::from(rate), gtx); let fault = Arc::new(Fault::default()); - use cpal::traits::StreamTrait; let built = match format { cpal::SampleFormat::I32 => build_output::(&device, &config, state, channels, &fault, &shared), cpal::SampleFormat::I16 => build_output::(&device, &config, state, channels, &fault, &shared), @@ -534,8 +533,9 @@ impl Player { }; match built { Ok(stream) => { - if let Err(e) = stream.play() { - log::warn!("audio stream failed to start: {e}"); + // A stream that builds but will not start is dropped, and the next config is tried. + if !start_stream(&stream) { + continue; } let buffer = match config.buffer_size { cpal::BufferSize::Fixed(n) => Some(n), @@ -862,6 +862,18 @@ fn fold_gains(s: &Session) -> [(f32, f32); MAX_CHANNELS] { g } +/// Start a freshly built output stream. `false` when the device refuses to start it, so the caller +/// treats it like a stream that failed to build and tries the next configuration. +fn start_stream(stream: &impl cpal::traits::StreamTrait) -> bool { + match stream.play() { + Ok(()) => true, + Err(e) => { + log::warn!("audio stream failed to start: {e}"); + false + } + } +} + /// The output stream: the mix renders f32 into a scratch buffer allocated here (so the callback /// never allocates), converted to the device's sample type. fn build_output( @@ -967,6 +979,35 @@ fn open_device(want_rate: u32, want_ch: u16, cfg: &AudioConfig) -> Result<(cpal: mod tests { use super::*; + /// A stream that builds but refuses to start, as a device can after a reconnect. + struct RefusesToStart; + + impl cpal::traits::StreamTrait for RefusesToStart { + fn play(&self) -> Result<(), cpal::PlayStreamError> { + Err(cpal::PlayStreamError::DeviceNotAvailable) + } + fn pause(&self) -> Result<(), cpal::PauseStreamError> { + Ok(()) + } + } + + struct Starts; + + impl cpal::traits::StreamTrait for Starts { + fn play(&self) -> Result<(), cpal::PlayStreamError> { + Ok(()) + } + fn pause(&self) -> Result<(), cpal::PauseStreamError> { + Ok(()) + } + } + + #[test] + fn a_stream_that_will_not_start_is_reported_as_failed() { + assert!(!start_stream(&RefusesToStart)); + assert!(start_stream(&Starts)); + } + #[test] fn reusable_meter_snapshot_updates_and_drops_stale_tracks() { let keep = TrackId(1); From 79adfc0b57fc423cfba3f8c9da51e6af89b08e0b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:30:29 +0000 Subject: [PATCH 21/55] playback: test the stream fallback order through first_started Build and start each attempt in one helper, so the fallback order can be tested without an audio device. The first config that builds and starts wins; a build error or a refused start moves on to the next; none gives None, which leads to the silent clock. Claude-Session: https://claude.ai/code/session_014JH3FxAq7VHhaKjG9U3vfA --- crates/playback/src/lib.rs | 181 ++++++++++++++++++++++++------------- 1 file changed, 120 insertions(+), 61 deletions(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index c16713d4..9d0d1579 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -519,54 +519,58 @@ impl Player { if config.buffer_size != cpal::BufferSize::Default { attempts.push(cpal::StreamConfig { buffer_size: cpal::BufferSize::Default, ..config }); } - for config in attempts { - let rate = config.sample_rate.0; - let channels = usize::from(config.channels); - let (tx, rx) = channel(); - let (gtx, grx) = channel(); - let state = AudioState::new(rx, Arc::clone(&shared), Arc::clone(&session), f64::from(rate), gtx); - let fault = Arc::new(Fault::default()); - let built = match format { - cpal::SampleFormat::I32 => build_output::(&device, &config, state, channels, &fault, &shared), - cpal::SampleFormat::I16 => build_output::(&device, &config, state, channels, &fault, &shared), - _ => build_output::(&device, &config, state, channels, &fault, &shared), - }; - match built { - Ok(stream) => { - // A stream that builds but will not start is dropped, and the next config is tried. - if !start_stream(&stream) { - continue; - } - let buffer = match config.buffer_size { - cpal::BufferSize::Fixed(n) => Some(n), - cpal::BufferSize::Default => None, - }; - log::info!( - "audio device: {name} @ {rate} Hz, {channels} ch, {format}, buffer {}", - buffer.map_or("default".to_string(), |n| n.to_string()) - ); - return Player { - shared, - fault, - #[cfg(not(target_arch = "wasm32"))] - watch: Mutex::new(None), - tx, - side: new_side(grx), - _stream: Some(stream), - device_name: name, - device_rate: rate, - device_channels: config.channels, - device_buffer: buffer, - silent: false, - }; + let opened = first_started( + attempts, + |config: &cpal::StreamConfig| -> Result { + let rate = config.sample_rate.0; + let channels = usize::from(config.channels); + let (tx, rx) = channel(); + let (gtx, grx) = channel(); + let state = AudioState::new(rx, Arc::clone(&shared), Arc::clone(&session), f64::from(rate), gtx); + let fault = Arc::new(Fault::default()); + let stream = match format { + cpal::SampleFormat::I32 => build_output::(&device, config, state, channels, &fault, &shared), + cpal::SampleFormat::I16 => build_output::(&device, config, state, channels, &fault, &shared), + _ => build_output::(&device, config, state, channels, &fault, &shared), + }?; + Ok(Opened { stream, tx, grx, fault }) + }, + |opened| start_stream(&opened.stream), + ); + match opened { + Some((config, Opened { stream, tx, grx, fault })) => { + let rate = config.sample_rate.0; + let channels = usize::from(config.channels); + let buffer = match config.buffer_size { + cpal::BufferSize::Fixed(n) => Some(n), + cpal::BufferSize::Default => None, + }; + log::info!( + "audio device: {name} @ {rate} Hz, {channels} ch, {format}, buffer {}", + buffer.map_or("default".to_string(), |n| n.to_string()) + ); + Player { + shared, + fault, + #[cfg(not(target_arch = "wasm32"))] + watch: Mutex::new(None), + tx, + side: new_side(grx), + _stream: Some(stream), + device_name: name, + device_rate: rate, + device_channels: config.channels, + device_buffer: buffer, + silent: false, } - Err(e) => log::warn!("cannot open audio stream ({:?}): {e}", config.buffer_size), + } + None => { + log::warn!("no usable audio stream; using a silent clock"); + let (tx, rx) = channel(); + let (gtx, grx) = channel(); + Player::silent_clock(shared, tx, rx, want, new_side(grx), gtx) } } - log::warn!("no usable audio stream; using a silent clock"); - let (tx, rx) = channel(); - let (gtx, grx) = channel(); - Player::silent_clock(shared, tx, rx, want, new_side(grx), gtx) } Err(e) => { log::warn!("no audio output: {e}; using a silent clock"); @@ -874,6 +878,32 @@ fn start_stream(stream: &impl cpal::traits::StreamTrait) -> bool { } } +/// A built output stream and the channels and fault flag its callback shares with the player. +struct Opened { + stream: cpal::Stream, + tx: Sender, + grx: Receiver>>, + fault: Arc, +} + +/// Build and start the attempts in order; the first that builds and starts wins. A build error or +/// a stream that will not start is logged, and the next attempt is tried. `None` when none works. +fn first_started( + attempts: impl IntoIterator, + mut build: impl FnMut(&A) -> Result, + start: impl Fn(&S) -> bool, +) -> Option<(A, S)> { + for attempt in attempts { + match build(&attempt) { + Ok(built) if start(&built) => return Some((attempt, built)), + // `start` has already logged why the stream would not start. + Ok(_) => {} + Err(e) => log::warn!("cannot open audio stream ({attempt:?}): {e}"), + } + } + None +} + /// The output stream: the mix renders f32 into a scratch buffer allocated here (so the callback /// never allocates), converted to the device's sample type. fn build_output( @@ -979,23 +1009,15 @@ fn open_device(want_rate: u32, want_ch: u16, cfg: &AudioConfig) -> Result<(cpal: mod tests { use super::*; - /// A stream that builds but refuses to start, as a device can after a reconnect. - struct RefusesToStart; - - impl cpal::traits::StreamTrait for RefusesToStart { - fn play(&self) -> Result<(), cpal::PlayStreamError> { - Err(cpal::PlayStreamError::DeviceNotAvailable) - } - fn pause(&self) -> Result<(), cpal::PauseStreamError> { - Ok(()) - } + /// A stream that builds; `starts` says whether the device lets it play (it can refuse after a + /// reconnect). + struct Fake { + starts: bool, } - struct Starts; - - impl cpal::traits::StreamTrait for Starts { + impl cpal::traits::StreamTrait for Fake { fn play(&self) -> Result<(), cpal::PlayStreamError> { - Ok(()) + if self.starts { Ok(()) } else { Err(cpal::PlayStreamError::DeviceNotAvailable) } } fn pause(&self) -> Result<(), cpal::PauseStreamError> { Ok(()) @@ -1004,8 +1026,45 @@ mod tests { #[test] fn a_stream_that_will_not_start_is_reported_as_failed() { - assert!(!start_stream(&RefusesToStart)); - assert!(start_stream(&Starts)); + assert!(!start_stream(&Fake { starts: false })); + assert!(start_stream(&Fake { starts: true })); + } + + #[test] + fn the_first_config_that_starts_wins_and_later_ones_are_not_tried() { + let mut tried = Vec::new(); + let won = first_started( + [1u32, 2, 3], + |&n| { + tried.push(n); + Ok::<_, String>(Fake { starts: n != 1 }) + }, + start_stream, + ); + assert_eq!(won.map(|(n, _)| n), Some(2)); + assert_eq!(tried, vec![1, 2]); + } + + #[test] + fn a_config_that_fails_to_build_falls_through_to_the_next() { + let won = + first_started([1u32, 2], |&n| if n == 1 { Err("no such buffer size".to_string()) } else { Ok(Fake { starts: true }) }, start_stream); + assert_eq!(won.map(|(n, _)| n), Some(2)); + } + + #[test] + fn no_config_that_starts_yields_none_so_the_caller_falls_back_to_a_silent_clock() { + let mut tried = 0; + let won = first_started( + [1u32, 2], + |_| { + tried += 1; + Ok::<_, String>(Fake { starts: false }) + }, + start_stream, + ); + assert!(won.is_none()); + assert_eq!(tried, 2); } #[test] From 750db81371d175bfffb92aad554b3f7dba33e704 Mon Sep 17 00:00:00 2001 From: Saad Abdullah Date: Tue, 13 Oct 2026 22:04:57 +0000 Subject: [PATCH 22/55] fix(audio-io): poison stream encoders after a failed write so later calls cannot claim success (#464) --- crates/audio-io/src/aiff.rs | 16 ++++++++++-- crates/audio-io/src/flac.rs | 18 +++++++++++-- crates/audio-io/src/lib.rs | 11 +++++++- crates/audio-io/src/wav.rs | 29 +++++++++++++++++++-- crates/audio-io/tests/stream.rs | 46 ++++++++++++++++++++++++++++++++- 5 files changed, 112 insertions(+), 8 deletions(-) diff --git a/crates/audio-io/src/aiff.rs b/crates/audio-io/src/aiff.rs index d8462c0e..77ed6f80 100644 --- a/crates/audio-io/src/aiff.rs +++ b/crates/audio-io/src/aiff.rs @@ -185,6 +185,9 @@ pub(crate) struct Writer { quantizers: Vec, frames: u64, bytes: Vec, + /// Set when a block write has failed: the sink may hold a prefix of that block while + /// `frames` does not count it, so the writer refuses every later call. + failed: bool, } impl Writer { @@ -193,7 +196,7 @@ impl Writer { let start = out.stream_position()?; out.write_all(&header(channels, sample_rate, depth, 0)?)?; let quantizers = Quantizer::per_channel(u32::from(sample_format(depth).0), dither, channels); - Ok(Writer { out, start, channels, sample_rate, depth, quantizers, frames: 0, bytes: Vec::new() }) + Ok(Writer { out, start, channels, sample_rate, depth, quantizers, frames: 0, bytes: Vec::new(), failed: false }) } /// The frame count after `frames` more, or an error when AIFF can't hold that many (4 GiB). @@ -206,13 +209,22 @@ impl Writer { /// Appends `frames` frames of planar audio (missing samples are silence), unless they don't /// fit (see [`Writer::check_room`]). pub(crate) fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { + if self.failed { + return Err(crate::failed_write_error()); + } let total = self.check_room(frames as u64)?; - write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, sample_format(self.depth), true)?; + if let Err(e) = write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, sample_format(self.depth), true) { + self.failed = true; + return Err(AudioError::from(e)); + } self.frames = total; Ok(()) } pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(crate::failed_write_error()); + } let data_len = u64::from(sample_format(self.depth).0 / 8).saturating_mul(self.channels as u64).saturating_mul(self.frames); if data_len % 2 == 1 { self.out.write_all(&[0])?; diff --git a/crates/audio-io/src/flac.rs b/crates/audio-io/src/flac.rs index 8a8c0ce1..b119f09d 100644 --- a/crates/audio-io/src/flac.rs +++ b/crates/audio-io/src/flac.rs @@ -318,6 +318,10 @@ pub(crate) struct Writer { pending: Vec>, frames: u64, frame_no: u64, + /// Set when a frame write has failed: `write_frame` clears the pending samples before + /// writing, so a failed frame is lost while `frames` still counts it; the writer refuses + /// every later call. + failed: bool, } impl Writer { @@ -336,6 +340,7 @@ impl Writer { pending: (0..channels).map(|_| Vec::with_capacity(BLOCK_SIZE)).collect(), frames: 0, frame_no: 0, + failed: false, }) } @@ -351,6 +356,9 @@ impl Writer { /// Appends `frames` frames of planar audio (missing samples are silence), unless they don't /// fit (see [`Writer::check_room`]). pub(crate) fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { + if self.failed { + return Err(crate::failed_write_error()); + } let total = self.check_room(frames as u64)?; let mut done = 0; while done < frames { @@ -361,8 +369,11 @@ impl Writer { p.extend((0..n).map(|i| i64::from(q.quantize(src.get(i).copied().unwrap_or(0.0))))); } done += n; - if held + n == BLOCK_SIZE { - self.write_frame()?; + if held + n == BLOCK_SIZE + && let Err(e) = self.write_frame() + { + self.failed = true; + return Err(e); } } self.frames = total; @@ -408,6 +419,9 @@ impl Writer { } pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(crate::failed_write_error()); + } self.write_frame()?; let info = stream_info(self.channels, self.sample_rate, self.bps, self.frames); self.out.seek(SeekFrom::Start(self.start.saturating_add(4)))?; diff --git a/crates/audio-io/src/lib.rs b/crates/audio-io/src/lib.rs index 1b2b43f5..4a871946 100644 --- a/crates/audio-io/src/lib.rs +++ b/crates/audio-io/src/lib.rs @@ -294,6 +294,13 @@ pub fn encode_with_channel_mask(buf: &AudioBuffer, opts: &EncodeOptions, mask: u } } +/// The error a stream encoder returns for every call after one of its writes has failed: the +/// sink may already hold a prefix of the failed block, so the encoder's counters can no longer +/// describe the bytes on the sink, and later writes or `finish` must not claim success. +pub(crate) fn failed_write_error() -> AudioError { + AudioError::Encode("the stream encoder already failed on an earlier write; its output is incomplete".into()) +} + /// Writes WAV/BWF (RF64 above 4 GiB), AIFF / AIFF-C `fl32` or FLAC block by block, producing /// the same bytes [`encode_with_channel_mask`] would for the whole buffer. pub struct FileEncoder(Writer); @@ -329,7 +336,9 @@ impl FileEncoder { } /// Appends the first `frames` frames of `block`, one `Vec` per channel (missing samples are - /// silence), unless they don't fit (see [`FileEncoder::check_room`]). + /// silence), unless they don't fit (see [`FileEncoder::check_room`]). Once a write has + /// failed, the encoder is unusable: the sink may hold a prefix of the failed block, so + /// every later `write` and `finish` fails instead of completing an inconsistent file. pub fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { match &mut self.0 { Writer::Wav(w) => w.write(block, frames), diff --git a/crates/audio-io/src/wav.rs b/crates/audio-io/src/wav.rs index 1aa847d4..102b4eaa 100644 --- a/crates/audio-io/src/wav.rs +++ b/crates/audio-io/src/wav.rs @@ -321,6 +321,9 @@ pub(crate) struct Writer { quantizers: Vec, frames: u64, bytes: Vec, + /// Set when a block write has failed: the sink may hold a prefix of that block while + /// `frames` does not count it, so the writer refuses every later call. + failed: bool, } impl Writer { @@ -339,7 +342,20 @@ impl Writer { let start = out.stream_position()?; out.write_all(&header(channels, sample_rate, format, bwf, mask, 0, force_rf64)?)?; let quantizers = Quantizer::per_channel(u32::from(format.0), dither, channels); - Ok(Writer { out, start, channels, sample_rate, format, bwf: bwf.cloned(), mask, force_rf64, quantizers, frames: 0, bytes: Vec::new() }) + Ok(Writer { + out, + start, + channels, + sample_rate, + format, + bwf: bwf.cloned(), + mask, + force_rf64, + quantizers, + frames: 0, + bytes: Vec::new(), + failed: false, + }) } fn header(&self, frames: u64) -> Result> { @@ -355,13 +371,22 @@ impl Writer { /// Appends `frames` frames of planar audio (missing samples are silence). pub(crate) fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { + if self.failed { + return Err(crate::failed_write_error()); + } let total = self.check_room(frames as u64)?; - write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, self.format, false)?; + if let Err(e) = write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, self.format, false) { + self.failed = true; + return Err(AudioError::from(e)); + } self.frames = total; Ok(()) } pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(crate::failed_write_error()); + } let data_len = u64::from(self.format.0 / 8).saturating_mul(self.channels as u64).saturating_mul(self.frames); if data_len % 2 == 1 { self.out.write_all(&[0])?; diff --git a/crates/audio-io/tests/stream.rs b/crates/audio-io/tests/stream.rs index 61a28946..8d2c40fa 100644 --- a/crates/audio-io/tests/stream.rs +++ b/crates/audio-io/tests/stream.rs @@ -1,5 +1,5 @@ use soundcraft_audio_io::{AudioBuffer, AudioError, BitDepth, BwfInfo, EncodeOptions, FileEncoder, FileFormat, encode, encode_with_channel_mask}; -use std::io::Cursor; +use std::io::{Cursor, Seek, SeekFrom, Write}; fn tone(channels: usize, frames: usize) -> AudioBuffer { let channels = (0..channels).map(|c| (0..frames).map(|i| ((i * (c + 3)) as f32 * 0.013).sin() * 1.2).collect()).collect(); @@ -49,6 +49,50 @@ fn streamed_files_match_whole_buffer_encodes() { } } +/// A `Write + Seek` sink that accepts `budget` bytes in total and then fails every write, so +/// a `write_all` on it appends a prefix of the block and only then reports the error. +struct FailAfter { + inner: Cursor>, + budget: usize, +} + +impl Write for FailAfter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + if self.budget == 0 { + return Err(std::io::Error::other("sink is full")); + } + let n = buf.len().min(self.budget); + self.budget -= n; + self.inner.write(&buf[..n]) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } +} + +impl Seek for FailAfter { + fn seek(&mut self, pos: SeekFrom) -> std::io::Result { + self.inner.seek(pos) + } +} + +#[test] +fn a_failed_write_poisons_the_encoder() { + let buf = tone(2, 20_001); + for format in [FileFormat::Wav, FileFormat::Aiff, FileFormat::Flac] { + let opts = EncodeOptions { format, bit_depth: BitDepth::Int16, dither: false, bwf: None }; + let sink = FailAfter { inner: Cursor::new(Vec::new()), budget: 256 }; + let mut enc = FileEncoder::new(sink, &opts, 2, buf.sample_rate, None).unwrap(); + // The sink accepts a prefix of the first block, then fails. + assert!(enc.write(&buf.channels, buf.frames()).is_err(), "{format:?}"); + // Later calls must not claim success: the bytes on the sink no longer match the + // encoder's counters (and for FLAC the failed frame's samples are already gone). + assert!(matches!(enc.write(&buf.channels, 16), Err(AudioError::Encode(_))), "{format:?}"); + assert!(matches!(enc.finish(), Err(AudioError::Encode(_))), "{format:?}"); + } +} + #[test] fn blocks_past_a_format_limit_are_refused_before_anything_is_written() { let buf = tone(2, 100); From 6b4a7da010240e6f5ac96cd1788ca2139769ecf3 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:33:15 +0000 Subject: [PATCH 23/55] Apply the punch range before placing a recorded take Fixes #431 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/io.rs | 52 +++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..eab87b46 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1350,18 +1350,25 @@ pub struct Take { /// Place recorded take files as `plan` says, as one undo step ("Record"). pub fn place_recording(e: &mut Engine, plan: &Recording, takes: Vec<(TrackId, PathBuf)>) -> Vec { - let ids = add_recording(e, plan.start, takes, plan.pass); - if let Some(p) = plan.punch { - let clips: Vec = ids.iter().map(|c| c.0).collect(); - let _ = e.execute_merged("edit.trim_to_fill_selection", &json!({"clips": clips, "start": p.start, "end": p.end}), "record"); - } - ids + add_recording_in(e, plan.start, takes, plan.pass, plan.punch) } /// Turn recorded take files into clips at `start`, as one undo step ("Record"); each file is the /// source of its clips. With `pass` (loop recording), a take becomes one clip per pass (at most /// 1000), each pass after the first on a new playlist. pub fn add_recording(e: &mut Engine, start: Samples, takes: Vec<(TrackId, PathBuf)>, pass: Option) -> Vec { + add_recording_in(e, start, takes, pass, None) +} + +/// [`add_recording`] limited to the `punch` range: each clip is cut to it before it's placed, so +/// the material around the punch isn't overwritten. +fn add_recording_in( + e: &mut Engine, + start: Samples, + takes: Vec<(TrackId, PathBuf)>, + pass: Option, + punch: Option, +) -> Vec { let before = e.session_arc(); let mut out = Vec::new(); for (tid, path) in takes { @@ -1384,6 +1391,13 @@ pub fn add_recording(e: &mut Engine, start: Samples, takes: Vec<(TrackId, PathBu if offset > 0 && n < pass / 8 { break; } + let (clip_start, cut, clip_len) = match punch { + Some(p) => { + let Some(kept) = Range::new(start, start.saturating_add(n)).intersect(&p) else { continue }; + (kept.start, kept.start.saturating_sub(start), kept.len()) + } + None => (start, 0, n), + }; if offset > 0 && let Some(t) = s.track_mut(tid) { @@ -1391,7 +1405,7 @@ pub fn add_recording(e: &mut Engine, start: Samples, takes: Vec<(TrackId, PathBu t.active_playlist = t.playlists.len() - 1; } let cid = s.new_clip_id(); - crate::edit::place_clip(s, tid, Clip::audio(cid, name, src, offset, start, n)); + crate::edit::place_clip(s, tid, Clip::audio(cid, name, src, offset.saturating_add(cut), clip_start, clip_len)); out.push(cid); } } @@ -1473,6 +1487,30 @@ mod record_tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn punching_in_keeps_the_material_around_the_punch_range() { + let dir = std::env::temp_dir().join(format!("soundcraft-punch-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let mut e = Engine::default(); + let vox = armed(&mut e, "Vox", "mono", "In 1"); + let base = dir.join("Audio Files/Base.wav"); + let take = dir.join("Audio Files/Take.wav"); + write_take(&base, 3_000); + write_take(&take, 1_000); + add_recording(&mut e, 0, vec![(vox, base)], None); + let plan = Recording { + start: 1_000, + pass: None, + punch: Some(Range::new(1_400, 1_600)), + rate: 48_000, + takes: vec![Take { track: vox, path: take.clone(), channels: 1 }], + }; + place_recording(&mut e, &plan, vec![(vox, take)]); + let clips: Vec<_> = e.session().track(vox).unwrap().clips().iter().map(|c| (c.start, c.length, c.source_offset())).collect(); + assert_eq!(clips, vec![(0, 1_400, 0), (1_400, 200, 400), (1_600, 1_400, 1_600)]); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn a_take_shows_the_pass_under_way_within_the_punch_while_it_records() { assert_eq!(live_span(1_000, None, None, 0), None); From 6125f49ab1bd7c8cab025438ef813190e37868ff Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:54:16 +0000 Subject: [PATCH 24/55] Saturate midi.notes_move tick and pitch arithmetic Found while testing crates/engine/src/cmd/midi.rs:129-130 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/midi.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/midi.rs b/crates/engine/src/cmd/midi.rs index 10bcd767..129fd070 100644 --- a/crates/engine/src/cmd/midi.rs +++ b/crates/engine/src/cmd/midi.rs @@ -126,8 +126,8 @@ fn notes_move(e: &mut Engine, p: &Value) -> Result { let seq = seq_mut(e, id, "midi.notes_move")?; for (i, n) in seq.notes.iter_mut().enumerate() { if idx.contains(&i) { - n.start = (n.start + dt).max(0); - n.pitch = (i64::from(n.pitch) + ds).clamp(0, 127) as u8; + n.start = n.start.saturating_add(dt).max(0); + n.pitch = i64::from(n.pitch).saturating_add(ds).clamp(0, 127) as u8; } } seq.sort(); @@ -157,6 +157,21 @@ mod tests { assert!(e.execute("midi.note_edit", &json!({"clip": clip, "index": 99})).is_err()); } + #[test] + fn notes_move_huge_params_do_not_overflow() { + let mut e = Engine::default(); + let t = e.session_mut().add_track(TrackKind::Instrument, ChannelFormat::Stereo, Some("Keys")); + let r = e.execute("midi.new_clip", &json!({"track": t.0})).unwrap(); + let clip = r["clip"].as_u64().unwrap(); + e.execute("midi.note_add", &json!({"clip": clip, "pitch": 60, "start_ticks": 480})).unwrap(); + e.execute("midi.notes_move", &json!({"clip": clip, "indices": [0], "ticks": i64::MAX})).unwrap(); + let n = e.execute("midi.notes", &json!({"clip": clip})).unwrap(); + assert_eq!(n["notes"][0]["start"], i64::MAX); + e.execute("midi.notes_move", &json!({"clip": clip, "indices": [0], "semitones": i64::MAX})).unwrap(); + let n = e.execute("midi.notes", &json!({"clip": clip})).unwrap(); + assert_eq!(n["notes"][0]["pitch"], 127); + } + #[test] fn rejects_audio_tracks() { let mut e = Engine::default(); From 8a5da53870d57347bcad2715f21f6b27ceb39efe Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 19:03:00 +0000 Subject: [PATCH 25/55] Reject invalid send slot in automation.copy_to_send Found while testing crates/engine/src/cmd/mix.rs:697 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/mix.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index c0569429..f72f9ca0 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -694,7 +694,9 @@ fn clip_gain_to_vol(e: &mut Engine, p: &Value) -> Result { fn copy_to_send(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "automation.copy_to_send", p)?; - let slot = slot_of(p, "send", SEND_SLOTS).unwrap_or(0) as u8; + let slot = slot_of(p, "send", SEND_SLOTS).ok_or_else(|| { + bad("automation.copy_to_send", format!("`send` must be 0..{} or a..{}", SEND_SLOTS - 1, (b'a' + SEND_SLOTS as u8 - 1) as char)) + })? as u8; let s = e.session_mut(); for t in &tracks { if let Some(tr) = s.track_mut(*t) { @@ -874,6 +876,23 @@ mod tests { assert_eq!(e.session().tracks[0].mixer.inserts[0].as_ref().unwrap().preset, "Saved"); } + #[test] + fn copy_to_send_rejects_an_invalid_send_slot() { + let mut e = crate::demo::demo_engine(); + let history = e.undo_history().len(); + let lanes = |e: &Engine| e.session().tracks.iter().filter(|t| t.lane(&AutoParam::SendLevel(0)).is_some_and(|l| !l.points.is_empty())).count(); + let before = lanes(&e); + for bad_params in [json!({"track": "Kick", "send": 99}), json!({"track": "Kick", "send": "zz"}), json!({"track": "Kick"})] { + let r = e.execute("automation.copy_to_send", &bad_params); + assert!(r.is_err(), "{bad_params} accepted: {r:?}"); + assert_eq!(e.undo_history().len(), history, "{bad_params} pushed an undo step"); + assert_eq!(lanes(&e), before, "{bad_params} edited Send A"); + } + // A valid slot still copies. + e.execute("automation.copy_to_send", &json!({"track": "Kick", "send": "a"})).unwrap(); + assert_eq!(lanes(&e), before + 1); + } + #[test] fn retargeting_a_send_keeps_its_settings() { let mut e = Engine::default(); From 253707f65f656b73b5fcaca4c2b5b16e78911447 Mon Sep 17 00:00:00 2001 From: IanHollow <72767437+IanHollow@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:15:03 -0700 Subject: [PATCH 26/55] Add an editor screenshot to AppStream metadata Assisted-by: OpenAI Codex (GPT-6) Signed-off-by: IanHollow <72767437+IanHollow@users.noreply.github.com> --- packaging/linux/ai.storyteller.soundcraft.metainfo.xml.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packaging/linux/ai.storyteller.soundcraft.metainfo.xml.in b/packaging/linux/ai.storyteller.soundcraft.metainfo.xml.in index 9a4fdb5e..7dd14825 100644 --- a/packaging/linux/ai.storyteller.soundcraft.metainfo.xml.in +++ b/packaging/linux/ai.storyteller.soundcraft.metainfo.xml.in @@ -41,6 +41,12 @@ recording midi + + + Mixer with channel strips, sends, faders and an equalizer + https://raw.githubusercontent.com/storytold/soundcraft/main/docs/images/mix.png + + #8fd9e6 #0b7385 From c3d37f367df119926891c9f60ae852604d811f52 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:19:49 +0200 Subject: [PATCH 27/55] Fix Min:Secs showing 1 ms early at exact millisecond positions Found while testing crates/time/src/format.rs:46 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/time/src/format.rs | 3 ++- crates/time/tests/min_secs_exact_ms.rs | 36 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 crates/time/tests/min_secs_exact_ms.rs diff --git a/crates/time/src/format.rs b/crates/time/src/format.rs index bdd2fcb1..3c11c844 100644 --- a/crates/time/src/format.rs +++ b/crates/time/src/format.rs @@ -43,7 +43,8 @@ impl TimeFormat { fn min_secs(s: Samples, sr: SampleRate) -> String { let neg = s < 0; - let ms_total = (sr.seconds(s.saturating_abs()) * 1000.0).floor() as i64; + // Integer maths: a float `seconds * 1000` lands just below exact millisecond positions. + let ms_total = i64::try_from(i128::from(s).abs() * 1000 / i128::from(sr.hz().max(1))).unwrap_or(i64::MAX); let mins = ms_total / 60_000; let secs = (ms_total / 1000) % 60; let ms = ms_total % 1000; diff --git a/crates/time/tests/min_secs_exact_ms.rs b/crates/time/tests/min_secs_exact_ms.rs new file mode 100644 index 00000000..738a4bf4 --- /dev/null +++ b/crates/time/tests/min_secs_exact_ms.rs @@ -0,0 +1,36 @@ +//! Min:Secs shows exact millisecond positions exactly (no float-floor off-by-one). + +use soundcraft_time::{FrameRate, SampleRate, TempoMap, TimeFormat, format_position}; + +fn min_secs(s: i64, sr: SampleRate) -> String { + format_position(s, TimeFormat::MinSecs, sr, &TempoMap::default(), FrameRate::default(), 0, 0) +} + +fn expected(ms_total: i64) -> String { + format!("{}:{:02}.{:03}", ms_total / 60_000, (ms_total / 1000) % 60, ms_total % 1000) +} + +#[test] +fn known_positions_at_44100() { + assert_eq!(min_secs(88_641, SampleRate::HZ_44100), "0:02.010"); + assert_eq!(min_secs(89_523, SampleRate::HZ_44100), "0:02.030"); +} + +#[test] +fn negative_position_at_44100() { + assert_eq!(min_secs(-88_641, SampleRate::HZ_44100), "-0:02.010"); +} + +#[test] +fn every_exact_millisecond_displays_exactly() { + for sr in [SampleRate::HZ_44100, SampleRate::HZ_48000] { + let hz = i64::from(sr.hz()); + for ms in 0..=600_000_i64 { + if (ms * hz) % 1000 != 0 { + continue; + } + let s = ms * hz / 1000; + assert_eq!(min_secs(s, sr), expected(ms), "{hz} Hz, sample {s}"); + } + } +} From 039b9f4f03e1d3e7c0dcf2b84b3b47fc5536d899 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:50:59 +0000 Subject: [PATCH 28/55] Reject invalid slot in mix.insert and mix.send Found while testing crates/engine/src/cmd/mix.rs:300 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/mix.rs | 42 +++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index c0569429..66b1b431 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -287,17 +287,30 @@ fn slot_of(p: &Value, key: &str, max: usize) -> Option { (i < max).then_some(i) } +fn slot_error(cmd: &str, max: usize) -> EngineError { + bad(cmd, format!("`slot` must be 0..{} or a..{}", max - 1, (b'a' + max as u8 - 1) as char)) +} + fn track_slot(e: &Engine, p: &Value, cmd: &str, max: usize) -> Result<(TrackId, usize)> { let t = tracks_required(e, cmd, p)?.first().copied().ok_or_else(|| bad(cmd, "no track"))?; - let slot = slot_of(p, "slot", max).ok_or_else(|| bad(cmd, format!("`slot` must be 0..{} or a..{}", max - 1, (b'a' + max as u8 - 1) as char)))?; + let slot = slot_of(p, "slot", max).ok_or_else(|| slot_error(cmd, max))?; Ok((t, slot)) } +/// An optional `slot`: absent (or null) means "first free"; a present but invalid value is +/// an error, never silently treated as absent. +fn optional_slot(p: &Value, cmd: &str, max: usize) -> Result> { + match p.get("slot") { + None | Some(Value::Null) => Ok(None), + Some(_) => slot_of(p, "slot", max).map(Some).ok_or_else(|| slot_error(cmd, max)), + } +} + fn insert(e: &mut Engine, p: &Value) -> Result { let plugin = str_param(p, "plugin").ok_or_else(|| bad("mix.insert", "`plugin` required"))?.to_string(); let info = super::clap::plugin_info(&plugin).ok_or_else(|| bad("mix.insert", format!("unknown plugin `{plugin}`")))?; let t = tracks_required(e, "mix.insert", p)?.first().copied().ok_or_else(|| bad("mix.insert", "no track"))?; - let explicit = slot_of(p, "slot", INSERT_SLOTS); + let explicit = optional_slot(p, "mix.insert", INSERT_SLOTS)?; let s = e.session_mut(); let tr = s.track_mut(t).ok_or_else(|| bad("mix.insert", "no track"))?; let slot = @@ -416,7 +429,7 @@ fn insert_move(e: &mut Engine, p: &Value) -> Result { fn send(e: &mut Engine, p: &Value) -> Result { let bus = str_param(p, "bus").ok_or_else(|| bad("mix.send", "`bus` required"))?.to_string(); let t = tracks_required(e, "mix.send", p)?.first().copied().ok_or_else(|| bad("mix.send", "no track"))?; - let explicit = slot_of(p, "slot", SEND_SLOTS); + let explicit = optional_slot(p, "mix.send", SEND_SLOTS)?; let s = e.session_mut(); let bid = s.add_bus(&bus, soundcraft_model::ChannelFormat::Stereo); let tr = s.track_mut(t).ok_or_else(|| bad("mix.send", "no track"))?; @@ -886,4 +899,27 @@ mod tests { assert_eq!(sl.target, soundcraft_model::Route::Bus(delay)); assert_eq!((sl.level_db, sl.pan, sl.mute, sl.pre_fader), (-9.0, 0.5, true, true)); } + + #[test] + fn insert_and_send_reject_invalid_slot() { + let mut e = Engine::default(); + e.execute("track.new", &json!({"count": 1, "name": "Gtr", "format": "mono"})).unwrap(); + for slot in [json!(12), json!("k"), json!("x1")] { + let r = e.execute("mix.insert", &json!({"track": "Gtr", "slot": slot, "plugin": "compressor"})); + assert!(r.is_err(), "insert slot {slot} accepted: {r:?}"); + } + for slot in [json!(12), json!("k")] { + let r = e.execute("mix.send", &json!({"track": "Gtr", "slot": slot, "bus": "Verb"})); + assert!(r.is_err(), "send slot {slot} accepted: {r:?}"); + } + let mixer = &e.session().tracks[0].mixer; + assert!(mixer.inserts.iter().all(Option::is_none)); + assert!(mixer.sends.iter().all(Option::is_none)); + + // Leaving `slot` out still picks the first free slot. + let r = e.execute("mix.insert", &json!({"track": "Gtr", "plugin": "compressor"})).unwrap(); + assert_eq!(r["slot"], 0); + let r = e.execute("mix.send", &json!({"track": "Gtr", "bus": "Verb"})).unwrap(); + assert_eq!(r["slot"], 0); + } } From 0224ca1e248723586dcdb095c689525232fa201a Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 04:07:58 +0200 Subject: [PATCH 29/55] Reject CLAP port layouts beyond host limits instead of truncating Fixes #435 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/clap-host/src/ffi.rs | 25 ++++++-- crates/clap-host/src/plugin.rs | 17 ++++- crates/clap-host/tests/common/mod.rs | 2 +- crates/clap-host/tests/fixture/src/lib.rs | 78 +++++++++++++++++++++-- crates/clap-host/tests/fixture_e2e.rs | 23 ++++++- crates/engine/tests/clap_e2e.rs | 2 +- 6 files changed, 132 insertions(+), 15 deletions(-) diff --git a/crates/clap-host/src/ffi.rs b/crates/clap-host/src/ffi.rs index 82f57bf3..99062807 100644 --- a/crates/clap-host/src/ffi.rs +++ b/crates/clap-host/src/ffi.rs @@ -1241,25 +1241,38 @@ impl Instance { ok.then(|| read_name(&buf)).filter(|s| !s.is_empty()) } - pub fn audio_ports(&self, is_input: bool) -> Vec { + /// The plugin's audio ports in one direction. A layout beyond `MAX_PORTS` ports or + /// `MAX_PORT_CHANNELS` channels per port is an error: we never hand a plugin fewer ports or + /// channels than it advertised. + pub fn audio_ports(&self, is_input: bool) -> Result, ClapError> { // SAFETY: see `params`. - let Some(p) = (unsafe { self.ports.as_ref() }) else { return Vec::new() }; - let (Some(count), Some(get)) = (p.count, p.get) else { return Vec::new() }; + let Some(p) = (unsafe { self.ports.as_ref() }) else { return Ok(Vec::new()) }; + let (Some(count), Some(get)) = (p.count, p.get) else { return Ok(Vec::new()) }; + let dir = if is_input { "input" } else { "output" }; // SAFETY: vtable call with its instance. - let n = unsafe { count(self.plugin, is_input) }.min(MAX_PORTS); + let n = unsafe { count(self.plugin, is_input) }; + if n > MAX_PORTS { + return Err(ClapError::Process(format!("{n} audio {dir} ports (at most {MAX_PORTS} supported)"))); + } let mut out = Vec::new(); for i in 0..n { // SAFETY: plain data; all-zero (null port_type pointer) is valid. let mut info: clap_audio_port_info = unsafe { std::mem::zeroed() }; // SAFETY: index below the count; `info` is writable. if unsafe { get(self.plugin, i, is_input, &mut info) } { - out.push(PortInfo { channels: info.channel_count.min(MAX_PORT_CHANNELS), main: info.flags & CLAP_AUDIO_PORT_IS_MAIN != 0 }); + if info.channel_count > MAX_PORT_CHANNELS { + return Err(ClapError::Process(format!( + "audio {dir} port {i} has {} channels (at most {MAX_PORT_CHANNELS} supported)", + info.channel_count + ))); + } + out.push(PortInfo { channels: info.channel_count, main: info.flags & CLAP_AUDIO_PORT_IS_MAIN != 0 }); } else { // Keep port indices aligned: an unreadable port still needs a (silent) buffer slot. out.push(PortInfo { channels: 0, main: false }); } } - out + Ok(out) } pub fn note_port(&self) -> Option { diff --git a/crates/clap-host/src/plugin.rs b/crates/clap-host/src/plugin.rs index 89f8dbf8..2fbf2726 100644 --- a/crates/clap-host/src/plugin.rs +++ b/crates/clap-host/src/plugin.rs @@ -113,8 +113,21 @@ impl Plugin for ClapPlugin { self.channels = channels.clamp(1, MAX_CHANNELS); self.inst.deactivate(); self.inst.service_callback(); - let ins = self.inst.audio_ports(true); - let outs = self.inst.audio_ports(false); + let ports = self.inst.audio_ports(true).and_then(|ins| Ok((ins, self.inst.audio_ports(false)?))); + let (ins, outs) = match ports { + Ok(p) => p, + Err(e) => { + // Unsupported layout: stay inactive (`process` passes audio through untouched). + log::warn!("{}: {e}", self.info.id); + self.inputs = PortBuffers::new(&[], 0); + self.outputs = PortBuffers::new(&[], 0); + self.in_main = None; + self.out_main = None; + self.max_block = 0; + self.latency = 0; + return; + } + }; self.in_main = pick_main(&ins); self.out_main = pick_main(&outs); self.inputs = PortBuffers::new(&ins.iter().map(|p| p.channels).collect::>(), mb); diff --git a/crates/clap-host/tests/common/mod.rs b/crates/clap-host/tests/common/mod.rs index 2da23955..1318104d 100644 --- a/crates/clap-host/tests/common/mod.rs +++ b/crates/clap-host/tests/common/mod.rs @@ -40,7 +40,7 @@ pub fn fixture_dir() -> &'static Path { std::fs::create_dir_all(dir.join("vendor")).unwrap(); std::fs::copy(&built, dir.join("vendor/Fixture.clap")).unwrap(); } - assert_eq!(soundcraft_clap_host::add_search_dir(&dir), 2); + assert_eq!(soundcraft_clap_host::add_search_dir(&dir), 5); dir }) } diff --git a/crates/clap-host/tests/fixture/src/lib.rs b/crates/clap-host/tests/fixture/src/lib.rs index 85a7999b..b88f434f 100644 --- a/crates/clap-host/tests/fixture/src/lib.rs +++ b/crates/clap-host/tests/fixture/src/lib.rs @@ -6,6 +6,11 @@ //! to the velocity of the held note (0 when none), switching at the event's sample offset. A //! MIDI CC 7 (volume) sets the constant to its value / 127. //! +//! - `org.soundcraft.test.wide16`, `.wide17ports`, `.wide65ch`: effects advertising wide layouts +//! (16 ports of 64 channels, 17 ports of 2 channels, one port of 65 channels per direction). +//! `process` writes 1.0 into every output channel it was given when the host's buffers match the +//! advertised layout, -1.0 when they do not. +//! //! The gain plugin also implements `clap.state` (gain and mode as two little-endian f64s) and a //! floating-only `clap.gui` without a real window: showing it "edits" the gain to 0.25 in the GUI, //! reported as an output parameter event by the next `process`. @@ -36,6 +41,27 @@ unsafe impl Sync for Features {} static GAIN_FEATURES: Features<3> = Features([c"audio-effect".as_ptr(), c"stereo".as_ptr(), std::ptr::null()]); static SYNTH_FEATURES: Features<3> = Features([c"instrument".as_ptr(), c"synthesizer".as_ptr(), std::ptr::null()]); +static WIDE_FEATURES: Features<2> = Features([c"audio-effect".as_ptr(), std::ptr::null()]); + +const fn wide_desc(id: &'static CStr, name: &'static CStr) -> clap_plugin_descriptor { + clap_plugin_descriptor { + clap_version: CLAP_VERSION, + id: id.as_ptr(), + name: name.as_ptr(), + vendor: c"SoundCraft tests".as_ptr(), + url: std::ptr::null(), + manual_url: std::ptr::null(), + support_url: std::ptr::null(), + version: c"1.0.0".as_ptr(), + description: std::ptr::null(), + features: WIDE_FEATURES.0.as_ptr(), + } +} + +static WIDE16_DESC: clap_plugin_descriptor = wide_desc(c"org.soundcraft.test.wide16", c"Fixture Wide 16x64"); +static WIDE17_DESC: clap_plugin_descriptor = wide_desc(c"org.soundcraft.test.wide17ports", c"Fixture Wide 17 ports"); +static WIDE65_DESC: clap_plugin_descriptor = wide_desc(c"org.soundcraft.test.wide65ch", c"Fixture Wide 65 channels"); + static GAIN_DESC: clap_plugin_descriptor = clap_plugin_descriptor { clap_version: CLAP_VERSION, id: c"org.soundcraft.test.gain".as_ptr(), @@ -66,6 +92,8 @@ struct Fx { plugin: clap_plugin, host: *const clap_host, synth: bool, + /// `(ports per direction, channels per port)` for the wide fixtures. + wide: Option<(u32, u32)>, gain: f64, mode: f64, level: f64, @@ -168,6 +196,22 @@ unsafe extern "C" fn process(p: *const clap_plugin, pr: *const clap_process) -> let n = pr.frames_count as usize; let ev = unsafe { &*pr.in_events }; let count = unsafe { (ev.size.unwrap())(pr.in_events) }; + if let Some((ports, channels)) = f.wide { + let fits = |bufs: *const clap_audio_buffer, n: u32| { + n == ports && (0..n as usize).all(|i| unsafe { &*bufs.add(i) }.channel_count == channels) + }; + let ok = fits(pr.audio_inputs, pr.audio_inputs_count) && fits(pr.audio_outputs, pr.audio_outputs_count); + if pr.audio_outputs_count > 0 { + let out = unsafe { &*pr.audio_outputs }; + for c in 0..out.channel_count as usize { + let o = unsafe { *out.data32.add(c) }; + for i in 0..n { + unsafe { *o.add(i) = if ok { 1.0 } else { -1.0 } }; + } + } + } + return CLAP_PROCESS_CONTINUE; + } if pr.audio_outputs_count != 1 { return CLAP_PROCESS_ERROR; } @@ -324,10 +368,26 @@ static PARAMS: clap_plugin_params = clap_plugin_params { }; unsafe extern "C" fn ports_count(p: *const clap_plugin, is_input: bool) -> u32 { + if let Some((ports, _)) = unsafe { fx(p) }.wide { + return ports; + } if is_input && unsafe { fx(p) }.synth { 0 } else { 1 } } unsafe extern "C" fn ports_get(p: *const clap_plugin, index: u32, is_input: bool, info: *mut clap_audio_port_info) -> bool { + if let Some((ports, channels)) = unsafe { fx(p) }.wide { + if index >= ports { + return false; + } + let info = unsafe { &mut *info }; + info.id = index * 2 + u32::from(is_input); + write_name(&mut info.name, c"wide"); + info.flags = if index == 0 { CLAP_AUDIO_PORT_IS_MAIN } else { 0 }; + info.channel_count = channels; + info.port_type = std::ptr::null(); + info.in_place_pair = clap_sys::id::CLAP_INVALID_ID; + return true; + } if index != 0 || (is_input && unsafe { fx(p) }.synth) { return false; } @@ -498,23 +558,32 @@ unsafe extern "C" fn on_main_thread(_p: *const clap_plugin) {} // ---- factory + entry ---- unsafe extern "C" fn factory_count(_f: *const clap_plugin_factory) -> u32 { - 2 + 5 } unsafe extern "C" fn factory_desc(_f: *const clap_plugin_factory, i: u32) -> *const clap_plugin_descriptor { match i { 0 => &GAIN_DESC, 1 => &SYNTH_DESC, + 2 => &WIDE16_DESC, + 3 => &WIDE17_DESC, + 4 => &WIDE65_DESC, _ => std::ptr::null(), } } unsafe extern "C" fn factory_create(_f: *const clap_plugin_factory, host: *const clap_host, id: *const c_char) -> *const clap_plugin { let id = unsafe { CStr::from_ptr(id) }; - let (desc, synth) = if id == c"org.soundcraft.test.gain" { - (&GAIN_DESC as *const _, false) + let (desc, synth, wide) = if id == c"org.soundcraft.test.gain" { + (&GAIN_DESC as *const _, false, None) } else if id == c"org.soundcraft.test.synth" { - (&SYNTH_DESC as *const _, true) + (&SYNTH_DESC as *const _, true, None) + } else if id == c"org.soundcraft.test.wide16" { + (&WIDE16_DESC as *const _, false, Some((16, 64))) + } else if id == c"org.soundcraft.test.wide17ports" { + (&WIDE17_DESC as *const _, false, Some((17, 2))) + } else if id == c"org.soundcraft.test.wide65ch" { + (&WIDE65_DESC as *const _, false, Some((1, 65))) } else { return std::ptr::null(); }; @@ -535,6 +604,7 @@ unsafe extern "C" fn factory_create(_f: *const clap_plugin_factory, host: *const }, host, synth, + wide, gain: 1.0, mode: 0.0, level: 0.0, diff --git a/crates/clap-host/tests/fixture_e2e.rs b/crates/clap-host/tests/fixture_e2e.rs index a6c28cae..8525d740 100644 --- a/crates/clap-host/tests/fixture_e2e.rs +++ b/crates/clap-host/tests/fixture_e2e.rs @@ -13,7 +13,7 @@ const SYNTH: &str = "clap:org.soundcraft.test.synth"; fn scan_lists_both_fixture_plugins() { let dir = fixture_dir(); let found = soundcraft_clap_host::scan_paths(&[dir.to_path_buf()]); - assert_eq!(found.len(), 2); + assert_eq!(found.len(), 5); let g = found.iter().find(|d| d.id == GAIN).unwrap(); assert_eq!(g.name, "Fixture Gain"); assert_eq!(g.vendor, "SoundCraft tests"); @@ -110,6 +110,27 @@ fn synth_plays_notes_at_their_offsets() { assert!(io[0][19].abs() < 1e-6 && (io[0][20] - 64.0 / 127.0).abs() < 1e-6, "controllers arrive as MIDI at their offset"); } +#[test] +fn layouts_beyond_the_host_limits_are_rejected_not_truncated() { + fixture_dir(); + // Control: exactly at the limits (16 ports of 64 channels) activates and gets its full layout. + let mut p = soundcraft_clap_host::create("clap:org.soundcraft.test.wide16").unwrap(); + p.prepare(44_100.0, 64, 2); + assert_eq!(p.latency(), 3); + let mut io = vec![vec![0.5f32; 64]; 2]; + p.process(&mut io, 64); + assert!(io.iter().flatten().all(|&x| x == 1.0), "16x64 layout delivered as advertised: {:?}", &io[0][..4]); + // 17 ports, or a 65-channel port: never activated, never processed with a cut-down layout. + for id in ["clap:org.soundcraft.test.wide17ports", "clap:org.soundcraft.test.wide65ch"] { + let mut p = soundcraft_clap_host::create(id).unwrap(); + p.prepare(44_100.0, 64, 2); + assert_eq!(p.latency(), 0, "{id}: not activated"); + let mut io = vec![vec![0.5f32; 64]; 2]; + p.process(&mut io, 64); + assert!(io.iter().flatten().all(|&x| x == 0.5), "{id}: the plugin never ran: {:?}", &io[0][..4]); + } +} + #[test] fn many_instances_create_and_drop_cleanly() { fixture_dir(); diff --git a/crates/engine/tests/clap_e2e.rs b/crates/engine/tests/clap_e2e.rs index cd554a9d..96743198 100644 --- a/crates/engine/tests/clap_e2e.rs +++ b/crates/engine/tests/clap_e2e.rs @@ -40,7 +40,7 @@ fn install_fixture_now() { let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); std::fs::copy(target.join("debug").join(lib), dir.join("Fixture.clap")).unwrap(); - assert_eq!(soundcraft_clap_host::add_search_dir(&dir), 2); + assert_eq!(soundcraft_clap_host::add_search_dir(&dir), 5); } #[test] From fff096876d9866fcfaaa55515953c2061794f61c Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:16:17 +0200 Subject: [PATCH 30/55] Clamp retrospective record start_ticks to stay within MAX_POSITION Found while testing crates/engine/src/cmd/event_more.rs:753 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/event_more.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/event_more.rs b/crates/engine/src/cmd/event_more.rs index c0780a55..f8e403c3 100644 --- a/crates/engine/src/cmd/event_more.rs +++ b/crates/engine/src/cmd/event_more.rs @@ -730,13 +730,16 @@ fn retro_record(e: &mut Engine, p: &Value) -> Result { let t = midi_target(e, p, id)?; let s = e.session(); let sr = s.sample_rate; + // Latest start tick whose longest note still ends within MAX_POSITION. + let max_tick = s.tempo.samples_to_ticks(MAX_POSITION, sr).saturating_sub(TICKS_PER_QUARTER * 1024).max(0); let mut raw: Vec<(i64, Note)> = Vec::new(); for v in list.iter().take(100_000) { let tick = match (v.get("start_ticks").and_then(Value::as_i64), v.get("start")) { (Some(tk), _) => tk.max(0), (None, Some(_)) => s.tempo.samples_to_ticks(position_param(e, id, v, "start")?.unwrap_or(0).max(0), sr), _ => return Err(bad(id, "each note needs `start` or `start_ticks`")), - }; + } + .min(max_tick); let note = Note { pitch: i64_or(v, "pitch", 60).clamp(0, 127) as u8, velocity: i64_or(v, "velocity", 100).clamp(1, 127) as u8, @@ -750,7 +753,7 @@ fn retro_record(e: &mut Engine, p: &Value) -> Result { let first = raw.iter().map(|x| x.0).min().unwrap_or(0); // Start the clip on the bar line so the quantize grid lines up with the music. let clip_tick = s.tempo.bar_start_tick(s.tempo.bar_beat_at_tick(first).bar).min(first).max(0); - let end_tick = raw.iter().map(|(tk, n)| tk.saturating_add(n.length)).max().unwrap_or(first + 1); + let end_tick = raw.iter().map(|(tk, n)| tk.saturating_add(n.length)).max().unwrap_or_else(|| first.saturating_add(1)); let mut seq = soundcraft_midi::Sequence::default(); for (tk, mut n) in raw { n.start = tk - clip_tick; @@ -764,7 +767,7 @@ fn retro_record(e: &mut Engine, p: &Value) -> Result { seq.sort(); } let start = s.tempo.tick_to_samples(clip_tick, sr); - let end = s.tempo.tick_to_samples(end_tick, sr); + let end = s.tempo.tick_to_samples(end_tick, sr).min(MAX_POSITION); let n = seq.notes.len(); let s = e.session_mut(); let cid = s.new_clip_id(); @@ -943,6 +946,18 @@ mod tests { assert!(r["bpm"].as_f64().is_some()); } + #[test] + fn retro_record_huge_start_ticks_stay_in_range() { + for tk in [i64::MAX, 1_000_000_000_000_000] { + let (mut e, t) = midi_engine(); + let notes = json!([{"pitch": 60, "start_ticks": tk, "length_ticks": TICKS_PER_QUARTER * 1024}]); + e.execute("event.retrospective_record", &json!({"track": t.0, "notes": notes})).unwrap(); + let c = e.session().track(t).unwrap().clips()[0].clone(); + assert!(c.start >= 0 && c.length >= 1, "{tk}: {} {}", c.start, c.length); + assert!(c.start.saturating_add(c.length) <= MAX_POSITION, "{tk}: start {} len {}", c.start, c.length); + } + } + #[test] fn move_song_start_moves_tick_material() { let (mut e, t) = midi_engine(); From 0a6e08a9c30ed87cf25076fbb0f10c02983002bf Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 15 Oct 2026 11:31:52 +0200 Subject: [PATCH 31/55] Drag automation breakpoints and Alt-click to delete them in the Edit window (fixes #601) --- crates/ui-egui/src/edit_window.rs | 170 +++++++++++++++++-- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/tests/command_reachability.rs | 1 - 3 files changed, 155 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index f77560a5..7e24a4c7 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -1798,23 +1798,13 @@ fn crossfade_partner<'a>(track: &'a Track, clip: &Clip) -> Option<&'a Clip> { fn draw_automation(painter: &egui::Painter, s: &Session, track: &Track, p: &AutoParam, tl: Rect, lane: Rect) { let t = Tokens::current(); - let (lo, hi, _) = p.range(); - let (lo, hi) = if matches!(p, AutoParam::Volume | AutoParam::SendLevel(_)) { - (-60.0f32, 12.0f32) - } else if matches!(p, AutoParam::Plugin { .. }) { - plugin_range(track, p) - } else { - (lo, hi) - }; + let range = lane_range(track, p); let def = match p { AutoParam::Volume => track.mixer.volume_db, AutoParam::Pan(i) => track.mixer.pan.get(usize::from(*i)).copied().unwrap_or(0.0), _ => 0.0, }; - let yv = |v: f32| -> f32 { - let k = ((v.clamp(lo, hi) - lo) / (hi - lo).max(1e-6)).clamp(0.0, 1.0); - lane.max.y - 4.0 - k * (lane.height() - 8.0) - }; + let yv = |v: f32| auto_y(lane, range, v); let lanes = track.lane(p); let pts: Vec<(f32, f32)> = match lanes { Some(l) if !l.points.is_empty() => { @@ -1844,6 +1834,47 @@ fn draw_automation(painter: &egui::Painter, s: &Session, track: &Track, p: &Auto painter.text(pos2(lane.min.x + 6.0, lane.min.y + 9.0), Align2::LEFT_CENTER, p.label(), regular(10.0), t.text_dim); } +/// The values an automation lane spans, bottom to top. +fn lane_range(track: &Track, p: &AutoParam) -> (f32, f32) { + if matches!(p, AutoParam::Volume | AutoParam::SendLevel(_)) { + (-60.0, 12.0) + } else if matches!(p, AutoParam::Plugin { .. }) { + plugin_range(track, p) + } else { + let (lo, hi, _) = p.range(); + (lo, hi) + } +} + +fn auto_y(lane: Rect, (lo, hi): (f32, f32), v: f32) -> f32 { + let k = ((v.clamp(lo, hi) - lo) / (hi - lo).max(1e-6)).clamp(0.0, 1.0); + lane.max.y - 4.0 - k * (lane.height() - 8.0) +} + +fn auto_value(lane: Rect, (lo, hi): (f32, f32), y: f32) -> f32 { + let k = ((lane.max.y - 4.0 - y) / (lane.height() - 8.0)).clamp(0.0, 1.0); + lo + (hi - lo) * k +} + +/// The automation a track's lane draws: its view's, or volume on the tracks that always show it. +fn shown_automation(track: &Track) -> Option { + AutoParam::parse(&track.view).or_else(|| matches!(track.kind, TrackKind::Master | TrackKind::Aux | TrackKind::Vca).then_some(AutoParam::Volume)) +} + +/// The drawn breakpoint nearest `pos`, within grabbing distance. +fn breakpoint_at(s: &Session, tl: Rect, lane: Rect, track: &Track, p: &AutoParam, pos: Pos2) -> Option { + let range = lane_range(track, p); + let dist = |at: Samples, v: f32| pos2(x_of(s, tl, at), auto_y(lane, range, v)).distance(pos); + track + .lane(p)? + .points + .iter() + .map(|pt| (pt.at, dist(pt.at, pt.value))) + .filter(|(_, d)| *d <= 6.0) + .min_by(|a, b| a.1.total_cmp(&b.1)) + .map(|(at, _)| at) +} + fn plugin_range(track: &Track, p: &AutoParam) -> (f32, f32) { if let AutoParam::Plugin { slot, param } = p && let Some(Some(ins)) = track.mixer.inserts.get(usize::from(*slot)) @@ -1977,6 +2008,11 @@ fn lane_interaction(app: &mut SoundApp, ui: &mut Ui, track: &Track, lane: Rect, let origin = if resp.drag_started() { ui.input(|i| i.pointer.press_origin()).unwrap_or(p) } else { p }; let at_origin = sample_at(&s, tl, origin.x).max(0); let auto_view = AutoParam::parse(&track.view); + let point_hit = if matches!(tool, Tool::Smart | Tool::Grabber | Tool::Pencil) { + shown_automation(track).and_then(|param| breakpoint_at(&s, tl, lane, track, ¶m, origin).map(|at| (param, at))) + } else { + None + }; let body_hit = clip_at(track, at_origin); // Check fade endpoints before clip bodies: a full-length fade can end exactly // at the next clip's start, where `clip_at` would otherwise choose that neighbour. @@ -2008,8 +2044,9 @@ fn lane_interaction(app: &mut SoundApp, ui: &mut Ui, track: &Track, lane: Rect, None } }; - // Effective tool under the Smart tool. + // Effective tool under the Smart tool; on a breakpoint every tool that edits one grabs it. let eff = match tool { + _ if point_hit.is_some() => Tool::Grabber, Tool::Smart => { if let Some(c) = &hit { if near_edge(c).is_some() { @@ -2036,7 +2073,7 @@ fn lane_interaction(app: &mut SoundApp, ui: &mut Ui, track: &Track, lane: Rect, }; ui.ctx().set_cursor_icon(icon); } - let fade_corner = if tool == Tool::Smart { + let fade_corner = if tool == Tool::Smart && point_hit.is_none() { fade_handle.map(|(_, side)| side).or_else(|| { hit.as_ref().and_then(|c| { let xs = x_of(&s, tl, c.start); @@ -2074,7 +2111,11 @@ fn lane_interaction(app: &mut SoundApp, ui: &mut Ui, track: &Track, lane: Rect, place_insertion(app, ui, &s, track.id, at, mods.shift); } let gain_drag = if resp.drag_started() { gain_icon_at(origin) } else { None }; - if let Some(c) = gain_drag { + if resp.drag_started() + && let Some((param, at)) = &point_hit + { + app.gesture = Some(Gesture::Breakpoint { track: track.id, param: param.clone(), at: *at }); + } else if let Some(c) = gain_drag { // `clip.gain` refuses to run with nothing selected. if s.edit.selected_tracks.is_empty() && s.edit.selected_clips.is_empty() { let _ = app.engine.execute("edit.select", &json!({"clips": [c.id.0]})); @@ -2141,7 +2182,15 @@ fn lane_interaction(app: &mut SoundApp, ui: &mut Ui, track: &Track, lane: Rect, ui.ctx().set_cursor_icon(egui::CursorIcon::ResizeVertical); clip_gain_popup(ui.ctx(), *fader_bottom, *pos); } - if resp.clicked() && curve_handle.is_none() { + if resp.clicked() + && let Some((param, at)) = &point_hit + { + // Alt-click deletes a breakpoint, as in Pro Tools. + if mods.alt { + let _ = + app.run("automation.clear", json!({"track": track.id.0, "param": automation_id(param), "start": at, "end": at.saturating_add(1)})); + } + } else if resp.clicked() && curve_handle.is_none() { match eff { Tool::Grabber => { if let Some(c) = &hit { @@ -2187,6 +2236,7 @@ fn lane_interaction(app: &mut SoundApp, ui: &mut Ui, track: &Track, lane: Rect, } if resp.double_clicked() && curve_handle.is_none() + && point_hit.is_none() && eff != Tool::Pencil && let Some(c) = &hit { @@ -2300,6 +2350,29 @@ fn drive_drag(app: &mut SoundApp, ctx: &egui::Context) { *last = at; let _ = app.engine.execute("transport.locate", &json!({"at": at})); } + Some(Gesture::Breakpoint { track, param, at: from }) => { + if let Some(t) = s.track(*track) + && let Some((_, row)) = rows.iter().find(|(i, _)| *i == t.id.0) + && let Some(points) = t.lane(param).map(|l| l.points.as_slice()) + && let Some(i) = points.iter().position(|pt| pt.at == *from) + { + let lane = lane_rect(t, rect(*row), tl); + let after = i.checked_sub(1).and_then(|j| points.get(j)).map_or(0, |pt| pt.at.saturating_add(1)); + let before = points.get(i + 1).map_or(Samples::MAX, |pt| pt.at.saturating_sub(1)); + let to = snap(&s, at).clamp(after, before.max(after)); + let value = auto_value(lane, lane_range(t, param), p.y); + if to != *from || points.get(i).is_some_and(|pt| (pt.value - value).abs() > 1e-6) { + let (key, pid) = (format!("breakpoint:{}", track.0), automation_id(param)); + if to != *from { + let clear = json!({"track": track.0, "param": pid, "start": *from, "end": from.saturating_add(1)}); + let _ = app.engine.execute_merged("automation.clear", &clear, &key); + } + let _ = + app.engine.execute_merged("automation.set_point", &json!({"track": track.0, "param": pid, "at": to, "value": value}), &key); + *from = to; + } + } + } Some(Gesture::Pencil { track, points }) => { if let Some(t) = s.track(*track) && let Some(param) = AutoParam::parse(&t.view) @@ -3893,4 +3966,69 @@ mod tests { assert_eq!(points.len(), 1, "status: {}", h.state().ui.status); assert!((points[0].value + 12.0).abs() < 0.01, "{}", points[0].value); } + + /// A volume lane over a clip, with two breakpoints low enough that the Smart tool would grab + /// the clip there: the screen position of each, and the clips as they were. + fn breakpoints_over_a_clip(tool: &str) -> (Harness<'static, SoundApp>, TrackId, [Pos2; 2], Vec) { + let mut engine = soundcraft_engine::demo::demo_engine(); + engine.execute("edit.tool", &json!({"tool": tool})).unwrap(); + engine.execute("edit.mode", &json!({"mode": "shuffle"})).unwrap(); + let mut h = harness_with(engine); + let tl = timeline(&h); + let (clip, _) = clip_on_screen(&h, |xs, xe| xs > tl.min.x + 30.0 && xe < tl.max.x - 100.0 && xe - xs > 200.0); + let track = h.state().engine.session().find_clip(clip.id).unwrap().0; + let (xs, xe) = (x_of(h.state().engine.session(), tl, clip.start), x_of(h.state().engine.session(), tl, clip.end())); + let xs_at = [xs + (xe - xs) * 0.3, xs + (xe - xs) * 0.6]; + for x in xs_at { + let at = sample_at(h.state().engine.session(), tl, x); + h.state_mut().engine.execute("automation.set_point", &json!({"track": track.0, "param": "volume", "at": at, "value": -40.0})).unwrap(); + } + h.state_mut().engine.execute("track.view", &json!({"track": track.0, "view": "volume"})).unwrap(); + h.run_steps(2); + let s = h.state().engine.session_arc(); + let t = s.track(track).unwrap(); + let i = h.state().edit_layout.rows.iter().position(|(id, _)| *id == track.0).unwrap(); + let lane = lane_rect(t, row(&h, i), tl); + let y = lane.max.y - 4.0 - (lane.height() - 8.0) * (20.0 / 72.0); + assert!(y > lane.center().y, "the breakpoints sit where the Smart tool grabs"); + let at = t.lane(&AutoParam::Volume).unwrap().points.iter().map(|p| pos2(x_of(&s, tl, p.at), y)).collect::>(); + (h, track, [at[0], at[1]], t.clips().to_vec()) + } + + fn volume_points(h: &Harness, track: TrackId) -> Vec<(Samples, f32)> { + let s = h.state().engine.session(); + s.track(track).and_then(|t| t.lane(&AutoParam::Volume)).map(|l| l.points.iter().map(|p| (p.at, p.value)).collect()).unwrap_or_default() + } + + #[test] + fn dragging_a_breakpoint_moves_it_not_the_clip_under_it() { + for tool in ["smart", "grabber"] { + let (mut h, track, [a, b], clips) = breakpoints_over_a_clip(tool); + let before = volume_points(&h, track); + let to = b + vec2(-40.0, -15.0); + drag(&mut h, b, to); + let s = h.state().engine.session_arc(); + assert_eq!(s.track(track).unwrap().clips(), clips.as_slice(), "{tool}: the clip stays"); + let points = volume_points(&h, track); + assert_eq!(points.len(), 2, "{tool}: {points:?}"); + assert_eq!(points[0], before[0], "{tool}: the other breakpoint stays"); + let want = sample_at(&s, timeline(&h), to.x); + assert!((points[1].0 - want).abs() as f64 <= s.edit.zoom.samples_per_px, "{tool}: {} vs {want}", points[1].0); + assert!(points[1].1 > -40.0 + 1.0, "{tool}: dragged up, {}", points[1].1); + assert!(x_of(&s, timeline(&h), points[1].0) > a.x, "{tool}"); + h.state_mut().engine.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(volume_points(&h, track), before, "{tool}: one undo step"); + } + } + + #[test] + fn alt_click_on_a_breakpoint_deletes_it() { + for tool in ["smart", "grabber", "pencil"] { + let (mut h, track, [_, b], clips) = breakpoints_over_a_clip(tool); + let before = volume_points(&h, track); + click_with(&mut h, b, egui::Modifiers::ALT); + assert_eq!(volume_points(&h, track), before[..1], "{tool}"); + assert_eq!(h.state().engine.session().track(track).unwrap().clips(), clips.as_slice(), "{tool}"); + } + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ddd54275..092cb96b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -240,6 +240,7 @@ pub enum Gesture { Fader { track: TrackId, start_db: f32 }, ClipGain { clip: soundcraft_model::ClipId, track: TrackId, pos: f32, anchor: (f32, f32), fine: bool, fader_bottom: egui::Pos2 }, Pencil { track: TrackId, points: Vec<(Samples, f32)> }, + Breakpoint { track: TrackId, param: soundcraft_model::AutoParam, at: Samples }, MidiClip { track: TrackId, from: Samples, to: Samples }, ZoomBox { start: f32 }, } diff --git a/crates/ui-egui/tests/command_reachability.rs b/crates/ui-egui/tests/command_reachability.rs index 5328947b..d6907618 100644 --- a/crates/ui-egui/tests/command_reachability.rs +++ b/crates/ui-egui/tests/command_reachability.rs @@ -11,7 +11,6 @@ use std::path::Path; /// Known gaps: commands with no menu, shortcut or literal reference in the UI source today. /// Give each one a UI path and remove it from this list; don't add to it. const KNOWN_UNREACHED: &[&str] = &[ - "automation.clear", "edit.paste_clip_effects", "edit.paste_clip_gain", "event.midi_note", From 705c2b90773df24a147cba1516d535a7cce30ab5 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:46:37 +0000 Subject: [PATCH 32/55] Drop out-of-range tempo and meter events when sanitizing a session Found while testing crates/time/src/tempo.rs:254 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/model/src/session.rs | 11 +++++++++++ crates/time/src/tempo.rs | 21 ++++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 449d82da..c4f051a4 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -749,6 +749,17 @@ mod tests { assert_eq!((s.sample_rate, s.tempo), (SampleRate::HZ_48000, TempoMap::default())); } + #[test] + fn a_meter_change_far_beyond_the_position_range_is_dropped_on_load() { + let mut v = serde_json::to_value(Session::default()).unwrap(); + v["tempo"]["meters"] = serde_json::json!([ + {"tick": 0, "numerator": 4, "denominator": 4}, + {"tick": i64::MAX, "numerator": 3, "denominator": 4} + ]); + let s = Session::from_json(&v.to_string()).unwrap(); + assert_eq!(s.parse_position("2|1|000", TimeFormat::BarsBeats), Ok(96_000)); + } + #[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 6d16471b..24a8f49a 100644 --- a/crates/time/src/tempo.rs +++ b/crates/time/src/tempo.rs @@ -1,6 +1,6 @@ //! Tempo and meter maps: the conversion between musical time (ticks) and samples. -use crate::{SampleRate, Samples, TimeError, to_samples}; +use crate::{MAX_POSITION, SampleRate, Samples, TimeError, to_samples}; /// Ticks per quarter note (Pro Tools' resolution). pub const TICKS_PER_QUARTER: i64 = 960; @@ -74,17 +74,17 @@ 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. + /// Repair a map read from a file: only valid tempos and meters within `0..=MAX_POSITION` ticks, 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.retain(|e| (0..=MAX_POSITION).contains(&e.tick) && 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.retain(|m| (0..=MAX_POSITION).contains(&m.tick) && 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) { @@ -386,6 +386,17 @@ mod tests { assert_eq!(empty, TempoMap::default()); } + #[test] + fn sanitize_drops_events_beyond_the_position_range() { + let mut m = TempoMap { + tempos: vec![TempoEvent { tick: 0, bpm: 120.0 }, TempoEvent { tick: i64::MAX, bpm: 90.0 }], + meters: vec![MeterEvent { tick: 0, numerator: 4, denominator: 4 }, MeterEvent { tick: i64::MAX, numerator: 3, denominator: 4 }], + }; + m.sanitize(); + assert_eq!(m, TempoMap::default()); + assert_eq!(m.bar_start_tick(2), TICKS_PER_QUARTER * 4); + } + #[test] fn rejects_bad_values() { let mut m = TempoMap::default(); From f88748960378cf9ab6bcd5520c7fcbea09aa29a8 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 16:52:52 +0200 Subject: [PATCH 33/55] Fix step_input extending a MIDI clip over the adjacent next clip Found while testing crates/engine/src/cmd/event_more.rs:416-445 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/event_more.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/event_more.rs b/crates/engine/src/cmd/event_more.rs index c0780a55..083ae534 100644 --- a/crates/engine/src/cmd/event_more.rs +++ b/crates/engine/src/cmd/event_more.rs @@ -414,7 +414,9 @@ fn step_input(e: &mut Engine, p: &Value) -> Result { let end = s.tempo.tick_to_samples(end_tick, sr); // Continue the clip under (or ending at) the insertion point, else start a new one. let host = s.track(t).and_then(|tr| { - tr.clips().iter().filter(|c| matches!(c.content, ClipContent::Midi { .. })).find(|c| c.range().contains(at) || c.end() == at).map(|c| c.id) + let mut midi = tr.clips().iter().filter(|c| matches!(c.content, ClipContent::Midi { .. })); + // A clip starting at `at` wins over one that merely ends there. + midi.clone().find(|c| c.range().contains(at)).or_else(|| midi.find(|c| c.end() == at)).map(|c| c.id) }); let host = match host { Some(h) => h, @@ -862,6 +864,25 @@ mod tests { assert_eq!(e.session().edit.selection.start, 3 * 24_000); } + #[test] + fn step_input_at_clip_boundary_uses_next_clip() { + let (mut e, t) = midi_engine(); + let sr = 48_000; + e.execute("midi.new_clip", &json!({"track": t.0, "start": 0, "end": sr})).unwrap(); + let second = e.execute("midi.new_clip", &json!({"track": t.0, "start": sr, "end": 2 * sr})).unwrap()["clip"].as_u64().unwrap(); + e.execute("edit.select", &json!({"tracks": [t.0], "start": sr})).unwrap(); + let r = e.execute("event.step_input", &json!({"track": t.0, "pitch": 60, "duration": "1/4"})).unwrap(); + assert_eq!(r["clip"].as_u64(), Some(second)); + let mut ranges: Vec<(i64, i64)> = e.session().track(t).unwrap().clips().iter().map(|c| (c.start, c.end())).collect(); + ranges.sort(); + assert_eq!(ranges, vec![(0, sr), (sr, 2 * sr)]); + let c = e.session().find_clip(soundcraft_model::ClipId(second)).map(|(_, c)| c.clone()).unwrap(); + match &c.content { + ClipContent::Midi { sequence } => assert_eq!(sequence.notes.iter().map(|n| n.start).collect::>(), vec![0]), + _ => panic!(), + } + } + #[test] fn tempo_curves() { let mut e = Engine::default(); From eb4e2d21ba794d8d333eb5608c9b895bd715d9da Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:54:03 +0200 Subject: [PATCH 34/55] Make markers.edit apply its documented color parameter Found while testing crates/engine/src/cmd/event.rs:338 (edit_marker; params doc at line 97) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/event.rs | 41 +++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/event.rs b/crates/engine/src/cmd/event.rs index 205f038c..baf32f8e 100644 --- a/crates/engine/src/cmd/event.rs +++ b/crates/engine/src/cmd/event.rs @@ -94,7 +94,7 @@ pub fn specs() -> Vec { add_marker ), cmd!("markers.delete", "Delete Memory Location", [], None, "{number | id}", always, delete_marker), - cmd!("markers.edit", "Edit Memory Location", [], None, "{number | id, name?, comments?, at?, color?}", always, edit_marker), + cmd!("markers.edit", "Edit Memory Location", [], None, "{number | id, name?, comments?, at?, color?: [r,g,b] | null}", always, edit_marker), cmd!(noundo "markers.recall", "Recall Memory Location", [], Some("Period + n + Period"), "{number}", always, recall_marker), cmd!(noundo "markers.next", "Go to Next Marker", [], None, "{}", always, |e, _| step_marker(e, 1)), cmd!(noundo "markers.previous", "Go to Previous Marker", [], None, "{}", always, |e, _| step_marker(e, -1)), @@ -337,8 +337,23 @@ fn delete_marker(e: &mut Engine, p: &Value) -> Result { fn edit_marker(e: &mut Engine, p: &Value) -> Result { let i = marker_index(e, p, "markers.edit")?; let at = position_param(e, "markers.edit", p, "at")?; + let color = match p.get("color") { + None => None, + Some(Value::Null) => Some(None), + Some(Value::Array(a)) if a.len() == 3 => { + let ch = |i: usize| a.get(i).and_then(Value::as_u64).and_then(|v| u8::try_from(v).ok()); + match (ch(0), ch(1), ch(2)) { + (Some(r), Some(g), Some(b)) => Some(Some([r, g, b])), + _ => return Err(bad("markers.edit", "`color` components must be integers 0..=255")), + } + } + Some(_) => return Err(bad("markers.edit", "`color` must be [r,g,b] or null")), + }; let s = e.session_mut(); let Some(m) = s.markers.get_mut(i) else { return Err(bad("markers.edit", "gone")) }; + if let Some(c) = color { + m.color = c; + } // `name` is the documented new name; when it was the selector it already matches, so this is a no-op. if let Some(n) = str_param(p, "rename").or_else(|| str_param(p, "name")) { m.name = n.to_string(); @@ -443,6 +458,30 @@ mod tests { assert_eq!((bridge.name.as_str(), bridge.comments.as_str()), ("Bridge", "last")); } + /// `markers.edit` documents `color`: it sets the memory location's RGB colour, undoably; `null` clears it. + #[test] + fn edit_marker_sets_documented_color() { + let mut e = demo_engine(); + let n = e.session().markers.first().unwrap().number; + let color = |e: &crate::Engine| e.session().markers.iter().find(|m| m.number == n).unwrap().color; + assert_eq!(color(&e), None); + + e.execute("markers.edit", &json!({"number": n, "color": [255, 0, 0]})).unwrap(); + assert_eq!(color(&e), Some([255, 0, 0])); + assert!(e.undo()); + assert_eq!(color(&e), None); + assert!(e.redo()); + assert_eq!(color(&e), Some([255, 0, 0])); + + e.execute("markers.edit", &json!({"number": n, "color": null})).unwrap(); + assert_eq!(color(&e), None); + + for bad in [json!([1, 2]), json!([1, 2, 300]), json!("red"), json!([1, 2, -3])] { + assert!(e.execute("markers.edit", &json!({"number": n, "color": bad})).is_err()); + assert_eq!(color(&e), None); + } + } + /// Constant replaces the tempo over [start, end) only; the tempo at the old end tick carries on after it. #[test] fn tempo_constant_is_bounded_by_its_range() { From 0dfb0748204e5b7d4942a140a1935f098e6c70c4 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:07:39 +0000 Subject: [PATCH 35/55] Shift controller events by Delay in track.write_midi_rtp Found while testing crates/engine/src/cmd/track_more.rs:370-392 (write_rtp); playback side: crates/mix/src/lib.rs:1526-1527 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/track_more.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/crates/engine/src/cmd/track_more.rs b/crates/engine/src/cmd/track_more.rs index 290a8d5a..9f547597 100644 --- a/crates/engine/src/cmd/track_more.rs +++ b/crates/engine/src/cmd/track_more.rs @@ -383,6 +383,10 @@ fn write_rtp(e: &mut Engine, p: &Value) -> Result { n.start = n.start.saturating_add(r.delay_ticks).max(0); notes += 1; } + // Playback delays controller events too, but ignores those before tick 0 (hidden by a start trim). + for c in sequence.ctrls.iter_mut().filter(|c| c.tick >= 0) { + c.tick = c.tick.saturating_add(r.delay_ticks).max(0); + } sequence.sort(); } let pre = format!("rtp.{}.", t.0); @@ -635,6 +639,31 @@ mod tests { assert!(!e.session().edit.values.keys().any(|k| k.starts_with(&format!("rtp.{}.", keys.0)))); } + #[test] + fn rtp_delay_shifts_controller_events() { + use soundcraft_midi::{CtrlEvent, CtrlKind}; + let mut e = crate::demo::demo_engine(); + let keys = e.session().track_by_name("Keys").unwrap().id; + let s = e.session_mut(); + if let Some(pl) = s.track_mut(keys).and_then(Track::playlist_mut) + && let ClipContent::Midi { sequence } = &mut pl.clips[0].content + { + sequence.ctrls.clear(); + sequence.ctrls.push(CtrlEvent { tick: 100, channel: 0, kind: CtrlKind::Cc(7), value: 90 }); + sequence.ctrls.push(CtrlEvent { tick: 200, channel: 0, kind: CtrlKind::PitchBend, value: 10 }); + sequence.ctrls.push(CtrlEvent { tick: -30, channel: 0, kind: CtrlKind::Cc(1), value: 5 }); + } + s.edit.values.insert(track_key("rtp", keys, "delay"), 50.0); + e.execute("edit.select", &json!({"tracks": [keys.0]})).unwrap(); + e.execute("track.write_midi_rtp", &json!({})).unwrap(); + let ClipContent::Midi { sequence } = &e.session().track(keys).unwrap().clips()[0].content else { panic!() }; + let tick_of = |k: CtrlKind| sequence.ctrls.iter().find(|c| c.kind == k).map(|c| c.tick); + assert_eq!(tick_of(CtrlKind::Cc(7)), Some(150), "playback delayed the CC by 50 ticks; baked result must match"); + assert_eq!(tick_of(CtrlKind::PitchBend), Some(250)); + // Hidden by a start trim: playback ignores it, so baking leaves it alone. + assert_eq!(tick_of(CtrlKind::Cc(1)), Some(-30)); + } + #[test] fn trim_and_vca_coalesce() { let mut e = Engine::default(); From 29c014deef9abe85f9a23721490be9a5345b5332 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:01:15 +0200 Subject: [PATCH 36/55] Ask before a session opened in the browser replaces a modified one (fixes #432) Picking a session with Open Session or dropping one on the page in the browser replaced the document as soon as its bytes were read, without the Save Changes question the native Open goes through, so unsaved edits and their undo history were lost. The bytes now wait behind that question, asked when the read finishes: Don't Save opens them, Save saves first and then opens them, Cancel drops them and leaves the document alone. --- apps/soundcraft-web/src/web.rs | 20 +++------- crates/ui-egui/src/dialogs.rs | 46 ++++++++++++++++++++-- crates/ui-egui/tests/save_changes.rs | 58 +++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/apps/soundcraft-web/src/web.rs b/apps/soundcraft-web/src/web.rs index 3682fab7..0d9b6485 100644 --- a/apps/soundcraft-web/src/web.rs +++ b/apps/soundcraft-web/src/web.rs @@ -1,6 +1,5 @@ //! The browser shell: the eframe web runner around [`SoundApp`]. -use serde_json::json; use soundcraft_engine::{Engine, io}; use soundcraft_ui_egui::dialogs::Dialog; use soundcraft_ui_egui::sampler_window::SampleRead; @@ -166,30 +165,23 @@ impl WebShell { } } - fn handle(&mut self, purpose: Purpose, name: &str, bytes: &[u8]) { - if matches!(purpose, Purpose::Session) && soundcraft_ui_egui::bounce::refuses_session_switch(&mut self.app) { - return; - } + fn handle(&mut self, purpose: Purpose, name: String, bytes: Vec) { let e = &mut self.app.engine; let at = e.session().edit.selection.start.max(0); let result = match purpose { - Purpose::Session => { - let _ = self.app.run("transport.stop", json!({})); - io::open_session_bytes(&mut self.app.engine, name, bytes).map(|missing| { - if missing.is_empty() { format!("Opened {name}") } else { format!("Opened {name} ({} missing media)", missing.len()) } - }) - } + // Asks first when the open session has changes: the bytes wait for the answer. + Purpose::Session => return soundcraft_ui_egui::dialogs::open_session_bytes(&mut self.app, name, bytes), // Imports are edits: one undo step each, like the import commands. Purpose::Audio => { let before = e.session_arc(); - io::import_audio_bytes(e, name, bytes, None, None, at).map(|_| { + io::import_audio_bytes(e, &name, &bytes, None, None, at).map(|_| { e.push_undo("Import Audio", before); format!("Imported {name}") }) } Purpose::Midi => { let before = e.session_arc(); - io::import_midi_bytes(e, bytes, at, true).map(|_| { + io::import_midi_bytes(e, &bytes, at, true).map(|_| { e.push_undo("Import MIDI", before); format!("Imported {name}") }) @@ -205,7 +197,7 @@ impl eframe::App for WebShell { self.take_path_prompt(ctx); let pending = std::mem::take(&mut *self.inbox.borrow_mut()); for (purpose, name, bytes) in pending { - self.handle(purpose, &name, &bytes); + self.handle(purpose, name, bytes); } let samples = std::mem::take(&mut *self.samples.borrow_mut()); for (read, bytes) in samples { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 713e94b7..7b315ad5 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -34,6 +34,8 @@ pub struct Dialogs { pub open: Option, /// Runs once the session has been saved from `SaveChanges`. after_save: Option<(String, Value)>, + /// A session the browser read while the open one had changes, waiting for `SaveChanges`. + pending_session: Option<(String, Vec)>, } impl Dialogs { @@ -155,7 +157,7 @@ impl Dialogs { /// When `id` would close a session with unsaved changes, ask first; true when the question is /// open instead. A take in progress is stopped first, so it counts as a change. pub fn ask_to_save(app: &mut SoundApp, id: &str, params: &Value) -> bool { - if !matches!(id, "session.new" | "session.open" | "session.close" | "session.revert" | "app.quit") + if !matches!(id, "session.new" | "session.open" | "session.close" | "session.revert" | "app.quit" | OPEN_PENDING) || soundcraft_engine::find_command(id).is_some_and(|c| (c.enabled)(&app.engine).is_err()) { return false; @@ -178,6 +180,34 @@ pub fn run_or_ask(app: &mut SoundApp, id: &str, params: Value) -> Result) { + if crate::bounce::refuses_session_switch(app) { + return; + } + app.dialogs.pending_session = Some((name, bytes)); + if !ask_to_save(app, OPEN_PENDING, &json!({})) { + open_pending_session(app); + } +} + +fn open_pending_session(app: &mut SoundApp) { + let Some((name, bytes)) = app.dialogs.pending_session.take() else { return }; + if crate::bounce::refuses_session_switch(app) { + return; + } + let _ = app.run("transport.stop", json!({})); + app.ui.status = match soundcraft_engine::io::open_session_bytes(&mut app.engine, &name, &bytes) { + Ok(missing) if missing.is_empty() => format!("Opened {name}"), + Ok(missing) => format!("Opened {name} ({} missing media)", missing.len()), + Err(e) => e.to_string(), + }; +} + /// A path prompt for any command whose parameters start with a required `path` or `dir` (plus /// Print Score, whose path is optional), with a default file name from the command. pub fn takes_path(id: &str) -> bool { @@ -594,6 +624,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { if esc || ctx.memory(|m| m.data.get_temp::(egui::Id::new("dlg_cancel")).unwrap_or(false)) { keep = false; app.dialogs.after_save = None; + app.dialogs.pending_session = None; ctx.memory_mut(|m| m.data.remove::(egui::Id::new("dlg_cancel"))); } if let Some(then) = save_first { @@ -613,7 +644,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { let saving = action.as_ref().is_some_and(|(cmd, _)| cmd.starts_with("session.save")); if let Some((cmd, params)) = action { keep = false; - match app.run(&cmd, params) { + match run_then(app, &cmd, params) { Ok(v) if cmd == "file.export_clips" => { app.ui.status = format!("Exported {} clip(s) to {}", v["written"], v["dir"].as_str().unwrap_or_default()); } @@ -624,7 +655,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { if saving && let Some((then, params)) = app.dialogs.after_save.take() && !app.engine.is_dirty() - && let Err(e) = app.run(&then, params) + && let Err(e) = run_then(app, &then, params) { app.ui.status = e; } @@ -633,6 +664,15 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { } } +/// Runs a dialog's command, or what `SaveChanges` carries on with. +fn run_then(app: &mut SoundApp, cmd: &str, params: Value) -> Result { + if cmd == OPEN_PENDING { + open_pending_session(app); + return Ok(Value::Null); + } + app.run(cmd, params) +} + /// Cancel/OK row; returns true when OK was pressed (or Enter). fn buttons(ui: &mut egui::Ui, ok: &str, enter: bool) -> bool { let mut pressed = enter; diff --git a/crates/ui-egui/tests/save_changes.rs b/crates/ui-egui/tests/save_changes.rs index e500a8d5..caa0561d 100644 --- a/crates/ui-egui/tests/save_changes.rs +++ b/crates/ui-egui/tests/save_changes.rs @@ -2,7 +2,7 @@ use egui_kittest::Harness; use egui_kittest::kittest::Queryable; use serde_json::json; use soundcraft_ui_egui::dialogs::Dialog; -use soundcraft_ui_egui::{Services, SoundApp, menus}; +use soundcraft_ui_egui::{Services, SoundApp, dialogs, menus}; fn harness() -> Harness<'static, SoundApp> { let app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); @@ -95,3 +95,59 @@ fn save_in_the_question_saves_before_closing() { assert!(!h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute); let _ = std::fs::remove_dir_all(&dir); } + +/// A session the browser read: an empty one, so opening it is easy to see. +fn session_bytes() -> Vec { + soundcraft_engine::Engine::default().session().to_json().unwrap().into_bytes() +} + +fn open_bytes(h: &mut Harness<'_, SoundApp>) { + dialogs::open_session_bytes(h.state_mut(), "Other.scraft".into(), session_bytes()); + steps(h); +} + +#[test] +fn a_session_read_in_the_browser_asks_before_replacing_a_modified_one() { + let mut h = harness(); + open_bytes(&mut h); + assert!(h.state().dialogs.open.is_none(), "an unmodified session is replaced without asking"); + assert!(h.state().engine.session().tracks.is_empty()); + + let mut h = harness(); + let tracks = h.state().engine.session().tracks.len(); + mute_kick(&mut h); + open_bytes(&mut h); + assert_eq!(h.state().dialogs.open_name(), Some("save_changes")); + click(&mut h, "Cancel"); + assert!(h.state().dialogs.open.is_none()); + assert_eq!(h.state().engine.session().tracks.len(), tracks); + h.state_mut().run("edit.undo", json!({})).unwrap(); + assert!(!h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute, "Cancel keeps the undo history"); + + mute_kick(&mut h); + open_bytes(&mut h); + click(&mut h, "Don't Save"); + assert!(h.state().engine.session().tracks.is_empty()); +} + +#[test] +fn save_in_the_question_saves_before_opening_a_session_read_in_the_browser() { + let dir = std::env::temp_dir().join(format!("soundcraft-save-changes-bytes-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let path = dir.join("Song.scraft").to_string_lossy().into_owned(); + + let mut h = harness(); + mute_kick(&mut h); + open_bytes(&mut h); + click(&mut h, "Save"); + match &mut h.state_mut().dialogs.open { + Some(Dialog::PathPrompt { cmd, path: p, .. }) if cmd == "session.save_as" => p.clone_from(&path), + other => panic!("expected Save As, got {other:?}"), + } + steps(&mut h); + click(&mut h, "OK"); + assert!(h.state().engine.session().tracks.is_empty()); + h.state_mut().run("session.open", json!({"path": path})).unwrap(); + assert!(h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute); + let _ = std::fs::remove_dir_all(&dir); +} From 9477e7cb08ae51fdd19e41c48430f2e57885e1c6 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:20:21 +0200 Subject: [PATCH 37/55] Contributors: add my name for the About window --- contributors/people.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 1c344199..7799757f 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -50,3 +50,8 @@ sync_github_name = false [people.Momentum96] sync_github_name = true + +[people.marcoarnulfo] +real_name = "Marco Arnulfo" +display_name = "Marco Arnulfo" +sync_github_name = false From 6594badf377958725befde8313624726e28a7ca4 Mon Sep 17 00:00:00 2001 From: Audiosyncrasy <4401511+Audiosyncrasy@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:34:32 -0400 Subject: [PATCH 38/55] Modify Group: edit a group's name, tracks and Edit/Mix linking A track group's name, membership and Edit/Mix flags could only be set when it was created: nothing in the engine or the UI could change them afterwards, so a typo in a group name was permanent. Add track.group_modify {group: id|name, name?, tracks?, edit?, mix?}. Omitted fields keep their value. `tracks` replaces membership only when it is an array, so a mistyped param cannot silently swap the members for whatever happens to be selected; it dedupes, rejects an empty group, and rejects an empty or duplicate name, since find_group resolves by name. The group's letter is left alone: it is the stable id in the Groups list. Dialog::Group gains an optional group id. None creates, as before; Some pre-fills from the group and dispatches track.group_modify, so one dialog serves both. Reached from Modify Group... in the Groups list context menu. OK is now disabled while no track is checked, since both commands reject an empty group and the old OK could only raise an error. The command has no menu path, so it adds no menu leaf and the parity checklist is unchanged. --- crates/engine/src/cmd/track.rs | 79 ++++++++++++++++++++++++++++ crates/ui-egui/src/dialogs.rs | 50 ++++++++++++++---- crates/ui-egui/src/panels.rs | 4 ++ crates/ui-egui/tests/group_dialog.rs | 41 +++++++++++++++ 4 files changed, 163 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/track.rs b/crates/engine/src/cmd/track.rs index 87912389..cc496448 100644 --- a/crates/engine/src/cmd/track.rs +++ b/crates/engine/src/cmd/track.rs @@ -19,6 +19,7 @@ pub fn specs() -> Vec { cmd!("track.group", "Group...", ["Track"], Some("Cmd+G"), "{name?, tracks?, edit?: true, mix?: true}", has_selection, group), cmd!("track.ungroup", "Delete Group", [], None, "{group: id|name}", always, delete_group), cmd!("track.group_toggle", "Toggle Group Active", [], None, "{group: id|name}", always, toggle_group), + cmd!("track.group_modify", "Modify Group", [], None, "{group: id|name, name?, tracks?, edit?, mix?}", always, modify_group), cmd!( "track.duplicate", "Duplicate...", @@ -320,6 +321,55 @@ fn toggle_group(e: &mut Engine, p: &Value) -> Result { Ok(json!({"active": active})) } +/// Change an existing group's name, membership and Edit/Mix linking. Omitted fields keep their value. +fn modify_group(e: &mut Engine, p: &Value) -> Result { + const CMD: &str = "track.group_modify"; + let g = find_group(e, p, CMD)?; + // Absent `tracks` leaves membership alone; present, it replaces it. Only an array counts, so a + // mistyped param can't silently swap the members for whatever happens to be selected. + let members = match p.get("tracks") { + None => None, + Some(Value::Array(_)) => { + let mut t = tracks_param(e, CMD, p)?; + // A track listed twice would take every linked move twice over. + let mut seen = std::collections::HashSet::new(); + t.retain(|id| seen.insert(*id)); + if t.is_empty() { + return Err(bad(CMD, "a group needs at least one track")); + } + Some(t) + } + Some(_) => return Err(bad(CMD, "`tracks` must be an array of track ids or names")), + }; + let name = match str_param(p, "name") { + None => None, + Some(n) => { + let n = n.trim(); + if n.is_empty() { + return Err(bad(CMD, "`name` must not be empty")); + } + // `find_group` resolves by name, so duplicates would make a group unaddressable. + if e.session().groups.iter().any(|x| x.name == n && x.id != g) { + return Err(bad(CMD, format!("a group named `{n}` already exists"))); + } + let mut n = n.to_string(); + soundcraft_model::truncate_chars(&mut n, soundcraft_model::MAX_NAME_CHARS); + Some(n) + } + }; + let s = e.session_mut(); + let Some(gr) = s.groups.iter_mut().find(|x| x.id == g) else { return Err(bad(CMD, "unknown `group`")) }; + if let Some(n) = name { + gr.name = n; + } + if let Some(m) = members { + gr.members = m; + } + gr.edit = bool_or(p, "edit", gr.edit); + gr.mix = bool_or(p, "mix", gr.mix); + Ok(json!({"group": g, "name": gr.name, "tracks": gr.members, "edit": gr.edit, "mix": gr.mix})) +} + fn duplicate(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "track.duplicate", p)?; let (pl, auto, ins, snd) = @@ -872,6 +922,35 @@ mod tests { assert!(e.execute("track.playlist_promote", &json!({"track": kick, "playlist": 1, "start": 0, "end": 10})).is_err()); } + #[test] + fn group_modify_changes_name_membership_and_linking() { + let mut e = crate::demo::demo_engine(); + let g = e.execute("track.group", &json!({"tracks": ["Kick", "Snare"], "name": "Drums"})).unwrap()["group"].as_u64().unwrap(); + // Every field at once, the way the dialog submits it. + let r = e.execute("track.group_modify", &json!({"group": g, "name": "Rhythm", "tracks": ["Kick", "Bass"], "edit": false})).unwrap(); + assert_eq!(r["name"], "Rhythm"); + let gr = e.session().groups.iter().find(|x| x.id.0 == g).unwrap().clone(); + assert_eq!(gr.name, "Rhythm"); + assert_eq!(gr.members, vec![e.session().track_by_name("Kick").unwrap().id, e.session().track_by_name("Bass").unwrap().id]); + assert!(!gr.edit); + assert!(gr.mix, "an omitted field keeps its value"); + assert_eq!(gr.letter, 'a', "the letter is the group's stable id and never changes"); + // Omitting `tracks` leaves membership alone rather than falling back to the selection. + e.execute("edit.select", &json!({"tracks": ["Pad"], "exact": true})).unwrap(); + e.execute("track.group_modify", &json!({"group": g, "name": "Rhythm Section"})).unwrap(); + assert_eq!(e.session().groups.iter().find(|x| x.id.0 == g).unwrap().members, gr.members); + // Resolving the group by its new name keeps working, so names stay unique and non-empty. + e.execute("track.group", &json!({"tracks": ["Pad"], "name": "Pads"})).unwrap(); + assert!(e.execute("track.group_modify", &json!({"group": "Pads", "name": "Rhythm Section"})).is_err()); + assert!(e.execute("track.group_modify", &json!({"group": g, "name": " "})).is_err()); + assert!(e.execute("track.group_modify", &json!({"group": g, "tracks": []})).is_err()); + assert!(e.execute("track.group_modify", &json!({"group": g, "tracks": "Kick"})).is_err(), "`tracks` must be an array"); + assert!(e.execute("track.group_modify", &json!({"group": g, "tracks": ["Kick", "Kick"]})).is_ok()); + assert_eq!(e.session().groups.iter().find(|x| x.id.0 == g).unwrap().members.len(), 1, "a repeated track is listed once"); + assert!(e.execute("track.group_modify", &json!({"group": 9999, "name": "x"})).is_err()); + assert_eq!(e.session().groups.iter().find(|x| x.id.0 == g).unwrap().name, "Rhythm Section", "a rejected call changes nothing"); + } + #[test] fn folder_routes_members_through_its_bus() { let mut e = crate::demo::demo_engine(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 713e94b7..7b43afcb 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -5,7 +5,7 @@ use crate::theme::{Tokens, bold}; use egui::{Align2, vec2}; use serde_json::{Value, json}; use soundcraft_engine::Engine; -use soundcraft_model::{ClipId, Session, SourceId, TrackId}; +use soundcraft_model::{ClipId, GroupId, Session, SourceId, TrackId}; use soundcraft_time::Samples; #[derive(Debug, Clone)] @@ -23,7 +23,8 @@ pub enum Dialog { Spot(crate::spot::Spot), Fades { shape: String }, StripSilence { threshold: f64, min_ms: f64, pre_ms: f64, post_ms: f64 }, - Group { name: String, edit: bool, mix: bool, members: Vec<(u64, String, bool)> }, + // `id: None` creates (Track › Group…); `id: Some` modifies that group (Groups list › Modify Group…). + Group { id: Option, name: String, edit: bool, mix: bool, members: Vec<(u64, String, bool)> }, Session { frame_rate: String, bit_depth: String }, ScoreSetup { title: String, composer: String, bars_per_system: u32, show_track_names: bool }, SaveChanges { then: (String, Value) }, @@ -52,7 +53,8 @@ impl Dialogs { Dialog::Spot(_) => "spot", Dialog::Fades { .. } => "fades", Dialog::StripSilence { .. } => "strip_silence", - Dialog::Group { .. } => "group", + Dialog::Group { id: None, .. } => "group", + Dialog::Group { id: Some(_), .. } => "modify_group", Dialog::Session { .. } => "session", Dialog::ScoreSetup { .. } => "score_setup", Dialog::SaveChanges { .. } => "save_changes", @@ -71,6 +73,16 @@ impl Dialogs { self.open = Some(Dialog::RenameMarker { id, name: name.to_string() }); } + /// Groups list › Modify Group…: the creation dialog, pre-filled from an existing group. False + /// when the group is gone (deleted between the click and the menu). + pub fn open_modify_group(&mut self, e: &Engine, id: GroupId) -> bool { + let s = e.session(); + let Some(g) = s.group(id) else { return false }; + let members = s.tracks.iter().map(|t| (t.id.0, t.name.clone(), g.members.contains(&t.id))).collect(); + self.open = Some(Dialog::Group { id: Some(id), name: g.name.clone(), edit: g.edit, mix: g.mix, members }); + true + } + /// Clip List › Clear: confirm before clips and files leave the session. False when there is nothing to clear. pub fn open_clear(&mut self, e: &Engine, clips: Vec, sources: Vec) -> bool { let Some(message) = clear_message(e.session(), &clips, &sources) else { return false }; @@ -131,7 +143,7 @@ impl Dialogs { "track.group" => { let s = e.session(); let members = s.tracks.iter().map(|t| (t.id.0, t.name.clone(), s.edit.selected_tracks.contains(&t.id))).collect(); - Dialog::Group { name: format!("Group {}", s.groups.len() + 1), edit: true, mix: true, members } + Dialog::Group { id: None, name: format!("Group {}", s.groups.len() + 1), edit: true, mix: true, members } } "setup.session" => Dialog::Session { frame_rate: e.session().frame_rate.label().into(), bit_depth: "24".into() }, "file.score_setup" => { @@ -283,7 +295,8 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { Dialog::Spot(_) => "Spot", Dialog::Fades { .. } => "Fades", Dialog::StripSilence { .. } => "Strip Silence", - Dialog::Group { .. } => "Create Group", + Dialog::Group { id: None, .. } => "Create Group", + Dialog::Group { id: Some(_), .. } => "Modify Group", Dialog::Session { .. } => "Session Setup", Dialog::ScoreSetup { .. } => "Score Setup", Dialog::SaveChanges { then } if then.0 == "session.revert" => "Revert to Saved", @@ -508,7 +521,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { )); } } - Dialog::Group { name, edit, mix, members } => { + Dialog::Group { id, name, edit, mix, members } => { ui.horizontal(|ui| { ui.label("Name"); ui.text_edit_singleline(name); @@ -523,9 +536,18 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { crate::widgets::checkbox(ui, on, n.as_str()); } }); - if buttons(ui, "OK", enter) { - let ids: Vec = members.iter().filter(|m| m.2).map(|m| m.0).collect(); - action = Some(("track.group".into(), json!({"name": name, "edit": edit, "mix": mix, "tracks": ids}))); + let ids: Vec = members.iter().filter(|m| m.2).map(|m| m.0).collect(); + // Both commands reject an empty group, so don't offer an OK that can only fail. + if buttons_enabled(ui, "OK", enter, !ids.is_empty()) { + let mut params = json!({"name": name, "edit": edit, "mix": mix, "tracks": ids}); + let cmd = match id { + Some(g) => { + params["group"] = json!(g.0); + "track.group_modify" + } + None => "track.group", + }; + action = Some((cmd.into(), params)); } } Dialog::Session { frame_rate, bit_depth } => { @@ -635,13 +657,19 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { /// Cancel/OK row; returns true when OK was pressed (or Enter). fn buttons(ui: &mut egui::Ui, ok: &str, enter: bool) -> bool { - let mut pressed = enter; + buttons_enabled(ui, ok, enter, true) +} + +/// `buttons`, but a greyed-out OK when the dialog's values can't be submitted yet. Cancel always works. +fn buttons_enabled(ui: &mut egui::Ui, ok: &str, enter: bool, ready: bool) -> bool { + let mut pressed = enter && ready; ui.separator(); ui.horizontal(|ui| { if ui.button("Cancel").clicked() { ui.ctx().memory_mut(|m| m.data.insert_temp(egui::Id::new("dlg_cancel"), true)); } - if ui.add(egui::Button::new(egui::RichText::new(ok).strong()).fill(Tokens::current().accent)).clicked() { + let b = egui::Button::new(egui::RichText::new(ok).strong()).fill(Tokens::current().accent); + if ui.add_enabled(ready, b).clicked() { pressed = true; } }); diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index dda31ce0..de46e336 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -133,6 +133,10 @@ pub fn tracks_and_groups(app: &mut SoundApp, ui: &mut Ui) { let _ = app.run("track.group_toggle", json!({"group": id})); ui.close(); } + if ui.button("Modify Group…").clicked() { + let _ = app.dialogs.open_modify_group(&app.engine, soundcraft_model::GroupId(id)); + ui.close(); + } if ui.button("Delete Group").clicked() { let _ = app.run("track.ungroup", json!({"group": id})); ui.close(); diff --git a/crates/ui-egui/tests/group_dialog.rs b/crates/ui-egui/tests/group_dialog.rs index e91bfec1..bd17ede9 100644 --- a/crates/ui-egui/tests/group_dialog.rs +++ b/crates/ui-egui/tests/group_dialog.rs @@ -42,3 +42,44 @@ fn dragging_down_the_group_dialogs_track_list_adds_each_track() { let members: Vec = session.groups.last().map(|g| g.members.iter().map(|m| m.0).collect()).unwrap_or_default(); assert_eq!(members, tracks[..4].iter().map(|t| t.0).collect::>()); } + +/// The Groups list's Modify Group… reopens the creation dialog on an existing group: it comes up +/// pre-filled, and OK edits that group in place instead of adding a second one. +#[test] +fn modify_group_dialog_edits_the_existing_group() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let tracks: Vec<(u64, String)> = app.engine.session().tracks.iter().map(|t| (t.id.0, t.name.clone())).collect(); + assert!(tracks.len() >= 2); + let g = app.run("track.group", json!({"tracks": [tracks[0].0], "name": "Drums"})).unwrap()["group"].as_u64().unwrap(); + + assert!(app.dialogs.open_modify_group(&app.engine, soundcraft_model::GroupId(g))); + assert_eq!(app.dialogs.open_name(), Some("modify_group")); + // Pre-filled from the group, not from the track selection. + let Some(soundcraft_ui_egui::dialogs::Dialog::Group { id, name, edit, mix, members }) = &app.dialogs.open else { + panic!("Modify Group… did not open the group dialog"); + }; + assert_eq!((id.map(|i| i.0), name.as_str(), *edit, *mix), (Some(g), "Drums", true, true)); + assert_eq!(members.iter().filter(|m| m.2).map(|m| m.0).collect::>(), vec![tracks[0].0]); + + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_ui_state( + |ui, app: &mut SoundApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ); + h.run_steps(3); + // Add a second track and submit. + h.get_by_role_and_label(Role::CheckBox, &tracks[1].1).click(); + h.run_steps(2); + h.get_by_role_and_label(Role::Button, "OK").click(); + h.run_steps(2); + + let session = h.state().engine.session(); + assert_eq!(session.groups.len(), 1, "Modify edits the group in place rather than creating another"); + let gr = &session.groups[0]; + assert_eq!(gr.id.0, g); + assert_eq!(gr.name, "Drums"); + assert_eq!(gr.members.iter().map(|m| m.0).collect::>(), vec![tracks[0].0, tracks[1].0]); +} From d484a716adebf3bf9489fcc774c5a0456e0debe4 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 04:34:32 +0200 Subject: [PATCH 39/55] Tracks and Groups list rows publish their names to AccessKit (fixes #651) --- crates/ui-egui/src/panels.rs | 3 ++ .../ui-egui/tests/mix_instrument_contrast.rs | 2 +- crates/ui-egui/tests/side_lists_access.rs | 54 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/tests/side_lists_access.rs diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index dda31ce0..10c6d74f 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -66,6 +66,7 @@ pub fn tracks_and_groups(app: &mut SoundApp, ui: &mut Ui) { let out = lists.show(ui, |ui| { for (id, name, shown, color, sel) in tracks { let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 17.0), Sense::click_and_drag()); + resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::SelectableLabel, true, sel, &name)); if sel { ui.painter().rect_filled(r, 0.0, t.row_selected); } @@ -94,6 +95,7 @@ pub fn tracks_and_groups(app: &mut SoundApp, ui: &mut Ui) { let (r, _) = ui.allocate_exact_size(vec2(ui.available_width(), 17.0), Sense::hover()); ui.painter().text(pos2(r.min.x + 30.0, r.center().y), Align2::LEFT_CENTER, "", regular(11.5).clone(), t.text); let all = ui.interact(r, ui.id().with("group_all"), Sense::click()); + all.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, "")); if all.double_clicked() { let _ = app.run("edit.select_all", json!({})); } @@ -105,6 +107,7 @@ pub fn tracks_and_groups(app: &mut SoundApp, ui: &mut Ui) { }); for (id, letter, name, active, color) in groups { let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 17.0), Sense::click()); + resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::Checkbox, true, active, &name)); if active { ui.painter().rect_filled(Rect::from_min_size(pos2(r.min.x + 4.0, r.min.y + 2.0), vec2(14.0, 13.0)), 1.0, rgb(color)); } diff --git a/crates/ui-egui/tests/mix_instrument_contrast.rs b/crates/ui-egui/tests/mix_instrument_contrast.rs index 663cab3e..82ad099a 100644 --- a/crates/ui-egui/tests/mix_instrument_contrast.rs +++ b/crates/ui-egui/tests/mix_instrument_contrast.rs @@ -177,7 +177,7 @@ fn populated_mix_instrument_uses_readable_palette_in_real_frames() { harness.step(); } harness.step(); // Let the popup finish its initial sizing pass. - harness.get_by_label_contains("Built-in").hover(); + harness.get_by_label("Built-in ⏵").hover(); harness.step(); harness.step(); harness.get_by_label("Drum Synth").click(); diff --git a/crates/ui-egui/tests/side_lists_access.rs b/crates/ui-egui/tests/side_lists_access.rs new file mode 100644 index 00000000..c44bc4ec --- /dev/null +++ b/crates/ui-egui/tests/side_lists_access.rs @@ -0,0 +1,54 @@ +//! #651: the Tracks and Groups lists publish their rows to AccessKit, by name, so screen readers +//! can read them and tests can reach them by label instead of by geometry. + +use egui::accesskit::{Role, Toggled}; +use egui::vec2; +use egui_kittest::{ + Harness, + kittest::{NodeT, Queryable}, +}; +use serde_json::json; +use soundcraft_ui_egui::{Services, SoundApp}; + +fn harness() -> Harness<'static, SoundApp> { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let first = app.engine.session().tracks.first().map(|t| t.id.0).unwrap_or_default(); + app.run("track.group", json!({"name": "Rhythm", "tracks": [first]})).unwrap(); + app.engine.session_mut().edit.selected_tracks.clear(); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_ui_state( + |ui, app: &mut SoundApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ); + h.run_steps(3); + h +} + +#[test] +fn tracks_list_rows_are_named_and_report_their_selection() { + let mut h = harness(); + let tracks: Vec<(u64, String)> = h.state().engine.session().tracks.iter().map(|t| (t.id.0, t.name.clone())).collect(); + assert!(tracks.len() >= 2); + for (_, name) in &tracks { + assert_eq!(h.get_by_role_and_label(Role::Button, name).accesskit_node().toggled(), Some(Toggled::False), "{name}"); + } + let (id, name) = &tracks[1]; + h.get_by_role_and_label(Role::Button, name).click(); + h.run_steps(2); + assert_eq!(h.state().engine.session().edit.selected_tracks.iter().map(|t| t.0).collect::>(), vec![*id]); + assert_eq!(h.get_by_role_and_label(Role::Button, name).accesskit_node().toggled(), Some(Toggled::True)); +} + +#[test] +fn groups_list_rows_are_named_and_report_whether_the_group_is_active() { + let mut h = harness(); + h.get_by_label(""); + assert_eq!(h.get_by_role_and_label(Role::CheckBox, "Rhythm").accesskit_node().toggled(), Some(Toggled::True)); + h.get_by_role_and_label(Role::CheckBox, "Rhythm").click(); + h.run_steps(2); + assert!(h.state().engine.session().groups.iter().all(|g| !g.active)); + assert_eq!(h.get_by_role_and_label(Role::CheckBox, "Rhythm").accesskit_node().toggled(), Some(Toggled::False)); +} From 18dd68eb1d78617a2179740c86ef48dd3a420f3a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 15 Oct 2026 23:41:52 -0400 Subject: [PATCH 40/55] ci: switch to shared nix workflow from craft-common-actions --- .github/workflows/nix.yml | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 18392587..01cb9cd6 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -18,16 +18,6 @@ concurrency: cancel-in-progress: true jobs: - eval: - runs-on: ubuntu-latest - timeout-minutes: 15 - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: cachix/install-nix-action@13d8dd58da0234aa297dedd986986ccb8e7f3e24 # v31.11.1 - - name: nix flake check --no-build - run: | - if [ -f flake.nix ]; then - nix flake check --no-build --all-systems - else - echo "No flake.nix present in this repository; skipping check." - fi + nix: + # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. + uses: storytold/craft-common-actions/.github/workflows/nix.yml@main From f2ae345f2022e2da42935f5a7c035f53f4107bd2 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 07:43:12 +0200 Subject: [PATCH 41/55] Clip > Remove Sync Point takes the sync point off the selected clips --- crates/engine/src/cmd/clip.rs | 94 ++++++++++++++++++++ crates/ui-egui/src/menus.rs | 5 ++ crates/ui-egui/tests/command_reachability.rs | 18 ++++ 3 files changed, 117 insertions(+) diff --git a/crates/engine/src/cmd/clip.rs b/crates/engine/src/cmd/clip.rs index 9d68dc25..aff6d8cc 100644 --- a/crates/engine/src/cmd/clip.rs +++ b/crates/engine/src/cmd/clip.rs @@ -102,6 +102,7 @@ pub fn specs() -> Vec { } Ok(json!({})) }), + cmd!("clip.remove_sync_point", "Remove Sync Point", ["Clip"], None, "{clips?}", has_sync_point, remove_sync_point), cmd!("clip.quantize_to_grid", "Quantize to Grid", ["Clip"], Some("Cmd+0"), "{clips?}", has_selection, quantize_to_grid), cmd!("clip.color", "Clip Color", [], None, "{clips?, color: [r,g,b] | null}", has_selection, |e, p| { let ids = clip_ids_param(e, p); @@ -348,6 +349,31 @@ fn nudge_gain(e: &mut Engine, p: &Value, db: f64) -> Result { gain(e, &json!({"clips": clips, "delta_db": db})) } +/// Sync point 0 is "none" (edit_window only draws one when > 0). +fn has_sync_point(e: &Engine) -> std::result::Result<(), String> { + let s = e.session(); + if clip_ids_param(e, &json!({})).iter().any(|id| s.find_clip(*id).is_some_and(|(_, c)| c.sync_point != 0)) { + Ok(()) + } else { + Err("no selected clip has a sync point".into()) + } +} + +fn remove_sync_point(e: &mut Engine, p: &Value) -> Result { + let s = e.session(); + // Only clips that have a point are touched, so a call that changes nothing leaves no undo step. + let ids: Vec = clip_ids_param(e, p).into_iter().filter(|id| s.find_clip(*id).is_some_and(|(_, c)| c.sync_point != 0)).collect(); + if !ids.is_empty() { + let s = e.session_mut(); + for id in &ids { + if let Some(c) = s.find_clip_mut(*id) { + c.sync_point = 0 + } + } + } + Ok(json!({"clips": ids.len()})) +} + fn quantize_to_grid(e: &mut Engine, p: &Value) -> Result { let ids = clip_ids_param(e, p); let s = e.session(); @@ -452,4 +478,72 @@ mod tests { e.execute("clip.gain_nudge_up", &json!({})).unwrap(); assert_eq!(gain(&e), 36.0); } + + /// Three 10 000-sample clips side by side on one track; sync points are set by each test. + fn three_clips() -> (crate::Engine, [ClipId; 3]) { + use soundcraft_model::{ChannelFormat, Clip, Session, TrackKind}; + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, Some("Dialog")); + let buf = soundcraft_audio_io::AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.1; 10_000]] }; + let src = crate::io::add_source(&mut s, "Line", buf, None, soundcraft_audio_io::FileFormat::Wav); + let ids = [0, 1, 2].map(|k| { + let cid = s.new_clip_id(); + crate::edit::place_clip(&mut s, t, Clip::audio(cid, "Line", src, 0, k * 12_000, 10_000)); + cid + }); + (crate::Engine::new(s), ids) + } + + fn sync_of(e: &crate::Engine, id: ClipId) -> i64 { + e.session().find_clip(id).unwrap().1.sync_point + } + + #[test] + fn remove_sync_point_clears_the_point_identify_set() { + let (mut e, [a, ..]) = three_clips(); + e.execute("edit.select", &json!({"clips": [a.0]})).unwrap(); + e.execute("clip.identify_sync_point", &json!({"at": 4_000})).unwrap(); + assert_eq!(sync_of(&e, a), 4_000); + e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(sync_of(&e, a), 0); + } + + #[test] + fn remove_sync_point_clears_every_selected_clip_and_only_those() { + let (mut e, [a, b, c]) = three_clips(); + for (id, at) in [(a, 1_000), (b, 2_000), (c, 3_000)] { + e.session_mut().find_clip_mut(id).unwrap().sync_point = at; + } + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + let r = e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(r["clips"], 2); + assert_eq!([sync_of(&e, a), sync_of(&e, b), sync_of(&e, c)], [0, 0, 3_000]); + } + + #[test] + fn remove_sync_point_is_disabled_unless_a_selected_clip_has_one() { + let (mut e, [a, b, c]) = three_clips(); + e.session_mut().find_clip_mut(c).unwrap().sync_point = 3_000; + let enabled = |e: &crate::Engine| crate::find_command("clip.remove_sync_point").unwrap().info(e).enabled; + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + assert!(!enabled(&e), "neither selected clip has a sync point (the unselected one does)"); + e.execute("edit.select", &json!({"clips": [b.0, c.0]})).unwrap(); + assert!(enabled(&e)); + } + + #[test] + fn remove_sync_point_is_one_undo_step_that_puts_the_points_back() { + let (mut e, [a, b, _]) = three_clips(); + for (id, at) in [(a, 1_000), (b, 2_000)] { + e.session_mut().find_clip_mut(id).unwrap().sync_point = at; + } + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + let steps = e.undo_history().len(); + e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(e.undo_history().len(), steps + 1); + assert_eq!(e.undo_label(), Some("Remove Sync Point")); + assert_eq!([sync_of(&e, a), sync_of(&e, b)], [0, 0]); + assert!(e.undo()); + assert_eq!([sync_of(&e, a), sync_of(&e, b)], [1_000, 2_000]); + } } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9eef0b22..fbe274f1 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -202,6 +202,11 @@ pub fn tree() -> Vec { insert_path(&mut roots, line); } insert_path(&mut roots, "Window > Reset Panel Layout"); + // Not in the observed catalog either; Pro Tools lists it right under Identify Sync Point. + if let Some(clip) = roots.iter_mut().find(|n| n.label == "Clip") { + let at = clip.children.iter().position(|n| n.label == "Identify Sync Point").map_or(clip.children.len(), |i| i + 1); + clip.children.insert(at, MenuNode { label: "Remove Sync Point".into(), path: "Clip > Remove Sync Point".into(), children: Vec::new() }); + } // Not in the incumbent catalog, but the only UI path to an empty MIDI clip (#282). // It creates a clip rather than acting on the selected ones, so it leads the Clip menu. insert_path(&mut roots, "Clip > New MIDI Clip"); diff --git a/crates/ui-egui/tests/command_reachability.rs b/crates/ui-egui/tests/command_reachability.rs index 5328947b..ce7c6346 100644 --- a/crates/ui-egui/tests/command_reachability.rs +++ b/crates/ui-egui/tests/command_reachability.rs @@ -103,3 +103,21 @@ fn new_midi_clip_is_in_the_clip_menu() { let clip = tree.iter().find(|n| n.label == "Clip").unwrap(); assert_eq!(clip.children[0].label, "New MIDI Clip", "creates rather than edits, so it comes first"); } + +#[test] +fn remove_sync_point_follows_identify_and_is_greyed_out_without_a_point() { + let tree = menus::tree(); + let clip = tree.iter().find(|n| n.label == "Clip").unwrap(); + let labels: Vec<&str> = clip.children.iter().map(|n| n.label.as_str()).collect(); + let at = labels.iter().position(|l| *l == "Identify Sync Point").unwrap(); + assert_eq!(labels.get(at + 1), Some(&"Remove Sync Point")); + let path = &clip.children[at + 1].path; + let id = menus::command_for_path(path, &menus::ui_aliases()); + assert_eq!(id.as_deref(), Some("clip.remove_sync_point")); + + let mut app = soundcraft_ui_egui::SoundApp::new(soundcraft_engine::demo::demo_engine(), None, soundcraft_ui_egui::Services::default()); + app.engine.execute("edit.select", &serde_json::json!({"tracks": ["Kick"], "start": {"seconds": 12.0}})).unwrap(); + assert!(!menus::item_state(&app, path, id.as_deref()).0, "the selected Kick clip has no sync point yet"); + app.engine.execute("clip.identify_sync_point", &serde_json::json!({})).unwrap(); + assert!(menus::item_state(&app, path, id.as_deref()).0); +} From 783ce8f44bc76be0a42038b4a1baed7691d1d5e9 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 15:20:11 +0200 Subject: [PATCH 42/55] VST3: show the plugin's own text for identity-mapped parameters (fixes #599) --- crates/dsp/src/lib.rs | 6 ++++ crates/playback/src/lib.rs | 6 ++++ crates/ui-egui/src/panels.rs | 6 +++- crates/vst3-host/src/ffi.rs | 14 +++++++++ crates/vst3-host/src/plugin.rs | 31 +++++++++++++++++++- crates/vst3-host/tests/editor_main_thread.rs | 9 ++++++ crates/vst3-host/tests/fixture/src/lib.rs | 9 +++++- crates/vst3-host/tests/fixture_e2e.rs | 19 +++++++++++- 8 files changed, 96 insertions(+), 4 deletions(-) diff --git a/crates/dsp/src/lib.rs b/crates/dsp/src/lib.rs index d31ddac2..afa17dad 100644 --- a/crates/dsp/src/lib.rs +++ b/crates/dsp/src/lib.rs @@ -275,6 +275,12 @@ pub trait PluginEditor: Send { /// Tells the editor that a parameter changed elsewhere (generic editor, automation), so the /// GUI shows it. Values are in SoundCraft units. fn set_param(&mut self, _id: &str, _value: f32) {} + /// The plugin's own text for a parameter value (SoundCraft units) when SoundCraft's + /// [`ParamInfo::format`] would be wrong for it. Main thread only; `None` means "format it + /// yourself". + fn value_text(&mut self, _id: &str, _value: f32) -> Option { + None + } } /// Reads a plugin's state on the main (UI) thread while the instance processes on the audio diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index b945d4b6..e290727d 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -726,6 +726,12 @@ impl Player { } } + /// The plugin's own text for a parameter value when SoundCraft's format would be wrong for it + /// (see `PluginEditor::value_text`). Main thread only. + pub fn editor_value_text(&self, track: TrackId, slot: usize, id: &str, value: f32) -> Option { + self.side().editors.get_mut(&(track, slot))?.value_text(id, value) + } + /// Start playback at `from`, stopping at `end`, or looping `looped`. pub fn play(&self, from: Samples, end: Option, looped: Option) { self.shared.position.store(from, Ordering::Relaxed); diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index dda31ce0..57eda5b7 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -544,7 +544,11 @@ fn plugin_windows(app: &mut SoundApp, ctx: &egui::Context) { } } let _ = resp; - let shown = if instrument { crate::sampler_window::format_param(&ins.plugin, p, x) } else { p.format(x) }; + let shown = if instrument { + crate::sampler_window::format_param(&ins.plugin, p, x) + } else { + app.player.as_ref().and_then(|pl| pl.editor_value_text(tid, slot, p.id, x)).unwrap_or_else(|| p.format(x)) + }; ui.label(egui::RichText::new(shown).font(mono(11.0))); ui.end_row(); } diff --git a/crates/vst3-host/src/ffi.rs b/crates/vst3-host/src/ffi.rs index d1ddea23..bec9f167 100644 --- a/crates/vst3-host/src/ffi.rs +++ b/crates/vst3-host/src/ffi.rs @@ -1788,6 +1788,20 @@ impl EditorLink { v.is_finite().then(|| v.clamp(0.0, 1.0)) } + /// The plugin's own text for a normalized value (`getParamStringByValue`). Main thread only + /// (`None` elsewhere, and once the instance is gone). + pub fn value_text(&self, id: u32, n: f64) -> Option { + if !window::on_main_thread() || !n.is_finite() { + return None; + } + let g = self.lock(); + let c = g.controller.as_ref()?; + let mut s: sv::String128 = [0; 128]; + // SAFETY: valid controller (the instance clears it under this lock before it terminates), + // main thread; `s` is a writable String128. + ok(unsafe { c.getParamStringByValue(id, n.clamp(0.0, 1.0), &mut s) }).then(|| read_c16(&s)).filter(|t| !t.is_empty()) + } + /// Shows a value change in the editor (`setParamNormalized`, main thread). pub fn set_normalized(&self, id: u32, n: f64) { if !window::on_main_thread() || !n.is_finite() { diff --git a/crates/vst3-host/src/plugin.rs b/crates/vst3-host/src/plugin.rs index 2975465f..ed59fa8e 100644 --- a/crates/vst3-host/src/plugin.rs +++ b/crates/vst3-host/src/plugin.rs @@ -2,6 +2,7 @@ use crate::ffi::{AFTERTOUCH, BusBuffers, EVENT_CAP, EditorLink, HostContext, Instance, NoteEvent, PARAM_QUEUE_CAP, PITCH_BEND}; use soundcraft_dsp::{Plugin, PluginEditor, PluginInfo, Transport, Unit}; +use std::collections::HashMap; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use vst3::ComWrapper; @@ -399,7 +400,16 @@ impl Plugin for Vst3Plugin { fn editor(&mut self) -> Option> { let link = self.inst.editor()?; - Some(Box::new(Vst3Editor { link, host: self.inst.host_ref(), info: self.info, ids: self.ids.clone(), maps: self.maps.clone() })) + let mut units: HashMap = self.inst.params().into_iter().map(|p| (p.id, p.units)).collect(); + Some(Box::new(Vst3Editor { + link, + host: self.inst.host_ref(), + info: self.info, + index: self.info.params.iter().enumerate().map(|(i, p)| (p.id, i)).collect(), + units: self.ids.iter().map(|id| units.remove(id).unwrap_or_default()).collect(), + ids: self.ids.clone(), + maps: self.maps.clone(), + })) } fn state_reader(&mut self) -> Option> { @@ -423,6 +433,10 @@ struct Vst3Editor { link: Arc, host: ComWrapper, info: &'static PluginInfo, + /// Position of each SoundCraft param id in `info.params` (looked up once per row and frame). + index: HashMap<&'static str, usize>, + /// The plugin's own unit label of each param (same order as `ids`). + units: Vec, ids: Vec, maps: Vec, } @@ -487,4 +501,19 @@ impl PluginEditor for Vst3Editor { self.link.set_normalized(vid, n); } } + + fn value_text(&mut self, id: &str, value: f32) -> Option { + let &i = self.index.get(id)?; + let pi = self.info.params.get(i)?; + // A plugin whose plain mapping is the identity (JUCE, the SDK's default `Parameter`) leaves + // a 0..1 range with the units of its real one: only its own text says what the value is. + if !matches!(self.maps.get(i)?, Mapping::Plain) || pi.min != 0.0 || pi.max != 1.0 { + return None; + } + let text = self.link.value_text(*self.ids.get(i)?, self.normalized(i, value)?)?; + // VST3 keeps the units apart from the text (JUCE's label, the SDK's `RangeParameter`), + // but some plugins put them in both. + let units = self.units.get(i).map_or("", |u| u.trim()); + if units.is_empty() || text.trim_end().to_lowercase().ends_with(&units.to_lowercase()) { Some(text) } else { Some(format!("{text} {units}")) } + } } diff --git a/crates/vst3-host/tests/editor_main_thread.rs b/crates/vst3-host/tests/editor_main_thread.rs index 6bc732c7..c2399c81 100644 --- a/crates/vst3-host/tests/editor_main_thread.rs +++ b/crates/vst3-host/tests/editor_main_thread.rs @@ -23,7 +23,16 @@ fn main() { ed.set_param("7", 1.5); ed.set_param("nope", 1.0); assert!(ed.idle().is_empty(), "set_param is not reported back as an edit"); + // #599: an identity-mapped (JUCE-style) parameter shows the plugin's own text, verbatim, with + // its units appended once; correctly mapped parameters and choices keep SoundCraft's format. + assert_eq!(ed.value_text("13", 0.375).as_deref(), Some("\u{2212}6.0 dB")); + assert_eq!(ed.value_text("13", 1.0).as_deref(), Some("24.0 dB")); + assert_eq!(ed.value_text("14", 0.25).as_deref(), Some("3.0 dB"), "units already in the text"); + assert_eq!(ed.value_text("7", 1.5), None); + assert_eq!(ed.value_text("3", 1.0), None); + assert_eq!(ed.value_text("nope", 0.5), None); drop(p); + assert_eq!(ed.value_text("13", 0.375), None, "the plugin is gone"); assert!(!ed.is_open()); assert!(ed.open().is_err(), "the plugin is gone"); println!("editor_main_thread: ok"); diff --git a/crates/vst3-host/tests/fixture/src/lib.rs b/crates/vst3-host/tests/fixture/src/lib.rs index 935d4c66..2b7f73d9 100644 --- a/crates/vst3-host/tests/fixture/src/lib.rs +++ b/crates/vst3-host/tests/fixture/src/lib.rs @@ -298,12 +298,17 @@ impl IConnectionPointTrait for GainController { } } -const GAIN_PARAMS: [(u32, &str, &str, i32, f64, i32); 5] = [ +const GAIN_PARAMS: [(u32, &str, &str, i32, f64, i32); 7] = [ (7, "Gain", "dB", 0, 0.5, ParameterInfo_::ParameterFlags_::kCanAutomate), (9, "Secret", "", 0, 0.0, ParameterInfo_::ParameterFlags_::kIsHidden), (3, "Mode", "", 2, 0.0, ParameterInfo_::ParameterFlags_::kIsList), (11, "Bypass", "", 1, 0.0, ParameterInfo_::ParameterFlags_::kIsBypass), (12, "Freq", "Hz", 0, 0.5, 0), + // Like JUCE and the SDK's default `Parameter`: no plain mapping (identity), so the real range + // is only in the plugin's own text (-24..+24, the default 0.375 is -6), without the units. + (13, "Level", "dB", 0, 0.375, ParameterInfo_::ParameterFlags_::kCanAutomate), + // Identity too, but its text already carries the units (0..12 dB). + (14, "Drive", "dB", 0, 0.0, ParameterInfo_::ParameterFlags_::kCanAutomate), ]; impl IEditControllerTrait for GainController { @@ -351,6 +356,8 @@ impl IEditControllerTrait for GainController { let s = unsafe { &mut *string }; let text = match id { 3 => ["A", "B", "C"][((n * 2.0).round() as usize).min(2)].to_string(), + 13 => format!("{:.1}", 48.0 * n - 24.0).replace('-', "\u{2212}"), + 14 => format!("{:.1} dB", 12.0 * n), _ => format!("{:.2}", unsafe { self.normalizedParamToPlain(id, n) }), }; copy_c16(&text, s); diff --git a/crates/vst3-host/tests/fixture_e2e.rs b/crates/vst3-host/tests/fixture_e2e.rs index 5efd33cc..eb9037f5 100644 --- a/crates/vst3-host/tests/fixture_e2e.rs +++ b/crates/vst3-host/tests/fixture_e2e.rs @@ -42,7 +42,7 @@ fn info_exposes_visible_params_with_plain_ranges() { assert_eq!(info.id, gain); assert_eq!(info.short_name, "Fixture "); let ids: Vec<&str> = info.params.iter().map(|p| p.id).collect(); - assert_eq!(ids, vec!["7", "3", "12"], "hidden 9 and bypass 11 are skipped"); + assert_eq!(ids, vec!["7", "3", "12", "13", "14"], "hidden 9 and bypass 11 are skipped"); let g = info.param("7").unwrap(); assert_eq!((g.name, g.min, g.max, g.default, g.unit, g.taper), ("Gain", 0.0, 2.0, 1.0, Unit::Db, Taper::Linear)); let mode = info.param("3").unwrap(); @@ -55,6 +55,23 @@ fn info_exposes_visible_params_with_plain_ranges() { assert!(std::ptr::eq(info, soundcraft_vst3_host::plugin_info(&gain).unwrap())); } +/// #599: a plugin whose `normalizedParamToPlain` is the identity (JUCE, the SDK's default +/// `Parameter`) gives SoundCraft a 0..1 range; its real values are only in its own text, which the +/// editor handle reads from the controller on the main thread only (see `editor_main_thread`). +#[test] +fn identity_mapped_param_text_is_read_on_the_main_thread_only() { + let id = id_of("Fixture Gain"); + let level = soundcraft_vst3_host::plugin_info(&id).unwrap().param("13").unwrap(); + assert_eq!((level.min, level.max, level.unit), (0.0, 1.0, Unit::Db)); + let mut p = soundcraft_vst3_host::instantiate_plugin(&id).unwrap(); + let mut ed = soundcraft_dsp::Plugin::editor(&mut p).expect("editor handle"); + if cfg!(any(target_os = "macos", windows)) { + assert_eq!(ed.value_text("13", 0.375), None, "test threads are not the main thread"); + } else { + assert_eq!(ed.value_text("13", 0.375).as_deref(), Some("\u{2212}6.0 dB")); + } +} + #[test] fn gain_processes_audio_and_follows_params() { let mut p = soundcraft_vst3_host::create(&id_of("Fixture Gain")).unwrap(); From 86b31e71d36e48dbba4ab18b12652f169734e1f8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 17:32:35 +0000 Subject: [PATCH 43/55] Sessions: follow stored source paths only on this computer or the session's share Audio source and movie paths stored in a .scraft session go through io::stored_path (craft-docpath) before they reach the file system: a relative path is taken from the session's folder, and paths on other computers, devices and URLs are refused. A refused source is reported as missing with the reason; a refused movie is marked offline and the Video track and Video window show why instead of opening it. Import Session Data and crash recovery resolve the same way. --- Cargo.lock | 5 ++ Cargo.toml | 3 + crates/engine/Cargo.toml | 1 + crates/engine/src/autosave.rs | 12 ++- crates/engine/src/cmd/setup_more.rs | 30 +++++--- crates/engine/src/cmd/video.rs | 6 +- crates/engine/src/io.rs | 114 +++++++++++++++++++++++++--- crates/model/src/session.rs | 1 + crates/model/src/source.rs | 3 + crates/ui-egui/src/video_track.rs | 12 ++- crates/ui-egui/src/video_window.rs | 6 +- 11 files changed, 159 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b8eb7655..21b131eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -846,6 +846,10 @@ dependencies = [ "windows 0.54.0", ] +[[package]] +name = "craft-docpath" +version = "0.1.0" + [[package]] name = "craft-ui" version = "0.1.0" @@ -3654,6 +3658,7 @@ dependencies = [ name = "soundcraft-engine" version = "0.7.0" dependencies = [ + "craft-docpath", "log", "proptest", "serde", diff --git a/Cargo.toml b/Cargo.toml index 362e6a9a..8e950a9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,9 @@ rustfft = "6" zip = { version = "9", default-features = false } cpal = "0.16" +# Which file paths stored in a session may be opened. +craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } + # ui craft-ui = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] } egui = "0.36" diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 9b097a00..48a03408 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -23,6 +23,7 @@ serde_json.workspace = true thiserror.workspace = true log.workspace = true zip.workspace = true +craft-docpath.workspace = true [dev-dependencies] proptest.workspace = true diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index cb7faac1..01899d2f 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -1,7 +1,7 @@ //! Crash-recovery copies of open sessions, one folder each, written in the background, and the //! recovery of the ones left behind with unsaved work. -use crate::io::{Recording, open_session, sanitize_name}; +use crate::io::{Recording, open_session_linked, sanitize_name}; use crate::{Engine, EngineError, Result}; use soundcraft_audio_io::{BitDepth, EncodeOptions, FileFormat}; use soundcraft_model::{Session, SourceAudio, SourceId}; @@ -197,7 +197,7 @@ impl Autosave { let file = if Path::new(&src.path).is_absolute() { Some(PathBuf::from(&src.path)) } else { base.as_ref().map(|b| b.join(&src.path)) }; let audio = s.pool.get(src.id); if let Some(f) = &file - && (!src.unsaved && f.is_file() || audio.is_none()) + && (audio.is_none() || !src.unsaved && f.is_file()) { src.path = f.to_string_lossy().into_owned(); continue; @@ -291,7 +291,9 @@ pub fn leftovers(root: &Path) -> Vec { /// media that couldn't be loaded. pub fn recover(e: &mut Engine, left: &Leftover) -> Result> { let mut r = Engine::default(); - let mut missing = open_session(&mut r, &left.dir.join(SESSION).to_string_lossy())?; + let session = left.dir.join(SESSION); + let links = left.origin.as_deref().map_or(session.as_path(), Path::new); + let mut missing = open_session_linked(&mut r, &session.to_string_lossy(), links)?; let mut s = r.session().clone(); let unfinished = unfinished_takes(left, &s, &mut missing); if let Some(home) = left.origin.as_deref().and_then(|o| Path::new(o).parent()) { @@ -315,7 +317,8 @@ pub fn recover(e: &mut Engine, left: &Leftover) -> Result> { /// is left untouched and added to `skipped`. fn unfinished_takes(left: &Leftover, s: &Session, skipped: &mut Vec) -> Option<(Recording, Vec<(soundcraft_model::TrackId, PathBuf)>)> { let plan: Recording = serde_json::from_str(&std::fs::read_to_string(left.dir.join(RECORDING)).ok()?).ok()?; - let known: Vec = s.sources.iter().filter_map(|src| std::fs::canonicalize(&src.path).ok()).collect(); + let known: Vec = + s.sources.iter().filter(|src| s.pool.get(src.id).is_some()).filter_map(|src| std::fs::canonicalize(&src.path).ok()).collect(); let folders = take_folders(left); let mut takes = Vec::new(); for t in &plan.takes { @@ -413,6 +416,7 @@ fn io_error(path: &Path, err: std::io::Error) -> EngineError { #[cfg(test)] mod tests { use super::*; + use crate::io::open_session; use serde_json::json; use soundcraft_audio_io::AudioBuffer; use soundcraft_model::{ChannelFormat, Clip, TrackKind}; diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index 3d436759..aee13ba1 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -7,7 +7,7 @@ use crate::cmd; use serde_json::json; use soundcraft_model::{BusId, ChannelFormat, ClipContent, OutputPath, Route, Session, SourceId, TrackId, TrackKind}; use soundcraft_time::Timecode; -use std::path::{Path, PathBuf}; +use std::path::Path; macro_rules! option { ($id:literal, $label:literal, $flag:literal) => { @@ -407,7 +407,6 @@ fn import_session_data(e: &mut Engine, p: &Value) -> Result { let path = str_param(p, "path").filter(|x| !x.trim().is_empty()).ok_or_else(|| bad(id, "`path` required"))?.to_string(); 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> = p.get("tracks").and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_str).map(str::to_string).collect()); let picked: Vec = match &names { @@ -435,20 +434,27 @@ fn import_session_data(e: &mut Engine, p: &Value) -> Result { let mut map: std::collections::BTreeMap = Default::default(); let mut missing = Vec::new(); 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 = SourceId(s.alloc()); - ns.path = abs.to_string_lossy().into_owned(); - 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(ns.id, audio); + match crate::io::stored_path(&src.path, Path::new(&path)) { + Ok(abs) => { + ns.path = abs.to_string_lossy().into_owned(); + 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(ns.id, audio); + } + Err(_) => { + ns.unsaved = false; + missing.push(ns.path.clone()); + } + } } - Err(_) => { + Err(why) => { ns.unsaved = false; - missing.push(ns.path.clone()); + missing.push(format!("{}: {why}", ns.path)); } } map.insert(src.id.0, ns.id); diff --git a/crates/engine/src/cmd/video.rs b/crates/engine/src/cmd/video.rs index f361e0d4..c65e40d6 100644 --- a/crates/engine/src/cmd/video.rs +++ b/crates/engine/src/cmd/video.rs @@ -28,7 +28,7 @@ pub fn specs() -> Vec { always, offset ), - cmd!(query "video.list", "Movies", [], None, "{} — movies used by Video tracks (path, size, rate, codec, online file)", always, |e, _| { + cmd!(query "video.list", "Movies", [], None, "{} — movies used by Video tracks (path, size, rate, codec, online file, why it is offline)", always, |e, _| { let s = e.session(); let rows: Vec = s .videos @@ -36,7 +36,8 @@ pub fn specs() -> Vec { .map(|v| { let clips = s.tracks.iter().flat_map(|t| t.clips()).filter(|c| c.video_source() == Some(v.id)).count(); json!({"id": v.id, "name": v.name, "path": v.path, "width": v.width, "height": v.height, "frame_rate": v.frame_rate, - "duration": v.duration, "codec": v.codec, "clips": clips, "file_found": crate::io::file_found(&v.path)}) + "duration": v.duration, "codec": v.codec, "clips": clips, + "file_found": v.offline.is_none() && crate::io::file_found(&v.path), "offline": v.offline}) }) .collect(); Ok(json!({"videos": rows, "online": !s.edit.flag(OFFLINE_FLAG)})) @@ -92,6 +93,7 @@ mod tests { frame_rate: 24.0, duration: 2.0, codec: "h264".into(), + offline: None, }); let c = s.new_clip_id(); crate::edit::place_clip(s, t, Clip::video(c, "m", vid, 0, 1000, 96_000)); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..80e8e4f8 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -60,6 +60,23 @@ pub fn file_found(path: impl AsRef) -> bool { crate::file_access::check_read(path).is_ok() && path.is_file() } +/// The file a path stored in the session file `session` refers to, or why it is not opened. +/// +/// Every path a session names (audio sources, movies) goes through here before it reaches the +/// file system. A relative path is taken from the session's folder, and only files on this +/// computer or on the network share the session itself is on are opened (`craft_docpath`). +/// Paths the person picks in this session (imports) are not stored paths and skip this. +pub fn stored_path(stored: &str, session: &Path) -> std::result::Result { + let session = std::path::absolute(session).unwrap_or_else(|_| session.to_path_buf()); + let path = craft_docpath::resolve(stored, Some(&session)).map_err(|why| format!("not opened: {why}"))?; + // A path that is still relative would be taken from the working directory, which the session + // does not choose. + if path.is_relative() { + return Err("not opened: the path is not absolute".into()); + } + Ok(path) +} + /// Create `dir` and its parents, inside the automation write root when there is one. pub(crate) fn make_dir(dir: impl AsRef) -> Result<()> { let dir = dir.as_ref(); @@ -206,6 +223,7 @@ pub fn import_video_bytes(e: &mut Engine, path: &str, bytes: Vec, at: Sample frame_rate: info.frame_rate, duration: info.duration, codec: info.codec.clone(), + offline: None, }); // Reuse the first Video track when the new clip does not overlap its clips; else add one at the top. let range = Range::new(at, at.saturating_add(len)); @@ -235,22 +253,35 @@ pub fn import_video_bytes(e: &mut Engine, path: &str, bytes: Vec, at: Sample })) } -/// Resolve each referenced movie: keep it when its path exists, else look for a file of the same -/// name next to the session (moved projects) and relink. Returns the movies still missing. -pub fn relink_videos(s: &mut Session, session_dir: &Path) -> Vec { +/// Resolve each movie referenced by the session file `session` ([`stored_path`]): keep the +/// resolved path when its file exists, else look for a file of the same name next to the session +/// (moved projects) and relink. A movie whose stored path is refused and has no such copy is +/// marked offline. Returns the movies still missing. +pub fn relink_videos(s: &mut Session, session: &Path) -> Vec { + let session_dir = session.parent().unwrap_or(Path::new("")); let mut missing = Vec::new(); for v in &mut s.videos { - let p = if Path::new(&v.path).is_absolute() { PathBuf::from(&v.path) } else { session_dir.join(&v.path) }; - if file_found(&p) { + v.offline = None; + let p = stored_path(&v.path, session); + // The UI opens `path` as it is: keep the checked path, not one relative to the working + // directory. + if let Ok(found) = &p + && file_found(found) + { + v.path = found.to_string_lossy().into_owned(); continue; } let name = Path::new(&v.path).file_name().map(PathBuf::from).unwrap_or_default(); let near = [session_dir.join(&name), session_dir.join("Video Files").join(&name)] .into_iter() .find(|c| !name.as_os_str().is_empty() && file_found(c)); - match near { - Some(c) => v.path = c.to_string_lossy().into_owned(), - None => missing.push(format!("{} (video)", p.display())), + match (near, p) { + (Some(c), _) => v.path = c.to_string_lossy().into_owned(), + (None, Ok(p)) => missing.push(format!("{} (video)", p.display())), + (None, Err(why)) => { + missing.push(format!("{} (video): {why}", v.path)); + v.offline = Some(why); + } } } missing @@ -1096,19 +1127,26 @@ fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> { /// 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> { + open_session_linked(e, path, Path::new(path)) +} + +/// [`open_session`], following the media paths in it as if they were stored in the session file +/// `links` (crash recovery: the session written beside the autosave, its paths from the original). +pub(crate) fn open_session_linked(e: &mut Engine, path: &str, links: &Path) -> Result> { 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(); 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 decode_file(&mut files, &p, s.sample_rate.hz()) { + let decoded = + stored_path(&src.path, links).map_err(|why| format!("{}: {why}", src.path)).and_then(|p| decode_file(&mut files, &p, s.sample_rate.hz())); + match decoded { Ok(audio) => s.pool.insert(src.id, audio), Err(err) => missing.push(err), } } - missing.extend(relink_videos(&mut s, &dir)); + missing.extend(relink_videos(&mut s, links)); s.folder = Some(dir); e.replace_session(s); e.path = Some(path.to_string()); @@ -1731,6 +1769,60 @@ mod open_tests { assert_eq!(std::fs::read_dir(dir.join("copy/Audio Files")).unwrap().count(), 1); let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn stored_media_paths_are_followed_only_on_this_computer() { + let dir = std::env::temp_dir().join(format!("soundcraft-stored-paths-{}", 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 saved = Session::default(); + let t = saved.add_track(TrackKind::Audio, ChannelFormat::Mono, None); + let src = add_source(&mut saved, "k", audio, Some("../k.wav"), FileFormat::Wav); + let id = saved.new_clip_id(); + crate::edit::place_clip(&mut saved, t, Clip::audio(id, "k", src, 0, 0, 480)); + // In a folder of its own, so a movie named `k.wav` is not relinked to a file beside it. + std::fs::create_dir_all(dir.join("s")).unwrap(); + let path = dir.join("s").join("Song.scraft").to_string_lossy().into_owned(); + let mut stored = vec![r"\\192.0.2.1\share\x.wav".to_string(), "file://192.0.2.1/s/x.wav".to_string()]; + // Linux reads `//tmp/…` as the local `/tmp/…`, so only the path check keeps it from being read. + if cfg!(unix) { + stored.push(format!("/{}", dir.join("k.wav").display())); + } + let refused = |m: &str| m.contains("not opened: the path is on another computer"); + for p in stored { + let mut s = saved.clone(); + s.sources[0].path.clone_from(&p); + let id = SourceId(s.alloc()); + s.videos.push(soundcraft_model::VideoSource { + id, + name: "x".into(), + path: p.clone(), + width: 0, + height: 0, + frame_rate: 0.0, + duration: 0.0, + codec: String::new(), + offline: None, + }); + std::fs::write(&path, s.to_json().unwrap()).unwrap(); + let mut r = Engine::default(); + let missing = open_session(&mut r, &path).unwrap(); + assert_eq!(missing.iter().filter(|m| refused(m)).count(), 2, "{p}: {missing:?}"); + assert!(r.session().pool.get(r.session().sources[0].id).is_none(), "{p}"); + assert!(r.session().videos[0].offline.is_some(), "{p}"); + let mut into = Engine::default(); + let imported = into.execute("file.import_session_data", &json!({"path": path})).unwrap(); + assert!(imported["missing"].as_array().unwrap().iter().any(|m| m.as_str().is_some_and(refused)), "{p}: {imported}"); + assert!(into.session().pool.get(into.session().sources[0].id).is_none(), "{p}"); + } + // A path relative to the session opens. + std::fs::write(&path, saved.to_json().unwrap()).unwrap(); + let mut r = Engine::default(); + assert!(open_session(&mut r, &path).unwrap().is_empty()); + assert!(r.session().pool.get(r.session().sources[0].id).is_some()); + let _ = std::fs::remove_dir_all(&dir); + } } #[cfg(test)] diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 449d82da..df141a70 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -695,6 +695,7 @@ mod tests { frame_rate: 23.976, duration: 12.5, codec: "prores".into(), + offline: None, }); let cid = s.new_clip_id(); if let Some(p) = s.track_mut(t).and_then(Track::playlist_mut) { diff --git a/crates/model/src/source.rs b/crates/model/src/source.rs index 5da35e76..c7c8a341 100644 --- a/crates/model/src/source.rs +++ b/crates/model/src/source.rs @@ -46,6 +46,9 @@ pub struct VideoSource { /// Codec id (`h264`, `prores`, `mjpeg`, `hevc`, …). #[serde(default)] pub codec: String, + /// Why the movie is not opened, when its stored path was refused on opening the session. + #[serde(skip)] + pub offline: Option, } impl VideoSource { diff --git a/crates/ui-egui/src/video_track.rs b/crates/ui-egui/src/video_track.rs index fdf5d87b..eb6dfca1 100644 --- a/crates/ui-egui/src/video_track.rs +++ b/crates/ui-egui/src/video_track.rs @@ -404,8 +404,9 @@ pub fn movie_secs(s: &Session, clip_start: Samples, offset: Samples, at: Samples (at.saturating_sub(clip_start).saturating_add(offset) as f64 - sync) / s.sample_rate.as_f64().max(1.0) } -/// The picture at `at`: the topmost active Video track with an unmuted clip there. -pub fn picture_at(s: &Session, at: Samples) -> Option<(MovieKey, String, f64, f64)> { +/// The picture at `at`: the topmost active Video track with an unmuted clip there. The path is +/// the movie's, or why it is offline. +pub fn picture_at(s: &Session, at: Samples) -> Option<(MovieKey, Result, f64, f64)> { for t in s.tracks.iter().filter(|t| t.kind == soundcraft_model::TrackKind::Video && !t.inactive) { for c in t.clips() { if c.muted || !c.range().contains(at) { @@ -414,7 +415,8 @@ pub fn picture_at(s: &Session, at: Samples) -> Option<(MovieKey, String, f64, f6 if let ClipContent::Video { source, offset } = c.content && let Some(v) = s.video(source) { - return Some((MovieKey::new(source, &v.path), v.path.clone(), movie_secs(s, c.start, offset, at), v.frame_rate)); + let path = v.offline.clone().map_or_else(|| Ok(v.path.clone()), Err); + return Some((MovieKey::new(source, &v.path), path, movie_secs(s, c.start, offset, at), v.frame_rate)); } } } @@ -450,6 +452,10 @@ pub fn draw_lane(app: &mut SoundApp, painter: &Painter, s: &Session, track: &Tra p.text(vis.center(), egui::Align2::CENTER_CENTER, "Video Track Offline", regular(10.0), Color32::from_gray(170)); continue; } + if let Some(why) = &v.offline { + p.text(vis.center(), egui::Align2::CENTER_CENTER, format!("media offline: {why}"), regular(10.0), Color32::from_rgb(220, 120, 120)); + continue; + } let key = MovieKey::new(source, &v.path); if let Some(Err(e)) = app.video.info(key) { p.text(vis.center(), egui::Align2::CENTER_CENTER, format!("media offline: {e}"), regular(10.0), Color32::from_rgb(220, 120, 120)); diff --git a/crates/ui-egui/src/video_window.rs b/crates/ui-egui/src/video_window.rs index 2f2aa7f9..61809e78 100644 --- a/crates/ui-egui/src/video_window.rs +++ b/crates/ui-egui/src/video_window.rs @@ -24,7 +24,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { let s = app.engine.session_arc(); let pic = picture_at(&s, pos); let on = online(&s); - if on && let Some((key, path, secs, fps)) = &pic { + if on && let Some((key, Ok(path), secs, fps)) = &pic { app.video.request_frame(ctx, *key, path, *secs, *fps); } let aspect = pic @@ -39,6 +39,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { ui.horizontal(|ui| { crate::widgets::checkbox(ui, &mut burn, "Timecode burn-in"); let text = match &pic { + Some((_, Err(why), ..)) => why.clone(), Some((k, ..)) => match app.video.info(*k) { Some(Ok(i)) => format!("{}x{} {:.3} fps {}", i.width, i.height, i.frame_rate, i.codec_detail), Some(Err(e)) => e.clone(), @@ -64,6 +65,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { } else { match &pic { None => None, + Some((_, Err(why), ..)) => Some(why.clone()), Some((k, ..)) => match (app.video.current(*k), app.video.current_error(*k)) { (Some(tex), _) => { painter.image(tex.id(), img, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); @@ -130,7 +132,7 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) { let key = MovieKey::new(source, &v.path); let cr = Rect::from_min_max(pos2(x_at(c.start), r.min.y + 4.0), pos2(x_at(c.end()).max(x_at(c.start) + 2.0), r.max.y - 4.0)); painter.rect_filled(cr, 0.0, Color32::from_rgb(30, 30, 34)); - if !on { + if !on || v.offline.is_some() { continue; } let tw = (cr.height() * v.aspect() as f32).max(8.0); From bde132e0bf71c8e29281023dc5f116980d6854e5 Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 17:34:40 +0000 Subject: [PATCH 44/55] ProRes: check frame dimensions before allocating planes decode_frame_into refuses a frame header larger than the movie picture limit before sizing any plane, and Movie refuses a ProRes frame whose header size differs from the track's declared size before decoding it. --- crates/video/src/movie.rs | 7 ++++++- crates/video/src/prores/decode.rs | 4 ++++ crates/video/src/tests.rs | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/video/src/movie.rs b/crates/video/src/movie.rs index 00541407..c5755ca3 100644 --- a/crates/video/src/movie.rs +++ b/crates/video/src/movie.rs @@ -10,7 +10,7 @@ use std::sync::Arc; pub type SharedSource = Arc; /// Largest picture we decode (bounds every allocation derived from the file). -const MAX_PIXELS: u64 = 8192 * 8192; +pub(crate) const MAX_PIXELS: u64 = 8192 * 8192; /// A decoded picture, packed RGBA (8 bits per channel, `width * height * 4` bytes). #[derive(Debug, Clone, PartialEq)] @@ -395,6 +395,11 @@ impl Movie { self.decoded += 1; match self.codec { Codec::ProRes => { + let (hdr, ..) = crate::prores::FrameHeader::parse(&data).map_err(|e| VideoError::Decode(e.to_string()))?; + let (w, h) = (u32::from(hdr.width), u32::from(hdr.height)); + if (w, h) != (self.info.width, self.info.height) { + return Err(VideoError::Decode(format!("{w}x{h} frame in a {}x{} track", self.info.width, self.info.height))); + } let opts = crate::prores::DecodeOptions { bit_depth: Some(8), threads: false }; let f = crate::prores::decode_frame_with(&data, &opts).map_err(|e| VideoError::Decode(e.to_string()))?; if u64::from(f.width) * u64::from(f.height) > MAX_PIXELS { diff --git a/crates/video/src/prores/decode.rs b/crates/video/src/prores/decode.rs index 11f8fc1d..305a5b1b 100644 --- a/crates/video/src/prores/decode.rs +++ b/crates/video/src/prores/decode.rs @@ -109,6 +109,10 @@ pub fn decode_frame_with(data: &[u8], opts: &DecodeOptions) -> Result { /// a sequence). On error the contents of `out` are unspecified. pub fn decode_frame_into(data: &[u8], opts: &DecodeOptions, out: &mut Frame) -> Result<()> { let (hdr, mut off, frame_size) = FrameHeader::parse(data)?; + // The planes are sized from the header: refuse a size no movie may have before allocating. + if u64::from(hdr.width) * u64::from(hdr.height) > crate::movie::MAX_PIXELS { + return Err(Error::Unsupported("frame size")); + } let data = &data[..frame_size]; let depth = opts.bit_depth.unwrap_or(match hdr.chroma { ChromaFormat::Yuv422 => 10, diff --git a/crates/video/src/tests.rs b/crates/video/src/tests.rs index 58db4b3c..695a44ae 100644 --- a/crates/video/src/tests.rs +++ b/crates/video/src/tests.rs @@ -96,6 +96,28 @@ fn prores_and_mjpeg_decode() { } } +/// A 64x48 ProRes frame whose header claims `w`x`h`. +fn prores_frame_claiming(w: u16, h: u16) -> Vec { + use crate::prores::{ChromaFormat, Encoder, Frame, Profile}; + let mut data = Encoder::new(Profile::Hq, 64, 48).encode(&Frame::new(64, 48, ChromaFormat::Yuv422, 10, false)).unwrap(); + data[16..18].copy_from_slice(&w.to_be_bytes()); + data[18..20].copy_from_slice(&h.to_be_bytes()); + data +} + +#[test] +fn prores_frame_size_is_checked_before_decoding() { + use crate::prores::{DecodeOptions, Error, decode_frame_with}; + let huge = decode_frame_with(&prores_frame_claiming(65535, 65535), &DecodeOptions::default()); + assert!(matches!(huge, Err(Error::Unsupported("frame size"))), "{huge:?}"); + // A frame that differs from the track's declared size is refused, not decoded at its own size. + let mut mw = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mov)).unwrap(); + let t = mw.add_track(TrackConfig::new(SampleEntry::prores(crate::isobmff::FourCc(*b"apch"), 64, 48), 25)).unwrap(); + mw.write_sample(t, WriteSample { data: &prores_frame_claiming(56, 48), duration: 1, composition_offset: 0, is_sync: true }).unwrap(); + let mut m = Movie::open(mw.finish().unwrap().into_inner()).unwrap(); + assert!(matches!(m.frame(0), Err(VideoError::Decode(e)) if e.contains("56x48 frame in a 64x48 track"))); +} + #[test] fn unsupported_codec_opens_but_does_not_decode() { let mut mw = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mp4)).unwrap(); From 1e036eaa174b5d9db472ed2694ed1b4903294715 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 17 Oct 2026 02:20:54 +0000 Subject: [PATCH 45/55] Build: use craft-docpath from craft-libs --- Cargo.lock | 1 + Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 21b131eb..a1facf5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -849,6 +849,7 @@ dependencies = [ [[package]] name = "craft-docpath" version = "0.1.0" +source = "git+https://github.com/aiongg/craft-libs?rev=12281d07fedff5c955bb68344fb287aa5d87971e#12281d07fedff5c955bb68344fb287aa5d87971e" [[package]] name = "craft-ui" diff --git a/Cargo.toml b/Cargo.toml index 8e950a9c..a9e7f200 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ zip = { version = "9", default-features = false } cpal = "0.16" # Which file paths stored in a session may be opened. -craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } +craft-docpath = { git = "https://github.com/aiongg/craft-libs", version = "0.1.0", rev = "12281d07fedff5c955bb68344fb287aa5d87971e" } # ui craft-ui = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] } From 055ddbee7a6c8722d2a685eed543bf2305db90b7 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 05:26:51 +0200 Subject: [PATCH 46/55] Keep merging fader gestures once the undo history is full Found while testing crates/engine/src/lib.rs:368-381 (Engine::execute_merged, with push_undo at 280-291) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/lib.rs | 10 ++++- .../engine/tests/fader_merge_full_history.rs | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 crates/engine/tests/fader_merge_full_history.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6cc1c90a..03d173dd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -369,7 +369,15 @@ impl Engine { let key = self.merge_hold.clone().unwrap_or_else(|| key.to_string()); let continuing = self.merge_key.as_ref() == Some(&key); let before_len = self.undo.len(); - let r = self.execute(id, params)?; + // The step pushed for a continuing tick is dropped again below, so it must not trim the + // oldest entry first when the history is full. + let limit = self.history_limit; + if continuing && before_len > 0 { + self.history_limit = limit.max(before_len.saturating_add(1)); + } + let r = self.execute(id, params); + self.history_limit = limit; + let r = r?; if continuing && self.undo.len() == before_len + 1 && before_len > 0 { // Drop the step just pushed; the earlier one still holds the pre-gesture document. self.undo.pop(); diff --git a/crates/engine/tests/fader_merge_full_history.rs b/crates/engine/tests/fader_merge_full_history.rs new file mode 100644 index 00000000..c4656535 --- /dev/null +++ b/crates/engine/tests/fader_merge_full_history.rs @@ -0,0 +1,42 @@ +use serde_json::json; +use soundcraft_engine::Engine; +use soundcraft_model::{ChannelFormat, TrackKind}; + +fn engine_with_full_history(limit: usize) -> (Engine, u64) { + let mut e = Engine::default(); + let t = e.session_mut().add_track(TrackKind::Audio, ChannelFormat::Mono, None).0; + e.history_limit = limit; + for i in 0..limit + 1 { + e.execute("mix.pan", &json!({"track": t, "pan": 0.1 * (i as f64 + 1.0)})).unwrap(); + } + assert_eq!(e.undo_history().len(), limit); + (e, t) +} + +#[test] +fn a_fader_gesture_is_one_undo_step_when_the_history_is_full() { + let (mut e, t) = engine_with_full_history(3); + for db in [-1.0, -2.0, -3.0, -4.0, -5.0] { + e.execute_merged("mix.volume", &json!({"track": t, "db": db}), "fader").unwrap(); + } + e.end_merge(); + let volume = |e: &Engine| e.session().tracks.iter().find(|x| x.id.0 == t).map(|x| x.mixer.volume_db); + assert_eq!(volume(&e), Some(-5.0)); + // The gesture costs one step; it must not push further real edits out of the history. + assert_eq!(e.undo_history().len(), 3, "history: {:?}", e.undo_history()); + assert_eq!(e.undo_history().iter().filter(|l| l.as_str() == "Set Volume").count(), 1, "history: {:?}", e.undo_history()); + assert!(e.undo()); + assert_eq!(volume(&e), Some(0.0), "one undo should undo the whole fader gesture; history: {:?}", e.undo_history()); +} + +#[test] +fn a_zero_history_limit_keeps_no_undo_entry_for_a_gesture() { + let mut e = Engine::default(); + let t = e.session_mut().add_track(TrackKind::Audio, ChannelFormat::Mono, None).0; + e.history_limit = 0; + for db in [-1.0, -2.0, -3.0] { + e.execute_merged("mix.volume", &json!({"track": t, "db": db}), "fader").unwrap(); + } + e.end_merge(); + assert!(e.undo_history().is_empty(), "history: {:?}", e.undo_history()); +} From 4a4e82daba45992a237306713d2f64d75bd9ce17 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 09:54:08 +0200 Subject: [PATCH 47/55] Reject missing or non-numeric values in mix.pan and mix.trim Found while testing crates/engine/src/cmd/mix.rs:237 (pan); same pattern at mix.rs:76 (mix.trim) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/mix.rs | 6 ++-- crates/engine/tests/mix_required_params.rs | 42 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 crates/engine/tests/mix_required_params.rs diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index c0569429..faddc6ca 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -73,7 +73,8 @@ pub fn specs() -> Vec { )), cmd!("mix.trim", "Input Trim", [], None, "{tracks?, db}", has_selection, |e, p| { let tracks = tracks_required(e, "mix.trim", p)?; - let db = f32_or(p, "db", 0.0).clamp(-144.0, 24.0); + let db = (p.get("db").and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("mix.trim", "`db` (number) required"))? as f32) + .clamp(-144.0, 24.0); let s = e.session_mut(); for t in tracks { if let Some(tr) = s.track_mut(t) { @@ -234,7 +235,8 @@ fn volume(e: &mut Engine, p: &Value) -> Result { fn pan(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "mix.pan", p)?; - let v = f32_or(p, "pan", 0.0).clamp(-1.0, 1.0); + let v = (p.get("pan").and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("mix.pan", "`pan` (number) required"))? as f32) + .clamp(-1.0, 1.0); let idx = i64_or(p, "index", 0).clamp(0, 1) as usize; let s = e.session_mut(); for t in &tracks { diff --git a/crates/engine/tests/mix_required_params.rs b/crates/engine/tests/mix_required_params.rs new file mode 100644 index 00000000..f11e2f31 --- /dev/null +++ b/crates/engine/tests/mix_required_params.rs @@ -0,0 +1,42 @@ +use serde_json::json; +use soundcraft_engine::{Engine, EngineError}; + +fn engine_with_kick() -> Engine { + let mut e = Engine::default(); + e.execute("track.new", &json!({"name": "Kick", "format": "stereo"})).unwrap(); + e +} + +fn kick_pan(e: &Engine) -> Vec { + e.session().tracks.iter().find(|t| t.name == "Kick").unwrap().mixer.pan.to_vec() +} + +fn kick_trim(e: &Engine) -> f32 { + e.session().tracks.iter().find(|t| t.name == "Kick").unwrap().mixer.trim_db +} + +#[test] +fn mix_pan_rejects_a_missing_or_non_numeric_pan_and_keeps_the_pan() { + let mut e = engine_with_kick(); + e.execute("mix.pan", &json!({"tracks": ["Kick"], "pan": -1.0})).unwrap(); + assert_eq!(kick_pan(&e)[0], -1.0); + + for params in [json!({"tracks": ["Kick"]}), json!({"tracks": ["Kick"], "pan": "left"}), json!({"tracks": ["Kick"], "pan": null})] { + let r = e.execute("mix.pan", ¶ms); + assert!(matches!(r, Err(EngineError::BadParams(..))), "{params}: expected BadParams, got {r:?}"); + assert_eq!(kick_pan(&e)[0], -1.0, "{params}: the pan must be untouched"); + } +} + +#[test] +fn mix_trim_rejects_a_missing_or_non_numeric_db_and_keeps_the_trim() { + let mut e = engine_with_kick(); + e.execute("mix.trim", &json!({"tracks": ["Kick"], "db": -6.0})).unwrap(); + assert_eq!(kick_trim(&e), -6.0); + + for params in [json!({"tracks": ["Kick"]}), json!({"tracks": ["Kick"], "db": "loud"}), json!({"tracks": ["Kick"], "db": null})] { + let r = e.execute("mix.trim", ¶ms); + assert!(matches!(r, Err(EngineError::BadParams(..))), "{params}: expected BadParams, got {r:?}"); + assert_eq!(kick_trim(&e), -6.0, "{params}: the trim must be untouched"); + } +} From 52bd33bb06d5d001f7271d9272eca5d688f80ab5 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 12:04:05 -0600 Subject: [PATCH 48/55] Build: take craft-docpath from the merged craft-libs rev craft-libs#169 is merged, so the fork pin is no longer needed. Pinned to storytold/craft-libs 82a24b006eefa036b43c09a67fb9a3fbd17ffe63 (main after the merge) rather than the older 12281d07, which predates resolve_absolute and the Windows colon refusal. --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a1facf5e..463a213b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -849,7 +849,7 @@ dependencies = [ [[package]] name = "craft-docpath" version = "0.1.0" -source = "git+https://github.com/aiongg/craft-libs?rev=12281d07fedff5c955bb68344fb287aa5d87971e#12281d07fedff5c955bb68344fb287aa5d87971e" +source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63" [[package]] name = "craft-ui" diff --git a/Cargo.toml b/Cargo.toml index a9e7f200..4e2ed5b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ zip = { version = "9", default-features = false } cpal = "0.16" # Which file paths stored in a session may be opened. -craft-docpath = { git = "https://github.com/aiongg/craft-libs", version = "0.1.0", rev = "12281d07fedff5c955bb68344fb287aa5d87971e" } +craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # ui craft-ui = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] } From 2578ea69a388395ddadb6394d7b4ab40f21aa670 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 17 Oct 2026 17:16:15 -0400 Subject: [PATCH 49/55] Release 0.8.0 Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 36 ++++++++++++++++++------------------ Cargo.toml | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 463a213b..bae8187a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3572,7 +3572,7 @@ dependencies = [ [[package]] name = "soundcraft" -version = "0.7.0" +version = "0.8.0" dependencies = [ "eframe", "egui", @@ -3590,7 +3590,7 @@ dependencies = [ [[package]] name = "soundcraft-au-host" -version = "0.7.0" +version = "0.8.0" dependencies = [ "log", "serde", @@ -3600,7 +3600,7 @@ dependencies = [ [[package]] name = "soundcraft-audio-io" -version = "0.7.0" +version = "0.8.0" dependencies = [ "log", "proptest", @@ -3612,7 +3612,7 @@ dependencies = [ [[package]] name = "soundcraft-automation" -version = "0.7.0" +version = "0.8.0" dependencies = [ "log", "serde_json", @@ -3622,7 +3622,7 @@ dependencies = [ [[package]] name = "soundcraft-clap-host" -version = "0.7.0" +version = "0.8.0" dependencies = [ "clap-sys", "libloading", @@ -3635,7 +3635,7 @@ dependencies = [ [[package]] name = "soundcraft-cli" -version = "0.7.0" +version = "0.8.0" dependencies = [ "serde_json", "soundcraft-audio-io", @@ -3646,7 +3646,7 @@ dependencies = [ [[package]] name = "soundcraft-dsp" -version = "0.7.0" +version = "0.8.0" dependencies = [ "log", "proptest", @@ -3657,7 +3657,7 @@ dependencies = [ [[package]] name = "soundcraft-engine" -version = "0.7.0" +version = "0.8.0" dependencies = [ "craft-docpath", "log", @@ -3680,7 +3680,7 @@ dependencies = [ [[package]] name = "soundcraft-midi" -version = "0.7.0" +version = "0.8.0" dependencies = [ "proptest", "serde", @@ -3690,7 +3690,7 @@ dependencies = [ [[package]] name = "soundcraft-mix" -version = "0.7.0" +version = "0.8.0" dependencies = [ "log", "rayon", @@ -3706,7 +3706,7 @@ dependencies = [ [[package]] name = "soundcraft-model" -version = "0.7.0" +version = "0.8.0" dependencies = [ "proptest", "serde", @@ -3719,7 +3719,7 @@ dependencies = [ [[package]] name = "soundcraft-playback" -version = "0.7.0" +version = "0.8.0" dependencies = [ "cpal", "log", @@ -3734,7 +3734,7 @@ dependencies = [ [[package]] name = "soundcraft-time" -version = "0.7.0" +version = "0.8.0" dependencies = [ "proptest", "serde", @@ -3744,7 +3744,7 @@ dependencies = [ [[package]] name = "soundcraft-ui-egui" -version = "0.7.0" +version = "0.8.0" dependencies = [ "craft-ui", "egui", @@ -3770,7 +3770,7 @@ dependencies = [ [[package]] name = "soundcraft-video" -version = "0.7.0" +version = "0.8.0" dependencies = [ "image", "proptest", @@ -3781,7 +3781,7 @@ dependencies = [ [[package]] name = "soundcraft-vst3-host" -version = "0.7.0" +version = "0.8.0" dependencies = [ "libloading", "log", @@ -3797,7 +3797,7 @@ dependencies = [ [[package]] name = "soundcraft-web" -version = "0.7.0" +version = "0.8.0" dependencies = [ "eframe", "egui", @@ -5354,7 +5354,7 @@ checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7" [[package]] name = "xtask" -version = "0.7.0" +version = "0.8.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 4e2ed5b0..88bc4a3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/*", "xtask"] [workspace.package] -version = "0.7.0" +version = "0.8.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.90" From c1bf044a85b55e9aa083624c39fabddbb08dadeb Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 17 Oct 2026 21:43:22 -0400 Subject: [PATCH 50/55] Release 0.9.0 Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 36 ++++++++++++++++++------------------ Cargo.toml | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bae8187a..5ab3653d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3572,7 +3572,7 @@ dependencies = [ [[package]] name = "soundcraft" -version = "0.8.0" +version = "0.9.0" dependencies = [ "eframe", "egui", @@ -3590,7 +3590,7 @@ dependencies = [ [[package]] name = "soundcraft-au-host" -version = "0.8.0" +version = "0.9.0" dependencies = [ "log", "serde", @@ -3600,7 +3600,7 @@ dependencies = [ [[package]] name = "soundcraft-audio-io" -version = "0.8.0" +version = "0.9.0" dependencies = [ "log", "proptest", @@ -3612,7 +3612,7 @@ dependencies = [ [[package]] name = "soundcraft-automation" -version = "0.8.0" +version = "0.9.0" dependencies = [ "log", "serde_json", @@ -3622,7 +3622,7 @@ dependencies = [ [[package]] name = "soundcraft-clap-host" -version = "0.8.0" +version = "0.9.0" dependencies = [ "clap-sys", "libloading", @@ -3635,7 +3635,7 @@ dependencies = [ [[package]] name = "soundcraft-cli" -version = "0.8.0" +version = "0.9.0" dependencies = [ "serde_json", "soundcraft-audio-io", @@ -3646,7 +3646,7 @@ dependencies = [ [[package]] name = "soundcraft-dsp" -version = "0.8.0" +version = "0.9.0" dependencies = [ "log", "proptest", @@ -3657,7 +3657,7 @@ dependencies = [ [[package]] name = "soundcraft-engine" -version = "0.8.0" +version = "0.9.0" dependencies = [ "craft-docpath", "log", @@ -3680,7 +3680,7 @@ dependencies = [ [[package]] name = "soundcraft-midi" -version = "0.8.0" +version = "0.9.0" dependencies = [ "proptest", "serde", @@ -3690,7 +3690,7 @@ dependencies = [ [[package]] name = "soundcraft-mix" -version = "0.8.0" +version = "0.9.0" dependencies = [ "log", "rayon", @@ -3706,7 +3706,7 @@ dependencies = [ [[package]] name = "soundcraft-model" -version = "0.8.0" +version = "0.9.0" dependencies = [ "proptest", "serde", @@ -3719,7 +3719,7 @@ dependencies = [ [[package]] name = "soundcraft-playback" -version = "0.8.0" +version = "0.9.0" dependencies = [ "cpal", "log", @@ -3734,7 +3734,7 @@ dependencies = [ [[package]] name = "soundcraft-time" -version = "0.8.0" +version = "0.9.0" dependencies = [ "proptest", "serde", @@ -3744,7 +3744,7 @@ dependencies = [ [[package]] name = "soundcraft-ui-egui" -version = "0.8.0" +version = "0.9.0" dependencies = [ "craft-ui", "egui", @@ -3770,7 +3770,7 @@ dependencies = [ [[package]] name = "soundcraft-video" -version = "0.8.0" +version = "0.9.0" dependencies = [ "image", "proptest", @@ -3781,7 +3781,7 @@ dependencies = [ [[package]] name = "soundcraft-vst3-host" -version = "0.8.0" +version = "0.9.0" dependencies = [ "libloading", "log", @@ -3797,7 +3797,7 @@ dependencies = [ [[package]] name = "soundcraft-web" -version = "0.8.0" +version = "0.9.0" dependencies = [ "eframe", "egui", @@ -5354,7 +5354,7 @@ checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7" [[package]] name = "xtask" -version = "0.8.0" +version = "0.9.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 88bc4a3b..65b8ea5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/*", "xtask"] [workspace.package] -version = "0.8.0" +version = "0.9.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.90" From 31f038939715e055dc24d09d613e206d397d5fd4 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 18 Oct 2026 10:39:50 -0400 Subject: [PATCH 51/55] docs: drop references to competing commercial software (#743) --- README.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/README.md b/README.md index d88fee8a..8fd22ab7 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@

SoundCraft

- Recording, editing and mixing audio; an open-source, clean-room reimplementation of Avid Pro Tools, rebuilt in pure Rust. + Recording, editing and mixing audio; an open-source, modern digital audio workstation (DAW), built in pure Rust.

@@ -375,10 +375,6 @@ ArtCraft Team and are not covered by this license. They may be used only unmodif part of this repository and SoundCraft, under [`docs/brand/LICENSE-brand.txt`](docs/brand/LICENSE-brand.txt). Forks and modified versions must remove them. -Avid and Pro Tools are trademarks or registered trademarks of Avid Technology, Inc. in the United States and/or other countries. SoundCraft is an independent, open-source project and is not affiliated with, sponsored by or endorsed by Avid Technology, Inc.; these names are used only to describe the workflows it is compatible with. - -Adobe, Photoshop, Illustrator, Premiere Pro, Lightroom, Acrobat, After Effects and InDesign are trademarks or registered trademarks of Adobe Inc. in the United States and/or other countries. SoundCraft is an independent, open-source project and is not affiliated with, sponsored by or endorsed by Adobe Inc.; these names are used only to describe the workflows it is compatible with. -

ArtCraft
Made by the ArtCraft team and community. From 172dc4edc6c682077805bdc69c982094372dad75 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 18 Oct 2026 11:01:17 -0400 Subject: [PATCH 52/55] docs: add SonarCloud and CodeRabbit badges to README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 8fd22ab7..a4d2c972 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,8 @@ Platforms License: MIT OR Apache-2.0 Status: pre-alpha + Scanned by SonarCloud + Reviewed by CodeRabbit

From 2e9ad2f4215bd36e7edf30579e8ce520d5ce67c7 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 18 Oct 2026 14:31:49 -0400 Subject: [PATCH 53/55] ci: configure Dependabot for Cargo and GitHub Actions Co-authored-by: Dylan M. Taylor --- .github/dependabot.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..2c4b8cb1 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,30 @@ +version: 2 +updates: + # Maintain Rust / Cargo dependencies + - package-ecosystem: "cargo" + directory: "/" + schedule: + interval: "daily" + time: "06:00" + timezone: "America/New_York" + # Consolidate updates into a single PR rather than one PR per crate + groups: + cargo-dependencies: + patterns: + - "*" + commit-message: + prefix: "chore(deps)" + + # Maintain GitHub Actions workflows + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" + time: "06:00" + timezone: "America/New_York" + groups: + actions: + patterns: + - "*" + commit-message: + prefix: "ci(deps)" From 79e5bb6efb187587e3d3b370b591783722364c3a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 18 Oct 2026 14:47:46 -0400 Subject: [PATCH 54/55] docs: put SonarCloud and CodeRabbit badges on their own line --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a4d2c972..c683c34b 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ Platforms License: MIT OR Apache-2.0 Status: pre-alpha +
Scanned by SonarCloud Reviewed by CodeRabbit

From f53051a35f0421df7330f93c1d5a46c74e996e66 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 19 Oct 2026 02:42:40 -0400 Subject: [PATCH 55/55] copy_to_send: no send still means Send A, as the menu item calls it Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/mix.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index f72f9ca0..f5f50682 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -694,9 +694,13 @@ fn clip_gain_to_vol(e: &mut Engine, p: &Value) -> Result { fn copy_to_send(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "automation.copy_to_send", p)?; - let slot = slot_of(p, "send", SEND_SLOTS).ok_or_else(|| { - bad("automation.copy_to_send", format!("`send` must be 0..{} or a..{}", SEND_SLOTS - 1, (b'a' + SEND_SLOTS as u8 - 1) as char)) - })? as u8; + // No `send` means Send A (the menu item passes none); a given one must be valid. + let slot = match p.get("send") { + None | Some(Value::Null) => 0, + Some(_) => slot_of(p, "send", SEND_SLOTS).ok_or_else(|| { + bad("automation.copy_to_send", format!("`send` must be 0..{} or a..{}", SEND_SLOTS - 1, (b'a' + SEND_SLOTS as u8 - 1) as char)) + })?, + } as u8; let s = e.session_mut(); for t in &tracks { if let Some(tr) = s.track_mut(*t) { @@ -882,12 +886,15 @@ mod tests { let history = e.undo_history().len(); let lanes = |e: &Engine| e.session().tracks.iter().filter(|t| t.lane(&AutoParam::SendLevel(0)).is_some_and(|l| !l.points.is_empty())).count(); let before = lanes(&e); - for bad_params in [json!({"track": "Kick", "send": 99}), json!({"track": "Kick", "send": "zz"}), json!({"track": "Kick"})] { + for bad_params in [json!({"track": "Kick", "send": 99}), json!({"track": "Kick", "send": "zz"})] { let r = e.execute("automation.copy_to_send", &bad_params); assert!(r.is_err(), "{bad_params} accepted: {r:?}"); assert_eq!(e.undo_history().len(), history, "{bad_params} pushed an undo step"); assert_eq!(lanes(&e), before, "{bad_params} edited Send A"); } + // No `send` (the menu item's call) still means Send A. + e.execute("automation.copy_to_send", &json!({"track": "Kick"})).unwrap(); + assert_eq!(lanes(&e), before + 1); // A valid slot still copies. e.execute("automation.copy_to_send", &json!({"track": "Kick", "send": "a"})).unwrap(); assert_eq!(lanes(&e), before + 1);