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 01/32] =?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 02/32] 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 0c19777c8a90a86fffc4146323fc0597307c644d Mon Sep 17 00:00:00 2001 From: Royce Date: Tue, 13 Oct 2026 22:50:49 +0800 Subject: [PATCH 03/32] 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 04/32] 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 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 05/32] 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 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 06/32] 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 07/32] 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 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 08/32] 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 09/32] 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 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 10/32] 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 11/32] 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 12/32] 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 13/32] 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 14/32] 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 15/32] 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 16/32] 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 17/32] 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 18/32] 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 19/32] 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 20/32] 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 21/32] 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 783ce8f44bc76be0a42038b4a1baed7691d1d5e9 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 15:20:11 +0200 Subject: [PATCH 22/32] 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 23/32] 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 24/32] 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 25/32] 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 52bd33bb06d5d001f7271d9272eca5d688f80ab5 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 12:04:05 -0600 Subject: [PATCH 26/32] 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 27/32] 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 28/32] 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 29/32] 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 30/32] 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 31/32] 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 32/32] 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