[FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation

[Fu, Linjie]

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Michael Niedermayer
Sent: Friday, August 24, 2018 05:30
To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation

On Thu, Aug 16, 2018 at 03:07:50PM +0800, Linjie Fu wrote:
> Fix the live stream encoding problem using qsv when the first frame is 
> not an IDR frame.
> 
> Add the extradata information when the IDR frame is missing in the 
> first GOP.
> 
> Fix the bug reported in  ticket #6418.
> 
> [PATCH V2] Fix the coding style.
> 
> Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> ---
>  libavcodec/h264_mp4toannexb_bsf.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
> b/libavcodec/h264_mp4toannexb_bsf.c
> index 794c82e650..fbb9f1fe20 100644
> --- a/libavcodec/h264_mp4toannexb_bsf.c
> +++ b/libavcodec/h264_mp4toannexb_bsf.c
> @@ -33,6 +33,7 @@ typedef struct H264BSFContext {
>      int32_t  pps_offset;
>      uint8_t  length_size;
>      uint8_t  new_idr;
> +    uint8_t  new_nal_slice;
>      uint8_t  idr_sps_seen;
>      uint8_t  idr_pps_seen;
>      int      extradata_parsed;
> @@ -243,6 +244,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
>                                 buf, nal_size, 1)) < 0)
>                  goto fail;
>              s->new_idr = 0;
> +            s->new_nal_slice = 1;
>          /* if only SPS has been seen, also insert PPS */
>          } else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE && s->idr_sps_seen && !s->idr_pps_seen) {
>              if (s->pps_offset == -1) { @@ -253,6 +255,12 @@ static 
> int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
>                                          ctx->par_out->extradata + s->pps_offset, ctx->par_out->extradata_size - s->pps_offset,
>                                          buf, nal_size, 1)) < 0)
>                  goto fail;
> +        } else if (s->new_idr && !s->new_nal_slice && H264_NAL_SLICE == unit_type && !s->idr_sps_seen && !s->idr_pps_seen){
> +            av_log(ctx, AV_LOG_WARNING, "first H264_NAL_SLICE when there is no IDR.\n");
> +            if ((ret = alloc_and_copy(out, ctx->par_out->extradata, ctx->par_out->extradata_size, buf, nal_size, 1)) < 0)
> +                goto fail;
> +            s->new_nal_slice = 1;
> +            s->new_idr = 0;

The commit message is insufficient
this adds the extradata if new_idr is set not just in the absence of IDR at the begin

also teh addded code is identical to the first if() just a different NAL is checked for, this code duplication is not a good idea in already complex code

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus


Thanks for your review. 
I will adjust the code to reduce the duplication and make the commit message clearer.

I add " new_nal_slice " to indicate the newly coming H264_NAL_IDR_SLICE and H264_NAL_SLICE, because the new_idr is always "1" at the beginning of the filter regardless of the nal type of slice.  

This patch aims at the following issues:
1. the IDR frame is missing in the  first GOP of a stream (common in live stream, IDR.No.Inband.SPPS.mkv in ticket #6418)
2.  there is no IDR frame in the input stream.  (No.Inband.SPPS.No.IDR.mkv​ in ticket #6418)
In issue 1,  ffmpeg just drop all the frames in the first GOP without the patch then continue to decode the following frames. (qsv and libopenh264)
In issue 2,  ffmpeg could not decode any frames of the stream through h264_mp4toannexb_filter without the patch. (qsv and libopenh264)
After applying  this patch, all the frames could be decoded correctly  in both issues.  (qsv works well, libopenh264 has to check the slice_nal_type in the decode step in addition to the h264_mp4toannexb_filter, still can't decode.)

Thanks again.