From bc39b2b70d32ef692756459a06a1f88012667138 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Thu, 21 Dec 2023 17:48:09 -0800 Subject: [PATCH] `struct {D,R}av1dITUTT35`: Replace `Rav1dRef`s with `Option>>`s. --- include/dav1d/headers.rs | 4 ++-- include/dav1d/picture.rs | 46 ++++++++++------------------------------ src/internal.rs | 5 +++-- src/lib.rs | 8 +++---- src/obu.rs | 21 ++++++------------ src/picture.rs | 40 ++++++++-------------------------- tests/seek_stress.rs | 9 ++++---- tools/dav1d.rs | 5 ++--- 8 files changed, 40 insertions(+), 98 deletions(-) diff --git a/include/dav1d/headers.rs b/include/dav1d/headers.rs index 6bfe17d4d..e5ca51674 100644 --- a/include/dav1d/headers.rs +++ b/include/dav1d/headers.rs @@ -8,7 +8,7 @@ use strum::FromRepr; /// This is so we can store both `*mut D` and `*mut R` /// for maintaining `dav1d` ABI compatibility, /// where `D` is the `Dav1d*` type and `R` is the `Rav1d` type. -pub(crate) struct DRav1d { +pub struct DRav1d { pub rav1d: R, pub dav1d: D, } @@ -502,7 +502,7 @@ pub struct Dav1dITUTT35 { #[derive(Clone)] #[repr(C)] -pub(crate) struct Rav1dITUTT35 { +pub struct Rav1dITUTT35 { pub country_code: u8, pub country_code_extension_byte: u8, pub payload: Box<[u8]>, diff --git a/include/dav1d/picture.rs b/include/dav1d/picture.rs index b692839b6..51d5b66d0 100644 --- a/include/dav1d/picture.rs +++ b/include/dav1d/picture.rs @@ -19,6 +19,7 @@ use libc::ptrdiff_t; use libc::uintptr_t; use std::ffi::c_int; use std::ffi::c_void; +use std::marker::PhantomData; use std::ptr; use std::ptr::addr_of_mut; use std::ptr::NonNull; @@ -76,13 +77,13 @@ pub struct Dav1dPicture { pub m: Dav1dDataProps, pub content_light: Option>, pub mastering_display: Option>, - pub itut_t35: *mut Dav1dITUTT35, + pub itut_t35: Option>, pub reserved: [uintptr_t; 4], pub frame_hdr_ref: *mut Dav1dRef, pub seq_hdr_ref: *mut Dav1dRef, pub content_light_ref: Option>, // opaque, so we can change this pub mastering_display_ref: Option>, // opaque, so we can change this - pub itut_t35_ref: *mut Dav1dRef, + pub itut_t35_ref: Option>>>, // opaque, so we can change this pub reserved_ref: [uintptr_t; 4], pub r#ref: *mut Dav1dRef, pub allocator_data: *mut c_void, @@ -99,10 +100,9 @@ pub(crate) struct Rav1dPicture { pub m: Rav1dDataProps, pub content_light: Option>, pub mastering_display: Option>, - pub itut_t35: *mut Rav1dITUTT35, + pub itut_t35: Option>>, pub frame_hdr_ref: *mut Rav1dRef, pub seq_hdr_ref: *mut Rav1dRef, - pub itut_t35_ref: *mut Rav1dRef, pub r#ref: *mut Rav1dRef, pub allocator_data: *mut c_void, } @@ -118,7 +118,7 @@ impl From for Rav1dPicture { m, content_light: _, mastering_display: _, - itut_t35, + itut_t35: _, reserved: _, frame_hdr_ref, seq_hdr_ref, @@ -165,22 +165,10 @@ impl From for Rav1dPicture { content_light: content_light_ref.map(|raw| unsafe { Arc::from_raw(raw.as_ptr()) }), mastering_display: mastering_display_ref .map(|raw| unsafe { Arc::from_raw(raw.as_ptr()) }), - // We don't `.update_rav1d` [`Rav1dITUTT35`] because never read it. - itut_t35: if itut_t35.is_null() { - ptr::null_mut() - } else { - unsafe { - addr_of_mut!( - (*(itut_t35_ref.read()) - .data - .cast::>()) - .rav1d - ) - } - }, + // We don't `.update_rav1d()` [`Rav1dITUTT35`] because never read it. + itut_t35: itut_t35_ref.map(|raw| unsafe { Arc::from_raw(raw.as_ptr().cast()) }), frame_hdr_ref, seq_hdr_ref, - itut_t35_ref, r#ref, allocator_data, } @@ -201,7 +189,6 @@ impl From for Dav1dPicture { itut_t35, frame_hdr_ref, seq_hdr_ref, - itut_t35_ref, r#ref, allocator_data, } = value; @@ -241,18 +228,7 @@ impl From for Dav1dPicture { content_light: content_light.as_ref().map(|arc| arc.as_ref().into()), mastering_display: mastering_display.as_ref().map(|arc| arc.as_ref().into()), // `DRav1d::from_rav1d` is called in [`rav1d_parse_obus`]. - itut_t35: if itut_t35.is_null() { - ptr::null_mut() - } else { - unsafe { - addr_of_mut!( - (*(itut_t35_ref.read()) - .data - .cast::>()) - .dav1d - ) - } - }, + itut_t35: itut_t35.as_ref().map(|arc| (&arc.as_ref().dav1d).into()), reserved: Default::default(), frame_hdr_ref, seq_hdr_ref, @@ -260,7 +236,8 @@ impl From for Dav1dPicture { .map(|arc| unsafe { NonNull::new_unchecked(Arc::into_raw(arc).cast_mut()) }), mastering_display_ref: mastering_display .map(|arc| unsafe { NonNull::new_unchecked(Arc::into_raw(arc).cast_mut()) }), - itut_t35_ref, + itut_t35_ref: itut_t35 + .map(|arc| unsafe { NonNull::new_unchecked(Arc::into_raw(arc).cast_mut().cast()) }), reserved_ref: Default::default(), r#ref, allocator_data, @@ -284,10 +261,9 @@ impl Default for Rav1dPicture { m: Default::default(), content_light: None, mastering_display: None, - itut_t35: ptr::null_mut(), + itut_t35: None, frame_hdr_ref: ptr::null_mut(), seq_hdr_ref: ptr::null_mut(), - itut_t35_ref: ptr::null_mut(), r#ref: ptr::null_mut(), allocator_data: ptr::null_mut(), } diff --git a/src/internal.rs b/src/internal.rs index fce2f104e..0a43d5095 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -8,6 +8,8 @@ use crate::include::dav1d::data::Rav1dData; use crate::include::dav1d::dav1d::Rav1dDecodeFrameType; use crate::include::dav1d::dav1d::Rav1dEventFlags; use crate::include::dav1d::dav1d::Rav1dInloopFilterType; +use crate::include::dav1d::headers::DRav1d; +use crate::include::dav1d::headers::Dav1dITUTT35; use crate::include::dav1d::headers::Rav1dContentLightLevel; use crate::include::dav1d::headers::Rav1dFrameHeader; use crate::include::dav1d::headers::Rav1dITUTT35; @@ -211,8 +213,7 @@ pub struct Rav1dContext { pub(crate) frame_hdr: *mut Rav1dFrameHeader, pub(crate) content_light: Option>, pub(crate) mastering_display: Option>, - pub(crate) itut_t35_ref: *mut Rav1dRef, - pub(crate) itut_t35: *mut Rav1dITUTT35, + pub(crate) itut_t35: Option>>, // decoded output picture queue pub(crate) in_0: Rav1dData, diff --git a/src/lib.rs b/src/lib.rs index d5210c2ea..cb578936a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,7 +19,6 @@ use crate::include::dav1d::headers::Dav1dFrameHeader; use crate::include::dav1d::headers::Dav1dSequenceHeader; use crate::include::dav1d::headers::Rav1dFilmGrainData; use crate::include::dav1d::headers::Rav1dFrameHeader; -use crate::include::dav1d::headers::Rav1dITUTT35; use crate::include::dav1d::headers::Rav1dSequenceHeader; use crate::include::dav1d::picture::Dav1dPicture; use crate::include::dav1d::picture::Rav1dPicture; @@ -844,7 +843,7 @@ pub unsafe extern "C" fn dav1d_apply_grain( .cast::>()) .update_rav1d(); } - // Don't `.update_rav1d` [`Rav1dITUTT35`] because we never read it. + // Don't `.update_rav1d()` [`Rav1dITUTT35`] because we never read it. let mut out_rust = MaybeUninit::zeroed().assume_init(); // TODO(kkysen) Temporary until we return it directly. let in_rust = in_0.into(); let result = rav1d_apply_grain(c, &mut out_rust, &in_rust); @@ -879,8 +878,7 @@ pub(crate) unsafe fn rav1d_flush(c: *mut Rav1dContext) { rav1d_ref_dec(&mut (*c).seq_hdr_ref); let _ = mem::take(&mut (*c).content_light); let _ = mem::take(&mut (*c).mastering_display); - (*c).itut_t35 = 0 as *mut Rav1dITUTT35; - rav1d_ref_dec(&mut (*c).itut_t35_ref); + let _ = mem::take(&mut (*c).itut_t35); let _ = mem::take(&mut (*c).cached_error_props); if (*c).n_fc == 1 as c_uint && (*c).n_tc == 1 as c_uint { return; @@ -1086,7 +1084,7 @@ unsafe fn close_internal(c_out: &mut *mut Rav1dContext, flush: c_int) { rav1d_ref_dec(&mut (*c).frame_hdr_ref); let _ = mem::take(&mut (*c).mastering_display); let _ = mem::take(&mut (*c).content_light); - rav1d_ref_dec(&mut (*c).itut_t35_ref); + let _ = mem::take(&mut (*c).itut_t35); rav1d_mem_pool_end((*c).seq_hdr_pool); rav1d_mem_pool_end((*c).frame_hdr_pool); rav1d_mem_pool_end((*c).segmap_pool); diff --git a/src/obu.rs b/src/obu.rs index f95051a62..d34814324 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -2464,16 +2464,11 @@ unsafe fn parse_obus(c: &mut Rav1dContext, r#in: &Rav1dData, global: bool) -> Ra let country_code_extension_byte = country_code_extension_byte as u8; let payload = (0..payload_size).map(|_| gb.get_bits(8) as u8).collect(); // TODO(kkysen) fallible allocation - let itut_t35_metadatas = - (*r#ref).data.cast::>(); - itut_t35_metadatas.write(DRav1d::from_rav1d(Rav1dITUTT35 { + c.itut_t35 = Some(Arc::new(DRav1d::from_rav1d(Rav1dITUTT35 { country_code, country_code_extension_byte, payload, - })); - rav1d_ref_dec(&mut c.itut_t35_ref); - c.itut_t35 = &mut (*itut_t35_metadatas).rav1d; - c.itut_t35_ref = r#ref; + }))); // TODO(kkysen) fallible allocation } } OBU_META_SCALABILITY | OBU_META_TIMECODE => {} // Ignore metadata OBUs we don't care about. @@ -2542,13 +2537,11 @@ unsafe fn parse_obus(c: &mut Rav1dContext, r#in: &Rav1dData, global: bool) -> Ra &mut (*c).out.p, &c.content_light, &c.mastering_display, - c.itut_t35, - c.itut_t35_ref, + &c.itut_t35, &r#in.m, ); // Must be removed from the context after being attached to the frame - rav1d_ref_dec(&mut c.itut_t35_ref); - c.itut_t35 = 0 as *mut Rav1dITUTT35; + let _ = mem::take(&mut c.itut_t35); c.event_flags |= c.refs[(*c.frame_hdr).existing_frame_idx as usize] .p .flags @@ -2617,13 +2610,11 @@ unsafe fn parse_obus(c: &mut Rav1dContext, r#in: &Rav1dData, global: bool) -> Ra &mut (*out_delayed).p, &c.content_light, &c.mastering_display, - c.itut_t35, - c.itut_t35_ref, + &c.itut_t35, &r#in.m, ); // Must be removed from the context after being attached to the frame - rav1d_ref_dec(&mut c.itut_t35_ref); - c.itut_t35 = 0 as *mut Rav1dITUTT35; + let _ = mem::take(&mut c.itut_t35); pthread_mutex_unlock(&mut c.task_thread.lock); } if (*c.refs[(*c.frame_hdr).existing_frame_idx as usize] diff --git a/src/picture.rs b/src/picture.rs index e612040f2..256981f8e 100644 --- a/src/picture.rs +++ b/src/picture.rs @@ -1,6 +1,8 @@ use crate::include::common::validate::validate_input; use crate::include::dav1d::common::Rav1dDataProps; use crate::include::dav1d::dav1d::Rav1dEventFlags; +use crate::include::dav1d::headers::DRav1d; +use crate::include::dav1d::headers::Dav1dITUTT35; use crate::include::dav1d::headers::Rav1dContentLightLevel; use crate::include::dav1d::headers::Rav1dFrameHeader; use crate::include::dav1d::headers::Rav1dITUTT35; @@ -178,8 +180,7 @@ unsafe fn picture_alloc_with_edges( frame_hdr_ref: *mut Rav1dRef, content_light: &Option>, mastering_display: &Option>, - itut_t35: *mut Rav1dITUTT35, - itut_t35_ref: *mut Rav1dRef, + itut_t35: &Option>>, bpc: c_int, props: *const Rav1dDataProps, p_allocator: *mut Rav1dPicAllocator, @@ -239,14 +240,7 @@ unsafe fn picture_alloc_with_edges( if !frame_hdr_ref.is_null() { rav1d_ref_inc(frame_hdr_ref); } - rav1d_picture_copy_props( - p, - content_light, - mastering_display, - itut_t35, - itut_t35_ref, - props, - ); + rav1d_picture_copy_props(p, content_light, mastering_display, itut_t35, props); if extra != 0 && !extra_ptr.is_null() { *extra_ptr = &mut (*pic_ctx).extra_ptr as *mut *mut c_void as *mut c_void; @@ -259,21 +253,13 @@ pub unsafe fn rav1d_picture_copy_props( p: *mut Rav1dPicture, content_light: &Option>, mastering_display: &Option>, - itut_t35: *mut Rav1dITUTT35, - itut_t35_ref: *mut Rav1dRef, + itut_t35: &Option>>, props: *const Rav1dDataProps, ) { (*p).m = (*props).clone(); - (*p).content_light = content_light.clone(); (*p).mastering_display = mastering_display.clone(); - - rav1d_ref_dec(&mut (*p).itut_t35_ref); - (*p).itut_t35_ref = itut_t35_ref; - (*p).itut_t35 = itut_t35; - if !itut_t35_ref.is_null() { - rav1d_ref_inc(itut_t35_ref); - } + (*p).itut_t35 = itut_t35.clone(); } pub(crate) unsafe fn rav1d_thread_picture_alloc( @@ -294,8 +280,7 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc( (*f).frame_hdr_ref, &(*c).content_light, &(*c).mastering_display, - (*c).itut_t35, - (*c).itut_t35_ref, + &(*c).itut_t35, bpc, &mut (*f).tiles[0].data.m, &mut (*c).allocator, @@ -309,8 +294,7 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc( if res.is_err() { return res; } - rav1d_ref_dec(&mut (*c).itut_t35_ref); - (*c).itut_t35 = 0 as *mut Rav1dITUTT35; + let _ = mem::take(&mut (*c).itut_t35); let flags_mask = if (*(*f).frame_hdr).show_frame != 0 || (*c).output_invisible_frames { PictureFlags::empty() } else { @@ -345,8 +329,7 @@ pub(crate) unsafe fn rav1d_picture_alloc_copy( (*src).frame_hdr_ref, &(*src).content_light, &(*src).mastering_display, - (*src).itut_t35, - (*src).itut_t35_ref, + &(*src).itut_t35, (*src).p.bpc, &(*src).m, &mut (*pic_ctx).allocator, @@ -372,9 +355,6 @@ pub(crate) unsafe fn rav1d_picture_ref(dst: &mut Rav1dPicture, src: &Rav1dPictur if !src.seq_hdr_ref.is_null() { rav1d_ref_inc(src.seq_hdr_ref); } - if !src.itut_t35_ref.is_null() { - rav1d_ref_inc(src.itut_t35_ref); - } *dst = src.clone(); } @@ -415,7 +395,6 @@ pub(crate) unsafe fn rav1d_picture_unref_internal(p: &mut Rav1dPicture) { mut r#ref, mut frame_hdr_ref, mut seq_hdr_ref, - mut itut_t35_ref, .. } = mem::take(p); if !r#ref.is_null() { @@ -426,7 +405,6 @@ pub(crate) unsafe fn rav1d_picture_unref_internal(p: &mut Rav1dPicture) { } rav1d_ref_dec(&mut seq_hdr_ref); rav1d_ref_dec(&mut frame_hdr_ref); - rav1d_ref_dec(&mut itut_t35_ref); } pub(crate) unsafe fn rav1d_thread_picture_unref(p: *mut Rav1dThreadPicture) { diff --git a/tests/seek_stress.rs b/tests/seek_stress.rs index bd3ee36d1..fd1b63426 100644 --- a/tests/seek_stress.rs +++ b/tests/seek_stress.rs @@ -48,7 +48,6 @@ use rav1d::include::dav1d::dav1d::DAV1D_DECODEFRAMETYPE_ALL; use rav1d::include::dav1d::dav1d::DAV1D_INLOOPFILTER_NONE; use rav1d::include::dav1d::headers::Dav1dColorPrimaries; use rav1d::include::dav1d::headers::Dav1dFrameHeader; -use rav1d::include::dav1d::headers::Dav1dITUTT35; use rav1d::include::dav1d::headers::Dav1dSequenceHeader; use rav1d::include::dav1d::headers::Dav1dSequenceHeaderOperatingParameterInfo; use rav1d::include::dav1d::headers::Dav1dSequenceHeaderOperatingPoint; @@ -169,13 +168,13 @@ unsafe fn decode_rand( }, content_light: None, mastering_display: None, - itut_t35: 0 as *mut Dav1dITUTT35, + itut_t35: None, reserved: [0; 4], frame_hdr_ref: 0 as *mut Dav1dRef, seq_hdr_ref: 0 as *mut Dav1dRef, content_light_ref: None, mastering_display_ref: None, - itut_t35_ref: 0 as *mut Dav1dRef, + itut_t35_ref: None, reserved_ref: [0; 4], r#ref: 0 as *mut Dav1dRef, allocator_data: 0 as *mut c_void, @@ -224,13 +223,13 @@ unsafe fn decode_all( }, content_light: None, mastering_display: None, - itut_t35: 0 as *mut Dav1dITUTT35, + itut_t35: None, reserved: [0; 4], frame_hdr_ref: 0 as *mut Dav1dRef, seq_hdr_ref: 0 as *mut Dav1dRef, content_light_ref: None, mastering_display_ref: None, - itut_t35_ref: 0 as *mut Dav1dRef, + itut_t35_ref: None, reserved_ref: [0; 4], r#ref: 0 as *mut Dav1dRef, allocator_data: 0 as *mut c_void, diff --git a/tools/dav1d.rs b/tools/dav1d.rs index 2df26eb20..b03d2df85 100644 --- a/tools/dav1d.rs +++ b/tools/dav1d.rs @@ -65,7 +65,6 @@ use rav1d::include::dav1d::dav1d::DAV1D_DECODEFRAMETYPE_ALL; use rav1d::include::dav1d::dav1d::DAV1D_INLOOPFILTER_NONE; use rav1d::include::dav1d::headers::Dav1dColorPrimaries; use rav1d::include::dav1d::headers::Dav1dFrameHeader; -use rav1d::include::dav1d::headers::Dav1dITUTT35; use rav1d::include::dav1d::headers::Dav1dSequenceHeader; use rav1d::include::dav1d::headers::Dav1dSequenceHeaderOperatingParameterInfo; use rav1d::include::dav1d::headers::Dav1dSequenceHeaderOperatingPoint; @@ -326,13 +325,13 @@ unsafe fn main_0(argc: c_int, argv: *const *mut c_char) -> c_int { }, content_light: None, mastering_display: None, - itut_t35: 0 as *mut Dav1dITUTT35, + itut_t35: None, reserved: [0; 4], frame_hdr_ref: 0 as *mut Dav1dRef, seq_hdr_ref: 0 as *mut Dav1dRef, content_light_ref: None, mastering_display_ref: None, - itut_t35_ref: 0 as *mut Dav1dRef, + itut_t35_ref: None, reserved_ref: [0; 4], r#ref: 0 as *mut Dav1dRef, allocator_data: 0 as *mut c_void,