Skip to content

Commit

Permalink
struct Rav1dWarpedMotionParams: Make abcd: [i16; 4] field atomic,…
Browse files Browse the repository at this point in the history
… which eliminates the only mutation of `Rav1dFrameHeader`.
  • Loading branch information
kkysen committed Jan 3, 2024
1 parent 6ed6e8b commit cd8f316
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 34 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ path = "tests/seek_stress.rs"
name = "seek_stress"

[dependencies]
atomig = "0.4.0"
bitflags = "2.4.0"
cfg-if = "1.0.0"
libc = "0.2"
Expand Down
74 changes: 56 additions & 18 deletions include/dav1d/headers.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use crate::src::enum_map::EnumKey;
use atomig::Atom;
use atomig::Atomic;
use std::ffi::c_int;
use std::ffi::c_uint;
use std::ops::BitAnd;
use std::sync::atomic::Ordering;
use strum::EnumCount;
use strum::FromRepr;

Expand Down Expand Up @@ -163,33 +166,68 @@ impl Dav1dWarpedMotionParams {
}
}

#[derive(Clone, Copy, Default)]
pub(crate) struct InnerAbcd([i16; 4]);

impl Atom for InnerAbcd {
type Repr = u64;

fn pack(self) -> Self::Repr {
let [[b1, b2], [b3, b4], [b5, b6], [b7, b8]] = self.0.map(|x| x.to_ne_bytes());
Self::Repr::from_ne_bytes([b1, b2, b3, b4, b5, b6, b7, b8])
}

fn unpack(src: Self::Repr) -> Self {
let [b1, b2, b3, b4, b5, b6, b7, b8] = src.to_ne_bytes();
Self([[b1, b2], [b3, b4], [b5, b6], [b7, b8]].map(i16::from_ne_bytes))
}
}

#[derive(Default)]
pub struct Abcd(Atomic<InnerAbcd>);

impl Abcd {
pub fn new(abcd: [i16; 4]) -> Self {
Self(Atomic::new(InnerAbcd(abcd)))
}

pub fn get(&self) -> [i16; 4] {
self.0.load(Ordering::SeqCst).0
}

pub fn set(&self, abcd: [i16; 4]) {
self.0.store(InnerAbcd(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]
}
}

Expand All @@ -203,7 +241,7 @@ impl From<Dav1dWarpedMotionParams> for Rav1dWarpedMotionParams {
Self {
r#type,
matrix,
abcd,
abcd: Abcd::new(abcd),
}
}
}
Expand All @@ -218,14 +256,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,
Expand Down
13 changes: 6 additions & 7 deletions src/obu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/recon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
18 changes: 13 additions & 5 deletions src/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
4 changes: 2 additions & 2 deletions src/warpmv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down

0 comments on commit cd8f316

Please sign in to comment.