Merge pull request #1202 from storytold/codex/h264-filmcraft-parity

craft-video: preserve FilmCraft H.264 search starts

crates/video/src/h264/encode/mod.rs

@@ -418,7 +418,7 @@ impl<P: Pixel> Core<P> {
         )
     }
 
-    pub fn new(mut cfg: EncoderConfig) -> Result<Self, Error> {
+    fn new_with_extra_motion_search_starts(mut cfg: EncoderConfig, extra_motion_search_starts: bool) -> Result<Self, Error> {
         if cfg.width == 0
             || cfg.height == 0
             || !cfg.width.is_multiple_of(2)
@@ -468,6 +468,7 @@ impl<P: Pixel> Core<P> {
                 adaptive_t8: false,
                 always_intra: false,
                 cavlc_clamp: !cabac,
+                extra_motion_search_starts,
                 depth,
             },
             Preset::Balanced => EncParams {
@@ -487,6 +488,7 @@ impl<P: Pixel> Core<P> {
                 adaptive_t8: high,
                 always_intra: false,
                 cavlc_clamp: !cabac,
+                extra_motion_search_starts,
                 depth,
             },
             Preset::Quality => EncParams {
@@ -506,6 +508,7 @@ impl<P: Pixel> Core<P> {
                 adaptive_t8: high,
                 always_intra: true,
                 cavlc_clamp: !cabac,
+                extra_motion_search_starts,
                 depth,
             },
         };
@@ -1218,6 +1221,15 @@ pub struct Encoder {
     core: Cores,
 }
 
+/// Starting points considered by the integer motion search.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum MotionSearchStarts {
+    /// Include skip, zero and co-located vectors for 16x16 searches.
+    Extended,
+    /// Use only the skip seed and neighboring partition vector, matching the earlier FilmCraft encoder.
+    SeedAndNeighbour,
+}
+
 enum Cores {
     Eight(Box<Core<u8>>),
     Deep(Box<Core<u16>>),
@@ -1234,7 +1246,18 @@ macro_rules! core {
 
 impl Encoder {
     pub fn new(cfg: EncoderConfig) -> Result<Self, Error> {
-        let core = if cfg.bit_depth > 8 { Cores::Deep(Box::new(Core::new(cfg)?)) } else { Cores::Eight(Box::new(Core::new(cfg)?)) };
+        Self::new_with_motion_search_starts(cfg, MotionSearchStarts::Extended)
+    }
+
+    /// Construct an encoder with an explicit motion-search start policy. Existing callers should
+    /// use [`Encoder::new`], which selects [`MotionSearchStarts::Extended`].
+    pub fn new_with_motion_search_starts(cfg: EncoderConfig, starts: MotionSearchStarts) -> Result<Self, Error> {
+        let extra = starts == MotionSearchStarts::Extended;
+        let core = if cfg.bit_depth > 8 {
+            Cores::Deep(Box::new(Core::new_with_extra_motion_search_starts(cfg, extra)?))
+        } else {
+            Cores::Eight(Box::new(Core::new_with_extra_motion_search_starts(cfg, extra)?))
+        };
         Ok(Encoder { core })
     }
 
@@ -1450,7 +1473,7 @@ mod tests {
     /// A recycled `MbInfo` buffer must come back as all-default entries, whatever the previous picture left in it.
     #[test]
     fn recycled_mb_buffers_are_cleared() {
-        let mut core = Core::<u8>::new(EncoderConfig::new(64, 48, 30, 1)).unwrap();
+        let mut core = Core::<u8>::new_with_extra_motion_search_starts(EncoderConfig::new(64, 48, 30, 1), true).unwrap();
         let n = core.params.mbw * core.params.mbh;
         let mut m = core.new_mbs();
         assert_eq!(m.len(), n);

crates/video/src/h264/encode/slice.rs

@@ -47,6 +47,8 @@ pub struct EncParams {
     pub always_intra: bool,
     /// Maximum CAVLC level magnitude (Baseline).
     pub cavlc_clamp: bool,
+    /// Whether the 16x16 motion search tries skip, zero and co-located starts.
+    pub extra_motion_search_starts: bool,
     pub depth: Depth,
 }
 

crates/video/src/h264/encode/slice/motion.rs

@@ -296,10 +296,14 @@ impl<'a, 'b, P: Pixel> SliceEnc<'a, 'b, P> {
             skip_pred = Some((y, u, v));
         }
         // --- 16x16 search
-        let mut cands = vec![skip_mv, Mv::ZERO];
-        let colo = r.mbs[my * f.p.mbw + mx];
-        if colo.ref_idx[0][0] >= 0 {
-            cands.push(colo.mv[0][0]);
+        let mut cands = Vec::new();
+        if f.p.extra_motion_search_starts {
+            cands.push(skip_mv);
+            cands.push(Mv::ZERO);
+            let colo = r.mbs[my * f.p.mbw + mx];
+            if colo.ref_idx[0][0] >= 0 {
+                cands.push(colo.mv[0][0]);
+            }
         }
         let (mv16, mvp16, cost16) = {
             let (m, p, c) = self.search_part(r, &PartSearch { mx, my, part: Part::P16x16, list: 0, lam, seed: skip_mv }, src, &cands);

crates/video/tests/h264_encode/pinned_decisions.rs

@@ -7,8 +7,12 @@ use craft_video::h264::*;
 
 /// FNV-1a over the slice NAL units of `n` frames of [`synth`] encoded with `c`.
 fn slices_hash(c: EncoderConfig, n: usize) -> u64 {
+    slices_hash_with_starts(c, n, MotionSearchStarts::Extended)
+}
+
+fn slices_hash_with_starts(c: EncoderConfig, n: usize, starts: MotionSearchStarts) -> u64 {
     let (w, h) = (c.width as usize, c.height as usize);
-    let mut enc = Encoder::new(c).unwrap();
+    let mut enc = Encoder::new_with_motion_search_starts(c, starts).unwrap();
     let mut stream = Vec::new();
     for t in 0..n {
         let f = synth(w, h, t);
@@ -61,3 +65,27 @@ fn coded_slices_are_pinned() {
         assert_eq!(*got, *want, "{name}: the coded slices changed");
     }
 }
+
+#[test]
+fn filmcraft_writer_keeps_shared_search_decisions_pinned() {
+    let cases = [
+        ("high balanced crf", cfg(Profile::High, Preset::Balanced, RateControl::Crf(23.0)), 0x6223_db82_41eb_7da1u64),
+        ("high quality vbr", cfg(Profile::High, Preset::Quality, RateControl::Vbr { target_kbps: 800, max_kbps: 1200 }), 0x721d_6e68_3f96_5bf1),
+        ("main speed qp", cfg(Profile::Main, Preset::Speed, RateControl::Qp(30)), 0x7772_91d5_4c75_c320),
+        ("baseline balanced cbr", cfg(Profile::Baseline, Preset::Balanced, RateControl::Cbr { kbps: 600 }), 0x72ff_5e78_a3f0_e46e),
+    ];
+    let got: Vec<u64> = cases
+        .iter()
+        .map(|(_, c, _)| {
+            let mut c = c.clone();
+            c.writer = "filmcraft-h264enc 0.20.1".into();
+            slices_hash_with_starts(c, 20, MotionSearchStarts::SeedAndNeighbour)
+        })
+        .collect();
+    for ((name, _, _), got) in cases.iter().zip(&got) {
+        eprintln!("FilmCraft {name}: {got:#018x}");
+    }
+    for ((name, _, want), got) in cases.iter().zip(&got) {
+        assert_eq!(*got, *want, "{name}: FilmCraft's coded slices changed");
+    }
+}