[FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step

[Niklas Haas]

On Sat, 23 Mar 2024 19:02:26 +0100 Andreas Rheinhardt <andreas.rheinhardt at outlook.com> wrote:
> Niklas Haas:
> > From: Niklas Haas <git at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".