[FFmpeg-devel] [PATCH] libavcodec/hevc_mp4toannexb_bsf: insert extradata before non-AUD unit

[Andreas Rheinhardt]

Xiang, Haihao:
> From: Haihao Xiang <haihao.xiang at intel.com>
> 
> It is possible that an IRAP frame in input AVPacket contains VPS, SPS
> and PPS, and these headers should take effect. However the prepended
> extradata might override these headers. This patch inserts extradata
> before non-AUD unit, hence VPS, SPS and PPS from the input AVPacket will
> take effect if they are present.
> 
> This should fix #7799
> 
> Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
> ---
>  libavcodec/hevc_mp4toannexb_bsf.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
> index 790dfb0394..77551ba221 100644
> --- a/libavcodec/hevc_mp4toannexb_bsf.c
> +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> @@ -124,6 +124,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
>  
>      int got_irap = 0;
>      int i, ret = 0;
> +    int prev_nalu_is_aud = 0, extradata_offset = 0;
>  
>      ret = ff_bsf_get_packet(ctx, &in);
>      if (ret < 0)
> @@ -169,14 +170,21 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
>  
>          prev_size = out->size;
>  
> +        if (prev_nalu_is_aud)
> +            extradata_offset = prev_size;
> +
>          ret = av_grow_packet(out, 4 + nalu_size + extra_size);
>          if (ret < 0)
>              goto fail;
>  
> -        if (extra_size)
> -            memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size);
> +        if (extra_size) {
> +            memmove(out->data + extradata_offset + extra_size, out->data + extradata_offset, prev_size - extradata_offset);
> +            memcpy(out->data + extradata_offset, ctx->par_out->extradata, extra_size);
> +        }
> +
>          AV_WB32(out->data + prev_size + extra_size, 1);
>          bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size);
> +        prev_nalu_is_aud = nalu_type == HEVC_NAL_AUD;
>      }
>  
>      ret = av_packet_copy_props(out, in);

1. prev_nalu_is_aud is unnecessary: You can just use "if (nalu_type ==
HEVC_NAL_AUD) extradata_offset = out->size;" at the end of the loop.
2. This only mitigates a certain case of wrongly inserted extradata; it
does not fix the underlying issue which is that this BSF does not track
the current state of extradata and therefore inserts parameter sets that
have already been superseded by new in-band extradata. Your patch
ensures that the extradata parameter sets will be prepended to the
in-band extradata. Yet the already deactivated parameter sets will still
be inserted. The output can still be invalid, because 7.4.2.4.2 of the
HEVC spec requires the following: "Any SPS NAL unit with nuh_layer_id
equal to 0 containing the value of sps_seq_parameter_set_id for the
active SPS
RBSP for the base layer for a CVS shall have the same content as that of
the active SPS RBSP for the base layer for the CVS, unless it follows
the last access unit of the CVS and precedes the first VCL NAL unit and
the first SEI NAL unit containing an active parameter sets SEI message
(when present) of another CVS." Furthermore in case a preceding packet
contained updated parameter sets that (perhaps partially) overwrite
parameter sets from extradata and the current packet does not
contain/repeat these parameter sets, then the above code will still
insert the outdated and incorrect parameter set and these parameter sets
will not be overwritten before being used.
3. Andriy Gelman once proposed a patchset that tracked the parameter
sets and inserted only the needed ones. See
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20191016025040.31273-2-andriy.gelman@gmail.com/
The problem with this patchset was the complexity emanating from HEVC's
layers.
4. Lacking proper tracking of parameter sets we should probably err on
the side of caution and stop inserting parameter sets if the input
contained in-band parameter sets (similar to h264_mp4toannexb). I can
write a patch for this.

- Andreas