From d3ee1dfd22fa35572060369615aaa849e6e884b0 Mon Sep 17 00:00:00 2001 From: punkhop <1851839+punkhop@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:28:20 -0700 Subject: [PATCH] engine: Replace With Clip reports clips that run past the new media Problem: when the replacement is shorter than the clip's slot, the clip silently runs past the end of its media and the renderer holds the last frame. Nothing in the command's result says so, so a script or agent that swaps a take cannot tell. Solution: the result of the three Replace With Clip commands (`clip.replaceFromSource`, `clip.replaceFromSourceMatchFrame`, `clip.replaceFromBin`) gains `short`: one `{clip, shortByFrames}` per replaced clip that now plays past the end of its media, in sequence frames and never more than the clip's own length. It is empty when none does; stills and other sources without an end, and held frames, are never short. The edit itself is not changed, and the parameter docs are untouched (results are described in docs/control-protocol.md). Test: `replace_with_clip_reports_clips_that_run_past_the_new_media` (From Bin with a 12-frame subclip, undo / redo, From Source Monitor, Match Frame, the cases that are not short, and an In past the end of the new media capped at the clip's length). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/clip_ops.rs | 27 +++++++++- crates/engine/src/clip_ops_tests.rs | 79 ++++++++++++++++++++++++++++- docs/control-protocol.md | 5 +- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/clip_ops.rs b/crates/engine/src/clip_ops.rs index 701ba5a4..165fdb45 100644 --- a/crates/engine/src/clip_ops.rs +++ b/crates/engine/src/clip_ops.rs @@ -1724,10 +1724,35 @@ fn replace_with(s: &mut Session, p: &Value, how: Replace) -> Result { } Ok((out, clamped)) })?; - let mut result = json!({"clips": replaced, "item": new_item.0}); + let mut result = json!({"clips": replaced, "item": new_item.0, "short": short_clips(s, &replaced)}); if keep_in { // the clips whose In was outside a trims-restricted subclip and now sits on its nearest frame result["sourceInClamped"] = json!(clamped); } Ok(result) } + +/// The clips among `clips` that play past the end of their media, with how many sequence frames +/// of the clip have no media (at most the clip's own length). The edit is left as it is (the +/// renderer holds the last frame there); this is so whoever replaced the clip knows. +fn short_clips(s: &Session, clips: &[u64]) -> Vec { + let Some(q) = s.active_sequence() else { return Vec::new() }; + let frame = q.settings.frame_rate.frame_duration().0.max(1); + clips + .iter() + .filter_map(|c| { + let (_, it) = q.find_item(ClipId(*c))?; + // stills, mattes and other sources without an end are never short; nor is a held frame + let media_end = crate::media_duration(&s.project, &s.media, it.item)?; + let over = it.source_out().0.saturating_sub(media_end.0); + if over <= 0 || it.frame_hold.is_some() { + return None; + } + // media ticks past the end, as sequence ticks of this clip + let length = it.duration.0.max(0); + let ticks = ((over as f64 / it.speed.abs().max(1e-9)).round().min(length as f64) as i64).min(length); + let frames = ticks.saturating_add(frame - 1) / frame; + (frames > 0).then(|| json!({"clip": c, "shortByFrames": frames})) + }) + .collect() +} diff --git a/crates/engine/src/clip_ops_tests.rs b/crates/engine/src/clip_ops_tests.rs index 4121e0b7..a006647c 100644 --- a/crates/engine/src/clip_ops_tests.rs +++ b/crates/engine/src/clip_ops_tests.rs @@ -625,14 +625,14 @@ fn replace_from_bin_keeps_the_source_in_on_request() { // default: the replacement starts at the item's In point, and the result is what it was let r = s.execute("clip.replaceFromBin", json!({})).unwrap(); assert_eq!((clip(&s, c.id.0).item, clip(&s, c.id.0).source_in), (dunes, Tick::ZERO)); - assert_eq!(r, json!({"clips": [c.id.0], "item": dunes.0})); + assert_eq!(r, json!({"clips": [c.id.0], "item": dunes.0, "short": []})); s.execute("edit.undo", json!({})).unwrap(); assert_eq!(*s.project, *before); // keepSourceIn: the same place in the new media, everything else as before let r = s.execute("clip.replaceFromBin", json!({"keepSourceIn": true})).unwrap(); let got = clip(&s, c.id.0); assert_eq!((got.item, got.source_in, got.start, got.duration), (dunes, trimmed.source_in, trimmed.start, trimmed.duration)); - assert_eq!(r, json!({"clips": [c.id.0], "item": dunes.0, "sourceInClamped": []})); + assert_eq!(r, json!({"clips": [c.id.0], "item": dunes.0, "short": [], "sourceInClamped": []})); let after = s.project.clone(); s.execute("edit.undo", json!({})).unwrap(); assert_eq!(*s.project, *before); @@ -675,6 +675,81 @@ fn replace_from_bin_keeps_the_source_in_on_request() { } } +/// A replacement shorter than the clip's slot left the clip running past the end of its media +/// without a word. The three Replace With Clip commands now say which clips are short, by how +/// many sequence frames. +#[test] +fn replace_with_clip_reports_clips_that_run_past_the_new_media() { + let mut s = demo(); + let rate = s.sequence_rate(); + let demo_seq = s.state.active_sequence.unwrap(); + s.execute("sequence.linkedSelection", json!({"on": false})).unwrap(); + let c = v1(&s)[3].clone(); + let slot = rate.frame_at(c.duration); + let forest = item_named(&s, "Misty_Forest.mp4"); + let forest_len = s.project.item(forest).unwrap().as_media().unwrap().info.duration; + assert!(slot > 12 && forest_len > c.duration + rate.tick_of(40), "the clip is longer than 12 frames and shorter than the media"); + let state = |s: &Session| { + let it = clip(s, c.id.0); + (it.item, it.source_in, it.start, it.duration) + }; + select(&mut s, &[c.id.0]); + + // From Bin, with 12 frames of media (a subclip that restricts trims): the edit is what it + // always was, and the result says the clip is short + let r = s.execute("clip.makeSubclip", json!({"item": forest.0, "start": rate.tick_of(10).0, "end": rate.tick_of(22).0})).unwrap(); + let short = ItemId(r["item"].as_u64().unwrap()); + let before = s.project.clone(); + let r = s.execute("clip.replaceFromBin", json!({"item": short.0})).unwrap(); + assert_eq!(state(&s), (short, rate.tick_of(10), c.start, c.duration), "the edit itself is not changed"); + assert_eq!(r["short"], json!([{"clip": c.id.0, "shortByFrames": slot - 12}])); + let after = s.project.clone(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(*s.project, *before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(*s.project, *after); + s.execute("edit.undo", json!({})).unwrap(); + // long enough media: nothing is short + assert_eq!(s.execute("clip.replaceFromBin", json!({"item": forest.0})).unwrap()["short"], json!([])); + s.execute("edit.undo", json!({})).unwrap(); + + // From Source Monitor, with the Source In 20 frames before the end of the media + let frames = |t: Tick| (t.0 + rate.frame_duration().0 - 1) / rate.frame_duration().0; + s.execute("source.open", json!({"item": forest.0})).unwrap(); + let mark = forest_len - rate.tick_of(20); + s.execute("project.setMarks", json!({"item": forest.0, "in": mark.0})).unwrap(); + let r = s.execute("clip.replaceFromSource", json!({})).unwrap(); + let got = clip(&s, c.id.0); + assert_eq!(r["short"], json!([{"clip": c.id.0, "shortByFrames": frames(got.source_out() - forest_len)}])); + assert_eq!(frames(got.source_out() - forest_len), slot - 20); + s.execute("edit.undo", json!({})).unwrap(); + // Match Frame: the last frame of the media on the clip's first frame + s.execute("playhead.set", json!({"time": c.start.0})).unwrap(); + s.execute("source.setPlayhead", json!({"time": (forest_len - rate.tick_of(1)).0})).unwrap(); + let r = s.execute("clip.replaceFromSourceMatchFrame", json!({})).unwrap(); + assert_eq!(r["short"], json!([{"clip": c.id.0, "shortByFrames": slot - 1}])); + s.execute("edit.undo", json!({})).unwrap(); + // with enough media after the frame, nothing is short + s.execute("source.setPlayhead", json!({"frame": 5})).unwrap(); + assert_eq!(s.execute("clip.replaceFromSourceMatchFrame", json!({})).unwrap()["short"], json!([])); + s.execute("edit.undo", json!({})).unwrap(); + + // never more than the clip itself: an In that lies past the end of the new media altogether + s.execute("timeline.trim", json!({"clip": c.id.0, "edge": "in", "mode": "regular", "deltaFrames": 7})).unwrap(); + let trimmed = clip(&s, c.id.0); + s.execute("file.newSequence", json!({"name": "Brief"})).unwrap(); + let brief = s.state.active_sequence.unwrap(); + s.execute("timeline.place", json!({"item": forest.0, "time": 0})).unwrap(); + let placed = s.active_sequence().unwrap().video_tracks[0].items[0].clone(); + s.execute("timeline.trim", json!({"clip": placed.id.0, "edge": "out", "mode": "regular", "delta": (rate.tick_of(3) - placed.duration).0})).unwrap(); + s.execute("sequence.open", json!({"item": demo_seq.0})).unwrap(); + select(&mut s, &[c.id.0]); + assert!(trimmed.source_in > s.project.sequence(brief).unwrap().duration(), "the kept In is past the end of the nested sequence"); + let r = s.execute("clip.replaceFromBin", json!({"clips": [c.id.0], "item": brief.0, "keepSourceIn": true})).unwrap(); + assert_eq!(clip(&s, c.id.0).source_in, trimmed.source_in); + assert_eq!(r["short"], json!([{"clip": c.id.0, "shortByFrames": rate.frame_at(trimmed.duration)}]), "all of the clip, not more"); +} + #[test] fn audio_source_channels_pick_what_a_clip_plays() { let mut it = v1(&demo())[0].clone(); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index f8e95c8d..3d99e7b9 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -82,7 +82,10 @@ it shows. Dynamic trimming takes an explicit clock (seconds) so it is determinis **Replace With Clip**: `clip.replaceFromSource`, `clip.replaceFromSourceMatchFrame` and `clip.replaceFromBin {clips?, item?, keepSourceIn?}` swap the media of the clips and return -`{clips: [ids], item}`. From Bin starts each clip at the new item's In point; with +`{clips: [ids], item, short: [{clip, shortByFrames}]}`. `short` lists the replaced clips that now +run past the end of their media (the renderer holds the last frame there) with the number of +sequence frames that have no media, at most the clip's length; it is empty when the new media +covers every clip. The edit is the same either way. From Bin starts each clip at the new item's In point; with `keepSourceIn: true` each clip keeps its own source In instead. A subclip that restricts trims has no media outside its range, so there a kept In is moved to the subclip's first or last frame, and the result's `sourceInClamped: [ids]` (present with `keepSourceIn`) names the clips it was moved for.