LINUX.VALIDATION: honor preset shortcuts and retain MPEG oracle frames

Resolve inherited physical key collisions when loading built-in presets,
while preserving established explicit winners and deliberate custom conflicts.
Read every ffprobe display-frame timestamp and register the requested credit.
Verify exact frame-plane reuse without depending on concurrent global metrics.

Co-authored-by: Codex <[email protected]>

Cargo.lock

@@ -1816,6 +1816,7 @@ dependencies = [
  "filmcraft-vp9",
  "image",
  "log",
+ "serde_json",
  "symphonia",
  "thiserror 2.0.21",
  "web-time",

contributors/people.toml

@@ -21,3 +21,7 @@
 real_name = "Brandon Thomas"
 display_name = "Brandon Thomas"
 sync_github_name = false
+
+[people.sanyamsingh001]
+display_name = "sanyamsingh001"
+sync_github_name = false

crates/codecs/Cargo.toml

@@ -41,3 +41,4 @@ symphonia = { version = "0.5.5", default-features = false, features = ["aac", "m
 
 [dev-dependencies]
 filmcraft-testkit = { workspace = true }
+serde_json = { workspace = true }

crates/codecs/tests/mpeg_oracle.rs

@@ -177,15 +177,64 @@ fn open(p: &Path) -> MpegSource {
     MpegSource::open(p.file_name().unwrap().to_str().unwrap(), bytes(p)).unwrap()
 }
 
-/// ffprobe's video frames in display order: pts (90 kHz).
+/// ffprobe's video frames in display order: PTS (90 kHz). A program-stream picture
+/// need not carry its own PTS; use the decoder's presentation timestamp for that picture.
 fn ffprobe_frame_pts(file: &Path) -> Option<Vec<i64>> {
-    let fp = filmcraft_testkit::ffprobe()?;
+    let fp = filmcraft_testkit::ffprobe_or_skip("MPEG video frame presentation timestamps")?;
     let o = std::process::Command::new(fp)
-        .args(["-v", "error", "-select_streams", "v:0", "-show_entries", "frame=pts", "-of", "csv=p=0"])
+        .args(["-v", "error", "-select_streams", "v:0", "-show_entries", "frame=pts,best_effort_timestamp", "-of", "json"])
         .arg(file)
         .output()
-        .ok()?;
-    Some(String::from_utf8_lossy(&o.stdout).lines().filter_map(|l| l.trim().trim_end_matches(',').parse().ok()).collect())
+        .unwrap_or_else(|error| panic!("{}: cannot run ffprobe: {error}", file.display()));
+    assert!(o.status.success(), "{}: ffprobe failed: {}", file.display(), String::from_utf8_lossy(&o.stderr));
+    Some(parse_ffprobe_frame_pts(&o.stdout).unwrap_or_else(|error| panic!("{}: {error}", file.display())))
+}
+
+fn parse_ffprobe_frame_pts(bytes: &[u8]) -> Result<Vec<i64>, String> {
+    let data: serde_json::Value = serde_json::from_slice(bytes).map_err(|error| format!("invalid ffprobe JSON: {error}"))?;
+    let frames = data.get("frames").and_then(serde_json::Value::as_array).ok_or("ffprobe output is missing its frames array")?;
+    if frames.is_empty() {
+        return Err("ffprobe returned no video frames".into());
+    }
+    frames
+        .iter()
+        .enumerate()
+        .map(|(index, frame)| {
+            let field = if frame.get("pts").is_some() { "pts" } else { "best_effort_timestamp" };
+            frame.get(field).and_then(serde_json::Value::as_i64).ok_or_else(|| format!("ffprobe video frame {index} is missing an integer {field}"))
+        })
+        .collect()
+}
+
+#[test]
+fn ffprobe_pts_preserve_missing_raw_timestamps_and_side_data() {
+    let output = br#"{"frames":[
+        {"pts":131400,"best_effort_timestamp":131401,"side_data_list":[{},{}]},
+        {"best_effort_timestamp":135000,"side_data_list":[{}]},
+        {"pts":138600,"best_effort_timestamp":138600}
+    ]}"#;
+    assert_eq!(parse_ffprobe_frame_pts(output).unwrap(), [131400, 135000, 138600], "keep every display frame, with raw PTS preferred when present");
+    assert_eq!(parse_ffprobe_frame_pts(br#"{"frames":[{"pts":-3600},{"pts":0},{"pts":9223372036854775807}]}"#).unwrap(), [-3600, 0, i64::MAX]);
+}
+
+#[test]
+fn malformed_ffprobe_pts_fail_instead_of_dropping_frames() {
+    for output in [
+        "not JSON",
+        "{}",
+        r#"{"frames":{}}"#,
+        r#"{"frames":[]}"#,
+        r#"{"frames":[{}]}"#,
+        r#"{"frames":[null]}"#,
+        r#"{"frames":[{"pts":"N/A","best_effort_timestamp":135000}]}"#,
+        r#"{"frames":[{"pts":null,"best_effort_timestamp":135000}]}"#,
+        r#"{"frames":[{"pts":1.5}]}"#,
+        r#"{"frames":[{"pts":9223372036854775808}]}"#,
+        r#"{"frames":[{"best_effort_timestamp":"135000"}]}"#,
+        r#"{"frames":[{"pts":131400},{"best_effort_timestamp":null},{"pts":138600}]}"#,
+    ] {
+        assert!(parse_ffprobe_frame_pts(output.as_bytes()).is_err(), "malformed oracle output must fail: {output}");
+    }
 }
 
 /// Decode every frame in order and `seeks` random frames; compare with ffmpeg within `tol`.

crates/engine/src/shortcuts.rs

@@ -24,7 +24,7 @@
 //! Commands the frontend owns (tools, transport, panels, workspaces) are registered with
 //! [`Shortcuts::register_external`] so they can be listed, bound and resolved here too.
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::path::{Path, PathBuf};
 
 use serde::{Deserialize, Serialize};
@@ -593,22 +593,34 @@ impl Shortcuts {
             _ => return None,
         };
         self.apply_table(&mut b, &table);
+        // Canonical chords can become the same physical key on this platform. Keep the first
+        // binding, matching resolve(), when generating a built-in preset.
+        let platform = Platform::current();
+        let mut seen = BTreeSet::new();
+        b.retain(|binding| binding.chord().is_none_or(|chord| seen.insert((binding.context().to_string(), chord.effective(platform)))));
         Some(b)
     }
 
-    /// Apply a preset table: listed commands get exactly the listed keys, and a listed key is taken
-    /// away from whatever else had it in that context.
+    /// Apply a preset table: listed commands get the listed keys, and explicit physical keys take
+    /// precedence over inherited bindings in the same context. Within the table, keep canonical
+    /// replacement order so folded aliases retain their established first-match winner.
     fn apply_table(&self, b: &mut Vec<Binding>, table: &[Entry]) {
         let listed: Vec<&str> = table.iter().map(|e| e.0).filter(|c| self.known(c)).collect();
         b.retain(|x| !listed.contains(&x.command.as_str()));
+        let platform = Platform::current();
+        let mut explicit: Vec<Binding> = Vec::new();
         for (c, k, p) in table {
             if !self.known(c) {
                 continue;
             }
             let nb = Binding::new(c, k, panel_opt(p));
-            b.retain(|o| !(o.context() == nb.context() && o.chord() == nb.chord()));
-            b.push(nb);
+            let chord = nb.chord();
+            let physical = chord.as_ref().map(|chord| chord.effective(platform));
+            b.retain(|o| !(o.context() == nb.context() && o.chord().map(|chord| chord.effective(platform)) == physical));
+            explicit.retain(|o| !(o.context() == nb.context() && o.chord() == chord));
+            explicit.push(nb);
         }
+        b.extend(explicit);
     }
 
     fn presets_dir(&self) -> Option<PathBuf> {

crates/engine/src/shortcuts_tests.rs

@@ -218,3 +218,133 @@ fn list_searches_labels_and_keys() {
     let v = s.execute("shortcuts.list", json!({"panel": "History", "assigned": true})).unwrap();
     assert_eq!(v.as_array().unwrap().len(), 2, "{v}");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn engine_defaults_keep_the_existing_linux_winners_without_folded_conflicts() {
+    let s = Session::default();
+    assert!(s.shortcuts.conflicts(Platform::Linux).is_empty());
+    for (keys, command) in [("Cmd+9", "multicam.cutToCamera9"), ("Cmd+Shift+M", "markers.addRange"), ("Cmd+T", "trim.toggleType")] {
+        let chord = Chord::parse(keys).unwrap();
+        assert_eq!(s.shortcuts.resolve(&chord, None, Platform::Linux).unwrap().command, command, "{keys} keeps its existing first-match action");
+        let ctrl = keys.replacen("Cmd", "Ctrl", 1);
+        assert_eq!(s.shortcuts.resolve(&Chord::parse(&ctrl).unwrap(), None, Platform::Linux).unwrap().command, command);
+    }
+    assert!(!s.shortcuts.modified);
+    assert!(!s.shortcuts.can_undo());
+    assert!(!s.shortcuts.can_redo());
+}
+
+#[test]
+fn builtin_presets_are_unambiguous_with_engine_and_frontend_commands() {
+    let platform = Platform::current();
+    for frontend in [false, true] {
+        for name in crate::shortcuts::BUILTIN_PRESETS {
+            let mut s = Session::default();
+            if frontend {
+                with_ui_commands(&mut s);
+            }
+            s.execute("shortcuts.loadPreset", json!({"name": name})).unwrap();
+            assert!(s.shortcuts.conflicts(platform).is_empty(), "{name}, frontend={frontend}: {:?}", s.shortcuts.conflicts(platform));
+            let backspace = Chord::parse("Backspace").unwrap();
+            assert_eq!(s.shortcuts.resolve(&backspace, Some("Project"), platform).unwrap().command, "project.delete", "panel overrides remain independent");
+            s.execute("shortcuts.undo", json!({})).unwrap();
+            assert_eq!(s.shortcuts.preset, crate::shortcuts::DEFAULT_PRESET);
+            assert!(s.shortcuts.conflicts(platform).is_empty());
+            s.execute("shortcuts.redo", json!({})).unwrap();
+            assert_eq!(s.shortcuts.preset, name);
+            assert!(s.shortcuts.conflicts(platform).is_empty());
+        }
+    }
+}
+
+#[test]
+fn intentional_conflicts_survive_registration_persistence_and_shortcut_undo() {
+    let dir = temp_dir("intentional-conflicts");
+    let platform = Platform::current();
+    let mut s = Session::default();
+    s.shortcuts.set_dir(&dir);
+    let defaults = s.shortcuts.bindings.clone();
+    let result = s.execute("shortcuts.set", json!({"command": "edit.redo", "keys": "M", "keepConflicts": true})).unwrap();
+    assert_eq!(result["conflicts"].as_array().unwrap().len(), 1);
+    let edited = s.shortcuts.bindings.clone();
+    with_ui_commands(&mut s);
+    assert_eq!(s.shortcuts.bindings, edited, "registering the UI preserves an intentionally modified set");
+    let mut restored = Session::default();
+    restored.shortcuts.set_dir(&dir);
+    with_ui_commands(&mut restored);
+    assert_eq!(restored.shortcuts.bindings, edited, "a modified built-in set survives restart");
+    assert!(restored.shortcuts.modified);
+    assert_eq!(restored.shortcuts.conflicts(platform).len(), 1);
+    s.execute("shortcuts.undo", json!({})).unwrap();
+    assert_eq!(s.shortcuts.bindings, defaults);
+    assert!(!s.shortcuts.modified);
+    assert!(s.shortcuts.conflicts(platform).is_empty());
+    s.execute("shortcuts.redo", json!({})).unwrap();
+    assert_eq!(s.shortcuts.bindings, edited);
+    assert_eq!(s.shortcuts.conflicts(platform).len(), 1);
+    s.execute("shortcuts.savePreset", json!({"name": "Kept conflicts"})).unwrap();
+    assert!(!s.shortcuts.modified);
+    with_ui_commands(&mut s);
+    assert_eq!(s.shortcuts.bindings, edited, "an unmodified custom preset keeps its deliberate conflicts");
+    let mut custom = Session::default();
+    custom.shortcuts.set_dir(&dir);
+    with_ui_commands(&mut custom);
+    assert_eq!(custom.shortcuts.preset, "Kept conflicts");
+    assert_eq!(custom.shortcuts.bindings, edited);
+    assert_eq!(custom.shortcuts.conflicts(platform).len(), 1);
+    custom.execute("shortcuts.loadPreset", json!({"name": crate::shortcuts::DEFAULT_PRESET})).unwrap();
+    assert!(custom.shortcuts.conflicts(platform).is_empty());
+    custom.execute("shortcuts.loadPreset", json!({"name": "Kept conflicts"})).unwrap();
+    assert_eq!(custom.shortcuts.bindings, edited);
+    assert_eq!(custom.shortcuts.conflicts(platform).len(), 1);
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn an_explicit_final_cut_preset_binding_overrides_inherited_linux_physical_keys() {
+    for frontend in [false, true] {
+        let mut session = Session::default();
+        if frontend {
+            with_ui_commands(&mut session);
+        }
+        session.execute("shortcuts.loadPreset", json!({"name": crate::shortcuts::FCP_PRESET})).unwrap();
+        for keys in ["Cmd+T", "Ctrl+T"] {
+            let resolved = session.execute("shortcuts.resolve", json!({"keys": keys})).unwrap();
+            assert_eq!(
+                resolved["command"], "sequence.applyVideoTransition",
+                "the explicit Final Cut Pro table replaces inherited Trim Toggle Type: {resolved}"
+            );
+        }
+        assert!(session.shortcuts.conflicts(Platform::Linux).is_empty());
+        session.execute("shortcuts.undo", json!({})).unwrap();
+        let resolved = session.execute("shortcuts.resolve", json!({"keys": "Cmd+T"})).unwrap();
+        assert_eq!(resolved["command"], "trim.toggleType", "the FilmCraft default keeps its established Linux winner");
+        session.execute("shortcuts.redo", json!({})).unwrap();
+        let resolved = session.execute("shortcuts.resolve", json!({"keys": "Ctrl+T"})).unwrap();
+        assert_eq!(resolved["command"], "sequence.applyVideoTransition");
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn compatible_presets_keep_the_existing_first_explicit_linux_winners() {
+    let mut session = Session::default();
+    // Export is frontend-owned; use the same definition registered by the desktop menu.
+    session.shortcuts.register_external(vec![CommandInfo::new("mode.export", "Export", &["File", "Export"], Some("Cmd+M"))]);
+    for (preset, keys, command) in [
+        (crate::shortcuts::FCP_PRESET, "Cmd+R", "clip.speedDuration"),
+        (crate::shortcuts::PREMIERE_PRESET, "Cmd+M", "mode.export"),
+        (crate::shortcuts::PREMIERE_PRESET, "Cmd+Shift+M", "markers.goPrev"),
+        (crate::shortcuts::PREMIERE_PRESET, "Cmd+T", "trim.toggleType"),
+        (crate::shortcuts::PREMIERE_PRESET, "Cmd+9", "multicam.cutToCamera9"),
+    ] {
+        session.execute("shortcuts.loadPreset", json!({"name": preset})).unwrap();
+        for spelling in [keys.to_string(), keys.replacen("Cmd", "Ctrl", 1)] {
+            let resolved = session.execute("shortcuts.resolve", json!({"keys": spelling})).unwrap();
+            assert_eq!(resolved["command"], command, "{preset}: {spelling} keeps its existing first explicit action: {resolved}");
+        }
+        assert!(session.shortcuts.conflicts(Platform::Linux).is_empty());
+    }
+}

crates/frame/src/pool.rs

@@ -226,21 +226,37 @@ mod tests {
                 pts: filmcraft_time::Tick::ZERO,
             })
         };
-        let before = stats().reused;
-        // a frame someone else still holds is left alone
+        let plane_ptrs = |frame: &VideoFrame| match &frame.data {
+            PixelData::Yuv8 { planes, .. } => planes.each_ref().map(|plane| plane.as_ptr() as usize),
+            _ => panic!("fixture has three byte planes"),
+        };
+        // A frame someone else still holds is left alone. The reuse statistic is global:
+        // concurrent float-buffer tests can change it without touching these planes.
         let shared = yuv(1);
         let held = shared.clone();
+        let held_ptrs = plane_ptrs(&held);
         recycle(shared);
-        assert!(take_u8(n).capacity() >= n);
-        assert_eq!(stats().reused, before);
+        let fresh = take_u8(n);
+        assert!(fresh.capacity() >= n);
+        assert!(!held_ptrs.contains(&(fresh.as_ptr() as usize)), "a shared plane cannot be handed out");
         assert_eq!(held.byte_size(), n + 2 * c);
-        // an unshared frame's planes come back, empty
-        recycle(yuv(2));
+        // An unshared frame's exact allocations come back, empty. Equal-size chroma
+        // planes may be taken in either order.
+        let unshared = yuv(2);
+        let returned_ptrs = plane_ptrs(&unshared);
+        recycle(unshared);
         let (y, u, v) = (take_u8(n), take_u8(c), take_u8(c));
-        assert!(y.is_empty() && y.capacity() >= n && u.capacity() >= c && v.capacity() >= c);
-        assert_eq!(stats().reused, before + 3);
-        // and are not handed out twice
-        take_u8(n);
-        assert_eq!(stats().reused, before + 3);
+        assert!(y.is_empty() && u.is_empty() && v.is_empty());
+        assert!(y.capacity() >= n && u.capacity() >= c && v.capacity() >= c);
+        assert_eq!(y.as_ptr() as usize, returned_ptrs[0]);
+        let mut expected_chroma = [returned_ptrs[1], returned_ptrs[2]];
+        let mut actual_chroma = [u.as_ptr() as usize, v.as_ptr() as usize];
+        expected_chroma.sort_unstable();
+        actual_chroma.sort_unstable();
+        assert_eq!(actual_chroma, expected_chroma);
+        // The returned allocations remain owned by y/u/v and cannot be handed out twice.
+        let another = take_u8(n);
+        assert!(another.capacity() >= n);
+        assert!(!returned_ptrs.contains(&(another.as_ptr() as usize)));
     }
 }