[FFmpeg-devel] [PATCH] lavc/h264: warn on mixed non-IDR/IDR NAL units

[Michael Niedermayer]

On Tue, Sep 04, 2018 at 05:17:43PM +0100, joshdk at ob-encoder.com wrote:
> From: Josh de Kock <joshdk at obe.tv>
> 
> No segfault on sample ticket 4408.
> ---
>  libavcodec/h264dec.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 8d115fa040..2ab52f57c0 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -601,7 +601,7 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
>  {
>      AVCodecContext *const avctx = h->avctx;
>      int nals_needed = 0; ///< number of NALs that need decoding before the next frame thread starts
> -    int idr_cleared=0;
> +    int slice_run = 0;
>      int i, ret = 0;
>  
>      h->has_slice = 0;
> @@ -656,19 +656,23 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
>                  ret = -1;
>                  goto end;
>              }
> -            if(!idr_cleared) {
> -                if (h->current_slice && (avctx->active_thread_type & FF_THREAD_SLICE)) {
> -                    av_log(h, AV_LOG_ERROR, "invalid mixed IDR / non IDR frames cannot be decoded in slice multithreading mode\n");
> -                    ret = AVERROR_INVALIDDATA;
> -                    goto end;
> -                }
> -                idr(h); // FIXME ensure we don't lose some frames if there is reordering
> +            if (slice_run >= 0)
> +                slice_run++;
> +            if (slice_run < 0) {
> +                av_log(h, AV_LOG_WARNING, "encountered IDR slice after non-IDR slice before PPS (is PPS missing?)\n");
>              }
> -            idr_cleared = 1;
> +
> +            idr(h); // FIXME ensure we don't lose some frames if there is reordering

This patch does 2 things, one is described in the commit message
the other thing is to call idr() repeatly

I think the warnings are incorrect, pps can be placed earlier or in a
global header.

Calling idr() more often requires more computations, I do not know what
this is intended to fix so i cannot comment beyond that

I think i do not fully understand what the underlaying reason for these
changes is. The commit message speaks about what is done not why it is

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180904/21a49e7a/attachment.sig>