On Sat, 23 Mar 2024 19:02:26 +0100 Andreas Rheinhardt
<andreas.rheinha...@outlook.com> wrote:
> Niklas Haas:
> > From: Niklas Haas <g...@haasn.dev>
> >
> > 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
