From 72e3eeb5deaaed296d9e03fa594e8465e0c623e6 Mon Sep 17 00:00:00 2001 From: liangforstudy <36067395+liangforstudy@users.noreply.github.com> Date: Sat, 10 Oct 2026 05:22:05 +0800 Subject: [PATCH] Project files: each sequence reopens with its playhead where it was left The engine already keeps one playhead per sequence while a project is open, and the project file's view already stores each sequence's zoom, scroll and track heights, but not its playhead: after saving and reopening, every sequence started at zero. ProjectView gains `playheads` (sequence id -> Tick), written beside the tabs and views and restored with them when "Restore open sequences when opening projects" is on. Files written before it have none and open as they did. Values from a file are not trusted: ids that are not sequences are dropped, playheads are clamped to 0..ProjectView::MAX_PLAYHEAD and snapped to a frame of their sequence. Tests: each_sequence_reopens_with_its_playhead_where_it_was_left (fails without the fix), hostile playheads in a_damaged_view_in_a_project_file_is_cleaned_up_or_ignored, a deleted sequence's playhead is not written, and the format round trip carries it. Also adds my contributor credit entry (people.toml). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Qq8BK1LmohzE1ga4NgAdWM --- contributors/people.toml | 3 + crates/engine/src/lib.rs | 13 ++++- crates/engine/src/sequence_tabs_tests.rs | 71 +++++++++++++++++++++++- crates/format/src/lib.rs | 1 + crates/project/src/lib.rs | 17 +++++- docs/project-files.md | 6 +- 6 files changed, 99 insertions(+), 12 deletions(-) diff --git a/contributors/people.toml b/contributors/people.toml index 25cf020e..0acb9978 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -40,3 +40,6 @@ sync_github_name = true [people.ItzBloxy] display_name = "Bloxy" + +[people.liangforstudy] +sync_github_name = true diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 2578420b..d63e10e1 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -707,14 +707,15 @@ impl Session { open_sequences: self.state.open_sequences.iter().copied().filter(is_seq).collect(), active_sequence: self.state.active_sequence.filter(is_seq), sequences: self.state.timeline_views.iter().filter(|(id, _)| is_seq(id)).map(|(id, v)| (*id, *v)).collect(), + playheads: self.state.playheads.iter().filter(|(id, _)| is_seq(id)).map(|(id, t)| (*id, *t)).collect(), } } /// Open what was open when the project was saved. Nothing in `view` is trusted: ids that are /// not sequences of this project are dropped (also a second mention of the same sequence), - /// and numbers are brought into range. A view without any open sequence leaves the project - /// on its first sequence: a project saved by a session that never showed one (a script, the - /// CLI) should not open on an empty Timeline. + /// and numbers are brought into range (a playhead also lands on a frame of its sequence). A + /// view without any open sequence leaves the project on its first sequence: a project saved + /// by a session that never showed one (a script, the CLI) should not open on an empty Timeline. pub fn restore_project_view(&mut self, view: filmcraft_project::ProjectView) { let mut open: Vec = Vec::new(); for id in view.open_sequences { @@ -728,6 +729,12 @@ impl Session { } self.state.timeline_views = view.sequences.into_iter().filter(|(id, _)| self.project.sequence(*id).is_some()).filter_map(|(id, v)| Some((id, v.checked()?))).collect(); + for (id, t) in view.playheads { + if let Some(seq) = self.project.sequence(id) { + let t = t.clamp(Tick::ZERO, filmcraft_project::ProjectView::MAX_PLAYHEAD); + self.state.playheads.insert(id, seq.settings.frame_rate.snap(t)); + } + } } /// Every edit passes through here: one that would put a sequence inside itself (directly or diff --git a/crates/engine/src/sequence_tabs_tests.rs b/crates/engine/src/sequence_tabs_tests.rs index d8d6cd48..f3cb5f36 100644 --- a/crates/engine/src/sequence_tabs_tests.rs +++ b/crates/engine/src/sequence_tabs_tests.rs @@ -1,9 +1,10 @@ //! The Timeline's sequence tabs: closing the others, reordering, and what a project file keeps of -//! them (the open tabs in order, the active one, and each sequence's zoom, scroll and track -//! heights). Premiere's behaviour was observed in Premiere Pro 26.5.2. +//! them (the open tabs in order, the active one, each sequence's zoom, scroll and track heights, +//! and each sequence's playhead). Premiere's tab behaviour was observed in Premiere Pro 26.5.2. use super::*; use filmcraft_project::SequenceView; +use filmcraft_time::TICKS_PER_SECOND; use serde_json::json; /// The demo project with two more sequences open: tabs [main, b, c], `c` active. @@ -106,6 +107,52 @@ fn a_saved_project_reopens_with_its_tabs_and_their_views() { let _ = std::fs::remove_dir_all(&dir); } +/// Each sequence reopens with its playhead where it was left: the project file keeps one playhead +/// per sequence beside the tabs and views. +#[test] +fn each_sequence_reopens_with_its_playhead_where_it_was_left() { + let (mut s, [main, b, c]) = three_tabs(); + let at = |s: &Session, seq: ItemId, secs: i64| s.project.sequence(seq).unwrap().settings.frame_rate.snap(Tick(secs * TICKS_PER_SECOND)); + s.execute("sequence.open", json!({"item": main.0})).unwrap(); + s.execute("playhead.set", json!({"seconds": 7})).unwrap(); + s.execute("sequence.open", json!({"item": b.0})).unwrap(); + s.execute("playhead.set", json!({"seconds": 2})).unwrap(); + let (main_at, b_at) = (at(&s, main, 7), at(&s, b, 2)); + assert_eq!(s.playhead(), b_at); + assert_eq!(s.project_view().playheads.get(&c), None, "a playhead that never moved is not written"); + let dir = temp_dir("playheads"); + let path = dir.join("tabs.fcproj").to_string_lossy().to_string(); + s.execute("file.saveAs", json!({"path": path})).unwrap(); + + let mut t = Session::default(); + t.execute("file.open", json!({"path": path})).unwrap(); + assert_eq!(t.state.active_sequence, Some(b)); + assert_eq!(t.playhead(), b_at, "the sequence that was shown is back where it was"); + t.execute("sequence.open", json!({"item": main.0})).unwrap(); + assert_eq!(t.playhead(), main_at, "and so is every other sequence"); + t.execute("sequence.open", json!({"item": c.0})).unwrap(); + assert_eq!(t.playhead(), Tick::ZERO); + assert!(!t.is_dirty(), "a restored playhead is not an edit"); + + // with the preference off, nothing of the view comes back, playheads included + let mut u = Session::default(); + u.prefs.timeline.restore_open_sequences = false; + u.execute("file.open", json!({"path": path})).unwrap(); + assert_eq!(u.playhead(), Tick::ZERO); + + // a file written before playheads were kept opens as it did then: every playhead at zero + let mut doc: serde_json::Value = serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap(); + assert!(doc["view"]["playheads"].is_object()); + doc["view"].as_object_mut().unwrap().remove("playheads"); + std::fs::write(&path, serde_json::to_vec(&doc).unwrap()).unwrap(); + let mut w = Session::default(); + w.execute("file.open", json!({"path": path})).unwrap(); + assert_eq!(w.state.active_sequence, Some(b)); + assert_eq!(w.playhead(), Tick::ZERO); + assert!(w.state.playheads.is_empty()); + let _ = std::fs::remove_dir_all(&dir); +} + /// The view in a project file is not trusted: ids of things that are not sequences, repeats and /// numbers out of range are dropped or brought into range, and a view that cannot be read at all /// does not stop the project from opening. @@ -134,6 +181,13 @@ fn a_damaged_view_in_a_project_file_is_cleaned_up_or_ignored() { "999999": {"pps": 40.0, "scroll": 0.0, "video_track_h": 60.0, "audio_track_h": 56.0}, footage.0.to_string(): {"pps": 40.0, "scroll": 0.0, "video_track_h": 60.0, "audio_track_h": 56.0}, }, + "playheads": { + c.0.to_string(): -5, + b.0.to_string(): i64::MAX, + main.0.to_string(): TICKS_PER_SECOND + 1, + "999999": TICKS_PER_SECOND, + footage.0.to_string(): TICKS_PER_SECOND, + }, })); assert_eq!(t.state.open_sequences, [c, b]); assert_eq!(t.state.active_sequence, Some(c), "an active tab that is not open: the first tab"); @@ -141,6 +195,16 @@ fn a_damaged_view_in_a_project_file_is_cleaned_up_or_ignored() { let v = t.state.timeline_views[&b]; assert!(v.pps <= 1e5 && v.scroll == 0.0 && v.v_scroll <= 1e6 && v.a_scroll == 0.0); assert!(v.video_track_h >= 8.0 && v.audio_track_h <= 600.0); + // playheads: only sequences keep one, never before zero or past the bound, always on a frame + let rate = |seq: ItemId| t.project.sequence(seq).unwrap().settings.frame_rate; + assert_eq!(t.state.playheads.keys().copied().collect::>(), { + let mut ids = vec![main, b, c]; + ids.sort(); + ids + }); + assert_eq!(t.state.playheads[&c], Tick::ZERO); + assert_eq!(t.state.playheads[&b], rate(b).snap(filmcraft_project::ProjectView::MAX_PLAYHEAD)); + assert_eq!(t.state.playheads[&main], rate(main).snap(Tick(TICKS_PER_SECOND + 1))); // not a view at all: the project opens as it does without one for junk in [json!("tabs"), json!([1, 2, 3]), json!({"open_sequences": "all"}), json!({"sequences": {"x": 1}}), json!(null)] { @@ -159,6 +223,7 @@ fn the_view_of_a_deleted_sequence_is_forgotten() { s.execute("project.delete", json!({"items": [b.0]})).unwrap(); assert_eq!(s.state.timeline_views.keys().copied().collect::>(), [main, c]); assert_eq!(s.state.open_sequences, [main, c]); - // the saved view never names it either + // the saved view never names it either, nor keeps its playhead assert_eq!(s.project_view().open_sequences, [main, c]); + assert!(!s.project_view().playheads.contains_key(&b)); } diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 2c97f20a..1abdc666 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -418,6 +418,7 @@ mod tests { open_sequences: vec![seq], active_sequence: Some(seq), sequences: [(seq, SequenceView { pps: 80.0, scroll: 1.5, v_scroll: 0.0, a_scroll: 4.0, video_track_h: 60.0, audio_track_h: 56.0 })].into(), + playheads: [(seq, filmcraft_time::Tick(3 * filmcraft_time::TICKS_PER_SECOND))].into(), }; for pretty in [true, false] { let l = decode(&encode_with_view(&p, Some(&view), pretty)).unwrap(); diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index 40c2eb9b..650d42fc 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -1232,9 +1232,9 @@ impl SequenceView { } } -/// What was open when a project was saved: the Timeline's sequence tabs in order, the active one -/// and how each sequence was shown. Stored beside the project in its file, not in it: it is not -/// part of the edit and never an undo step. +/// What was open when a project was saved: the Timeline's sequence tabs in order, the active one, +/// how each sequence was shown and where its playhead was. Stored beside the project in its file, +/// not in it: it is not part of the edit and never an undo step. #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct ProjectView { #[serde(default)] @@ -1243,6 +1243,17 @@ pub struct ProjectView { pub active_sequence: Option, #[serde(default)] pub sequences: BTreeMap, + /// Each sequence's playhead, so a sequence reopens where it was left. Files + /// written before it existed have none, and every playhead starts at zero as it did then. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub playheads: BTreeMap, +} + +impl ProjectView { + /// The latest playhead a project file may ask for: ten million seconds (about 115 days), the + /// same bound as a sequence view's scroll position. Playheads are read from project files, so + /// nothing in them is trusted. + pub const MAX_PLAYHEAD: Tick = Tick(10_000_000 * TICKS_PER_SECOND); } /// A LUT imported into the project (`lut.import`). Lumetri refers to it as `lib:`. diff --git a/docs/project-files.md b/docs/project-files.md index c736ec31..c8827150 100644 --- a/docs/project-files.md +++ b/docs/project-files.md @@ -21,12 +21,12 @@ writing them is `crates/format` (`filmcraft-format`); the engine's `file.*` comm clips, effects, keyframes, markers). Media is referenced by path, never embedded. - Files are written compact (no indentation). A 2,000-clip project is about 1.4 MB. - `view` (optional) is what was open when the project was saved: the Timeline's sequence tabs in - order, the active one, and each sequence's zoom, scroll and track heights - (`filmcraft_project::ProjectView`). It is beside the project, not in it: it is not part of the + order, the active one, each sequence's zoom, scroll and track heights, and each sequence's + playhead (`filmcraft_project::ProjectView`). It is beside the project, not in it: it is not part of the edit, never an undo step, and changing it does not mark the project as changed. Opening a project restores it when the Timeline preference "Restore open sequences when opening projects" is on. It is outside the schema version: older builds ignore it, ids that are not sequences and - numbers out of range are dropped or clamped, and a `view` that cannot be read is ignored (the + numbers out of range are dropped or clamped (a playhead also snaps to a frame of its sequence), and a `view` that cannot be read is ignored (the project opens on its first sequence, as it does without one or with no open sequence in it). - A coordinate of an effect's point parameter may be `null`. Point parameters use NaN for "auto" (the frame centre, or the source centre for `anchor`), and JSON writes NaN as `null`; it reads back