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

[Andreas Rheinhardt]

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.)

- Andreas