From e10b24565212e0279dbe2f37d427d724c851c1a0 Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:56:29 +0000 Subject: [PATCH 01/23] Keep mute changes in place when thinning automation --- crates/model/src/automation.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/model/src/automation.rs b/crates/model/src/automation.rs index 3a68b041..a6d3963b 100644 --- a/crates/model/src/automation.rs +++ b/crates/model/src/automation.rs @@ -204,7 +204,8 @@ impl AutomationLane { /// Thin: remove points whose removal changes the curve by less than `tolerance`. pub fn thin(&mut self, tolerance: f32) -> usize { - if self.points.len() < 3 || tolerance.is_nan() { + // Linear thinning would move the transitions of step lanes. + if matches!(self.param, AutoParam::Mute | AutoParam::SendMute(_)) || self.points.len() < 3 || tolerance.is_nan() { return 0; } let tolerance = f64::from(tolerance); @@ -303,6 +304,19 @@ mod tests { } } + #[test] + fn thin_keeps_mute_changes_in_place() { + for param in [AutoParam::Mute, AutoParam::SendMute(0)] { + let mut lane = AutomationLane::new(param); + for (at, value) in [(0, 0.0), (9000, 1.0), (10000, 1.0)] { + lane.set_point(at, value); + } + lane.thin(0.1); + assert_eq!(lane.value_at(8999, 0.0), 0.0); + assert_eq!(lane.value_at(9000, 0.0), 1.0); + } + } + #[test] fn param_ids() { assert_eq!(AutoParam::parse("send_b_level"), Some(AutoParam::SendLevel(1))); From 1bebc52bf4633e76247b3b203423539aa3c716e9 Mon Sep 17 00:00:00 2001 From: Justin John <34035011+justinjohn0306@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:13:40 +0530 Subject: [PATCH 02/23] MIDI writer: clamp out-of-range pitches and controller numbers instead of wrapping (#472) Pitch, release velocity, CC number and poly-pressure pitch were masked with 0x7F, so 128 became 0 and 255 became 127. They now clamp to 127, as the write_smf doc promises and note-on velocity already did. Co-Authored-By: Claude Opus 5.5 --- crates/midi/src/smf.rs | 8 ++++---- crates/midi/tests/smf.rs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/midi/src/smf.rs b/crates/midi/src/smf.rs index 36c18a0b..7747a061 100644 --- a/crates/midi/src/smf.rs +++ b/crates/midi/src/smf.rs @@ -485,20 +485,20 @@ pub fn write_smf(smf: &Smf) -> Result, MidiError> { } for n in &track.sequence.notes { let ch = n.channel & 0x0F; - let pitch = n.pitch & 0x7F; + let pitch = n.pitch.min(127); let start = clamp_tick(n.start); let end = clamp_tick(n.end()).max(start.saturating_add(1)); ev.push(WEvent { tick: start, order: 3, bytes: vec![0x90 | ch, pitch, n.velocity.clamp(1, 127)] }); - ev.push(WEvent { tick: end, order: 1, bytes: vec![0x80 | ch, pitch, n.release_velocity & 0x7F] }); + ev.push(WEvent { tick: end, order: 1, bytes: vec![0x80 | ch, pitch, n.release_velocity.min(127)] }); } for c in &track.sequence.ctrls { let ch = c.channel & 0x0F; let v7 = c.value.clamp(0, 127) as u8; let bytes = match c.kind { - CtrlKind::Cc(num) => vec![0xB0 | ch, num & 0x7F, v7], + CtrlKind::Cc(num) => vec![0xB0 | ch, num.min(127), v7], CtrlKind::Program => vec![0xC0 | ch, v7], CtrlKind::ChannelPressure => vec![0xD0 | ch, v7], - CtrlKind::PolyPressure(p) => vec![0xA0 | ch, p & 0x7F, v7], + CtrlKind::PolyPressure(p) => vec![0xA0 | ch, p.min(127), v7], CtrlKind::PitchBend => { let v = (c.value.saturating_add(8192)).clamp(0, 16383); vec![0xE0 | ch, (v & 0x7F) as u8, ((v >> 7) & 0x7F) as u8] diff --git a/crates/midi/tests/smf.rs b/crates/midi/tests/smf.rs index f1861d83..f069be39 100644 --- a/crates/midi/tests/smf.rs +++ b/crates/midi/tests/smf.rs @@ -332,3 +332,21 @@ proptest! { prop_assert_eq!(a, b); } } + +/// #472: out-of-range pitches, release velocities and controller numbers clamp to 127 instead of +/// wrapping into other notes and controllers (128 & 0x7F would be 0). +#[test] +fn writer_clamps_out_of_range_data_bytes() { + for raw in [127u8, 128, 255] { + let mut seq = Sequence::default(); + seq.notes.push(Note { pitch: raw, velocity: 100, release_velocity: raw, channel: 0, start: 0, length: 960 }); + seq.ctrls.push(CtrlEvent { tick: 0, channel: 0, kind: CtrlKind::Cc(raw), value: 64 }); + seq.ctrls.push(CtrlEvent { tick: 0, channel: 0, kind: CtrlKind::PolyPressure(raw), value: 64 }); + let smf = Smf { tracks: vec![SmfTrack { name: String::new(), sequence: seq }], ..Smf::default() }; + let back = read_smf(&write_smf(&smf).unwrap()).unwrap(); + let s = &back.tracks.last().unwrap().sequence; + assert_eq!((s.notes[0].pitch, s.notes[0].release_velocity), (127, 127), "input {raw}"); + let kinds: Vec = s.ctrls.iter().map(|c| c.kind).collect(); + assert!(kinds.contains(&CtrlKind::Cc(127)) && kinds.contains(&CtrlKind::PolyPressure(127)), "input {raw}: {kinds:?}"); + } +} From bf3cd7ac09be0f09162e8f944b8f601dd047567f Mon Sep 17 00:00:00 2001 From: Jafz2001 Date: Tue, 13 Oct 2026 11:40:34 -0300 Subject: [PATCH 03/23] fix(routing): remove folder bus on delete --- crates/engine/src/cmd/track.rs | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/engine/src/cmd/track.rs b/crates/engine/src/cmd/track.rs index 87912389..5e19182c 100644 --- a/crates/engine/src/cmd/track.rs +++ b/crates/engine/src/cmd/track.rs @@ -493,6 +493,15 @@ fn folder_bus(s: &Session, folder: TrackId) -> Option { s.track(folder).map(|folder| folder.mixer.input.clone()).filter(|route| matches!(route, Route::Bus(_))) } +/// The summing bus owned by a routing folder. +fn owned_folder_bus(s: &Session, folder: TrackId) -> Option { + let folder = s.track(folder).filter(|track| track.kind == TrackKind::Folder)?; + match folder.mixer.input { + Route::Bus(id) => Some(id), + _ => None, + } +} + /// Keep only the selected roots so selecting a folder and one of its children preserves the subtree. fn root_tracks(s: &Session, tracks: Vec) -> Vec { tracks.iter().copied().filter(|track| !tracks.iter().any(|root| root != track && in_folder(s, *track, *root))).collect() @@ -577,6 +586,20 @@ fn change_width(e: &mut Engine, p: &Value) -> Result { fn delete(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "track.delete", p)?; let s = e.session_mut(); + let removed_busses: Vec<_> = tracks + .iter() + .filter_map(|folder| { + let bus = owned_folder_bus(s, *folder)?; + let mut parent = s.track(*folder).and_then(|folder| folder.folder); + for _ in 0..64 { + match parent { + Some(candidate) if tracks.contains(&candidate) => parent = s.track(candidate).and_then(|folder| folder.folder), + _ => break, + } + } + Some((bus, parent.and_then(|parent| folder_bus(s, parent)).unwrap_or(Route::Main))) + }) + .collect(); // Promote children of deleted folders and repair only routes that used the deleted // folder's own bus. Custom outputs remain untouched. let mut promoted = Vec::new(); @@ -607,6 +630,22 @@ fn delete(e: &mut Engine, p: &Value) -> Result { for g in &mut s.groups { g.members.retain(|t| !tracks.contains(t)); } + for (bus, replacement) in removed_busses { + s.busses.retain(|candidate| candidate.id != bus); + for track in &mut s.tracks { + if track.mixer.input == Route::Bus(bus) { + track.mixer.input = Route::None; + } + if track.mixer.output == Route::Bus(bus) { + track.mixer.output = replacement.clone(); + } + for send in &mut track.mixer.sends { + if send.as_ref().is_some_and(|send| send.target == Route::Bus(bus)) { + *send = None; + } + } + } + } Ok(json!({"deleted": tracks.len()})) } @@ -920,6 +959,14 @@ mod tests { e.execute("track.move_to_folder", &json!({"tracks": ["Kick"], "folder": inner.0})).unwrap(); e.execute("track.move_to_folder", &json!({"tracks": [inner.0], "folder": outer.0})).unwrap(); + let inner_bus = match e.session().track(inner).unwrap().mixer.input { + Route::Bus(bus) => bus, + _ => panic!("folder did not receive a bus"), + }; + let outer_bus = match e.session().track(outer).unwrap().mixer.input { + Route::Bus(bus) => bus, + _ => panic!("folder did not receive a bus"), + }; assert!(e.execute("track.move_to_folder", &json!({"tracks": [outer.0], "folder": inner.0})).is_err()); let kick = e.session().track_by_name("Kick").unwrap(); @@ -946,6 +993,8 @@ mod tests { let kick = e.session().track_by_name("Kick").unwrap(); assert_eq!(kick.folder, None); assert_eq!(kick.mixer.output, Route::Main); + assert!(e.session().bus(inner_bus).is_none(), "the deleted folder's owned bus must be removed"); + assert!(e.session().bus(outer_bus).is_some(), "the surviving parent folder's bus must remain"); } #[test] From 8ac95607b718d16e21d057270f974b4728c494f2 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:14 +0200 Subject: [PATCH 04/23] Return TooLarge when the WAV header size overflows u64 (#467) --- crates/audio-io/src/wav.rs | 5 ++++- crates/audio-io/tests/roundtrip.rs | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/audio-io/src/wav.rs b/crates/audio-io/src/wav.rs index 1aa847d4..7dcd36b8 100644 --- a/crates/audio-io/src/wav.rs +++ b/crates/audio-io/src/wav.rs @@ -442,7 +442,10 @@ pub(crate) fn header( if is_float { push_chunk(&mut meta, b"fact", &u32::try_from(frames).unwrap_or(u32::MAX).to_le_bytes()); } - let total = (4 + (8 + rf64_body_len) + meta.len() + 8) as u64 + data_len + (data_len & 1); + let total = ((4 + (8 + rf64_body_len) + meta.len() + 8) as u64) + .checked_add(data_len) + .and_then(|t| t.checked_add(data_len & 1)) + .ok_or_else(|| AudioError::TooLarge("WAV size overflow".into()))?; let rf64 = force_rf64 || u32::try_from(total).is_err(); if rf64 { diff --git a/crates/audio-io/tests/roundtrip.rs b/crates/audio-io/tests/roundtrip.rs index a53c272a..10ba2ecf 100644 --- a/crates/audio-io/tests/roundtrip.rs +++ b/crates/audio-io/tests/roundtrip.rs @@ -284,3 +284,10 @@ fn float_wav_header_is_rewritten_in_place_as_the_file_grows() { assert_eq!((&big[..4], big.len()), (&b"RF64"[..], len)); assert!(wav_float_header(0, 48_000, 0).is_err()); } + +#[test] +fn float_wav_header_rejects_a_frame_count_whose_total_size_overflows() { + // 4 bytes per frame: the data length fits in u64, but adding the header to it does not. + let frames = u64::MAX / 4; + assert!(matches!(wav_float_header(1, 48_000, frames), Err(soundcraft_audio_io::AudioError::TooLarge(_)))); +} From edd2b0ef73b6e9472db6e106ef6993814679a591 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:22 +0200 Subject: [PATCH 05/23] Keep audio-io stream test limits as u64 so it compiles on 32-bit targets (#462) --- crates/audio-io/tests/stream.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/audio-io/tests/stream.rs b/crates/audio-io/tests/stream.rs index 61a28946..7b149ca6 100644 --- a/crates/audio-io/tests/stream.rs +++ b/crates/audio-io/tests/stream.rs @@ -52,13 +52,15 @@ fn streamed_files_match_whole_buffer_encodes() { #[test] fn blocks_past_a_format_limit_are_refused_before_anything_is_written() { let buf = tone(2, 100); - for (format, limit) in [(FileFormat::Aiff, 1usize << 30), (FileFormat::Flac, 1 << 36)] { + for (format, limit) in [(FileFormat::Aiff, 1u64 << 30), (FileFormat::Flac, 1u64 << 36)] { let opts = EncodeOptions { format, bit_depth: BitDepth::Int16, dither: false, bwf: None }; let mut enc = FileEncoder::new(Cursor::new(Vec::new()), &opts, 2, buf.sample_rate, None).unwrap(); enc.write(&buf.channels, 100).unwrap(); - assert!(enc.check_room(limit as u64 / 2).is_ok(), "{format:?}"); - assert!(matches!(enc.check_room(limit as u64), Err(AudioError::TooLarge(_))), "{format:?}"); - assert!(matches!(enc.write(&[], limit), Err(AudioError::TooLarge(_))), "{format:?}"); + assert!(enc.check_room(limit / 2).is_ok(), "{format:?}"); + assert!(matches!(enc.check_room(limit), Err(AudioError::TooLarge(_))), "{format:?}"); + if let Ok(frames) = usize::try_from(limit) { + assert!(matches!(enc.write(&[], frames), Err(AudioError::TooLarge(_))), "{format:?}"); + } assert_eq!(enc.finish().unwrap().into_inner(), encode(&buf, &opts).unwrap()); } } From 9209909ff78da930fa8d934315f3c13460379bbb Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:15 +0200 Subject: [PATCH 06/23] Count the partial quartet in the base64 decoded-size cap (#468) --- crates/model/src/b64.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/model/src/b64.rs b/crates/model/src/b64.rs index 8288c466..1435d313 100644 --- a/crates/model/src/b64.rs +++ b/crates/model/src/b64.rs @@ -37,15 +37,19 @@ fn value(c: u8) -> Option { /// Decodes padded (or unpadded) standard base64. `None` for malformed input or a result larger /// than [`MAX_DECODED_BYTES`]. pub fn decode(text: &str) -> Option> { + if text.bytes().filter(|c| !c.is_ascii_whitespace()).count() > MAX_DECODED_BYTES.div_ceil(3).saturating_mul(4) { + return None; + } let syms: Vec = text.bytes().filter(|c| !c.is_ascii_whitespace()).collect(); let body = syms.strip_suffix(b"==").or_else(|| syms.strip_suffix(b"=")).unwrap_or(&syms); if body.contains(&b'=') || (syms.len() != body.len() && !syms.len().is_multiple_of(4)) || body.len() % 4 == 1 { return None; } - if body.len() / 4 * 3 > MAX_DECODED_BYTES { + let decoded_len = body.len() / 4 * 3 + body.len() % 4 * 3 / 4; + if decoded_len > MAX_DECODED_BYTES { return None; } - let mut out = Vec::with_capacity(body.len() / 4 * 3 + 2); + let mut out = Vec::with_capacity(decoded_len); for chunk in body.chunks(4) { let mut n = 0u32; for (i, &c) in chunk.iter().enumerate() { @@ -81,4 +85,17 @@ mod tests { assert!(decode(bad).is_none(), "{bad}"); } } + + #[test] + fn size_cap_counts_the_partial_quartet() { + for (extra, ok) in [(-1, true), (0, true), (1, false), (2, false)] { + let data = vec![7u8; MAX_DECODED_BYTES.saturating_add_signed(extra)]; + let padded = encode(&data); + let unpadded = padded.trim_end_matches('='); + assert_eq!(decode(&padded).is_some(), ok, "padded, {extra}"); + assert_eq!(decode(unpadded).is_some(), ok, "unpadded, {extra}"); + let spaced = format!("{}\n {}", &padded[..8], &padded[8..]); + assert_eq!(decode(&spaced).is_some(), ok, "whitespace, {extra}"); + } + } } From 7bcb4e48026e0aa3a41101b89233cdf7417c4404 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:27 +0200 Subject: [PATCH 07/23] Do not retry remote-control commands that may already have run (#458) --- crates/automation/src/lib.rs | 74 ++++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 12 deletions(-) diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 17c3a5bc..1172f7be 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -106,37 +106,48 @@ impl Remote { Ok(()) } - fn roundtrip(&mut self, method: &str, params: &Value) -> Result { - self.connect()?; + /// One request/reply exchange. On failure the flag says whether the request line was already + /// written to the socket (so the app may have executed it). + fn roundtrip(&mut self, method: &str, params: &Value) -> Result { + self.connect().map_err(|e| (e, false))?; let id = self.next_id; self.next_id += 1; let line = json!({"id": id, "method": method, "params": params}).to_string(); - let (w, r) = self.conn.as_mut().ok_or("not connected")?; - writeln!(w, "{line}").map_err(|e| e.to_string())?; + let (w, r) = self.conn.as_mut().ok_or_else(|| ("not connected".to_string(), false))?; + writeln!(w, "{line}").map_err(|e| (e.to_string(), false))?; let mut buf = String::new(); - r.read_line(&mut buf).map_err(|e| e.to_string())?; + r.read_line(&mut buf).map_err(|e| (e.to_string(), true))?; if buf.is_empty() { - return Err("connection closed".into()); + return Err(("connection closed".into(), true)); } - let v: Value = serde_json::from_str(&buf).map_err(|e| e.to_string())?; + let v: Value = serde_json::from_str(&buf).map_err(|e| (e.to_string(), true))?; if v.get("ok").and_then(Value::as_bool).unwrap_or(false) { Ok(v.get("result").cloned().unwrap_or(Value::Null)) } else { - Err(v.get("error").and_then(Value::as_str).unwrap_or("error").to_string()) + Err((v.get("error").and_then(Value::as_str).unwrap_or("error").to_string(), true)) } } } +/// Methods that only read state, so repeating them after a lost reply is harmless. +fn is_read_only(method: &str) -> bool { + matches!(method, "session.inspect" | "document.inspect" | "engine.commands" | "engine.parity" | "ui.inspect" | "ui.menu.list") +} + impl Backend for Remote { fn call(&mut self, method: &str, params: Value) -> Result { match self.roundtrip(method, ¶ms) { Ok(v) => Ok(v), - Err(e) if e.contains("closed") || e.contains("Broken pipe") || e.contains("reset") => { - // Retry once on a fresh connection. + Err((e, sent)) if e.contains("closed") || e.contains("Broken pipe") || e.contains("reset") => { self.conn = None; - self.roundtrip(method, ¶ms) + if sent && !is_read_only(method) { + // The app may already have executed the request; repeating it could apply it twice. + return Err(format!("{e}; `{method}` may or may not have been executed, check the session before repeating it")); + } + // Retry once on a fresh connection. + self.roundtrip(method, ¶ms).map_err(|(e, _)| e) } - Err(e) => Err(e), + Err((e, _)) => Err(e), } } fn has_ui(&self) -> bool { @@ -167,4 +178,43 @@ mod tests { let mut r = Remote::new("127.0.0.1:1"); assert!(r.call("session.inspect", json!({})).is_err()); } + + /// Fake app: accepts one connection per entry in `replies`, counts the request line it reads, + /// then either drops the connection (`false`) or answers ok (`true`). + fn fake_app(replies: Vec) -> (String, Arc) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let seen = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let counter = seen.clone(); + std::thread::spawn(move || { + for answer in replies { + let Ok((mut sock, _)) = listener.accept() else { return }; + let mut line = String::new(); + if BufReader::new(sock.try_clone().unwrap()).read_line(&mut line).unwrap_or(0) > 0 { + counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + if answer { + let _ = writeln!(sock, "{}", json!({"id": 1, "ok": true, "result": {"done": true}})); + } + } + } + }); + (addr, seen) + } + + #[test] + fn remote_does_not_repeat_a_mutation_after_a_lost_reply() { + let (addr, seen) = fake_app(vec![false, true]); + let mut r = Remote::new(&addr); + let err = r.call("engine.execute", json!({"command": "track.new"})).unwrap_err(); + assert!(err.contains("may or may not have been executed"), "{err}"); + assert_eq!(seen.load(std::sync::atomic::Ordering::SeqCst), 1); + } + + #[test] + fn remote_retries_a_read_after_a_lost_reply() { + let (addr, seen) = fake_app(vec![false, true]); + let mut r = Remote::new(&addr); + assert!(r.call("session.inspect", json!({})).is_ok()); + assert_eq!(seen.load(std::sync::atomic::Ordering::SeqCst), 2); + } } From 807230167d1fa218590f98e93356f6ac0cf88881 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:26 +0200 Subject: [PATCH 08/23] Reset clip stretch after AudioSuite bakes it into the new source (#451) --- crates/engine/src/cmd/audiosuite.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/engine/src/cmd/audiosuite.rs b/crates/engine/src/cmd/audiosuite.rs index 585f2bec..911ccca5 100644 --- a/crates/engine/src/cmd/audiosuite.rs +++ b/crates/engine/src/cmd/audiosuite.rs @@ -117,6 +117,8 @@ fn process(e: &mut Engine, p: &Value) -> Result { nc.name = new_name; nc.content = ClipContent::Audio { source: src, offset: 0 }; nc.length = i64::try_from(frames).unwrap_or(nc.length); + // The render already has the clip's stretch applied; keeping it would stretch the new file again. + nc.stretch = 1.0; // Invert and Duplicate are phase/copy operations. The clip's gain stays a clip gain, // the same way its fades stay editable. Other processes bake a new file at unity gain. if !matches!(name.as_str(), "invert" | "duplicate") { @@ -216,6 +218,21 @@ mod tests { assert!((sample - 0.5).abs() < 1e-3, "duplicate keeps the file level and the clip gain, got {sample}"); } + #[test] + fn processed_clip_does_not_keep_the_stretch_it_baked() { + for stretch in [0.5, 2.0] { + for process in ["invert", "duplicate"] { + let (mut e, id) = gained_clip(); + e.session_mut().find_clip_mut(id).unwrap().stretch = stretch; + e.execute("audiosuite.process", &json!({"process": process, "clips": [id.0]})).unwrap(); + let c = e.session().tracks[0].clips()[0].clone(); + let frames = e.session().pool.get(c.source().unwrap()).unwrap().buffer.channels[0].len(); + assert_eq!(c.stretch, 1.0, "{process} at {stretch}"); + assert_eq!(c.length, i64::try_from(frames).unwrap(), "{process} at {stretch}"); + } + } + } + #[test] fn signal_generator_fills_the_selection_with_a_clip() { let (mut e, _) = gained_clip(); From 6c84416c3cffb72d8d350ddfefc4fc9ee954ca1a Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:23 +0200 Subject: [PATCH 09/23] Reject invalid numeric prereleases and restore Cargo.toml when version update fails (#471) --- xtask/src/version.rs | 75 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 13 deletions(-) diff --git a/xtask/src/version.rs b/xtask/src/version.rs index d9bb7aa5..d469c2e9 100644 --- a/xtask/src/version.rs +++ b/xtask/src/version.rs @@ -46,11 +46,20 @@ pub fn validate(v: &str) -> Result<(), String> { None => (v, None), }; let nums: Vec<&str> = core.split('.').collect(); - if nums.len() != 3 || nums.iter().any(|n| n.is_empty() || !n.bytes().all(|b| b.is_ascii_digit()) || (n.len() > 1 && n.starts_with('0'))) { + // Numeric parts must fit in u64, like the semver crate Cargo uses. + if nums.len() != 3 + || nums.iter().any(|n| !n.bytes().all(|b| b.is_ascii_digit()) || (n.len() > 1 && n.starts_with('0')) || n.parse::().is_err()) + { return bad(); } + // A numeric prerelease identifier has no leading zero (any length is fine); alphanumeric ones may start with `0`. + let bad_id = |id: &str| { + id.is_empty() + || !id.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') + || (id.len() > 1 && id.starts_with('0') && id.bytes().all(|b| b.is_ascii_digit())) + }; if let Some(p) = pre - && (p.is_empty() || p.split('.').any(|id| id.is_empty() || !id.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-'))) + && (p.is_empty() || p.split('.').any(bad_id)) { return bad(); } @@ -91,6 +100,31 @@ fn cargo_update(root: &Path, offline: bool) -> Result<(), String> { if st.success() { Ok(()) } else { Err(format!("cargo update --workspace{} failed", if offline { " --offline" } else { "" })) } } +/// Write atomically: a half-written root manifest breaks every build in the workspace. +fn write_manifest(root: &Path, path: &Path, content: &str) -> Result<(), String> { + let dir = root.join("target"); + std::fs::create_dir_all(&dir).map_err(|e| format!("create {}: {e}", dir.display()))?; + let tmp = dir.join("Cargo.toml.xtask-version"); + std::fs::write(&tmp, content).map_err(|e| format!("write {}: {e}", tmp.display()))?; + std::fs::rename(&tmp, path).map_err(|e| format!("replace {}: {e}", path.display())) +} + +/// Writes version `new` into the manifest at `path` (current text `text`), then runs `update`. +/// If `update` fails the old manifest is put back. Returns the old version. +fn set(root: &Path, path: &Path, text: &str, new: &str, update: impl FnOnce() -> Result<(), String>) -> Result { + let old = read(text)?; + let updated = replace(text, new)?; + write_manifest(root, path, &updated)?; + if let Err(e) = update() { + // Put the old manifest back so a failed update does not leave the workspace changed. + return Err(match write_manifest(root, path, text) { + Ok(()) => format!("{e}; Cargo.toml restored to {old}"), + Err(r) => format!("{e}; could not restore Cargo.toml: {r}"), + }); + } + Ok(old) +} + /// `cargo xtask version [set X.Y.Z[-pre]]`; `args` are the words after `version`. pub fn run(args: &[String]) -> Result<(), String> { let root = root(); @@ -103,18 +137,8 @@ pub fn run(args: &[String]) -> Result<(), String> { Ok(()) } ["set", new] => { - let old = read(&text)?; - let updated = replace(&text, new)?; - // Write atomically: a half-written root manifest breaks every build in the workspace. - let dir = root.join("target"); - std::fs::create_dir_all(&dir).map_err(|e| format!("create {}: {e}", dir.display()))?; - let tmp = dir.join("Cargo.toml.xtask-version"); - std::fs::write(&tmp, updated).map_err(|e| format!("write {}: {e}", tmp.display()))?; - std::fs::rename(&tmp, &path).map_err(|e| format!("replace {}: {e}", path.display()))?; // Refresh the workspace members' entries in Cargo.lock (no dependency upgrades). - if cargo_update(&root, true).is_err() { - cargo_update(&root, false)?; - } + let old = set(&root, &path, &text, new, || cargo_update(&root, true).or_else(|_| cargo_update(&root, false)))?; println!("version: {old} -> {new}"); Ok(()) } @@ -160,6 +184,31 @@ mod tests { assert!(replace(MANIFEST, "nope").is_err()); } + #[test] + fn rejects_what_cargo_rejects() { + for bad in ["1.0.0-01", "1.0.0-rc.01", "1.0.0-0.00", "1.0.0-rc.1.007", "99999999999999999999.0.0", "1.18446744073709551616.0"] { + assert!(validate(bad).is_err(), "{bad}"); + assert!(replace(MANIFEST, bad).is_err(), "{bad}"); + } + for ok in ["1.0.0-0", "1.0.0-rc.0", "1.0.0-0a", "1.0.0-01a", "1.0.0-99999999999999999999", "18446744073709551615.0.0"] { + assert!(validate(ok).is_ok(), "{ok}"); + } + } + + #[test] + fn a_failed_update_restores_the_manifest() { + let root = std::env::temp_dir().join(format!("xtask-version-restore-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let path = root.join("Cargo.toml"); + std::fs::write(&path, MANIFEST).unwrap(); + let err = set(&root, &path, MANIFEST, "2.0.0", || Err("cargo update failed".into())).unwrap_err(); + assert!(err.contains("restored to 0.1.0"), "{err}"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), MANIFEST); + assert_eq!(set(&root, &path, MANIFEST, "2.0.0", || Ok(())).unwrap(), "0.1.0"); + assert_eq!(read(&std::fs::read_to_string(&path).unwrap()).unwrap(), "2.0.0"); + std::fs::remove_dir_all(&root).unwrap(); + } + #[test] fn the_real_manifest_has_a_valid_version() { let text = std::fs::read_to_string(root().join("Cargo.toml")).unwrap(); From 2a43710e526570c6e459dbe2554c90f78ac8349d Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:51:48 +0200 Subject: [PATCH 10/23] Scale fades, sync point and gain envelope when importing a clip group at another sample rate (#453) --- crates/engine/src/clip_group_file.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index e20caf0c..ee1f3764 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -160,6 +160,10 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - c.id = s.new_clip_id(); c.start = at.saturating_add(scale(c.start.max(0))); c.length = scale(c.length); + c.fade_in.len = scale(c.fade_in.len); + c.fade_out.len = scale(c.fade_out.len); + c.sync_point = scale(c.sync_point); + c.gain_env.iter_mut().for_each(|(at, _)| *at = scale(*at)); c.group = Some(group_id); c.sanitize(); placed.push(c.id); @@ -229,6 +233,28 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn fades_sync_point_and_envelope_follow_the_sample_rate() { + let dir = std::env::temp_dir().join(format!("sc-grp-rate-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + let p = dir.join("rate.scgrp"); + let mut e = crate::demo::demo_engine(); + // The file was saved at half the session rate, so every position doubles. + let src_rate = e.session().sample_rate.hz() / 2; + let fade = |len: i64| json!({"len": len, "shape": "Linear"}); + let clip = json!({ + "id": 1, "name": "m", "start": 0, "length": 10_000, "content": {"Midi": {"sequence": {"notes": [], "ctrls": []}}}, + "fade_in": fade(1000), "fade_out": fade(2000), "sync_point": 500, "gain_env": [[100, -6.0], [4000, 0.0]] + }); + let file = json!({"format": "soundcraft.clipgroup", "version": 1, "sample_rate": src_rate, "length": 10_000, "lanes": [[clip]], "audio": {}}); + std::fs::write(&p, file.to_string()).unwrap(); + let r = e.execute("file.import_clip_groups", &json!({"path": p.to_string_lossy()})).unwrap(); + let c = e.session().find_clip(soundcraft_model::ClipId(r["clips"][0].as_u64().unwrap())).unwrap().1; + assert_eq!((c.length, c.fade_in.len, c.fade_out.len, c.sync_point), (20_000, 2000, 4000, 1000)); + assert_eq!(c.gain_env.iter().map(|&(at, _)| at).collect::>(), vec![200, 8000]); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn a_file_named_by_many_ids_becomes_one_source() { let dir = std::env::temp_dir().join(format!("sc-grp-shared-{}", std::process::id())); From 75eb152f03ddbd03249ca81cacc23b06f742a760 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:37 +0200 Subject: [PATCH 11/23] Create session backups exclusively and stop when backup numbers run out (#426) --- crates/engine/src/io.rs | 62 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..aee56ad1 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1010,6 +1010,18 @@ fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> { f.sync_all() } +/// Copies `from` to `to`, which must not exist yet; an existing entry, even a link, is never followed. +fn copy_new(from: &Path, to: &Path) -> std::io::Result<()> { + let mut src = std::fs::File::open(from)?; + let mut dst = std::fs::OpenOptions::new().write(true).create_new(true).open(to)?; + if let Err(err) = std::io::copy(&mut src, &mut dst) { + drop(dst); + let _ = std::fs::remove_file(to); + return Err(err); + } + Ok(()) +} + /// Copy the session file about to be replaced to `Backups/ backup .`, keeping the /// newest ten. fn back_up(file: &Path) { @@ -1030,9 +1042,16 @@ fn back_up(file: &Path) { .flatten() .filter_map(|f| f.file_name().to_str()?.strip_prefix(&prefix)?.strip_suffix(&suffix)?.parse().ok()) .collect(); - let next = numbers.iter().max().map_or(1, |n| n.saturating_add(1)); - let copied = std::fs::create_dir_all(&backups).and_then(|()| std::fs::copy(file, backups.join(format!("{prefix}{next}{suffix}")))); - if let Err(err) = copied { + let Some(next) = numbers.iter().max().map_or(Some(1), |n| n.checked_add(1)) else { + log::warn!("no backup of {}: backup numbers are exhausted", file.display()); + return; + }; + let dest = backups.join(format!("{prefix}{next}{suffix}")); + if let Err(err) = crate::file_access::check_write(&dest) { + log::warn!("no backup of {}: {err}", file.display()); + return; + } + if let Err(err) = std::fs::create_dir_all(&backups).and_then(|()| copy_new(file, &dest)) { log::warn!("no backup of {}: {err}", file.display()); return; } @@ -1648,6 +1667,43 @@ mod save_tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn backup_never_overwrites_an_existing_entry_when_numbers_run_out() { + let dir = std::env::temp_dir().join(format!("soundcraft-backups-full-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Backups")).unwrap(); + let path = dir.join("Song.scraft"); + std::fs::write(&path, b"session").unwrap(); + let last = dir.join(format!("Backups/Song backup {}.scraft", u32::MAX)); + std::fs::write(&last, b"sentinel").unwrap(); + back_up(&path); + assert_eq!(std::fs::read(&last).unwrap(), b"sentinel"); + let dest = dir.join("Backups/Song backup 1.scraft"); + std::fs::write(&dest, b"taken").unwrap(); + assert!(copy_new(&path, &dest).is_err()); + assert_eq!(std::fs::read(&dest).unwrap(), b"taken"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn backup_never_writes_through_a_link_to_an_outside_file() { + let dir = std::env::temp_dir().join(format!("soundcraft-backups-link-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Project/Backups")).unwrap(); + let outside = dir.join("outside.txt"); + std::fs::write(&outside, b"sentinel").unwrap(); + let path = dir.join("Project/Song.scraft"); + std::fs::write(&path, b"session").unwrap(); + std::os::unix::fs::symlink(&outside, dir.join(format!("Project/Backups/Song backup {}.scraft", u32::MAX))).unwrap(); + back_up(&path); + let link = dir.join("Project/Backups/Song backup 1.scraft"); + std::os::unix::fs::symlink(&outside, &link).unwrap(); + assert!(copy_new(&path, &link).is_err()); + assert_eq!(std::fs::read(&outside).unwrap(), b"sentinel"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn save_gives_sources_that_share_a_path_their_own_files() { let mut s = Session::default(); From 9afb266fcee3505a931194ba5539a249f545b95f Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:42 +0200 Subject: [PATCH 12/23] Fix bundle opening losing audio for sources that share one file (#446) --- crates/engine/src/io.rs | 55 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..29198daa 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1248,14 +1248,32 @@ pub fn open_session_bytes(e: &mut Engine, name: &str, bytes: &[u8]) -> Result>> = std::collections::HashMap::new(); for src in &s.sources { - match files.remove(&src.path) { - Some(audio) => match decode_source(&audio, Path::new(&src.path).extension().and_then(|x| x.to_str()), s.sample_rate.hz()) { - Ok(audio) => s.pool.insert(src.id, audio), - Err(err) => missing.push(format!("{}: {err}", src.path)), - }, - None => missing.push(src.path.clone()), + if let Some(audio) = decoded.get(&src.path) { + if let Some(audio) = audio { + s.pool.insert(src.id, Arc::clone(audio)); + } + continue; } + let audio = match files.remove(&src.path) { + Some(bytes) => match decode_source(&bytes, Path::new(&src.path).extension().and_then(|x| x.to_str()), s.sample_rate.hz()) { + Ok(audio) => Some(audio), + Err(err) => { + missing.push(format!("{}: {err}", src.path)); + None + } + }, + None => { + missing.push(src.path.clone()); + None + } + }; + if let Some(audio) = &audio { + s.pool.insert(src.id, Arc::clone(audio)); + } + decoded.insert(src.path.clone(), audio); } // Movies are referenced by path, never copied into a session: none can be found here. missing.extend(s.videos.iter().map(|v| v.path.clone())); @@ -1886,6 +1904,31 @@ mod bundle_tests { assert_eq!(r.path.as_deref(), Some("Midnight Groove.scraftz")); } + #[test] + fn sources_sharing_one_file_all_load_from_a_bundle() { + use std::io::Write as _; + let e = crate::demo::demo_engine(); + let (bytes, _) = session_bundle(e.session()).unwrap(); + let (text, files) = read_bundle(&bytes).unwrap(); + let mut s = Session::from_json(&text).unwrap(); + s.sources[1].path = s.sources[0].path.clone(); + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + for path in s.sources.iter().map(|x| &x.path).collect::>() { + w.start_file(path.as_str(), opts).unwrap(); + w.write_all(&files[path]).unwrap(); + } + w.start_file("Song.scraft", opts).unwrap(); + w.write_all(s.to_json().unwrap().as_bytes()).unwrap(); + let bundle = w.finish().unwrap().into_inner(); + let mut r = Engine::default(); + let missing = open_session_bytes(&mut r, "Song.scraftz", &bundle).unwrap(); + assert!(missing.is_empty(), "{missing:?}"); + for src in &r.session().sources { + assert!(r.session().pool.get(src.id).is_some(), "{} has no audio", src.name); + } + } + #[test] fn same_named_sources_get_distinct_files() { let mut e = crate::demo::demo_engine(); From 33d59b26a644a33b7fbe21afb24fd14b99d7913b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:51:12 +0200 Subject: [PATCH 13/23] Keep controller-only tracks when importing MIDI (#454) --- crates/engine/src/io.rs | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..d07f3eea 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -278,17 +278,19 @@ pub fn import_midi_bytes(e: &mut Engine, bytes: &[u8], at: Samples, tempo_map: b let base_tick = s.tempo.samples_to_ticks(at, s.sample_rate); let mut out = Vec::new(); for (i, tr) in smf.tracks.iter().enumerate() { - if tr.sequence.notes.is_empty() { + if tr.sequence.notes.is_empty() && tr.sequence.ctrls.is_empty() { continue; } let mut name = if tr.name.trim().is_empty() { format!("MIDI {}", i + 1) } else { tr.name.clone() }; soundcraft_model::truncate_chars(&mut name, soundcraft_model::MAX_NAME_CHARS); let id = s.add_track(TrackKind::Instrument, ChannelFormat::Stereo, Some(&name)); - let drums = tr.sequence.notes.iter().all(|n| n.channel == 9); + let drums = !tr.sequence.notes.is_empty() && tr.sequence.notes.iter().all(|n| n.channel == 9); if let Some(t) = s.track_mut(id) { t.instrument = Some(soundcraft_model::Insert::new(if drums { "drum_synth" } else { "subtractive_synth" })); } - let end_tick = base_tick.saturating_add(tr.sequence.end_tick().max(1)); + // A controller on the clip's last tick would not play, so the clip runs one tick past the last one. + let past_ctrls = tr.sequence.ctrls.iter().map(|c| c.tick.saturating_add(1)).max().unwrap_or(0); + let end_tick = base_tick.saturating_add(tr.sequence.end_tick().max(past_ctrls).max(1)); let len = s.tempo.tick_to_samples(end_tick, s.sample_rate).saturating_sub(at); if len > s.sample_rate.samples(86_400.0) { return Err(EngineError::Io(format!("MIDI track {}: longer than 24 hours", i + 1))); @@ -1748,6 +1750,28 @@ mod name_tests { assert_eq!((track.name.len(), track.clips()[0].name.len()), (soundcraft_model::MAX_NAME_CHARS, soundcraft_model::MAX_NAME_CHARS)); } + #[test] + fn a_controller_only_midi_track_is_imported_with_its_events() { + let ctrls = vec![ + soundcraft_midi::CtrlEvent { tick: 0, channel: 0, kind: soundcraft_midi::CtrlKind::Cc(7), value: 100 }, + soundcraft_midi::CtrlEvent { tick: 480, channel: 0, kind: soundcraft_midi::CtrlKind::Cc(7), value: 64 }, + soundcraft_midi::CtrlEvent { tick: 960, channel: 0, kind: soundcraft_midi::CtrlKind::Cc(7), value: 20 }, + ]; + let sequence = soundcraft_midi::Sequence { ctrls: ctrls.clone(), ..Default::default() }; + let smf = soundcraft_midi::Smf { tracks: vec![soundcraft_midi::SmfTrack { name: "Vol".into(), sequence }], ..Default::default() }; + let mut e = Engine::default(); + let ids = import_midi_bytes(&mut e, &soundcraft_midi::write_smf(&smf).unwrap(), 0, false).unwrap(); + assert_eq!(ids.len(), 1); + let track = e.session().track(ids[0]).unwrap(); + assert_eq!(track.instrument.as_ref().map(|i| i.plugin.as_str()), Some("subtractive_synth")); + // The clip must reach past the last event, or playback skips it. + assert!(track.clips()[0].end() > e.session().tempo.tick_to_samples(960, e.session().sample_rate)); + match &track.clips()[0].content { + ClipContent::Midi { sequence } => assert_eq!(sequence.ctrls, ctrls), + _ => panic!("expected a MIDI clip"), + } + } + #[test] fn midi_longer_than_a_day_is_refused() { let held = soundcraft_midi::Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start: 0, length: (1 << 28) - 1 }; From 52dc4f9822775b1f34e5e4ad23d3813120d6c20c Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:50:50 +0200 Subject: [PATCH 14/23] Reset clip effect EQ/compressor state on stop and seek (#442) --- crates/mix/src/lib.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index e8c4fbad..3d78bde5 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -515,6 +515,9 @@ impl MixEngine { s.align.clear(); s.align_pre.clear(); s.out_extra.clear(); + for fx in s.clip_fx.values_mut() { + fx.reset(); + } } self.meters.clear(); self.main_meter = StripMeter::default(); @@ -1149,6 +1152,12 @@ impl ClipFx { fn new(sr: f32, ch: usize) -> ClipFx { ClipFx { eq: None, comp: None, gain_db: 0.0, sr, ch } } + /// Clear filter, detector and lookahead history (transport stop / locate). + fn reset(&mut self) { + for p in self.eq.iter_mut().chain(self.comp.iter_mut()) { + p.reset(); + } + } fn make(&self, id: &str) -> Option> { soundcraft_dsp::create(id).map(|mut p| { p.prepare(self.sr, 16_384, self.ch.max(1)); @@ -2902,6 +2911,29 @@ mod tests { assert!(out.iter().flatten().all(|x| *x == 0.0), "old aligned audio after a reset"); } + /// A reset (stop, locate) must clear the cached clip EQ/compressor history too. + #[test] + fn reset_clears_clip_effect_history() { + // Compressor lookahead delay, then EQ high-pass filter state, each primed with DC. + for (param, value) in [("comp.lookahead", 5.0), ("eq.hpf_on", 1.0)] { + let (mut s, t) = session_with_dc(0.5, 9600); + s.edit.click = false; + let cid = s.track(t).unwrap().clips()[0].id.0; + s.edit.values.insert(format!("clip_fx.{cid}.{param}"), value); + let mut out = vec![vec![0.0f32; 512]; 2]; + let mut eng = MixEngine::new(48_000.0, 512); + for b in 0..4 { + eng.render(&s, b * 512, 512, &mut out); + } + eng.reset(); + eng.render(&s, 4096, 512, &mut out); + let mut fresh = MixEngine::new(48_000.0, 512); + let mut expect = vec![vec![0.0f32; 512]; 2]; + fresh.render(&s, 4096, 512, &mut expect); + assert_eq!(out, expect, "{param}: clip effect state survived the reset"); + } + } + fn impulse_at(ch: &[f32]) -> Vec { ch.iter().enumerate().filter(|(_, v)| v.abs() > 1e-4).map(|(i, _)| i).collect() } From 90318fd6d6ac9e329c11d6dfc27191bda882d6a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:28:26 +0000 Subject: [PATCH 15/23] playback: retry the next stream config when a built stream won't start --- crates/playback/src/lib.rs | 47 +++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index b945d4b6..c16713d4 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -526,7 +526,6 @@ impl Player { let (gtx, grx) = channel(); let state = AudioState::new(rx, Arc::clone(&shared), Arc::clone(&session), f64::from(rate), gtx); let fault = Arc::new(Fault::default()); - use cpal::traits::StreamTrait; let built = match format { cpal::SampleFormat::I32 => build_output::(&device, &config, state, channels, &fault, &shared), cpal::SampleFormat::I16 => build_output::(&device, &config, state, channels, &fault, &shared), @@ -534,8 +533,9 @@ impl Player { }; match built { Ok(stream) => { - if let Err(e) = stream.play() { - log::warn!("audio stream failed to start: {e}"); + // A stream that builds but will not start is dropped, and the next config is tried. + if !start_stream(&stream) { + continue; } let buffer = match config.buffer_size { cpal::BufferSize::Fixed(n) => Some(n), @@ -862,6 +862,18 @@ fn fold_gains(s: &Session) -> [(f32, f32); MAX_CHANNELS] { g } +/// Start a freshly built output stream. `false` when the device refuses to start it, so the caller +/// treats it like a stream that failed to build and tries the next configuration. +fn start_stream(stream: &impl cpal::traits::StreamTrait) -> bool { + match stream.play() { + Ok(()) => true, + Err(e) => { + log::warn!("audio stream failed to start: {e}"); + false + } + } +} + /// The output stream: the mix renders f32 into a scratch buffer allocated here (so the callback /// never allocates), converted to the device's sample type. fn build_output( @@ -967,6 +979,35 @@ fn open_device(want_rate: u32, want_ch: u16, cfg: &AudioConfig) -> Result<(cpal: mod tests { use super::*; + /// A stream that builds but refuses to start, as a device can after a reconnect. + struct RefusesToStart; + + impl cpal::traits::StreamTrait for RefusesToStart { + fn play(&self) -> Result<(), cpal::PlayStreamError> { + Err(cpal::PlayStreamError::DeviceNotAvailable) + } + fn pause(&self) -> Result<(), cpal::PauseStreamError> { + Ok(()) + } + } + + struct Starts; + + impl cpal::traits::StreamTrait for Starts { + fn play(&self) -> Result<(), cpal::PlayStreamError> { + Ok(()) + } + fn pause(&self) -> Result<(), cpal::PauseStreamError> { + Ok(()) + } + } + + #[test] + fn a_stream_that_will_not_start_is_reported_as_failed() { + assert!(!start_stream(&RefusesToStart)); + assert!(start_stream(&Starts)); + } + #[test] fn reusable_meter_snapshot_updates_and_drops_stale_tracks() { let keep = TrackId(1); From 79adfc0b57fc423cfba3f8c9da51e6af89b08e0b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:30:29 +0000 Subject: [PATCH 16/23] playback: test the stream fallback order through first_started Build and start each attempt in one helper, so the fallback order can be tested without an audio device. The first config that builds and starts wins; a build error or a refused start moves on to the next; none gives None, which leads to the silent clock. Claude-Session: https://claude.ai/code/session_014JH3FxAq7VHhaKjG9U3vfA --- crates/playback/src/lib.rs | 181 ++++++++++++++++++++++++------------- 1 file changed, 120 insertions(+), 61 deletions(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index c16713d4..9d0d1579 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -519,54 +519,58 @@ impl Player { if config.buffer_size != cpal::BufferSize::Default { attempts.push(cpal::StreamConfig { buffer_size: cpal::BufferSize::Default, ..config }); } - for config in attempts { - let rate = config.sample_rate.0; - let channels = usize::from(config.channels); - let (tx, rx) = channel(); - let (gtx, grx) = channel(); - let state = AudioState::new(rx, Arc::clone(&shared), Arc::clone(&session), f64::from(rate), gtx); - let fault = Arc::new(Fault::default()); - let built = match format { - cpal::SampleFormat::I32 => build_output::(&device, &config, state, channels, &fault, &shared), - cpal::SampleFormat::I16 => build_output::(&device, &config, state, channels, &fault, &shared), - _ => build_output::(&device, &config, state, channels, &fault, &shared), - }; - match built { - Ok(stream) => { - // A stream that builds but will not start is dropped, and the next config is tried. - if !start_stream(&stream) { - continue; - } - let buffer = match config.buffer_size { - cpal::BufferSize::Fixed(n) => Some(n), - cpal::BufferSize::Default => None, - }; - log::info!( - "audio device: {name} @ {rate} Hz, {channels} ch, {format}, buffer {}", - buffer.map_or("default".to_string(), |n| n.to_string()) - ); - return Player { - shared, - fault, - #[cfg(not(target_arch = "wasm32"))] - watch: Mutex::new(None), - tx, - side: new_side(grx), - _stream: Some(stream), - device_name: name, - device_rate: rate, - device_channels: config.channels, - device_buffer: buffer, - silent: false, - }; + let opened = first_started( + attempts, + |config: &cpal::StreamConfig| -> Result { + let rate = config.sample_rate.0; + let channels = usize::from(config.channels); + let (tx, rx) = channel(); + let (gtx, grx) = channel(); + let state = AudioState::new(rx, Arc::clone(&shared), Arc::clone(&session), f64::from(rate), gtx); + let fault = Arc::new(Fault::default()); + let stream = match format { + cpal::SampleFormat::I32 => build_output::(&device, config, state, channels, &fault, &shared), + cpal::SampleFormat::I16 => build_output::(&device, config, state, channels, &fault, &shared), + _ => build_output::(&device, config, state, channels, &fault, &shared), + }?; + Ok(Opened { stream, tx, grx, fault }) + }, + |opened| start_stream(&opened.stream), + ); + match opened { + Some((config, Opened { stream, tx, grx, fault })) => { + let rate = config.sample_rate.0; + let channels = usize::from(config.channels); + let buffer = match config.buffer_size { + cpal::BufferSize::Fixed(n) => Some(n), + cpal::BufferSize::Default => None, + }; + log::info!( + "audio device: {name} @ {rate} Hz, {channels} ch, {format}, buffer {}", + buffer.map_or("default".to_string(), |n| n.to_string()) + ); + Player { + shared, + fault, + #[cfg(not(target_arch = "wasm32"))] + watch: Mutex::new(None), + tx, + side: new_side(grx), + _stream: Some(stream), + device_name: name, + device_rate: rate, + device_channels: config.channels, + device_buffer: buffer, + silent: false, } - Err(e) => log::warn!("cannot open audio stream ({:?}): {e}", config.buffer_size), + } + None => { + log::warn!("no usable audio stream; using a silent clock"); + let (tx, rx) = channel(); + let (gtx, grx) = channel(); + Player::silent_clock(shared, tx, rx, want, new_side(grx), gtx) } } - log::warn!("no usable audio stream; using a silent clock"); - let (tx, rx) = channel(); - let (gtx, grx) = channel(); - Player::silent_clock(shared, tx, rx, want, new_side(grx), gtx) } Err(e) => { log::warn!("no audio output: {e}; using a silent clock"); @@ -874,6 +878,32 @@ fn start_stream(stream: &impl cpal::traits::StreamTrait) -> bool { } } +/// A built output stream and the channels and fault flag its callback shares with the player. +struct Opened { + stream: cpal::Stream, + tx: Sender, + grx: Receiver>>, + fault: Arc, +} + +/// Build and start the attempts in order; the first that builds and starts wins. A build error or +/// a stream that will not start is logged, and the next attempt is tried. `None` when none works. +fn first_started( + attempts: impl IntoIterator, + mut build: impl FnMut(&A) -> Result, + start: impl Fn(&S) -> bool, +) -> Option<(A, S)> { + for attempt in attempts { + match build(&attempt) { + Ok(built) if start(&built) => return Some((attempt, built)), + // `start` has already logged why the stream would not start. + Ok(_) => {} + Err(e) => log::warn!("cannot open audio stream ({attempt:?}): {e}"), + } + } + None +} + /// The output stream: the mix renders f32 into a scratch buffer allocated here (so the callback /// never allocates), converted to the device's sample type. fn build_output( @@ -979,23 +1009,15 @@ fn open_device(want_rate: u32, want_ch: u16, cfg: &AudioConfig) -> Result<(cpal: mod tests { use super::*; - /// A stream that builds but refuses to start, as a device can after a reconnect. - struct RefusesToStart; - - impl cpal::traits::StreamTrait for RefusesToStart { - fn play(&self) -> Result<(), cpal::PlayStreamError> { - Err(cpal::PlayStreamError::DeviceNotAvailable) - } - fn pause(&self) -> Result<(), cpal::PauseStreamError> { - Ok(()) - } + /// A stream that builds; `starts` says whether the device lets it play (it can refuse after a + /// reconnect). + struct Fake { + starts: bool, } - struct Starts; - - impl cpal::traits::StreamTrait for Starts { + impl cpal::traits::StreamTrait for Fake { fn play(&self) -> Result<(), cpal::PlayStreamError> { - Ok(()) + if self.starts { Ok(()) } else { Err(cpal::PlayStreamError::DeviceNotAvailable) } } fn pause(&self) -> Result<(), cpal::PauseStreamError> { Ok(()) @@ -1004,8 +1026,45 @@ mod tests { #[test] fn a_stream_that_will_not_start_is_reported_as_failed() { - assert!(!start_stream(&RefusesToStart)); - assert!(start_stream(&Starts)); + assert!(!start_stream(&Fake { starts: false })); + assert!(start_stream(&Fake { starts: true })); + } + + #[test] + fn the_first_config_that_starts_wins_and_later_ones_are_not_tried() { + let mut tried = Vec::new(); + let won = first_started( + [1u32, 2, 3], + |&n| { + tried.push(n); + Ok::<_, String>(Fake { starts: n != 1 }) + }, + start_stream, + ); + assert_eq!(won.map(|(n, _)| n), Some(2)); + assert_eq!(tried, vec![1, 2]); + } + + #[test] + fn a_config_that_fails_to_build_falls_through_to_the_next() { + let won = + first_started([1u32, 2], |&n| if n == 1 { Err("no such buffer size".to_string()) } else { Ok(Fake { starts: true }) }, start_stream); + assert_eq!(won.map(|(n, _)| n), Some(2)); + } + + #[test] + fn no_config_that_starts_yields_none_so_the_caller_falls_back_to_a_silent_clock() { + let mut tried = 0; + let won = first_started( + [1u32, 2], + |_| { + tried += 1; + Ok::<_, String>(Fake { starts: false }) + }, + start_stream, + ); + assert!(won.is_none()); + assert_eq!(tried, 2); } #[test] From 750db81371d175bfffb92aad554b3f7dba33e704 Mon Sep 17 00:00:00 2001 From: Saad Abdullah Date: Tue, 13 Oct 2026 22:04:57 +0000 Subject: [PATCH 17/23] fix(audio-io): poison stream encoders after a failed write so later calls cannot claim success (#464) --- crates/audio-io/src/aiff.rs | 16 ++++++++++-- crates/audio-io/src/flac.rs | 18 +++++++++++-- crates/audio-io/src/lib.rs | 11 +++++++- crates/audio-io/src/wav.rs | 29 +++++++++++++++++++-- crates/audio-io/tests/stream.rs | 46 ++++++++++++++++++++++++++++++++- 5 files changed, 112 insertions(+), 8 deletions(-) diff --git a/crates/audio-io/src/aiff.rs b/crates/audio-io/src/aiff.rs index d8462c0e..77ed6f80 100644 --- a/crates/audio-io/src/aiff.rs +++ b/crates/audio-io/src/aiff.rs @@ -185,6 +185,9 @@ pub(crate) struct Writer { quantizers: Vec, frames: u64, bytes: Vec, + /// Set when a block write has failed: the sink may hold a prefix of that block while + /// `frames` does not count it, so the writer refuses every later call. + failed: bool, } impl Writer { @@ -193,7 +196,7 @@ impl Writer { let start = out.stream_position()?; out.write_all(&header(channels, sample_rate, depth, 0)?)?; let quantizers = Quantizer::per_channel(u32::from(sample_format(depth).0), dither, channels); - Ok(Writer { out, start, channels, sample_rate, depth, quantizers, frames: 0, bytes: Vec::new() }) + Ok(Writer { out, start, channels, sample_rate, depth, quantizers, frames: 0, bytes: Vec::new(), failed: false }) } /// The frame count after `frames` more, or an error when AIFF can't hold that many (4 GiB). @@ -206,13 +209,22 @@ impl Writer { /// Appends `frames` frames of planar audio (missing samples are silence), unless they don't /// fit (see [`Writer::check_room`]). pub(crate) fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { + if self.failed { + return Err(crate::failed_write_error()); + } let total = self.check_room(frames as u64)?; - write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, sample_format(self.depth), true)?; + if let Err(e) = write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, sample_format(self.depth), true) { + self.failed = true; + return Err(AudioError::from(e)); + } self.frames = total; Ok(()) } pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(crate::failed_write_error()); + } let data_len = u64::from(sample_format(self.depth).0 / 8).saturating_mul(self.channels as u64).saturating_mul(self.frames); if data_len % 2 == 1 { self.out.write_all(&[0])?; diff --git a/crates/audio-io/src/flac.rs b/crates/audio-io/src/flac.rs index 8a8c0ce1..b119f09d 100644 --- a/crates/audio-io/src/flac.rs +++ b/crates/audio-io/src/flac.rs @@ -318,6 +318,10 @@ pub(crate) struct Writer { pending: Vec>, frames: u64, frame_no: u64, + /// Set when a frame write has failed: `write_frame` clears the pending samples before + /// writing, so a failed frame is lost while `frames` still counts it; the writer refuses + /// every later call. + failed: bool, } impl Writer { @@ -336,6 +340,7 @@ impl Writer { pending: (0..channels).map(|_| Vec::with_capacity(BLOCK_SIZE)).collect(), frames: 0, frame_no: 0, + failed: false, }) } @@ -351,6 +356,9 @@ impl Writer { /// Appends `frames` frames of planar audio (missing samples are silence), unless they don't /// fit (see [`Writer::check_room`]). pub(crate) fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { + if self.failed { + return Err(crate::failed_write_error()); + } let total = self.check_room(frames as u64)?; let mut done = 0; while done < frames { @@ -361,8 +369,11 @@ impl Writer { p.extend((0..n).map(|i| i64::from(q.quantize(src.get(i).copied().unwrap_or(0.0))))); } done += n; - if held + n == BLOCK_SIZE { - self.write_frame()?; + if held + n == BLOCK_SIZE + && let Err(e) = self.write_frame() + { + self.failed = true; + return Err(e); } } self.frames = total; @@ -408,6 +419,9 @@ impl Writer { } pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(crate::failed_write_error()); + } self.write_frame()?; let info = stream_info(self.channels, self.sample_rate, self.bps, self.frames); self.out.seek(SeekFrom::Start(self.start.saturating_add(4)))?; diff --git a/crates/audio-io/src/lib.rs b/crates/audio-io/src/lib.rs index 1b2b43f5..4a871946 100644 --- a/crates/audio-io/src/lib.rs +++ b/crates/audio-io/src/lib.rs @@ -294,6 +294,13 @@ pub fn encode_with_channel_mask(buf: &AudioBuffer, opts: &EncodeOptions, mask: u } } +/// The error a stream encoder returns for every call after one of its writes has failed: the +/// sink may already hold a prefix of the failed block, so the encoder's counters can no longer +/// describe the bytes on the sink, and later writes or `finish` must not claim success. +pub(crate) fn failed_write_error() -> AudioError { + AudioError::Encode("the stream encoder already failed on an earlier write; its output is incomplete".into()) +} + /// Writes WAV/BWF (RF64 above 4 GiB), AIFF / AIFF-C `fl32` or FLAC block by block, producing /// the same bytes [`encode_with_channel_mask`] would for the whole buffer. pub struct FileEncoder(Writer); @@ -329,7 +336,9 @@ impl FileEncoder { } /// Appends the first `frames` frames of `block`, one `Vec` per channel (missing samples are - /// silence), unless they don't fit (see [`FileEncoder::check_room`]). + /// silence), unless they don't fit (see [`FileEncoder::check_room`]). Once a write has + /// failed, the encoder is unusable: the sink may hold a prefix of the failed block, so + /// every later `write` and `finish` fails instead of completing an inconsistent file. pub fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { match &mut self.0 { Writer::Wav(w) => w.write(block, frames), diff --git a/crates/audio-io/src/wav.rs b/crates/audio-io/src/wav.rs index 1aa847d4..102b4eaa 100644 --- a/crates/audio-io/src/wav.rs +++ b/crates/audio-io/src/wav.rs @@ -321,6 +321,9 @@ pub(crate) struct Writer { quantizers: Vec, frames: u64, bytes: Vec, + /// Set when a block write has failed: the sink may hold a prefix of that block while + /// `frames` does not count it, so the writer refuses every later call. + failed: bool, } impl Writer { @@ -339,7 +342,20 @@ impl Writer { let start = out.stream_position()?; out.write_all(&header(channels, sample_rate, format, bwf, mask, 0, force_rf64)?)?; let quantizers = Quantizer::per_channel(u32::from(format.0), dither, channels); - Ok(Writer { out, start, channels, sample_rate, format, bwf: bwf.cloned(), mask, force_rf64, quantizers, frames: 0, bytes: Vec::new() }) + Ok(Writer { + out, + start, + channels, + sample_rate, + format, + bwf: bwf.cloned(), + mask, + force_rf64, + quantizers, + frames: 0, + bytes: Vec::new(), + failed: false, + }) } fn header(&self, frames: u64) -> Result> { @@ -355,13 +371,22 @@ impl Writer { /// Appends `frames` frames of planar audio (missing samples are silence). pub(crate) fn write(&mut self, block: &[Vec], frames: usize) -> Result<()> { + if self.failed { + return Err(crate::failed_write_error()); + } let total = self.check_room(frames as u64)?; - write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, self.format, false)?; + if let Err(e) = write_interleaved(&mut self.out, &mut self.bytes, block, frames, &mut self.quantizers, self.format, false) { + self.failed = true; + return Err(AudioError::from(e)); + } self.frames = total; Ok(()) } pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(crate::failed_write_error()); + } let data_len = u64::from(self.format.0 / 8).saturating_mul(self.channels as u64).saturating_mul(self.frames); if data_len % 2 == 1 { self.out.write_all(&[0])?; diff --git a/crates/audio-io/tests/stream.rs b/crates/audio-io/tests/stream.rs index 61a28946..8d2c40fa 100644 --- a/crates/audio-io/tests/stream.rs +++ b/crates/audio-io/tests/stream.rs @@ -1,5 +1,5 @@ use soundcraft_audio_io::{AudioBuffer, AudioError, BitDepth, BwfInfo, EncodeOptions, FileEncoder, FileFormat, encode, encode_with_channel_mask}; -use std::io::Cursor; +use std::io::{Cursor, Seek, SeekFrom, Write}; fn tone(channels: usize, frames: usize) -> AudioBuffer { let channels = (0..channels).map(|c| (0..frames).map(|i| ((i * (c + 3)) as f32 * 0.013).sin() * 1.2).collect()).collect(); @@ -49,6 +49,50 @@ fn streamed_files_match_whole_buffer_encodes() { } } +/// A `Write + Seek` sink that accepts `budget` bytes in total and then fails every write, so +/// a `write_all` on it appends a prefix of the block and only then reports the error. +struct FailAfter { + inner: Cursor>, + budget: usize, +} + +impl Write for FailAfter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + if self.budget == 0 { + return Err(std::io::Error::other("sink is full")); + } + let n = buf.len().min(self.budget); + self.budget -= n; + self.inner.write(&buf[..n]) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } +} + +impl Seek for FailAfter { + fn seek(&mut self, pos: SeekFrom) -> std::io::Result { + self.inner.seek(pos) + } +} + +#[test] +fn a_failed_write_poisons_the_encoder() { + let buf = tone(2, 20_001); + for format in [FileFormat::Wav, FileFormat::Aiff, FileFormat::Flac] { + let opts = EncodeOptions { format, bit_depth: BitDepth::Int16, dither: false, bwf: None }; + let sink = FailAfter { inner: Cursor::new(Vec::new()), budget: 256 }; + let mut enc = FileEncoder::new(sink, &opts, 2, buf.sample_rate, None).unwrap(); + // The sink accepts a prefix of the first block, then fails. + assert!(enc.write(&buf.channels, buf.frames()).is_err(), "{format:?}"); + // Later calls must not claim success: the bytes on the sink no longer match the + // encoder's counters (and for FLAC the failed frame's samples are already gone). + assert!(matches!(enc.write(&buf.channels, 16), Err(AudioError::Encode(_))), "{format:?}"); + assert!(matches!(enc.finish(), Err(AudioError::Encode(_))), "{format:?}"); + } +} + #[test] fn blocks_past_a_format_limit_are_refused_before_anything_is_written() { let buf = tone(2, 100); From 8a5da53870d57347bcad2715f21f6b27ceb39efe Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 19:03:00 +0000 Subject: [PATCH 18/23] Reject invalid send slot in automation.copy_to_send Found while testing crates/engine/src/cmd/mix.rs:697 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/mix.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index c0569429..f72f9ca0 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -694,7 +694,9 @@ fn clip_gain_to_vol(e: &mut Engine, p: &Value) -> Result { fn copy_to_send(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "automation.copy_to_send", p)?; - let slot = slot_of(p, "send", SEND_SLOTS).unwrap_or(0) as u8; + let slot = slot_of(p, "send", SEND_SLOTS).ok_or_else(|| { + bad("automation.copy_to_send", format!("`send` must be 0..{} or a..{}", SEND_SLOTS - 1, (b'a' + SEND_SLOTS as u8 - 1) as char)) + })? as u8; let s = e.session_mut(); for t in &tracks { if let Some(tr) = s.track_mut(*t) { @@ -874,6 +876,23 @@ mod tests { assert_eq!(e.session().tracks[0].mixer.inserts[0].as_ref().unwrap().preset, "Saved"); } + #[test] + fn copy_to_send_rejects_an_invalid_send_slot() { + let mut e = crate::demo::demo_engine(); + let history = e.undo_history().len(); + let lanes = |e: &Engine| e.session().tracks.iter().filter(|t| t.lane(&AutoParam::SendLevel(0)).is_some_and(|l| !l.points.is_empty())).count(); + let before = lanes(&e); + for bad_params in [json!({"track": "Kick", "send": 99}), json!({"track": "Kick", "send": "zz"}), json!({"track": "Kick"})] { + let r = e.execute("automation.copy_to_send", &bad_params); + assert!(r.is_err(), "{bad_params} accepted: {r:?}"); + assert_eq!(e.undo_history().len(), history, "{bad_params} pushed an undo step"); + assert_eq!(lanes(&e), before, "{bad_params} edited Send A"); + } + // A valid slot still copies. + e.execute("automation.copy_to_send", &json!({"track": "Kick", "send": "a"})).unwrap(); + assert_eq!(lanes(&e), before + 1); + } + #[test] fn retargeting_a_send_keeps_its_settings() { let mut e = Engine::default(); From 039b9f4f03e1d3e7c0dcf2b84b3b47fc5536d899 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:50:59 +0000 Subject: [PATCH 19/23] Reject invalid slot in mix.insert and mix.send Found while testing crates/engine/src/cmd/mix.rs:300 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/mix.rs | 42 +++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index c0569429..66b1b431 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -287,17 +287,30 @@ fn slot_of(p: &Value, key: &str, max: usize) -> Option { (i < max).then_some(i) } +fn slot_error(cmd: &str, max: usize) -> EngineError { + bad(cmd, format!("`slot` must be 0..{} or a..{}", max - 1, (b'a' + max as u8 - 1) as char)) +} + fn track_slot(e: &Engine, p: &Value, cmd: &str, max: usize) -> Result<(TrackId, usize)> { let t = tracks_required(e, cmd, p)?.first().copied().ok_or_else(|| bad(cmd, "no track"))?; - let slot = slot_of(p, "slot", max).ok_or_else(|| bad(cmd, format!("`slot` must be 0..{} or a..{}", max - 1, (b'a' + max as u8 - 1) as char)))?; + let slot = slot_of(p, "slot", max).ok_or_else(|| slot_error(cmd, max))?; Ok((t, slot)) } +/// An optional `slot`: absent (or null) means "first free"; a present but invalid value is +/// an error, never silently treated as absent. +fn optional_slot(p: &Value, cmd: &str, max: usize) -> Result> { + match p.get("slot") { + None | Some(Value::Null) => Ok(None), + Some(_) => slot_of(p, "slot", max).map(Some).ok_or_else(|| slot_error(cmd, max)), + } +} + fn insert(e: &mut Engine, p: &Value) -> Result { let plugin = str_param(p, "plugin").ok_or_else(|| bad("mix.insert", "`plugin` required"))?.to_string(); let info = super::clap::plugin_info(&plugin).ok_or_else(|| bad("mix.insert", format!("unknown plugin `{plugin}`")))?; let t = tracks_required(e, "mix.insert", p)?.first().copied().ok_or_else(|| bad("mix.insert", "no track"))?; - let explicit = slot_of(p, "slot", INSERT_SLOTS); + let explicit = optional_slot(p, "mix.insert", INSERT_SLOTS)?; let s = e.session_mut(); let tr = s.track_mut(t).ok_or_else(|| bad("mix.insert", "no track"))?; let slot = @@ -416,7 +429,7 @@ fn insert_move(e: &mut Engine, p: &Value) -> Result { fn send(e: &mut Engine, p: &Value) -> Result { let bus = str_param(p, "bus").ok_or_else(|| bad("mix.send", "`bus` required"))?.to_string(); let t = tracks_required(e, "mix.send", p)?.first().copied().ok_or_else(|| bad("mix.send", "no track"))?; - let explicit = slot_of(p, "slot", SEND_SLOTS); + let explicit = optional_slot(p, "mix.send", SEND_SLOTS)?; let s = e.session_mut(); let bid = s.add_bus(&bus, soundcraft_model::ChannelFormat::Stereo); let tr = s.track_mut(t).ok_or_else(|| bad("mix.send", "no track"))?; @@ -886,4 +899,27 @@ mod tests { assert_eq!(sl.target, soundcraft_model::Route::Bus(delay)); assert_eq!((sl.level_db, sl.pan, sl.mute, sl.pre_fader), (-9.0, 0.5, true, true)); } + + #[test] + fn insert_and_send_reject_invalid_slot() { + let mut e = Engine::default(); + e.execute("track.new", &json!({"count": 1, "name": "Gtr", "format": "mono"})).unwrap(); + for slot in [json!(12), json!("k"), json!("x1")] { + let r = e.execute("mix.insert", &json!({"track": "Gtr", "slot": slot, "plugin": "compressor"})); + assert!(r.is_err(), "insert slot {slot} accepted: {r:?}"); + } + for slot in [json!(12), json!("k")] { + let r = e.execute("mix.send", &json!({"track": "Gtr", "slot": slot, "bus": "Verb"})); + assert!(r.is_err(), "send slot {slot} accepted: {r:?}"); + } + let mixer = &e.session().tracks[0].mixer; + assert!(mixer.inserts.iter().all(Option::is_none)); + assert!(mixer.sends.iter().all(Option::is_none)); + + // Leaving `slot` out still picks the first free slot. + let r = e.execute("mix.insert", &json!({"track": "Gtr", "plugin": "compressor"})).unwrap(); + assert_eq!(r["slot"], 0); + let r = e.execute("mix.send", &json!({"track": "Gtr", "bus": "Verb"})).unwrap(); + assert_eq!(r["slot"], 0); + } } From f2ae345f2022e2da42935f5a7c035f53f4107bd2 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 07:43:12 +0200 Subject: [PATCH 20/23] Clip > Remove Sync Point takes the sync point off the selected clips --- crates/engine/src/cmd/clip.rs | 94 ++++++++++++++++++++ crates/ui-egui/src/menus.rs | 5 ++ crates/ui-egui/tests/command_reachability.rs | 18 ++++ 3 files changed, 117 insertions(+) diff --git a/crates/engine/src/cmd/clip.rs b/crates/engine/src/cmd/clip.rs index 9d68dc25..aff6d8cc 100644 --- a/crates/engine/src/cmd/clip.rs +++ b/crates/engine/src/cmd/clip.rs @@ -102,6 +102,7 @@ pub fn specs() -> Vec { } Ok(json!({})) }), + cmd!("clip.remove_sync_point", "Remove Sync Point", ["Clip"], None, "{clips?}", has_sync_point, remove_sync_point), cmd!("clip.quantize_to_grid", "Quantize to Grid", ["Clip"], Some("Cmd+0"), "{clips?}", has_selection, quantize_to_grid), cmd!("clip.color", "Clip Color", [], None, "{clips?, color: [r,g,b] | null}", has_selection, |e, p| { let ids = clip_ids_param(e, p); @@ -348,6 +349,31 @@ fn nudge_gain(e: &mut Engine, p: &Value, db: f64) -> Result { gain(e, &json!({"clips": clips, "delta_db": db})) } +/// Sync point 0 is "none" (edit_window only draws one when > 0). +fn has_sync_point(e: &Engine) -> std::result::Result<(), String> { + let s = e.session(); + if clip_ids_param(e, &json!({})).iter().any(|id| s.find_clip(*id).is_some_and(|(_, c)| c.sync_point != 0)) { + Ok(()) + } else { + Err("no selected clip has a sync point".into()) + } +} + +fn remove_sync_point(e: &mut Engine, p: &Value) -> Result { + let s = e.session(); + // Only clips that have a point are touched, so a call that changes nothing leaves no undo step. + let ids: Vec = clip_ids_param(e, p).into_iter().filter(|id| s.find_clip(*id).is_some_and(|(_, c)| c.sync_point != 0)).collect(); + if !ids.is_empty() { + let s = e.session_mut(); + for id in &ids { + if let Some(c) = s.find_clip_mut(*id) { + c.sync_point = 0 + } + } + } + Ok(json!({"clips": ids.len()})) +} + fn quantize_to_grid(e: &mut Engine, p: &Value) -> Result { let ids = clip_ids_param(e, p); let s = e.session(); @@ -452,4 +478,72 @@ mod tests { e.execute("clip.gain_nudge_up", &json!({})).unwrap(); assert_eq!(gain(&e), 36.0); } + + /// Three 10 000-sample clips side by side on one track; sync points are set by each test. + fn three_clips() -> (crate::Engine, [ClipId; 3]) { + use soundcraft_model::{ChannelFormat, Clip, Session, TrackKind}; + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, Some("Dialog")); + let buf = soundcraft_audio_io::AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.1; 10_000]] }; + let src = crate::io::add_source(&mut s, "Line", buf, None, soundcraft_audio_io::FileFormat::Wav); + let ids = [0, 1, 2].map(|k| { + let cid = s.new_clip_id(); + crate::edit::place_clip(&mut s, t, Clip::audio(cid, "Line", src, 0, k * 12_000, 10_000)); + cid + }); + (crate::Engine::new(s), ids) + } + + fn sync_of(e: &crate::Engine, id: ClipId) -> i64 { + e.session().find_clip(id).unwrap().1.sync_point + } + + #[test] + fn remove_sync_point_clears_the_point_identify_set() { + let (mut e, [a, ..]) = three_clips(); + e.execute("edit.select", &json!({"clips": [a.0]})).unwrap(); + e.execute("clip.identify_sync_point", &json!({"at": 4_000})).unwrap(); + assert_eq!(sync_of(&e, a), 4_000); + e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(sync_of(&e, a), 0); + } + + #[test] + fn remove_sync_point_clears_every_selected_clip_and_only_those() { + let (mut e, [a, b, c]) = three_clips(); + for (id, at) in [(a, 1_000), (b, 2_000), (c, 3_000)] { + e.session_mut().find_clip_mut(id).unwrap().sync_point = at; + } + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + let r = e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(r["clips"], 2); + assert_eq!([sync_of(&e, a), sync_of(&e, b), sync_of(&e, c)], [0, 0, 3_000]); + } + + #[test] + fn remove_sync_point_is_disabled_unless_a_selected_clip_has_one() { + let (mut e, [a, b, c]) = three_clips(); + e.session_mut().find_clip_mut(c).unwrap().sync_point = 3_000; + let enabled = |e: &crate::Engine| crate::find_command("clip.remove_sync_point").unwrap().info(e).enabled; + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + assert!(!enabled(&e), "neither selected clip has a sync point (the unselected one does)"); + e.execute("edit.select", &json!({"clips": [b.0, c.0]})).unwrap(); + assert!(enabled(&e)); + } + + #[test] + fn remove_sync_point_is_one_undo_step_that_puts_the_points_back() { + let (mut e, [a, b, _]) = three_clips(); + for (id, at) in [(a, 1_000), (b, 2_000)] { + e.session_mut().find_clip_mut(id).unwrap().sync_point = at; + } + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + let steps = e.undo_history().len(); + e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(e.undo_history().len(), steps + 1); + assert_eq!(e.undo_label(), Some("Remove Sync Point")); + assert_eq!([sync_of(&e, a), sync_of(&e, b)], [0, 0]); + assert!(e.undo()); + assert_eq!([sync_of(&e, a), sync_of(&e, b)], [1_000, 2_000]); + } } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9eef0b22..fbe274f1 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -202,6 +202,11 @@ pub fn tree() -> Vec { insert_path(&mut roots, line); } insert_path(&mut roots, "Window > Reset Panel Layout"); + // Not in the observed catalog either; Pro Tools lists it right under Identify Sync Point. + if let Some(clip) = roots.iter_mut().find(|n| n.label == "Clip") { + let at = clip.children.iter().position(|n| n.label == "Identify Sync Point").map_or(clip.children.len(), |i| i + 1); + clip.children.insert(at, MenuNode { label: "Remove Sync Point".into(), path: "Clip > Remove Sync Point".into(), children: Vec::new() }); + } // Not in the incumbent catalog, but the only UI path to an empty MIDI clip (#282). // It creates a clip rather than acting on the selected ones, so it leads the Clip menu. insert_path(&mut roots, "Clip > New MIDI Clip"); diff --git a/crates/ui-egui/tests/command_reachability.rs b/crates/ui-egui/tests/command_reachability.rs index 5328947b..ce7c6346 100644 --- a/crates/ui-egui/tests/command_reachability.rs +++ b/crates/ui-egui/tests/command_reachability.rs @@ -103,3 +103,21 @@ fn new_midi_clip_is_in_the_clip_menu() { let clip = tree.iter().find(|n| n.label == "Clip").unwrap(); assert_eq!(clip.children[0].label, "New MIDI Clip", "creates rather than edits, so it comes first"); } + +#[test] +fn remove_sync_point_follows_identify_and_is_greyed_out_without_a_point() { + let tree = menus::tree(); + let clip = tree.iter().find(|n| n.label == "Clip").unwrap(); + let labels: Vec<&str> = clip.children.iter().map(|n| n.label.as_str()).collect(); + let at = labels.iter().position(|l| *l == "Identify Sync Point").unwrap(); + assert_eq!(labels.get(at + 1), Some(&"Remove Sync Point")); + let path = &clip.children[at + 1].path; + let id = menus::command_for_path(path, &menus::ui_aliases()); + assert_eq!(id.as_deref(), Some("clip.remove_sync_point")); + + let mut app = soundcraft_ui_egui::SoundApp::new(soundcraft_engine::demo::demo_engine(), None, soundcraft_ui_egui::Services::default()); + app.engine.execute("edit.select", &serde_json::json!({"tracks": ["Kick"], "start": {"seconds": 12.0}})).unwrap(); + assert!(!menus::item_state(&app, path, id.as_deref()).0, "the selected Kick clip has no sync point yet"); + app.engine.execute("clip.identify_sync_point", &serde_json::json!({})).unwrap(); + assert!(menus::item_state(&app, path, id.as_deref()).0); +} From 055ddbee7a6c8722d2a685eed543bf2305db90b7 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 05:26:51 +0200 Subject: [PATCH 21/23] Keep merging fader gestures once the undo history is full Found while testing crates/engine/src/lib.rs:368-381 (Engine::execute_merged, with push_undo at 280-291) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/lib.rs | 10 ++++- .../engine/tests/fader_merge_full_history.rs | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 crates/engine/tests/fader_merge_full_history.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6cc1c90a..03d173dd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -369,7 +369,15 @@ impl Engine { let key = self.merge_hold.clone().unwrap_or_else(|| key.to_string()); let continuing = self.merge_key.as_ref() == Some(&key); let before_len = self.undo.len(); - let r = self.execute(id, params)?; + // The step pushed for a continuing tick is dropped again below, so it must not trim the + // oldest entry first when the history is full. + let limit = self.history_limit; + if continuing && before_len > 0 { + self.history_limit = limit.max(before_len.saturating_add(1)); + } + let r = self.execute(id, params); + self.history_limit = limit; + let r = r?; if continuing && self.undo.len() == before_len + 1 && before_len > 0 { // Drop the step just pushed; the earlier one still holds the pre-gesture document. self.undo.pop(); diff --git a/crates/engine/tests/fader_merge_full_history.rs b/crates/engine/tests/fader_merge_full_history.rs new file mode 100644 index 00000000..c4656535 --- /dev/null +++ b/crates/engine/tests/fader_merge_full_history.rs @@ -0,0 +1,42 @@ +use serde_json::json; +use soundcraft_engine::Engine; +use soundcraft_model::{ChannelFormat, TrackKind}; + +fn engine_with_full_history(limit: usize) -> (Engine, u64) { + let mut e = Engine::default(); + let t = e.session_mut().add_track(TrackKind::Audio, ChannelFormat::Mono, None).0; + e.history_limit = limit; + for i in 0..limit + 1 { + e.execute("mix.pan", &json!({"track": t, "pan": 0.1 * (i as f64 + 1.0)})).unwrap(); + } + assert_eq!(e.undo_history().len(), limit); + (e, t) +} + +#[test] +fn a_fader_gesture_is_one_undo_step_when_the_history_is_full() { + let (mut e, t) = engine_with_full_history(3); + for db in [-1.0, -2.0, -3.0, -4.0, -5.0] { + e.execute_merged("mix.volume", &json!({"track": t, "db": db}), "fader").unwrap(); + } + e.end_merge(); + let volume = |e: &Engine| e.session().tracks.iter().find(|x| x.id.0 == t).map(|x| x.mixer.volume_db); + assert_eq!(volume(&e), Some(-5.0)); + // The gesture costs one step; it must not push further real edits out of the history. + assert_eq!(e.undo_history().len(), 3, "history: {:?}", e.undo_history()); + assert_eq!(e.undo_history().iter().filter(|l| l.as_str() == "Set Volume").count(), 1, "history: {:?}", e.undo_history()); + assert!(e.undo()); + assert_eq!(volume(&e), Some(0.0), "one undo should undo the whole fader gesture; history: {:?}", e.undo_history()); +} + +#[test] +fn a_zero_history_limit_keeps_no_undo_entry_for_a_gesture() { + let mut e = Engine::default(); + let t = e.session_mut().add_track(TrackKind::Audio, ChannelFormat::Mono, None).0; + e.history_limit = 0; + for db in [-1.0, -2.0, -3.0] { + e.execute_merged("mix.volume", &json!({"track": t, "db": db}), "fader").unwrap(); + } + e.end_merge(); + assert!(e.undo_history().is_empty(), "history: {:?}", e.undo_history()); +} From 4a4e82daba45992a237306713d2f64d75bd9ce17 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 09:54:08 +0200 Subject: [PATCH 22/23] Reject missing or non-numeric values in mix.pan and mix.trim Found while testing crates/engine/src/cmd/mix.rs:237 (pan); same pattern at mix.rs:76 (mix.trim) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/mix.rs | 6 ++-- crates/engine/tests/mix_required_params.rs | 42 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 crates/engine/tests/mix_required_params.rs diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index c0569429..faddc6ca 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -73,7 +73,8 @@ pub fn specs() -> Vec { )), cmd!("mix.trim", "Input Trim", [], None, "{tracks?, db}", has_selection, |e, p| { let tracks = tracks_required(e, "mix.trim", p)?; - let db = f32_or(p, "db", 0.0).clamp(-144.0, 24.0); + let db = (p.get("db").and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("mix.trim", "`db` (number) required"))? as f32) + .clamp(-144.0, 24.0); let s = e.session_mut(); for t in tracks { if let Some(tr) = s.track_mut(t) { @@ -234,7 +235,8 @@ fn volume(e: &mut Engine, p: &Value) -> Result { fn pan(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "mix.pan", p)?; - let v = f32_or(p, "pan", 0.0).clamp(-1.0, 1.0); + let v = (p.get("pan").and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("mix.pan", "`pan` (number) required"))? as f32) + .clamp(-1.0, 1.0); let idx = i64_or(p, "index", 0).clamp(0, 1) as usize; let s = e.session_mut(); for t in &tracks { diff --git a/crates/engine/tests/mix_required_params.rs b/crates/engine/tests/mix_required_params.rs new file mode 100644 index 00000000..f11e2f31 --- /dev/null +++ b/crates/engine/tests/mix_required_params.rs @@ -0,0 +1,42 @@ +use serde_json::json; +use soundcraft_engine::{Engine, EngineError}; + +fn engine_with_kick() -> Engine { + let mut e = Engine::default(); + e.execute("track.new", &json!({"name": "Kick", "format": "stereo"})).unwrap(); + e +} + +fn kick_pan(e: &Engine) -> Vec { + e.session().tracks.iter().find(|t| t.name == "Kick").unwrap().mixer.pan.to_vec() +} + +fn kick_trim(e: &Engine) -> f32 { + e.session().tracks.iter().find(|t| t.name == "Kick").unwrap().mixer.trim_db +} + +#[test] +fn mix_pan_rejects_a_missing_or_non_numeric_pan_and_keeps_the_pan() { + let mut e = engine_with_kick(); + e.execute("mix.pan", &json!({"tracks": ["Kick"], "pan": -1.0})).unwrap(); + assert_eq!(kick_pan(&e)[0], -1.0); + + for params in [json!({"tracks": ["Kick"]}), json!({"tracks": ["Kick"], "pan": "left"}), json!({"tracks": ["Kick"], "pan": null})] { + let r = e.execute("mix.pan", ¶ms); + assert!(matches!(r, Err(EngineError::BadParams(..))), "{params}: expected BadParams, got {r:?}"); + assert_eq!(kick_pan(&e)[0], -1.0, "{params}: the pan must be untouched"); + } +} + +#[test] +fn mix_trim_rejects_a_missing_or_non_numeric_db_and_keeps_the_trim() { + let mut e = engine_with_kick(); + e.execute("mix.trim", &json!({"tracks": ["Kick"], "db": -6.0})).unwrap(); + assert_eq!(kick_trim(&e), -6.0); + + for params in [json!({"tracks": ["Kick"]}), json!({"tracks": ["Kick"], "db": "loud"}), json!({"tracks": ["Kick"], "db": null})] { + let r = e.execute("mix.trim", ¶ms); + assert!(matches!(r, Err(EngineError::BadParams(..))), "{params}: expected BadParams, got {r:?}"); + assert_eq!(kick_trim(&e), -6.0, "{params}: the trim must be untouched"); + } +} From f53051a35f0421df7330f93c1d5a46c74e996e66 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 19 Oct 2026 02:42:40 -0400 Subject: [PATCH 23/23] copy_to_send: no send still means Send A, as the menu item calls it Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/mix.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/mix.rs b/crates/engine/src/cmd/mix.rs index f72f9ca0..f5f50682 100644 --- a/crates/engine/src/cmd/mix.rs +++ b/crates/engine/src/cmd/mix.rs @@ -694,9 +694,13 @@ fn clip_gain_to_vol(e: &mut Engine, p: &Value) -> Result { fn copy_to_send(e: &mut Engine, p: &Value) -> Result { let tracks = tracks_required(e, "automation.copy_to_send", p)?; - let slot = slot_of(p, "send", SEND_SLOTS).ok_or_else(|| { - bad("automation.copy_to_send", format!("`send` must be 0..{} or a..{}", SEND_SLOTS - 1, (b'a' + SEND_SLOTS as u8 - 1) as char)) - })? as u8; + // No `send` means Send A (the menu item passes none); a given one must be valid. + let slot = match p.get("send") { + None | Some(Value::Null) => 0, + Some(_) => slot_of(p, "send", SEND_SLOTS).ok_or_else(|| { + bad("automation.copy_to_send", format!("`send` must be 0..{} or a..{}", SEND_SLOTS - 1, (b'a' + SEND_SLOTS as u8 - 1) as char)) + })?, + } as u8; let s = e.session_mut(); for t in &tracks { if let Some(tr) = s.track_mut(*t) { @@ -882,12 +886,15 @@ mod tests { let history = e.undo_history().len(); let lanes = |e: &Engine| e.session().tracks.iter().filter(|t| t.lane(&AutoParam::SendLevel(0)).is_some_and(|l| !l.points.is_empty())).count(); let before = lanes(&e); - for bad_params in [json!({"track": "Kick", "send": 99}), json!({"track": "Kick", "send": "zz"}), json!({"track": "Kick"})] { + for bad_params in [json!({"track": "Kick", "send": 99}), json!({"track": "Kick", "send": "zz"})] { let r = e.execute("automation.copy_to_send", &bad_params); assert!(r.is_err(), "{bad_params} accepted: {r:?}"); assert_eq!(e.undo_history().len(), history, "{bad_params} pushed an undo step"); assert_eq!(lanes(&e), before, "{bad_params} edited Send A"); } + // No `send` (the menu item's call) still means Send A. + e.execute("automation.copy_to_send", &json!({"track": "Kick"})).unwrap(); + assert_eq!(lanes(&e), before + 1); // A valid slot still copies. e.execute("automation.copy_to_send", &json!({"track": "Kick", "send": "a"})).unwrap(); assert_eq!(lanes(&e), before + 1);