Merge pull request #506 from liangforstudy/fix/save-playhead

Project files: each sequence reopens with its playhead where it was left

contributors/people.toml

@@ -40,3 +40,6 @@ sync_github_name = true
 
 [people.ItzBloxy]
 display_name = "Bloxy"
+
+[people.liangforstudy]
+sync_github_name = true

crates/engine/src/lib.rs

@@ -708,14 +708,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<ItemId> = Vec::new();
         for id in view.open_sequences {
@@ -729,6 +730,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

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::<Vec<_>>(), {
+        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::<Vec<_>>(), [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));
 }

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();

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<ItemId>,
     #[serde(default)]
     pub sequences: BTreeMap<ItemId, SequenceView>,
+    /// 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<ItemId, Tick>,
+}
+
+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:<id>`.

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