From fe2a24035e88ac074af186ac87778ce08f44bd63 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Fri, 1 Dec 2023 17:53:53 -0500 Subject: [PATCH 1/5] `mod obu`: Translate `offsetof`. --- lib.rs | 1 + src/obu.rs | 13 ++++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib.rs b/lib.rs index 84b0c5370..52cf43f9e 100644 --- a/lib.rs +++ b/lib.rs @@ -3,6 +3,7 @@ #![allow(non_upper_case_globals)] #![feature(c_variadic)] #![feature(core_intrinsics)] +#![feature(offset_of)] // TODO(kkysen) Will be removed shortly. #![cfg_attr(target_arch = "arm", feature(stdsimd))] #![allow(clippy::all)] diff --git a/src/obu.rs b/src/obu.rs index 9979ab1cd..4c9baefab 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -112,6 +112,7 @@ use std::ffi::c_int; use std::ffi::c_uint; use std::ffi::c_ulong; use std::ffi::c_void; +use std::mem; use std::ptr::addr_of_mut; #[inline] @@ -1609,7 +1610,17 @@ pub(crate) unsafe fn rav1d_parse_obus( if c.seq_hdr.is_null() { c.frame_hdr = 0 as *mut Rav1dFrameHeader; c.frame_flags |= PICTURE_FLAG_NEW_SEQUENCE; - } else if memcmp(seq_hdr as *const c_void, c.seq_hdr as *const c_void, 1100) != 0 { + } else if memcmp( + seq_hdr as *const c_void, + c.seq_hdr as *const c_void, + // TODO(kkysen) Remove unstable feature. + // Doing it this way also prevents us from removing the `#[repr(C)]`. + // We should split [`Rav1dSequenceHeader`] into an inner `struct` + // without the `operating_parameter_info` field, + // or at least offer safe field-by-field comparison methods. + mem::offset_of!(Rav1dSequenceHeader, operating_parameter_info), + ) != 0 + { // See 7.5, `operating_parameter_info` is allowed to change in // sequence headers of a single sequence. c.frame_hdr = 0 as *mut Rav1dFrameHeader; From 50d391bd9c8c85c87bb52dedbdbe05bdba7a1b83 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Fri, 1 Dec 2023 18:10:16 -0500 Subject: [PATCH 2/5] `mod obu`: Deduplicate `is_key_or_intra` calls. --- src/obu.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/obu.rs b/src/obu.rs index 4c9baefab..1b38bee61 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -1,3 +1,4 @@ +use crate::include::common::frame::is_key_or_intra; use crate::include::common::intops::iclip_u8; use crate::include::common::intops::ulog2; use crate::include::dav1d::data::Rav1dData; @@ -514,7 +515,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult hdr.force_integer_mv = 0; } - if hdr.frame_type & 1 == 0 { + if is_key_or_intra(hdr) { hdr.force_integer_mv = 1; } @@ -558,7 +559,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult } } - if hdr.frame_type & 1 == 0 { + if is_key_or_intra(hdr) { hdr.refresh_frame_flags = if hdr.frame_type == RAV1D_FRAME_TYPE_KEY && hdr.show_frame != 0 { 0xff } else { From 1b63072e7f706a24b2fd877dc05d2be747be37ed Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Fri, 1 Dec 2023 18:12:35 -0500 Subject: [PATCH 3/5] `mod obu`: Deduplicate `is_inter_or_switch` calls. --- src/obu.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/obu.rs b/src/obu.rs index 1b38bee61..790891ff3 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -1,3 +1,4 @@ +use crate::include::common::frame::is_inter_or_switch; use crate::include::common::frame::is_key_or_intra; use crate::include::common::intops::iclip_u8; use crate::include::common::intops::ulog2; @@ -535,7 +536,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult } else { 0 }; - hdr.primary_ref_frame = if hdr.error_resilient_mode == 0 && hdr.frame_type as c_uint & 1 != 0 { + hdr.primary_ref_frame = if hdr.error_resilient_mode == 0 && is_inter_or_switch(hdr) { rav1d_get_bits(gb, 3) as c_int } else { 7 @@ -736,7 +737,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult hdr.use_ref_frame_mvs = (hdr.error_resilient_mode == 0 && seqhdr.ref_frame_mvs != 0 && seqhdr.order_hint != 0 - && hdr.frame_type & 1 != 0 + && is_inter_or_switch(hdr) && rav1d_get_bit(gb) != 0) as c_int; } @@ -1145,14 +1146,13 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult } else { RAV1D_TX_LARGEST }; - hdr.switchable_comp_refs = if hdr.frame_type as c_uint & 1 != 0 { + hdr.switchable_comp_refs = if is_inter_or_switch(hdr) { rav1d_get_bit(gb) as c_int } else { 0 }; hdr.skip_mode_allowed = 0; - if hdr.switchable_comp_refs != 0 && hdr.frame_type as c_uint & 1 != 0 && seqhdr.order_hint != 0 - { + if hdr.switchable_comp_refs != 0 && is_inter_or_switch(hdr) && seqhdr.order_hint != 0 { let poc = hdr.frame_offset as c_uint; let mut off_before = 0xffffffff; let mut off_after = -1; @@ -1236,7 +1236,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult 0 }; hdr.warp_motion = (hdr.error_resilient_mode == 0 - && hdr.frame_type & 1 != 0 + && is_inter_or_switch(hdr) && seqhdr.warped_motion != 0 && rav1d_get_bit(gb) != 0) as c_int; hdr.reduced_txtp_set = rav1d_get_bit(gb) as c_int; @@ -1245,7 +1245,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult hdr.gmv[i as usize] = dav1d_default_wm_params.clone(); } - if hdr.frame_type & 1 != 0 { + if is_inter_or_switch(hdr) { for i in 0..7 { hdr.gmv[i as usize].r#type = if rav1d_get_bit(gb) == 0 { RAV1D_WM_TYPE_IDENTITY From 1b962f3dedb55ccc5ea95a68be18736f6e3cb3ba Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Fri, 1 Dec 2023 23:46:34 -0500 Subject: [PATCH 4/5] `const {D,R}AV1D_MAX_SEGMENTS`: Deduplicate. --- include/dav1d/headers.rs | 12 ++++++------ src/cdf.rs | 5 +++-- src/internal.rs | 5 +++-- src/obu.rs | 7 ++++--- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/dav1d/headers.rs b/include/dav1d/headers.rs index ccd1ec8db..81a5330e9 100644 --- a/include/dav1d/headers.rs +++ b/include/dav1d/headers.rs @@ -1035,7 +1035,7 @@ impl From for Dav1dSegmentationData { #[derive(Clone)] #[repr(C)] pub struct Dav1dSegmentationDataSet { - pub d: [Dav1dSegmentationData; 8], + pub d: [Dav1dSegmentationData; DAV1D_MAX_SEGMENTS as usize], pub preskip: c_int, pub last_active_segid: c_int, } @@ -1043,7 +1043,7 @@ pub struct Dav1dSegmentationDataSet { #[derive(Clone)] #[repr(C)] pub(crate) struct Rav1dSegmentationDataSet { - pub d: [Rav1dSegmentationData; 8], + pub d: [Rav1dSegmentationData; RAV1D_MAX_SEGMENTS as usize], pub preskip: c_int, pub last_active_segid: c_int, } @@ -1607,8 +1607,8 @@ pub struct Dav1dFrameHeader_segmentation { pub temporal: c_int, pub update_data: c_int, pub seg_data: Dav1dSegmentationDataSet, - pub lossless: [c_int; 8], - pub qidx: [c_int; 8], + pub lossless: [c_int; DAV1D_MAX_SEGMENTS as usize], + pub qidx: [c_int; DAV1D_MAX_SEGMENTS as usize], } #[derive(Clone)] @@ -1619,8 +1619,8 @@ pub(crate) struct Rav1dFrameHeader_segmentation { pub temporal: c_int, pub update_data: c_int, pub seg_data: Rav1dSegmentationDataSet, - pub lossless: [c_int; 8], - pub qidx: [c_int; 8], + pub lossless: [c_int; RAV1D_MAX_SEGMENTS as usize], + pub qidx: [c_int; RAV1D_MAX_SEGMENTS as usize], } impl From for Rav1dFrameHeader_segmentation { diff --git a/src/cdf.rs b/src/cdf.rs index 5270fdff3..8b9c4ef1b 100644 --- a/src/cdf.rs +++ b/src/cdf.rs @@ -1,4 +1,5 @@ use crate::include::dav1d::headers::Rav1dFrameHeader; +use crate::include::dav1d::headers::RAV1D_MAX_SEGMENTS; use crate::include::dav1d::headers::RAV1D_N_SWITCHABLE_FILTERS; use crate::include::stdatomic::atomic_uint; use crate::src::align::Align16; @@ -89,7 +90,7 @@ pub struct CdfModeContext { pub angle_delta: Align16<[[u16; 8]; 8]>, pub filter_intra: Align16<[u16; 8]>, pub comp_inter_mode: Align16<[[u16; N_COMP_INTER_PRED_MODES]; 8]>, - pub seg_id: Align16<[[u16; 8]; 3]>, + pub seg_id: Align16<[[u16; RAV1D_MAX_SEGMENTS as usize]; 3]>, pub pal_sz: Align16<[[[u16; 8]; 7]; 2]>, pub color_map: Align16<[[[[u16; 8]; 5]; 7]; 2]>, pub filter: Align8<[[[u16; 4]; 8]; 2]>, @@ -5186,7 +5187,7 @@ pub(crate) unsafe fn rav1d_cdf_thread_update( ((*src).m.seg_id[j_18 as usize]).as_ptr() as *const c_void, ::core::mem::size_of::<[u16; 8]>(), ); - (*dst).m.seg_id[j_18 as usize][(8 - 1) as usize] = 0 as c_int as u16; + (*dst).m.seg_id[j_18 as usize][(RAV1D_MAX_SEGMENTS - 1) as usize] = 0 as c_int as u16; j_18 += 1; } memcpy( diff --git a/src/internal.rs b/src/internal.rs index 930350c4f..aeff711d9 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -15,6 +15,7 @@ use crate::include::dav1d::headers::Rav1dITUTT35; use crate::include::dav1d::headers::Rav1dMasteringDisplay; use crate::include::dav1d::headers::Rav1dSequenceHeader; use crate::include::dav1d::headers::Rav1dWarpedMotionParams; +use crate::include::dav1d::headers::RAV1D_MAX_SEGMENTS; use crate::include::dav1d::picture::Rav1dPicAllocator; use crate::include::dav1d::picture::Rav1dPicture; use crate::include::stdatomic::atomic_int; @@ -455,7 +456,7 @@ pub(crate) struct Rav1dFrameContext { pub sb_shift: c_int, pub sb_step: c_int, pub sr_sb128w: c_int, - pub dq: [[[u16; 2]; 3]; 8], + pub dq: [[[u16; 2]; 3]; RAV1D_MAX_SEGMENTS as usize], pub qm: [[*const u8; 3]; 19], pub a: *mut BlockContext, pub a_sz: c_int, @@ -492,7 +493,7 @@ pub struct Rav1dTileState { pub progress: [atomic_int; 2], pub frame_thread: [Rav1dTileState_frame_thread; 2], pub lowest_pixel: *mut [[c_int; 2]; 7], - pub dqmem: [[[u16; 2]; 3]; 8], + pub dqmem: [[[u16; 2]; 3]; RAV1D_MAX_SEGMENTS as usize], pub dq: *const [[u16; 2]; 3], pub last_qidx: c_int, pub last_delta_lf: [i8; 4], diff --git a/src/obu.rs b/src/obu.rs index 790891ff3..13d7046aa 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -37,6 +37,7 @@ use crate::include::dav1d::headers::RAV1D_FRAME_TYPE_INTER; use crate::include::dav1d::headers::RAV1D_FRAME_TYPE_INTRA; use crate::include::dav1d::headers::RAV1D_FRAME_TYPE_KEY; use crate::include::dav1d::headers::RAV1D_FRAME_TYPE_SWITCH; +use crate::include::dav1d::headers::RAV1D_MAX_SEGMENTS; use crate::include::dav1d::headers::RAV1D_MC_IDENTITY; use crate::include::dav1d::headers::RAV1D_MC_UNKNOWN; use crate::include::dav1d::headers::RAV1D_OBU_FRAME; @@ -912,7 +913,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult if hdr.segmentation.update_data != 0 { hdr.segmentation.seg_data.preskip = 0; hdr.segmentation.seg_data.last_active_segid = -1; - for i in 0..8 { + for i in 0..RAV1D_MAX_SEGMENTS as c_int { let seg = &mut hdr.segmentation.seg_data.d[i as usize]; if rav1d_get_bit(gb) != 0 { seg.delta_q = rav1d_get_sbits(gb, 9); @@ -981,7 +982,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult 0, ::core::mem::size_of::(), ); - for i in 0..8 { + for i in 0..RAV1D_MAX_SEGMENTS { hdr.segmentation.seg_data.d[i as usize].r#ref = -1; } } @@ -1017,7 +1018,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult && hdr.quant.vdc_delta == 0 && hdr.quant.vac_delta == 0) as c_int; hdr.all_lossless = 1; - for i in 0..8 { + for i in 0..RAV1D_MAX_SEGMENTS { hdr.segmentation.qidx[i as usize] = if hdr.segmentation.enabled != 0 { iclip_u8(hdr.quant.yac + hdr.segmentation.seg_data.d[i as usize].delta_q) } else { From 63ab93f582b3c2b26c89e947162c3c1148a233fe Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Fri, 1 Dec 2023 23:56:10 -0500 Subject: [PATCH 5/5] `const {D,R}AV1D_PRIMARY_REF_NONE`: Deduplicate. --- include/dav1d/headers.rs | 2 ++ src/decode.rs | 9 +++++---- src/obu.rs | 11 ++++++----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/include/dav1d/headers.rs b/include/dav1d/headers.rs index 81a5330e9..46db7b9e8 100644 --- a/include/dav1d/headers.rs +++ b/include/dav1d/headers.rs @@ -421,8 +421,10 @@ pub(crate) const RAV1D_CHR_UNKNOWN: Rav1dChromaSamplePosition = DAV1D_CHR_UNKNOW // Constants from Section 3. "Symbols and abbreviated terms" pub const DAV1D_MAX_SEGMENTS: u8 = 8; +pub const DAV1D_PRIMARY_REF_NONE: c_int = 7; pub(crate) const RAV1D_MAX_SEGMENTS: u8 = DAV1D_MAX_SEGMENTS; +pub(crate) const RAV1D_PRIMARY_REF_NONE: c_int = DAV1D_PRIMARY_REF_NONE; #[repr(C)] pub struct Rav1dContentLightLevel { diff --git a/src/decode.rs b/src/decode.rs index c36008ff0..7fbfe1a70 100644 --- a/src/decode.rs +++ b/src/decode.rs @@ -19,6 +19,7 @@ use crate::include::dav1d::headers::RAV1D_FILTER_8TAP_REGULAR; use crate::include::dav1d::headers::RAV1D_FILTER_SWITCHABLE; use crate::include::dav1d::headers::RAV1D_MAX_SEGMENTS; use crate::include::dav1d::headers::RAV1D_N_SWITCHABLE_FILTERS; +use crate::include::dav1d::headers::RAV1D_PRIMARY_REF_NONE; use crate::include::dav1d::headers::RAV1D_RESTORATION_NONE; use crate::include::dav1d::headers::RAV1D_RESTORATION_SGRPROJ; use crate::include::dav1d::headers::RAV1D_RESTORATION_SWITCHABLE; @@ -1160,7 +1161,7 @@ unsafe fn get_prev_frame_segid( ref_seg_map: *const u8, stride: ptrdiff_t, ) -> u8 { - assert!((*f.frame_hdr).primary_ref_frame != 7); + assert!((*f.frame_hdr).primary_ref_frame != RAV1D_PRIMARY_REF_NONE); // Need checked casts here because an overflowing cast // would give a too large `len` to [`std::slice::from_raw_parts`], which would UB. @@ -5099,7 +5100,7 @@ pub unsafe fn rav1d_submit_frame(c: &mut Rav1dContext) -> Rav1dResult { let mut ref_coded_width = <[i32; 7]>::default(); if is_inter_or_switch(&*f.frame_hdr) { - if (*f.frame_hdr).primary_ref_frame != 7 { + if (*f.frame_hdr).primary_ref_frame != RAV1D_PRIMARY_REF_NONE { let pri_ref = (*f.frame_hdr).refidx[(*f.frame_hdr).primary_ref_frame as usize] as usize; if c.refs[pri_ref].p.p.data[0].is_null() { on_error(f, c, out_delayed); @@ -5143,7 +5144,7 @@ pub unsafe fn rav1d_submit_frame(c: &mut Rav1dContext) -> Rav1dResult { } // setup entropy - if (*f.frame_hdr).primary_ref_frame == 7 { + if (*f.frame_hdr).primary_ref_frame == RAV1D_PRIMARY_REF_NONE { rav1d_cdf_thread_init_static(&mut f.in_cdf, (*f.frame_hdr).quant.yac); } else { let pri_ref = (*f.frame_hdr).refidx[(*f.frame_hdr).primary_ref_frame as usize] as usize; @@ -5292,7 +5293,7 @@ pub unsafe fn rav1d_submit_frame(c: &mut Rav1dContext) -> Rav1dResult { if (*f.frame_hdr).segmentation.temporal != 0 || (*f.frame_hdr).segmentation.update_map == 0 { let pri_ref = (*f.frame_hdr).primary_ref_frame as usize; - assert!(pri_ref != 7); + assert!(pri_ref != RAV1D_PRIMARY_REF_NONE as usize); let ref_w = (ref_coded_width[pri_ref] + 7 >> 3) << 1; let ref_h = (f.refp[pri_ref].p.p.h + 7 >> 3) << 1; if ref_w == f.bw && ref_h == f.bh { diff --git a/src/obu.rs b/src/obu.rs index 13d7046aa..1f0374e27 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -48,6 +48,7 @@ use crate::include::dav1d::headers::RAV1D_OBU_REDUNDANT_FRAME_HDR; use crate::include::dav1d::headers::RAV1D_OBU_SEQ_HDR; use crate::include::dav1d::headers::RAV1D_OBU_TD; use crate::include::dav1d::headers::RAV1D_OBU_TILE_GRP; +use crate::include::dav1d::headers::RAV1D_PRIMARY_REF_NONE; use crate::include::dav1d::headers::RAV1D_RESTORATION_NONE; use crate::include::dav1d::headers::RAV1D_TRC_SRGB; use crate::include::dav1d::headers::RAV1D_TRC_UNKNOWN; @@ -540,7 +541,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult hdr.primary_ref_frame = if hdr.error_resilient_mode == 0 && is_inter_or_switch(hdr) { rav1d_get_bits(gb, 3) as c_int } else { - 7 + RAV1D_PRIMARY_REF_NONE }; if seqhdr.decoder_model_info_present != 0 { @@ -896,7 +897,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult // segmentation data hdr.segmentation.enabled = rav1d_get_bit(gb) as c_int; if hdr.segmentation.enabled != 0 { - if hdr.primary_ref_frame == 7 { + if hdr.primary_ref_frame == RAV1D_PRIMARY_REF_NONE { hdr.segmentation.update_map = 1; hdr.segmentation.temporal = 0; hdr.segmentation.update_data = 1; @@ -966,7 +967,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult } else { // segmentation.update_data was false so we should copy // segmentation data from the reference frame. - assert!(hdr.primary_ref_frame != 7); + assert!(hdr.primary_ref_frame != RAV1D_PRIMARY_REF_NONE); let pri_ref = hdr.refidx[hdr.primary_ref_frame as usize]; if (c.refs[pri_ref as usize].p.p.frame_hdr).is_null() { return error(c); @@ -1050,7 +1051,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult } hdr.loopfilter.sharpness = rav1d_get_bits(gb, 3) as c_int; - if hdr.primary_ref_frame == 7 { + if hdr.primary_ref_frame == RAV1D_PRIMARY_REF_NONE { hdr.loopfilter.mode_ref_deltas = default_mode_ref_deltas.clone(); } else { let r#ref = hdr.refidx[hdr.primary_ref_frame as usize]; @@ -1259,7 +1260,7 @@ unsafe fn parse_frame_hdr(c: &mut Rav1dContext, gb: &mut GetBits) -> Rav1dResult }; if !(hdr.gmv[i as usize].r#type == RAV1D_WM_TYPE_IDENTITY) { let ref_gmv; - if hdr.primary_ref_frame == 7 { + if hdr.primary_ref_frame == RAV1D_PRIMARY_REF_NONE { ref_gmv = &dav1d_default_wm_params; } else { let pri_ref = hdr.refidx[hdr.primary_ref_frame as usize];