struct {D,R}av1dITUTT35: Replace Rav1dRefs with `Option<Arc<DRav1…

…d<_, _>>>`s (#661)

Note that we skip `.update_rav1d()`ing it as we never read it.

include/dav1d/headers.rs

@@ -2,14 +2,13 @@ use crate::src::enum_map::EnumKey;
 use std::ffi::c_int;
 use std::ffi::c_uint;
 use std::ops::BitAnd;
-use std::slice;
 use strum::EnumCount;
 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<R, D> {
+pub struct DRav1d<R, D> {
     pub rav1d: R,
     pub dav1d: D,
 }
@@ -503,30 +502,12 @@ 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]>,
 }
 
-impl From<Dav1dITUTT35> for Rav1dITUTT35 {
-    fn from(value: Dav1dITUTT35) -> Self {
-        let Dav1dITUTT35 {
-            country_code,
-            country_code_extension_byte,
-            payload_size,
-            payload,
-        } = value;
-        let payload = unsafe { slice::from_raw_parts_mut(payload, payload_size) };
-        let payload = unsafe { Box::from_raw(payload) };
-        Self {
-            country_code,
-            country_code_extension_byte,
-            payload,
-        }
-    }
-}
-
 impl From<Rav1dITUTT35> for Dav1dITUTT35 {
     fn from(value: Rav1dITUTT35) -> Self {
         let Rav1dITUTT35 {

include/dav1d/picture.rs

@@ -77,13 +77,13 @@ pub struct Dav1dPicture {
     pub m: Dav1dDataProps,
     pub content_light: Option<NonNull<Rav1dContentLightLevel>>,
     pub mastering_display: Option<NonNull<Rav1dMasteringDisplay>>,
-    pub itut_t35: *mut Dav1dITUTT35,
+    pub itut_t35: Option<NonNull<Dav1dITUTT35>>,
     pub reserved: [uintptr_t; 4],
     pub frame_hdr_ref: *mut Dav1dRef,
     pub seq_hdr_ref: *mut Dav1dRef,
     pub content_light_ref: Option<RawArc<Rav1dContentLightLevel>>, // opaque, so we can change this
     pub mastering_display_ref: Option<RawArc<Rav1dMasteringDisplay>>, // opaque, so we can change this
-    pub itut_t35_ref: *mut Dav1dRef,
+    pub itut_t35_ref: Option<RawArc<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>>, // opaque, so we can change this
     pub reserved_ref: [uintptr_t; 4],
     pub r#ref: *mut Dav1dRef,
     pub allocator_data: *mut c_void,
@@ -100,10 +100,9 @@ pub(crate) struct Rav1dPicture {
     pub m: Rav1dDataProps,
     pub content_light: Option<Arc<Rav1dContentLightLevel>>,
     pub mastering_display: Option<Arc<Rav1dMasteringDisplay>>,
-    pub itut_t35: *mut Rav1dITUTT35,
+    pub itut_t35: Option<Arc<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>>,
     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,
 }
@@ -119,7 +118,7 @@ impl From<Dav1dPicture> for Rav1dPicture {
             m,
             content_light: _,
             mastering_display: _,
-            itut_t35,
+            itut_t35: _,
             reserved: _,
             frame_hdr_ref,
             seq_hdr_ref,
@@ -167,22 +166,11 @@ impl From<Dav1dPicture> for Rav1dPicture {
             content_light: content_light_ref.map(|raw| unsafe { raw.into_arc() }),
             // Safety: `raw` came from [`RawArc::from_arc`].
             mastering_display: mastering_display_ref.map(|raw| unsafe { raw.into_arc() }),
-            // `.update_rav1d()` happens in `#[no_mangle] extern "C"`/`DAV1D_API` calls
-            itut_t35: if itut_t35.is_null() {
-                ptr::null_mut()
-            } else {
-                unsafe {
-                    addr_of_mut!(
-                        (*(itut_t35_ref.read())
-                            .data
-                            .cast::<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>())
-                        .rav1d
-                    )
-                }
-            },
+            // We don't `.update_rav1d` [`Rav1dITUTT35`] because never read it.
+            // Safety: `raw` came from [`RawArc::from_arc`].
+            itut_t35: itut_t35_ref.map(|raw| unsafe { raw.into_arc() }),
             frame_hdr_ref,
             seq_hdr_ref,
-            itut_t35_ref,
             r#ref,
             allocator_data,
         }
@@ -203,7 +191,6 @@ impl From<Rav1dPicture> for Dav1dPicture {
             itut_t35,
             frame_hdr_ref,
             seq_hdr_ref,
-            itut_t35_ref,
             r#ref,
             allocator_data,
         } = value;
@@ -243,24 +230,13 @@ impl From<Rav1dPicture> 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::<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>())
-                        .dav1d
-                    )
-                }
-            },
+            itut_t35: itut_t35.as_ref().map(|arc| (&arc.as_ref().dav1d).into()),
             reserved: Default::default(),
             frame_hdr_ref,
             seq_hdr_ref,
             content_light_ref: content_light.map(RawArc::from_arc),
             mastering_display_ref: mastering_display.map(RawArc::from_arc),
-            itut_t35_ref,
+            itut_t35_ref: itut_t35.map(RawArc::from_arc),
             reserved_ref: Default::default(),
             r#ref,
             allocator_data,
@@ -284,10 +260,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(),
         }

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<Arc<Rav1dContentLightLevel>>,
     pub(crate) mastering_display: Option<Arc<Rav1dMasteringDisplay>>,
-    pub(crate) itut_t35_ref: *mut Rav1dRef,
-    pub(crate) itut_t35: *mut Rav1dITUTT35,
+    pub(crate) itut_t35: Option<Arc<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>>,
 
     // decoded output picture queue
     pub(crate) in_0: Rav1dData,

src/lib.rs

@@ -16,11 +16,9 @@ use crate::include::dav1d::dav1d::RAV1D_DECODEFRAMETYPE_KEY;
 use crate::include::dav1d::dav1d::RAV1D_INLOOPFILTER_ALL;
 use crate::include::dav1d::headers::DRav1d;
 use crate::include::dav1d::headers::Dav1dFrameHeader;
-use crate::include::dav1d::headers::Dav1dITUTT35;
 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;
@@ -845,13 +843,7 @@ pub unsafe extern "C" fn dav1d_apply_grain(
                 .cast::<DRav1d<Rav1dFrameHeader, Dav1dFrameHeader>>())
             .update_rav1d();
         }
-        if let Some(mut itut_t35_ref) = NonNull::new(in_0.itut_t35_ref) {
-            (*itut_t35_ref
-                .as_mut()
-                .data
-                .cast::<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>())
-            .update_rav1d();
-        }
+        // 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);
@@ -886,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;
@@ -1093,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);

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::<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>();
-                        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]

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<Arc<Rav1dContentLightLevel>>,
     mastering_display: &Option<Arc<Rav1dMasteringDisplay>>,
-    itut_t35: *mut Rav1dITUTT35,
-    itut_t35_ref: *mut Rav1dRef,
+    itut_t35: &Option<Arc<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>>,
     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<Arc<Rav1dContentLightLevel>>,
     mastering_display: &Option<Arc<Rav1dMasteringDisplay>>,
-    itut_t35: *mut Rav1dITUTT35,
-    itut_t35_ref: *mut Rav1dRef,
+    itut_t35: &Option<Arc<DRav1d<Rav1dITUTT35, Dav1dITUTT35>>>,
     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) {