From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 95F044A1C7 for ; Sat, 23 Mar 2024 18:06:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 80E3D68D3BB; Sat, 23 Mar 2024 20:06:41 +0200 (EET) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5A0D968CC1C for ; Sat, 23 Mar 2024 20:06:35 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1711217195; bh=NyW841QUHmQvIyOcyM/w1C6JEksoe5at/OqIEjCYVTg=; h=Date:From:To:Subject:In-Reply-To:References:From; b=l/Tr4TwNrtoyDybhj8xC4ArV3FLKnK1x5+dTMwHkjCnZ7pJruz6ilLv+zaEdhBaR7 IWfWQByVwBvI7iaZiwTUZZ6oYbvU73JZaKP8xs7AHne9thhwcx+S+CyqKKtNdvTgAW A7ooY6J0OLNBE1EJJlM81071uzxZ3AEg68+Vtq78= Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id 1203C40015 for ; Sat, 23 Mar 2024 19:06:35 +0100 (CET) Date: Sat, 23 Mar 2024 19:06:34 +0100 Message-ID: <20240323190634.GB56135@haasn.xyz> From: Niklas Haas To: ffmpeg-devel@ffmpeg.org In-Reply-To: References: <20240323173735.26224-1-ffmpeg@haasn.xyz> <20240323173735.26224-3-ffmpeg@haasn.xyz> MIME-Version: 1.0 Content-Disposition: inline Subject: Re: [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, 23 Mar 2024 19:02:26 +0100 Andreas Rheinhardt wrote: > Niklas Haas: > > From: Niklas Haas > > > > This ensures that `gb` in the following section is fully byte-aligned, > > points at the start of the actual RPU, and ends on the CRC terminator. > > > > This is important for both calculation of the CRC, as well as dovi > > extension block parsing (which aligns to byte boundaries in various > > places). > > --- > > libavcodec/dovi_rpu.c | 48 +++++++++++++++++++++++++++++++++++-------- > > libavcodec/dovi_rpu.h | 2 ++ > > 2 files changed, 41 insertions(+), 9 deletions(-) > > > > diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c > > index c84a942f476..24a1cdf39a8 100644 > > --- a/libavcodec/dovi_rpu.c > > +++ b/libavcodec/dovi_rpu.c > > @@ -23,6 +23,7 @@ > > > > #include "libavutil/buffer.h" > > > > +#include "avcodec.h" > > What is that used for? av_fast_padded_malloc > > > #include "dovi_rpu.h" > > #include "golomb.h" > > #include "get_bits.h" > > @@ -45,6 +46,7 @@ void ff_dovi_ctx_unref(DOVIContext *s) > > { > > for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++) > > ff_refstruct_unref(&s->vdr[i]); > > + av_free(s->rpu_buf); > > > > *s = (DOVIContext) { > > .logctx = s->logctx, > > @@ -202,17 +204,17 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) > > DOVIVdr *vdr; > > int ret; > > > > - uint8_t nal_prefix; > > uint8_t rpu_type; > > uint8_t vdr_seq_info_present; > > uint8_t vdr_dm_metadata_present; > > uint8_t use_prev_vdr_rpu; > > uint8_t use_nlq; > > uint8_t profile; > > - if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0) > > - return ret; > > > > - /* Container header */ > > + if (rpu_size < 5) > > + goto fail; > > + > > + /* Container */ > > if (s->dv_profile == 10 /* dav1.10 */) { > > /* DV inside AV1 re-uses an EMDF container skeleton, but with fixed > > * values - so we can effectively treat this as a magic byte sequence. > > @@ -229,18 +231,46 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) > > * discard_unknown_payload : f(1) = 1 > > */ > > const unsigned header_magic = 0x01be6841u; > > - unsigned header, emdf_payload_size; > > - header = get_bits_long(gb, 27); > > - VALIDATE(header, header_magic, header_magic); > > + unsigned emdf_header, emdf_payload_size, emdf_protection; > > + if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0) > > + return ret; > > + emdf_header = get_bits_long(gb, 27); > > + VALIDATE(emdf_header, header_magic, header_magic); > > emdf_payload_size = get_variable_bits(gb, 8); > > VALIDATE(emdf_payload_size, 6, 512); > > if (emdf_payload_size * 8 > get_bits_left(gb)) > > return AVERROR_INVALIDDATA; > > + > > + /* The payload is not byte-aligned (off by *one* bit, curse Dolby), > > + * so copy into a fresh buffer to preserve byte alignment of the > > + * RPU struct */ > > + av_fast_padded_malloc(&s->rpu_buf, &s->rpu_buf_sz, emdf_payload_size); > > + if (!s->rpu_buf) > > + return AVERROR(ENOMEM); > > + for (int i = 0; i < emdf_payload_size; i++) > > + s->rpu_buf[i] = get_bits(gb, 8); > > + rpu = s->rpu_buf; > > + rpu_size = emdf_payload_size; > > + > > + /* Validate EMDF footer */ > > + emdf_protection = get_bits(gb, 5 + 12); > > + VALIDATE(emdf_protection, 0x400, 0x400); > > + > > + if ((ret = init_get_bits8(gb, s->rpu_buf, emdf_payload_size)) < 0) > > + return ret; > > } else { > > - nal_prefix = get_bits(gb, 8); > > - VALIDATE(nal_prefix, 25, 25); > > + /* NAL RBSP with prefix and trailing zeroes */ > > + VALIDATE(rpu[0], 25, 25); /* NAL prefix */ > > + rpu++; > > + rpu_size--; > > + /* Strip trailing padding bytes */ > > + while (rpu_size && rpu[rpu_size - 1] == 0) > > + rpu_size--; > > } > > > > + if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0) > > + return ret; > > + > > /* RPU header */ > > rpu_type = get_bits(gb, 6); > > if (rpu_type != 2) { > > diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h > > index 51c5fdbb879..506974a74bf 100644 > > --- a/libavcodec/dovi_rpu.h > > +++ b/libavcodec/dovi_rpu.h > > @@ -49,6 +49,8 @@ typedef struct DOVIContext { > > */ > > struct DOVIVdr *vdr[DOVI_MAX_DM_ID+1]; ///< RefStruct references > > uint8_t dv_profile; > > + uint8_t *rpu_buf; ///< temporary buffer > > + unsigned rpu_buf_sz; > > The order here introduces unnecessary padding. Fixed. > > > > > } DOVIContext; > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".