From a44dbc59dd42231ffc21690359e906a91a2d7059 Mon Sep 17 00:00:00 2001 From: James Almer Date: Tue, 18 Apr 2023 16:10:01 -0300 Subject: [PATCH 1/6] picture: allow storing an array of Dav1dITUTT35 entries Nothing in the spec prevents a Temporal Unit from having more than one Metadata OBU of type ITU-T T.35, so export them as an array instead of only exporting the last one we parse. This is backwards compatible with the previous implementation, as users unaware of this change can ignore the n_itut_t35 field and still access the first (or only) entry in the array as they have been doing until now. --- include/dav1d/picture.h | 9 +++++++-- meson.build | 2 +- src/internal.h | 1 + src/lib.c | 1 + src/obu.c | 44 ++++++++++++++++++++++++++++++----------- src/picture.c | 40 +++++++++++++++++++++++-------------- src/picture.h | 8 +++++++- 7 files changed, 75 insertions(+), 30 deletions(-) diff --git a/include/dav1d/picture.h b/include/dav1d/picture.h index 2eb0b62e3..04d46df93 100644 --- a/include/dav1d/picture.h +++ b/include/dav1d/picture.h @@ -78,11 +78,16 @@ typedef struct Dav1dPicture { */ Dav1dMasteringDisplay *mastering_display; /** - * ITU-T T.35 metadata as defined in section 5.8.2 and 6.7.2 + * Array of ITU-T T.35 metadata as defined in section 5.8.2 and 6.7.2 */ Dav1dITUTT35 *itut_t35; - uintptr_t reserved[4]; ///< reserved for future use + /** + * Number of ITU-T T35 metadata entries in the array + */ + size_t n_itut_t35; + + uintptr_t reserved[3]; ///< reserved for future use struct Dav1dRef *frame_hdr_ref; ///< Dav1dFrameHeader allocation origin struct Dav1dRef *seq_hdr_ref; ///< Dav1dSequenceHeader allocation origin diff --git a/meson.build b/meson.build index c55a94fd6..ca439f729 100644 --- a/meson.build +++ b/meson.build @@ -30,7 +30,7 @@ project('dav1d', ['c'], 'b_ndebug=if-release'], meson_version: '>= 0.49.0') -dav1d_soname_version = '6.8.0' +dav1d_soname_version = '6.9.0' dav1d_api_version_array = dav1d_soname_version.split('.') dav1d_api_version_major = dav1d_api_version_array[0] dav1d_api_version_minor = dav1d_api_version_array[1] diff --git a/src/internal.h b/src/internal.h index 7ade788d9..3bfb3dfc8 100644 --- a/src/internal.h +++ b/src/internal.h @@ -115,6 +115,7 @@ struct Dav1dContext { Dav1dMasteringDisplay *mastering_display; Dav1dRef *itut_t35_ref; Dav1dITUTT35 *itut_t35; + int n_itut_t35; // decoded output picture queue Dav1dData in; diff --git a/src/lib.c b/src/lib.c index 833862ffc..111074058 100644 --- a/src/lib.c +++ b/src/lib.c @@ -543,6 +543,7 @@ void dav1d_flush(Dav1dContext *const c) { c->mastering_display = NULL; c->content_light = NULL; c->itut_t35 = NULL; + c->n_itut_t35 = 0; dav1d_ref_dec(&c->mastering_display_ref); dav1d_ref_dec(&c->content_light_ref); dav1d_ref_dec(&c->itut_t35_ref); diff --git a/src/obu.c b/src/obu.c index 2afbd5246..e417860e3 100644 --- a/src/obu.c +++ b/src/obu.c @@ -1566,22 +1566,42 @@ int dav1d_parse_obus(Dav1dContext *const c, Dav1dData *const in) { break; } - Dav1dRef *ref = dav1d_ref_create(sizeof(Dav1dITUTT35) + payload_size * sizeof(uint8_t)); - if (!ref) return DAV1D_ERR(ENOMEM); - Dav1dITUTT35 *const itut_t35_metadata = ref->data; + if ((c->n_itut_t35 + 1) > INT_MAX / (int)sizeof(*c->itut_t35)) goto error; + struct Dav1dITUTT35 *itut_t35 = realloc(c->itut_t35, (c->n_itut_t35 + 1) * sizeof(*c->itut_t35)); + if (!itut_t35) goto error; + c->itut_t35 = itut_t35; + memset(c->itut_t35 + c->n_itut_t35, 0, sizeof(*c->itut_t35)); + + struct itut_t35_ctx_context *itut_t35_ctx; + if (!c->n_itut_t35) { + assert(!c->itut_t35_ref); + itut_t35_ctx = malloc(sizeof(struct itut_t35_ctx_context)); + if (!itut_t35_ctx) goto error; + c->itut_t35_ref = dav1d_ref_wrap((uint8_t *)c->itut_t35, dav1d_picture_free_itut_t35, + itut_t35_ctx); + if (!c->itut_t35_ref) { + free(itut_t35_ctx); + goto error; + } + } else { + assert(c->itut_t35_ref && atomic_load(&c->itut_t35_ref->ref_cnt) == 1); + itut_t35_ctx = c->itut_t35_ref->user_data; + c->itut_t35_ref->const_data = (uint8_t *)c->itut_t35; + } + itut_t35_ctx->itut_t35 = c->itut_t35; + itut_t35_ctx->n_itut_t35 = c->n_itut_t35 + 1; + + Dav1dITUTT35 *const itut_t35_metadata = &c->itut_t35[c->n_itut_t35]; + itut_t35_metadata->payload = malloc(payload_size); + if (!itut_t35_metadata->payload) goto error; - // We need our public headers to be C++ compatible, so payload can't be - // a flexible array member - itut_t35_metadata->payload = (uint8_t *) &itut_t35_metadata[1]; itut_t35_metadata->country_code = country_code; itut_t35_metadata->country_code_extension_byte = country_code_extension_byte; for (int i = 0; i < payload_size; i++) itut_t35_metadata->payload[i] = dav1d_get_bits(&gb, 8); itut_t35_metadata->payload_size = payload_size; - dav1d_ref_dec(&c->itut_t35_ref); - c->itut_t35 = itut_t35_metadata; - c->itut_t35_ref = ref; + c->n_itut_t35++; break; } case OBU_META_SCALABILITY: @@ -1636,11 +1656,12 @@ int dav1d_parse_obus(Dav1dContext *const c, Dav1dData *const in) { dav1d_picture_copy_props(&c->out.p, c->content_light, c->content_light_ref, c->mastering_display, c->mastering_display_ref, - c->itut_t35, c->itut_t35_ref, + c->itut_t35, c->itut_t35_ref, c->n_itut_t35, &in->m); // Must be removed from the context after being attached to the frame dav1d_ref_dec(&c->itut_t35_ref); c->itut_t35 = NULL; + c->n_itut_t35 = 0; c->event_flags |= dav1d_picture_get_event_flags(&c->refs[c->frame_hdr->existing_frame_idx].p); } else { pthread_mutex_lock(&c->task_thread.lock); @@ -1689,11 +1710,12 @@ int dav1d_parse_obus(Dav1dContext *const c, Dav1dData *const in) { dav1d_picture_copy_props(&out_delayed->p, c->content_light, c->content_light_ref, c->mastering_display, c->mastering_display_ref, - c->itut_t35, c->itut_t35_ref, + c->itut_t35, c->itut_t35_ref, c->n_itut_t35, &in->m); // Must be removed from the context after being attached to the frame dav1d_ref_dec(&c->itut_t35_ref); c->itut_t35 = NULL; + c->n_itut_t35 = 0; pthread_mutex_unlock(&c->task_thread.lock); } diff --git a/src/picture.c b/src/picture.c index e6ffb931b..5309c9f19 100644 --- a/src/picture.c +++ b/src/picture.c @@ -100,14 +100,20 @@ static void free_buffer(const uint8_t *const data, void *const user_data) { free(pic_ctx); } +void dav1d_picture_free_itut_t35(const uint8_t *const data, void *const user_data) { + struct itut_t35_ctx_context *itut_t35_ctx = user_data; + + for (size_t i = 0; i < itut_t35_ctx->n_itut_t35; i++) + free(itut_t35_ctx->itut_t35[i].payload); + free(itut_t35_ctx->itut_t35); + free(itut_t35_ctx); +} + static int picture_alloc_with_edges(Dav1dContext *const c, Dav1dPicture *const p, const int w, const int h, Dav1dSequenceHeader *const seq_hdr, Dav1dRef *const seq_hdr_ref, Dav1dFrameHeader *const frame_hdr, Dav1dRef *const frame_hdr_ref, - Dav1dContentLightLevel *const content_light, Dav1dRef *const content_light_ref, - Dav1dMasteringDisplay *const mastering_display, Dav1dRef *const mastering_display_ref, - Dav1dITUTT35 *const itut_t35, Dav1dRef *const itut_t35_ref, const int bpc, const Dav1dDataProps *const props, Dav1dPicAllocator *const p_allocator, @@ -152,10 +158,6 @@ static int picture_alloc_with_edges(Dav1dContext *const c, p->frame_hdr_ref = frame_hdr_ref; if (frame_hdr_ref) dav1d_ref_inc(frame_hdr_ref); - dav1d_picture_copy_props(p, content_light, content_light_ref, - mastering_display, mastering_display_ref, - itut_t35, itut_t35_ref, props); - if (extra && extra_ptr) *extra_ptr = &pic_ctx->extra_ptr; @@ -165,7 +167,7 @@ static int picture_alloc_with_edges(Dav1dContext *const c, void dav1d_picture_copy_props(Dav1dPicture *const p, Dav1dContentLightLevel *const content_light, Dav1dRef *const content_light_ref, Dav1dMasteringDisplay *const mastering_display, Dav1dRef *const mastering_display_ref, - Dav1dITUTT35 *const itut_t35, Dav1dRef *const itut_t35_ref, + Dav1dITUTT35 *const itut_t35, Dav1dRef *itut_t35_ref, size_t n_itut_t35, const Dav1dDataProps *const props) { dav1d_data_props_copy(&p->m, props); @@ -183,6 +185,7 @@ void dav1d_picture_copy_props(Dav1dPicture *const p, dav1d_ref_dec(&p->itut_t35_ref); p->itut_t35_ref = itut_t35_ref; p->itut_t35 = itut_t35; + p->n_itut_t35 = n_itut_t35; if (itut_t35_ref) dav1d_ref_inc(itut_t35_ref); } @@ -196,17 +199,20 @@ int dav1d_thread_picture_alloc(Dav1dContext *const c, Dav1dFrameContext *const f picture_alloc_with_edges(c, &p->p, f->frame_hdr->width[1], f->frame_hdr->height, f->seq_hdr, f->seq_hdr_ref, f->frame_hdr, f->frame_hdr_ref, - c->content_light, c->content_light_ref, - c->mastering_display, c->mastering_display_ref, - c->itut_t35, c->itut_t35_ref, bpc, &f->tile[0].data.m, &c->allocator, have_frame_mt ? sizeof(atomic_int) * 2 : 0, (void **) &p->progress); if (res) return res; + dav1d_picture_copy_props(&p->p, c->content_light, c->content_light_ref, + c->mastering_display, c->mastering_display_ref, + c->itut_t35, c->itut_t35_ref, c->n_itut_t35, + &f->tile[0].data.m); + // Must be removed from the context after being attached to the frame dav1d_ref_dec(&c->itut_t35_ref); c->itut_t35 = NULL; + c->n_itut_t35 = 0; // Don't clear these flags from c->frame_flags if the frame is not visible. // This way they will be added to the next visible frame too. @@ -231,12 +237,16 @@ int dav1d_picture_alloc_copy(Dav1dContext *const c, Dav1dPicture *const dst, con const int res = picture_alloc_with_edges(c, dst, w, src->p.h, src->seq_hdr, src->seq_hdr_ref, src->frame_hdr, src->frame_hdr_ref, - src->content_light, src->content_light_ref, - src->mastering_display, src->mastering_display_ref, - src->itut_t35, src->itut_t35_ref, src->p.bpc, &src->m, &pic_ctx->allocator, 0, NULL); - return res; + if (res) return res; + + dav1d_picture_copy_props(dst, src->content_light, src->content_light_ref, + src->mastering_display, src->mastering_display_ref, + src->itut_t35, src->itut_t35_ref, src->n_itut_t35, + &src->m); + + return 0; } void dav1d_picture_ref(Dav1dPicture *const dst, const Dav1dPicture *const src) { diff --git a/src/picture.h b/src/picture.h index 0c3a0ec56..02f3664c7 100644 --- a/src/picture.h +++ b/src/picture.h @@ -101,10 +101,16 @@ int dav1d_default_picture_alloc(Dav1dPicture *p, void *cookie); void dav1d_default_picture_release(Dav1dPicture *p, void *cookie); void dav1d_picture_unref_internal(Dav1dPicture *p); +struct itut_t35_ctx_context { + Dav1dITUTT35 *itut_t35; + size_t n_itut_t35; +}; + +void dav1d_picture_free_itut_t35(const uint8_t *data, void *user_data); void dav1d_picture_copy_props(Dav1dPicture *p, Dav1dContentLightLevel *content_light, Dav1dRef *content_light_ref, Dav1dMasteringDisplay *mastering_display, Dav1dRef *mastering_display_ref, - Dav1dITUTT35 *itut_t35, Dav1dRef *itut_t35_ref, + Dav1dITUTT35 *itut_t35, Dav1dRef *itut_t35_ref, size_t n_itut_t35, const Dav1dDataProps *props); /** From 23049af727c08325307d28ac8710af4ad8b53eaa Mon Sep 17 00:00:00 2001 From: Frank Bossen Date: Sun, 15 Oct 2023 03:58:05 -0400 Subject: [PATCH 2/6] Replace `goto error` by `return dav1d_parse_obus_error(c, in)` --- src/obu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/obu.c b/src/obu.c index e417860e3..cbb500303 100644 --- a/src/obu.c +++ b/src/obu.c @@ -1566,9 +1566,9 @@ int dav1d_parse_obus(Dav1dContext *const c, Dav1dData *const in) { break; } - if ((c->n_itut_t35 + 1) > INT_MAX / (int)sizeof(*c->itut_t35)) goto error; + if ((c->n_itut_t35 + 1) > INT_MAX / (int)sizeof(*c->itut_t35)) return dav1d_parse_obus_error(c, in); struct Dav1dITUTT35 *itut_t35 = realloc(c->itut_t35, (c->n_itut_t35 + 1) * sizeof(*c->itut_t35)); - if (!itut_t35) goto error; + if (!itut_t35) return dav1d_parse_obus_error(c, in); c->itut_t35 = itut_t35; memset(c->itut_t35 + c->n_itut_t35, 0, sizeof(*c->itut_t35)); @@ -1576,12 +1576,12 @@ int dav1d_parse_obus(Dav1dContext *const c, Dav1dData *const in) { if (!c->n_itut_t35) { assert(!c->itut_t35_ref); itut_t35_ctx = malloc(sizeof(struct itut_t35_ctx_context)); - if (!itut_t35_ctx) goto error; + if (!itut_t35_ctx) return dav1d_parse_obus_error(c, in); c->itut_t35_ref = dav1d_ref_wrap((uint8_t *)c->itut_t35, dav1d_picture_free_itut_t35, itut_t35_ctx); if (!c->itut_t35_ref) { free(itut_t35_ctx); - goto error; + return dav1d_parse_obus_error(c, in); } } else { assert(c->itut_t35_ref && atomic_load(&c->itut_t35_ref->ref_cnt) == 1); @@ -1593,7 +1593,7 @@ int dav1d_parse_obus(Dav1dContext *const c, Dav1dData *const in) { Dav1dITUTT35 *const itut_t35_metadata = &c->itut_t35[c->n_itut_t35]; itut_t35_metadata->payload = malloc(payload_size); - if (!itut_t35_metadata->payload) goto error; + if (!itut_t35_metadata->payload) return dav1d_parse_obus_error(c, in); itut_t35_metadata->country_code = country_code; itut_t35_metadata->country_code_extension_byte = country_code_extension_byte; From 221ab8d3b539a6d0962731b925d2e4d800ce0073 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 19 Mar 2024 18:05:44 -0700 Subject: [PATCH 3/6] `struct Rav1dContext::itut_t35`: Replace `Option::take`s with the more generic and equivalent `mem::take`. --- src/decode.rs | 2 +- src/obu.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/decode.rs b/src/decode.rs index a6ce08f48..f82799afd 100644 --- a/src/decode.rs +++ b/src/decode.rs @@ -4798,7 +4798,7 @@ pub unsafe fn rav1d_submit_frame(c: &mut Rav1dContext) -> Rav1dResult { // We must take itut_t35 out of the context before the call so borrowck can // see we mutably borrow `c.itut_t35` disjointly from the task thread lock. - let itut_t35 = c.itut_t35.take(); + let itut_t35 = mem::take(&mut c.itut_t35); let res = rav1d_thread_picture_alloc(c, f, bpc, itut_t35); if res.is_err() { on_error(f, c, out); diff --git a/src/obu.rs b/src/obu.rs index 18def8e5b..9cccc7a27 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -2529,7 +2529,7 @@ unsafe fn parse_obus( c.content_light.clone(), c.mastering_display.clone(), // Must be moved from the context to the frame. - c.itut_t35.take(), + mem::take(&mut c.itut_t35), props.clone(), ); c.event_flags |= c.refs[frame_hdr.existing_frame_idx as usize].p.flags.into(); @@ -2593,7 +2593,7 @@ unsafe fn parse_obus( c.content_light.clone(), c.mastering_display.clone(), // Must be moved from the context to the frame. - c.itut_t35.take(), + mem::take(&mut c.itut_t35), props.clone(), ); } From 185aa41f084ba111bafa5fb17f115a60626c4337 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 23 Apr 2024 12:03:38 -0700 Subject: [PATCH 4/6] `struct Rav1d{Context,Picture}::itut_t35`: Port C code to Rust, making it an `Arc, Vec<_>>>>`. Due to the now shared access to the `itut_t35` `Vec` to push new ones during parsing, I added a zero-contention `Mutex` around it. This also adds a lot of explicit initialization that was previously just zero initialized, since the `Arc` is not all zeros. Co-authored-by: Frank Bossen --- include/dav1d/headers.rs | 23 +++++++++++++++++++++++ include/dav1d/picture.rs | 32 ++++++++++++++++++++++++++------ src/internal.rs | 3 ++- src/lib.rs | 9 ++++++++- src/obu.rs | 6 +++--- src/picture.rs | 34 ++++++++++++++++++++-------------- 6 files changed, 82 insertions(+), 25 deletions(-) diff --git a/include/dav1d/headers.rs b/include/dav1d/headers.rs index c11873bd1..9ad7b0577 100644 --- a/include/dav1d/headers.rs +++ b/include/dav1d/headers.rs @@ -42,6 +42,29 @@ impl Deref for DRav1d { } } +impl Default for DRav1d +where + R: Default, + D: Default, +{ + fn default() -> Self { + Self { + rav1d: Default::default(), + dav1d: Default::default(), + } + } +} + +impl DRav1d, Vec> +where + R: Clone + Into, +{ + pub fn push(&mut self, value: R) { + self.rav1d.push(value.clone()); + self.dav1d.push(value.into()); + } +} + // Constants from Section 3. "Symbols and abbreviated terms" pub const DAV1D_MAX_CDEF_STRENGTHS: usize = 8; pub const DAV1D_MAX_OPERATING_POINTS: usize = 32; diff --git a/include/dav1d/picture.rs b/include/dav1d/picture.rs index 91e0e8cc3..de0446f95 100644 --- a/include/dav1d/picture.rs +++ b/include/dav1d/picture.rs @@ -25,6 +25,7 @@ use std::ffi::c_void; use std::ptr; use std::ptr::NonNull; use std::sync::Arc; +use std::sync::Mutex; pub(crate) const RAV1D_PICTURE_ALIGNMENT: usize = 64; pub const DAV1D_PICTURE_ALIGNMENT: usize = RAV1D_PICTURE_ALIGNMENT; @@ -84,12 +85,13 @@ pub struct Dav1dPicture { pub content_light: Option>, pub mastering_display: Option>, pub itut_t35: Option>, - pub reserved: [uintptr_t; 4], + pub n_itut_t35: usize, + pub reserved: [uintptr_t; 3], pub frame_hdr_ref: Option>>, // opaque, so we can change this pub seq_hdr_ref: Option>>, // opaque, so we can change this 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: Option>>, // opaque, so we can change this + pub itut_t35_ref: Option, Vec>>>>, // opaque, so we can change this pub reserved_ref: [uintptr_t; 4], pub r#ref: Option>, pub allocator_data: Option>, @@ -126,7 +128,14 @@ pub(crate) struct Rav1dPicture { pub m: Rav1dDataProps, pub content_light: Option>, pub mastering_display: Option>, - pub itut_t35: Option>>, + + /// [`Option`] wasn't needed here since [`Vec`]`: `[`Default`], + /// but this does necessitate an allocation of `Vec::default()` every time, + /// and an [`Arc::clone`] on every clone, + /// even though having a [`Rav1dITUTT35`] is fairly rare. + /// This is a small cost compared to pixel data, however, + /// so until we notice a performance impact, this simpler way should suffice. + pub itut_t35: Arc, Vec>>>, pub r#ref: Option>, } @@ -142,6 +151,7 @@ impl From for Rav1dPicture { content_light: _, mastering_display: _, itut_t35: _, + n_itut_t35: _, reserved: _, frame_hdr_ref, seq_hdr_ref, @@ -172,7 +182,9 @@ impl From for Rav1dPicture { mastering_display: mastering_display_ref.map(|raw| unsafe { raw.into_arc() }), // 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() }), + itut_t35: itut_t35_ref + .map(|raw| unsafe { raw.into_arc() }) + .unwrap_or_default(), r#ref, } } @@ -196,6 +208,13 @@ impl From for Dav1dPicture { itut_t35, r#ref, } = value; + let (itut_t35_dav1d, n_itut_t35) = { + let itut_t35 = &*itut_t35.try_lock().unwrap(); + let itut_t35_dav1d = Some(NonNull::new(itut_t35.dav1d.as_ptr().cast_mut()).unwrap()); + let n_itut_t35 = itut_t35.len(); + (itut_t35_dav1d, n_itut_t35) + }; + Self { // [`DRav1d::from_rav1d`] is called right after [`parse_seq_hdr`]. seq_hdr: seq_hdr.as_ref().map(|arc| (&arc.as_ref().dav1d).into()), @@ -208,13 +227,14 @@ 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: itut_t35.as_ref().map(|arc| (&arc.as_ref().dav1d).into()), + itut_t35: itut_t35_dav1d, + n_itut_t35, reserved: Default::default(), frame_hdr_ref: frame_hdr.map(RawArc::from_arc), seq_hdr_ref: seq_hdr.map(RawArc::from_arc), content_light_ref: content_light.map(RawArc::from_arc), mastering_display_ref: mastering_display.map(RawArc::from_arc), - itut_t35_ref: itut_t35.map(RawArc::from_arc), + itut_t35_ref: Some(itut_t35).map(RawArc::from_arc), reserved_ref: Default::default(), r#ref, allocator_data, diff --git a/src/internal.rs b/src/internal.rs index 4d01fec23..812ff41ad 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -272,6 +272,7 @@ pub(crate) struct TaskThreadData { pub delayed_fg: Mutex, } +#[derive(Default)] #[repr(C)] pub(crate) struct Rav1dContext_refs { pub p: Rav1dThreadPicture, @@ -324,7 +325,7 @@ pub struct Rav1dContext { pub(crate) frame_hdr: Option>>, // TODO(kkysen) Previously pooled. pub(crate) content_light: Option>, pub(crate) mastering_display: Option>, - pub(crate) itut_t35: Option>>, + pub(crate) itut_t35: Arc, Vec>>>, // decoded output picture queue pub(crate) in_0: Rav1dData, diff --git a/src/lib.rs b/src/lib.rs index 288572ad5..89350ebde 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -101,7 +101,7 @@ pub unsafe extern "C" fn dav1d_version() -> *const c_char { } pub const DAV1D_API_VERSION_MAJOR: u8 = 6; -pub const DAV1D_API_VERSION_MINOR: u8 = 8; +pub const DAV1D_API_VERSION_MINOR: u8 = 9; pub const DAV1D_API_VERSION_PATCH: u8 = 0; /// Get the `dav1d` library C API version. @@ -277,6 +277,10 @@ pub(crate) unsafe fn rav1d_open(c_out: &mut *mut Rav1dContext, s: &Rav1dSettings } else { Box::new([]) }); + addr_of_mut!((*c).itut_t35).write(Arc::new(Mutex::new(Default::default()))); + addr_of_mut!((*c).out).write(Default::default()); + addr_of_mut!((*c).cache).write(Default::default()); + addr_of_mut!((*c).refs).write(Default::default()); for n in 0..n_fc { let f: &mut Rav1dFrameData = &mut *((*c).fc).offset(n as isize); f.index = n; @@ -297,6 +301,9 @@ pub(crate) unsafe fn rav1d_open(c_out: &mut *mut Rav1dContext, s: &Rav1dSettings (&mut f.task_thread.ttd as *mut Arc).write(Arc::clone(&(*c).task_thread)); f.lf.last_sharpness = -(1 as c_int); rav1d_refmvs_init(&mut f.rf); + addr_of_mut!(f.refp).write(Default::default()); + addr_of_mut!(f.cur).write(Default::default()); + addr_of_mut!(f.sr_cur).write(Default::default()); } (*c).tc = (0..n_tc) .map(|n| { diff --git a/src/obu.rs b/src/obu.rs index 9cccc7a27..cd7955b5c 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -2463,12 +2463,12 @@ unsafe fn parse_obus( let country_code = country_code as u8; 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 - - c.itut_t35 = Some(Arc::new(DRav1d::from_rav1d(Rav1dITUTT35 { + let itut_t35 = Rav1dITUTT35 { country_code, country_code_extension_byte, payload, - }))); // TODO(kkysen) fallible allocation + }; + c.itut_t35.try_lock().unwrap().push(itut_t35); // TODO fallible allocation } } Some(ObuMetaType::Scalability | ObuMetaType::Timecode) => {} // Ignore metadata OBUs we don't care about. diff --git a/src/picture.rs b/src/picture.rs index e221b256c..904380df3 100644 --- a/src/picture.rs +++ b/src/picture.rs @@ -45,6 +45,7 @@ use std::slice; use std::sync::atomic::AtomicU32; use std::sync::atomic::Ordering; use std::sync::Arc; +use std::sync::Mutex; use to_method::To as _; #[derive(Clone, Copy, PartialEq, Eq, Hash, Default, Atom, AtomLogic)] @@ -231,11 +232,7 @@ unsafe fn picture_alloc_with_edges( h: c_int, seq_hdr: Option>>, frame_hdr: Option>>, - content_light: Option>, - mastering_display: Option>, - itut_t35: Option>>, bpc: c_int, - props: Rav1dDataProps, p_allocator: &Rav1dPicAllocator, ) -> Rav1dResult { if !p.data.data[0].is_null() { @@ -243,8 +240,7 @@ unsafe fn picture_alloc_with_edges( return Err(EGeneric); } assert!(bpc > 0 && bpc <= 16); - let mut pic = p_allocator.alloc_picture_data(w, h, seq_hdr.unwrap(), frame_hdr)?; - rav1d_picture_copy_props(&mut pic, content_light, mastering_display, itut_t35, props); + let pic = p_allocator.alloc_picture_data(w, h, seq_hdr.unwrap(), frame_hdr)?; *p = pic; Ok(()) @@ -254,7 +250,7 @@ pub fn rav1d_picture_copy_props( p: &mut Rav1dPicture, content_light: Option>, mastering_display: Option>, - itut_t35: Option>>, + itut_t35: Arc, Vec>>>, props: Rav1dDataProps, ) { p.m = props; @@ -269,7 +265,7 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc( c: &Rav1dContext, f: &mut Rav1dFrameData, bpc: c_int, - itut_t35: Option>>, + itut_t35: Arc, Vec>>>, ) -> Rav1dResult { let p = &mut f.sr_cur; let have_frame_mt = c.n_fc > 1; @@ -281,13 +277,18 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc( frame_hdr.size.height, f.seq_hdr.clone(), f.frame_hdr.clone(), + bpc, + &c.allocator, + )?; + + rav1d_picture_copy_props( + &mut p.p, c.content_light.clone(), c.mastering_display.clone(), itut_t35, - bpc, f.tiles[0].data.m.clone(), - &c.allocator, - )?; + ); + let flags_mask = if frame_hdr.show_frame != 0 || c.output_invisible_frames { PictureFlags::empty() } else { @@ -319,13 +320,18 @@ pub(crate) unsafe fn rav1d_picture_alloc_copy( src.p.h, src.seq_hdr.clone(), src.frame_hdr.clone(), + src.p.bpc, + &mut (*pic_ctx).allocator, + )?; + + rav1d_picture_copy_props( + dst, src.content_light.clone(), src.mastering_display.clone(), src.itut_t35.clone(), - src.p.bpc, src.m.clone(), - &mut (*pic_ctx).allocator, - ) + ); + Ok(()) } pub(crate) unsafe fn rav1d_picture_ref(dst: &mut Rav1dPicture, src: &Rav1dPicture) { From 72cde019085596da1a232924ae68a5dc51a8a6fc Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Mon, 22 Apr 2024 15:56:01 -0700 Subject: [PATCH 5/6] `struct Rav1dPicture::itut_t35`: Remove `Mutex` and make `Vec`s into `Box<[_]>`s since the mutation only happens on `Rav1dContext::itut_t35`. --- include/dav1d/headers.rs | 13 +++++++++++++ include/dav1d/picture.rs | 23 ++++------------------- src/obu.rs | 4 ++-- src/picture.rs | 4 ++-- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/include/dav1d/headers.rs b/include/dav1d/headers.rs index 9ad7b0577..4055b131d 100644 --- a/include/dav1d/headers.rs +++ b/include/dav1d/headers.rs @@ -10,6 +10,8 @@ use std::ops::Deref; use std::ops::Sub; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; +use std::sync::Arc; +use std::sync::Mutex; use strum::EnumCount; use strum::FromRepr; @@ -840,6 +842,17 @@ impl From for Dav1dITUTT35 { } } +impl Rav1dITUTT35 { + pub fn to_immut( + mutable: Arc, Vec>>>, + ) -> Arc, Box<[Dav1dITUTT35]>>> { + let DRav1d { rav1d, dav1d: _ } = Arc::into_inner(mutable).unwrap().into_inner().unwrap(); + let rav1d = rav1d.into_boxed_slice(); + let dav1d = rav1d.iter().cloned().map(Dav1dITUTT35::from).collect(); + Arc::new(DRav1d { rav1d, dav1d }) + } +} + #[derive(Clone, Copy)] #[repr(C)] pub struct Dav1dSequenceHeaderOperatingPoint { diff --git a/include/dav1d/picture.rs b/include/dav1d/picture.rs index de0446f95..0acc9b448 100644 --- a/include/dav1d/picture.rs +++ b/include/dav1d/picture.rs @@ -25,7 +25,6 @@ use std::ffi::c_void; use std::ptr; use std::ptr::NonNull; use std::sync::Arc; -use std::sync::Mutex; pub(crate) const RAV1D_PICTURE_ALIGNMENT: usize = 64; pub const DAV1D_PICTURE_ALIGNMENT: usize = RAV1D_PICTURE_ALIGNMENT; @@ -91,7 +90,7 @@ pub struct Dav1dPicture { pub seq_hdr_ref: Option>>, // opaque, so we can change this 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: Option, Vec>>>>, // opaque, so we can change this + pub itut_t35_ref: Option, Box<[Dav1dITUTT35]>>>>, // opaque, so we can change this pub reserved_ref: [uintptr_t; 4], pub r#ref: Option>, pub allocator_data: Option>, @@ -128,14 +127,7 @@ pub(crate) struct Rav1dPicture { pub m: Rav1dDataProps, pub content_light: Option>, pub mastering_display: Option>, - - /// [`Option`] wasn't needed here since [`Vec`]`: `[`Default`], - /// but this does necessitate an allocation of `Vec::default()` every time, - /// and an [`Arc::clone`] on every clone, - /// even though having a [`Rav1dITUTT35`] is fairly rare. - /// This is a small cost compared to pixel data, however, - /// so until we notice a performance impact, this simpler way should suffice. - pub itut_t35: Arc, Vec>>>, + pub itut_t35: Arc, Box<[Dav1dITUTT35]>>>, pub r#ref: Option>, } @@ -208,13 +200,6 @@ impl From for Dav1dPicture { itut_t35, r#ref, } = value; - let (itut_t35_dav1d, n_itut_t35) = { - let itut_t35 = &*itut_t35.try_lock().unwrap(); - let itut_t35_dav1d = Some(NonNull::new(itut_t35.dav1d.as_ptr().cast_mut()).unwrap()); - let n_itut_t35 = itut_t35.len(); - (itut_t35_dav1d, n_itut_t35) - }; - Self { // [`DRav1d::from_rav1d`] is called right after [`parse_seq_hdr`]. seq_hdr: seq_hdr.as_ref().map(|arc| (&arc.as_ref().dav1d).into()), @@ -227,8 +212,8 @@ 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: itut_t35_dav1d, - n_itut_t35, + itut_t35: Some(NonNull::new(itut_t35.dav1d.as_ptr().cast_mut()).unwrap()), + n_itut_t35: itut_t35.len(), reserved: Default::default(), frame_hdr_ref: frame_hdr.map(RawArc::from_arc), seq_hdr_ref: seq_hdr.map(RawArc::from_arc), diff --git a/src/obu.rs b/src/obu.rs index cd7955b5c..b4c3bf55d 100644 --- a/src/obu.rs +++ b/src/obu.rs @@ -2529,7 +2529,7 @@ unsafe fn parse_obus( c.content_light.clone(), c.mastering_display.clone(), // Must be moved from the context to the frame. - mem::take(&mut c.itut_t35), + Rav1dITUTT35::to_immut(mem::take(&mut c.itut_t35)), props.clone(), ); c.event_flags |= c.refs[frame_hdr.existing_frame_idx as usize].p.flags.into(); @@ -2593,7 +2593,7 @@ unsafe fn parse_obus( c.content_light.clone(), c.mastering_display.clone(), // Must be moved from the context to the frame. - mem::take(&mut c.itut_t35), + Rav1dITUTT35::to_immut(mem::take(&mut c.itut_t35)), props.clone(), ); } diff --git a/src/picture.rs b/src/picture.rs index 904380df3..3f92dff87 100644 --- a/src/picture.rs +++ b/src/picture.rs @@ -250,7 +250,7 @@ pub fn rav1d_picture_copy_props( p: &mut Rav1dPicture, content_light: Option>, mastering_display: Option>, - itut_t35: Arc, Vec>>>, + itut_t35: Arc, Box<[Dav1dITUTT35]>>>, props: Rav1dDataProps, ) { p.m = props; @@ -285,7 +285,7 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc( &mut p.p, c.content_light.clone(), c.mastering_display.clone(), - itut_t35, + Rav1dITUTT35::to_immut(itut_t35), f.tiles[0].data.m.clone(), ); From 1c24cee7815d069cd6c303e1e2355dc39143de46 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Mon, 22 Apr 2024 15:57:45 -0700 Subject: [PATCH 6/6] `struct Rav1dContext::itut_t35`: Remove `DRav1d`, as only `Rav1dPicture::itut_t35` needs it. --- include/dav1d/headers.rs | 17 ++++------------- src/internal.rs | 3 +-- src/picture.rs | 2 +- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/include/dav1d/headers.rs b/include/dav1d/headers.rs index 4055b131d..3762a0d80 100644 --- a/include/dav1d/headers.rs +++ b/include/dav1d/headers.rs @@ -57,16 +57,6 @@ where } } -impl DRav1d, Vec> -where - R: Clone + Into, -{ - pub fn push(&mut self, value: R) { - self.rav1d.push(value.clone()); - self.dav1d.push(value.into()); - } -} - // Constants from Section 3. "Symbols and abbreviated terms" pub const DAV1D_MAX_CDEF_STRENGTHS: usize = 8; pub const DAV1D_MAX_OPERATING_POINTS: usize = 32; @@ -844,10 +834,11 @@ impl From for Dav1dITUTT35 { impl Rav1dITUTT35 { pub fn to_immut( - mutable: Arc, Vec>>>, + mutable: Arc>>, ) -> Arc, Box<[Dav1dITUTT35]>>> { - let DRav1d { rav1d, dav1d: _ } = Arc::into_inner(mutable).unwrap().into_inner().unwrap(); - let rav1d = rav1d.into_boxed_slice(); + let mutable = Arc::into_inner(mutable).unwrap().into_inner().unwrap(); + let immutable = mutable.into_boxed_slice(); + let rav1d = immutable; let dav1d = rav1d.iter().cloned().map(Dav1dITUTT35::from).collect(); Arc::new(DRav1d { rav1d, dav1d }) } diff --git a/src/internal.rs b/src/internal.rs index 812ff41ad..bc9d05b24 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -12,7 +12,6 @@ use crate::include::dav1d::dav1d::Rav1dEventFlags; use crate::include::dav1d::dav1d::Rav1dInloopFilterType; 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::Rav1dContentLightLevel; use crate::include::dav1d::headers::Rav1dFrameHeader; @@ -325,7 +324,7 @@ pub struct Rav1dContext { pub(crate) frame_hdr: Option>>, // TODO(kkysen) Previously pooled. pub(crate) content_light: Option>, pub(crate) mastering_display: Option>, - pub(crate) itut_t35: Arc, Vec>>>, + pub(crate) itut_t35: Arc>>, // decoded output picture queue pub(crate) in_0: Rav1dData, diff --git a/src/picture.rs b/src/picture.rs index 3f92dff87..36d56007a 100644 --- a/src/picture.rs +++ b/src/picture.rs @@ -265,7 +265,7 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc( c: &Rav1dContext, f: &mut Rav1dFrameData, bpc: c_int, - itut_t35: Arc, Vec>>>, + itut_t35: Arc>>, ) -> Rav1dResult { let p = &mut f.sr_cur; let have_frame_mt = c.n_fc > 1;