Merge pull request #1139 from itrycodinganditsbad/fix-1101

fix(timeline): snap sub-frame timeline.place to the frame grid; inspect counts export's frames (#1101)

crates/engine/src/commands.rs

@@ -843,6 +843,13 @@ fn place_one(s: &mut Session, p: &Value) -> Result<Value> {
     };
     let sin = p.get("sourceIn").and_then(Value::as_i64).map(Tick).or(mi).unwrap_or_default();
     let dur = p.get("duration").and_then(Value::as_i64).map(Tick).unwrap_or_else(|| mo.unwrap_or(full) - sin);
+    // The clip goes on the frame grid: its start and its end (`time + duration`) each snap to the
+    // nearest frame, so a sub-frame `time` never pushes the end past the one asked for and the
+    // sequence never ends between frames (#1101).
+    let rate = s.sequence_rate();
+    let end = rate.snap_nearest(Tick(at.0.saturating_add(dur.0)));
+    let at = rate.snap_nearest(at);
+    let dur = end - at;
     let ids = place_item(s, item, TimeRange::new(sin, dur), at, v, a, bool_p(p, "insert").unwrap_or(false), "Place Clip", None)?;
     Ok(json!({"clips": ids.iter().map(|c| c.0).collect::<Vec<_>>()}))
 }
@@ -4125,12 +4132,14 @@ pub fn inspect_sequence(s: &Session, id: ItemId, q: &filmcraft_project::Sequence
             "transitions": t.transitions.iter().map(|x| json!({"id": x.id.0, "effect": x.effect.effect, "start": x.start.0, "duration": x.duration.0, "from": x.from.map(|c| c.0), "to": x.to.map(|c| c.0), "reverse": x.reverse, "align": filmcraft_edit::transitions::bounds(t, x).map(|b| filmcraft_edit::transitions::alignment(x, b.1, rate.frame_duration()).id()), "params": x.effect.params.iter().map(|(k, v)| (k.clone(), serde_json::to_value(&v.value).unwrap_or_default())).collect::<serde_json::Map<_, _>>()})).collect::<Vec<_>>(),
         })
     };
+    // the frames export renders, a partial last frame included
+    let (f0, f1) = filmcraft_export::frame_span(rate, TimeRange::new(Tick::ZERO, q.duration()));
     json!({
         "id": id.0,
         "name": s.project.item(id).map(|i| i.name.clone()),
         "settings": q.settings,
         "duration": q.duration().0,
-        "durationFrames": rate.frame_at(q.duration()),
+        "durationFrames": f1 - f0,
         "playhead": s.state.playheads.get(&id).map(|t| t.0),
         "in": q.mark_in.map(|t| t.0),
         "out": q.mark_out.map(|t| t.0),

crates/engine/src/source_placement_tests.rs

@@ -197,3 +197,45 @@ fn malformed_source_spans_are_bounded() {
     view.start = Tick::from_seconds_f64(1.0);
     assert_eq!(view.selected_range().duration, Tick::ZERO);
 }
+
+/// #1101: a placement that starts between frames must not run past `time + duration`, and the
+/// frame count `sequence.inspect` reports is the one export renders.
+#[test]
+fn sub_frame_place_ends_at_requested_end_and_inspect_matches_export_frames() {
+    let mut s = Session::default();
+    s.execute("file.openDemoProject", json!({})).unwrap();
+    s.execute("file.newSequence", json!({"name":"f","width":640,"height":360,"fps":24,"video":1,"audio":2})).unwrap();
+    let rate = s.sequence_rate();
+    let fd = rate.frame_duration().0;
+    let end = rate.tick_of(120).0;
+    s.execute("timeline.place", json!({"item":5,"track":"V1","audioTrack":"A1","frame":0,"sourceIn":0,"duration":end})).unwrap();
+    // a third of a frame after frame 60, ending exactly on frame 120
+    let at = rate.tick_of(60).0 + fd / 3;
+    let placed = s.execute("timeline.place", json!({"item":5,"track":"A2","time":at,"sourceIn":0,"duration":end - at})).unwrap();
+    let clip = placed["clips"][0].as_u64().unwrap();
+    let js = s.execute("sequence.inspect", json!({})).unwrap();
+    let c = js["audio"][1]["items"].as_array().unwrap().iter().find(|c| c["clip"] == clip).unwrap().clone();
+    assert!(c["end"].as_i64().unwrap() <= end, "clip runs past the requested end: {c}");
+    assert_eq!(rate.snap(Tick(c["start"].as_i64().unwrap())).0, c["start"].as_i64().unwrap(), "clip starts between frames: {c}");
+    assert_eq!(js["duration"], end);
+    let frames = s.execute("export.resolve", json!({})).unwrap()["frames"].clone();
+    assert_eq!(js["durationFrames"], frames);
+    assert_eq!(frames, 120);
+}
+
+/// A sequence that ends between frames (a clip moved to a sub-frame time) still reports the
+/// frame count export renders, the partial last frame included.
+#[test]
+fn inspect_duration_frames_counts_a_partial_last_frame_like_export() {
+    let mut s = setup();
+    let rate = s.sequence_rate();
+    let placed = s.execute("timeline.place", json!({"item":5,"track":"V1","audioTrack":"A1","frame":0,"sourceIn":0,"duration":rate.tick_of(48).0})).unwrap();
+    let clips: Vec<u64> = placed["clips"].as_array().unwrap().iter().filter_map(|v| v.as_u64()).collect();
+    let tracks = s.active_sequence().map(|q| (q.video_tracks[0].id.0, q.audio_tracks[0].id.0)).unwrap();
+    let t = rate.frame_duration().0 / 3;
+    s.execute("timeline.move", json!({"moves":[{"clip":clips[0],"track":tracks.0,"time":t},{"clip":clips[1],"track":tracks.1,"time":t}]})).unwrap();
+    let js = s.execute("sequence.inspect", json!({})).unwrap();
+    let frames = s.execute("export.resolve", json!({})).unwrap()["frames"].clone();
+    assert_eq!(js["durationFrames"], frames);
+    assert_eq!(frames, 49);
+}

docs/agents.md

@@ -169,6 +169,10 @@ Notes:
   an "invalid parameters" error, never a different track.
   In `timeline.place`, `track` is where the picture goes and `audioTrack` where the sound goes;
   an audio track given as `track` places the sound alone, on that track.
+  The clip lands on the frame grid: its start and its end (`time + duration`) each snap to the
+  nearest frame.
+- `sequence.inspect` reports `durationFrames` as the number of frames export renders: a sequence
+  that ends partway through a frame counts that frame.
 - `engine.execute` also runs UI-only commands (`tool.razor`, `playback.toggle`,
   `window.workspace.color`, `window.panel.<name>`).
 - A command that works on the selection is disabled ("no clips selected", "select a clip in the