[FFmpeg-devel] [PATCH] libavcodec/hevc_mp4toannexb_bsf: update the extradata in codec par if change detected

[Linjie Fu]

Andreas:
On Sun, Dec 5, 2021 at 7:34 PM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:

> Linjie Fu:
> > From: Linjie Fu <linjie.justin.fu at gmail.com>
> >
> > Container may support multiple sample descriptions in a single
> > bitstream, like multiple stsd in mov, which introduces different
> > sequence header(e.g.profile/bit_depth) in the middle of the bitstream.
> >
> > Update the extradata field in context parameter once packet with
> > different extradata is got.
> >
> > Signed-off-by: Linjie Fu <linjie.justin.fu at gmail.com>
> > ---
> >  libavcodec/hevc_mp4toannexb_bsf.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> b/libavcodec/hevc_mp4toannexb_bsf.c
> > index 790dfb0394..36a83d0c95 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -125,6 +125,9 @@ static int hevc_mp4toannexb_filter(AVBSFContext
> *ctx, AVPacket *out)
> >      int got_irap = 0;
> >      int i, ret = 0;
> >
> > +    uint8_t *extradata;
> > +    size_t extradata_size = 0;
> > +
> >      ret = ff_bsf_get_packet(ctx, &in);
> >      if (ret < 0)
> >          return ret;
> > @@ -135,6 +138,26 @@ static int hevc_mp4toannexb_filter(AVBSFContext
> *ctx, AVPacket *out)
> >          return 0;
> >      }
> >
> > +    extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA,
> &extradata_size);
> > +    if (extradata && extradata_size &&
> > +        (ctx->par_in->extradata_size != extradata_size ||
> > +         memcmp(ctx->par_in->extradata, extradata, extradata_size))) {
> > +        av_log(ctx, AV_LOG_VERBOSE, "Update extradata, size = %zu\n",
> extradata_size);
>
> We use SIZE_SPECIFIER instead of zu due to MSVC compatibility. I don't
> know if we actually support MSVC versions that old any more, but it is
> still used.
>
> +        /* Update extradata */
> > +        av_freep(&ctx->par_in->extradata);
> > +        ctx->par_in->extradata_size = extradata_size;
> > +        ctx->par_in->extradata      = av_mallocz(extradata_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> > +        if (!ctx->par_in->extradata)
> > +            return AVERROR(ENOMEM);
> > +        memcpy(ctx->par_in->extradata, extradata, extradata_size);
> > +        /* Reinit */
> > +        ret = hevc_extradata_to_annexb(ctx);
> > +        if (ret < 0)
> > +            return ret;
> > +        s->length_size      = ret;
> > +        s->extradata_parsed = 1;
> > +    }
> > +
> >      bytestream2_init(&gb, in->data, in->size);
> >
> >      while (bytestream2_get_bytes_left(&gb)) {
> >
>
> 1. This is an API violation: par_in is set by the user before
> initializing the BSF, par_out is set by the BSF in init. After init,
> both are immutable.
> 2. Instead you should make hevc_extradata_to_annexb() accept a buffer
> with length field (or maybe even factor everything that
> hevc_mp4toannexb_init() does out of it and make that function accept a
> buffer and a length field; and another field to return the reformatted
> extradata and its size).
> 3. Notice that you need to actually modify the packet's
> AV_PKT_DATA_NEW_EXTRADATA side-data to make it Annex B. Otherwise the
> output will be incorrect.
> 4. And in case this BSF actually inserts the extradata in-band, it would
> need to retain an internal copy of this new extradata.
> (Andriy Gelman once sent a good patch that never got applied that added
> the correct extradata in-band even when there are in-band updates to the
> extradata. I suggest you look at this patch if you want to tackle this
> problem.)
>

Will check Andriy‘s patch first, seems that it may handle this case
properly, thanks for the review comments and pointing this out.

 - Linjie