struct Rav1dWarpedMotionParams: Make abcd: [i16; 4] field atomic (#…

…662)

This eliminates the only mutation of `Rav1dFrameHeader`, thus allowing
it to be stored in an `Arc` without any borrowck problems.

This uses the [`atomig`](https://docs.rs/atomig) crate to make the
`[i16; 4]` field atomic. The crate is quite simple, offering an
`Atomic<T>` type that can only be implemented for actually lock-free
atomic types through a simple `pack` and `unpack` into an atomic type
(`u64` in this case for `[i16; 4]`).

The alternatives would to use the [`atomic`](https://docs.rs/atomic)
crate, which also offers an `Atomic<T>`, but doesn't guarantee a
lock-free implementation, and whose implementation seems more complex.
The other alternative would be to basically recreate what `atomig` is
doing of packing and unpacking into an `u64` for atomic operations, so I
think using the existing `atomig` crate is simpler.

Update: I replaced `atomig` with an inline solution of about the same
complexity.

include/dav1d/headers.rs

@@ -2,6 +2,8 @@ use crate::src::enum_map::EnumKey;
 use std::ffi::c_int;
 use std::ffi::c_uint;
 use std::ops::BitAnd;
+use std::sync::atomic::AtomicU64;
+use std::sync::atomic::Ordering;
 use strum::EnumCount;
 use strum::FromRepr;
 
@@ -163,33 +165,61 @@ impl Dav1dWarpedMotionParams {
     }
 }
 
+#[derive(Default)]
+pub struct Abcd(AtomicU64);
+
+impl Abcd {
+    fn pack(unpacked: [i16; 4]) -> u64 {
+        let [[b1, b2], [b3, b4], [b5, b6], [b7, b8]] = unpacked.map(|x| x.to_ne_bytes());
+        u64::from_ne_bytes([b1, b2, b3, b4, b5, b6, b7, b8])
+    }
+
+    fn unpack(packed: u64) -> [i16; 4] {
+        let [b1, b2, b3, b4, b5, b6, b7, b8] = packed.to_ne_bytes();
+        [[b1, b2], [b3, b4], [b5, b6], [b7, b8]].map(i16::from_ne_bytes)
+    }
+
+    pub fn new(abcd: [i16; 4]) -> Self {
+        Self(AtomicU64::new(Self::pack(abcd)))
+    }
+
+    pub fn get(&self) -> [i16; 4] {
+        Self::unpack(self.0.load(Ordering::SeqCst))
+    }
+
+    pub fn set(&self, abcd: [i16; 4]) {
+        self.0.store(Self::pack(abcd), Ordering::SeqCst);
+    }
+}
+
+impl Clone for Abcd {
+    fn clone(&self) -> Self {
+        Self::new(self.get())
+    }
+}
+
 #[derive(Clone)]
-#[repr(C)]
-pub(crate) struct Rav1dWarpedMotionParams {
+pub struct Rav1dWarpedMotionParams {
     pub r#type: Rav1dWarpedMotionType,
     pub matrix: [i32; 6],
-    pub abcd: [i16; 4],
+    pub abcd: Abcd,
 }
 
 impl Rav1dWarpedMotionParams {
-    #[allow(dead_code)] // TODO(kkysen) remove when we use this
-    pub const fn alpha(&self) -> i16 {
-        self.abcd[0]
+    pub fn alpha(&self) -> i16 {
+        self.abcd.get()[0]
     }
 
-    #[allow(dead_code)] // TODO(kkysen) remove when we use this
-    pub const fn beta(&self) -> i16 {
-        self.abcd[1]
+    pub fn beta(&self) -> i16 {
+        self.abcd.get()[1]
     }
 
-    #[allow(dead_code)] // TODO(kkysen) remove when we use this
-    pub const fn gamma(&self) -> i16 {
-        self.abcd[2]
+    pub fn gamma(&self) -> i16 {
+        self.abcd.get()[2]
     }
 
-    #[allow(dead_code)] // TODO(kkysen) remove when we use this
-    pub const fn delta(&self) -> i16 {
-        self.abcd[3]
+    pub fn delta(&self) -> i16 {
+        self.abcd.get()[3]
     }
 }
 
@@ -203,7 +233,7 @@ impl From<Dav1dWarpedMotionParams> for Rav1dWarpedMotionParams {
         Self {
             r#type,
             matrix,
-            abcd,
+            abcd: Abcd::new(abcd),
         }
     }
 }
@@ -218,14 +248,14 @@ impl From<Rav1dWarpedMotionParams> for Dav1dWarpedMotionParams {
         Self {
             r#type,
             matrix,
-            abcd,
+            abcd: abcd.get(),
         }
     }
 }
 
 // TODO(kkysen) Eventually the [`impl Default`] might not be needed.
 #[derive(Clone, Copy, PartialEq, Eq, EnumCount, FromRepr, Default)]
-pub(crate) enum Rav1dPixelLayout {
+pub enum Rav1dPixelLayout {
     #[default]
     I400 = 0,
     I420 = 1,

src/obu.rs

@@ -106,7 +106,6 @@ use crate::src::r#ref::rav1d_ref_create_using_pool;
 use crate::src::r#ref::rav1d_ref_dec;
 use crate::src::r#ref::rav1d_ref_inc;
 use crate::src::r#ref::rav1d_ref_is_writable;
-use crate::src::tables::dav1d_default_wm_params;
 use crate::src::thread_task::FRAME_ERROR;
 use libc::pthread_cond_wait;
 use libc::pthread_mutex_lock;
@@ -1487,7 +1486,7 @@ unsafe fn parse_gmv(
     debug: &Debug,
     gb: &mut GetBits,
 ) -> Rav1dResult<[Rav1dWarpedMotionParams; RAV1D_REFS_PER_FRAME]> {
-    let mut gmv = array::from_fn(|_| dav1d_default_wm_params.clone());
+    let mut gmv = array::from_fn(|_| Rav1dWarpedMotionParams::default());
 
     if frame_type.is_inter_or_switch() {
         for (i, gmv) in gmv.iter_mut().enumerate() {
@@ -1504,16 +1503,16 @@ unsafe fn parse_gmv(
                 continue;
             }
 
-            let ref_gmv;
-            if primary_ref_frame == RAV1D_PRIMARY_REF_NONE {
-                ref_gmv = &dav1d_default_wm_params;
+            let default_gmv = Default::default();
+            let ref_gmv = if primary_ref_frame == RAV1D_PRIMARY_REF_NONE {
+                &default_gmv
             } else {
                 let pri_ref = refidx[primary_ref_frame as usize];
                 if (c.refs[pri_ref as usize].p.p.frame_hdr).is_null() {
                     return Err(EINVAL);
                 }
-                ref_gmv = &(*c.refs[pri_ref as usize].p.p.frame_hdr).gmv[i];
-            }
+                &(*c.refs[pri_ref as usize].p.p.frame_hdr).gmv[i]
+            };
             let mat = &mut gmv.matrix;
             let ref_mat = &ref_gmv.matrix;
             let bits;

src/recon.rs

@@ -2471,7 +2471,7 @@ unsafe fn warp_affine<BD: BitDepth>(
                     dstride,
                     ref_ptr.cast(),
                     ref_stride,
-                    ((*wmp).abcd).as_ptr(),
+                    (*wmp).abcd.get().as_ptr(),
                     mx,
                     my,
                     (*f).bitdepth_max,
@@ -2482,7 +2482,7 @@ unsafe fn warp_affine<BD: BitDepth>(
                     dstride,
                     ref_ptr.cast(),
                     ref_stride,
-                    ((*wmp).abcd).as_ptr(),
+                    (*wmp).abcd.get().as_ptr(),
                     mx,
                     my,
                     (*f).bitdepth_max,

src/tables.rs

@@ -745,11 +745,19 @@ pub const interintra_allowed_mask: c_uint = 0
     | 1 << BS_8x8
     | 0;
 
-pub(crate) static dav1d_default_wm_params: Rav1dWarpedMotionParams = Rav1dWarpedMotionParams {
-    r#type: RAV1D_WM_TYPE_IDENTITY,
-    matrix: [0, 0, 1 << 16, 0, 0, 1 << 16],
-    abcd: [0; 4],
-};
+/// This can't be `const` because [`Atomic`] uses `trait fn`s,
+/// which can't be `const`.
+///
+/// [`Atomic`]: atomig::Atomic
+impl Default for Rav1dWarpedMotionParams {
+    fn default() -> Self {
+        Self {
+            r#type: RAV1D_WM_TYPE_IDENTITY,
+            matrix: [0, 0, 1 << 16, 0, 0, 1 << 16],
+            abcd: Default::default(),
+        }
+    }
+}
 
 pub static dav1d_cdef_directions: [[i8; 2]; 12] = [
     [1 * 12 + 0, 2 * 12 + 0],

src/warpmv.rs

@@ -48,7 +48,7 @@ fn resolve_divisor_32(d: u32) -> (c_int, c_int) {
     (shift + 14, div_lut[f as usize] as c_int)
 }
 
-pub(crate) fn rav1d_get_shear_params(wm: &mut Rav1dWarpedMotionParams) -> bool {
+pub(crate) fn rav1d_get_shear_params(wm: &Rav1dWarpedMotionParams) -> bool {
     let mat = &wm.matrix;
 
     if mat[2] <= 0 {
@@ -67,7 +67,7 @@ pub(crate) fn rav1d_get_shear_params(wm: &mut Rav1dWarpedMotionParams) -> bool {
     let delta =
         iclip_wmp(mat[5] - apply_sign64((v2.abs() + rnd as i64 >> shift) as c_int, v2) - 0x10000)
             as i16;
-    wm.abcd = [alpha, beta, gamma, delta];
+    wm.abcd.set([alpha, beta, gamma, delta]);
 
     4 * (alpha as i32).abs() + 7 * (beta as i32).abs() >= 0x10000
         || 4 * (gamma as i32).abs() + 4 * (delta as i32).abs() >= 0x10000