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

Submitted by joshdk@ob-encoder.com on Sept. 4, 2018, 4:17 p.m.

Details

Message ID 20180904161743.29608-1-joshdk@obe.tv
State New
Headers show

Commit Message

joshdk@ob-encoder.com Sept. 4, 2018, 4:17 p.m.
From: Josh de Kock <joshdk@obe.tv>

No segfault on sample ticket 4408.
---
 libavcodec/h264dec.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Comments

Michael Niedermayer Sept. 4, 2018, 7:46 p.m.
On Tue, Sep 04, 2018 at 05:17:43PM +0100, joshdk@ob-encoder.com wrote:
> From: Josh de Kock <joshdk@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

[...]

Patch hide | download patch | download mbox

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
             h->has_recovery_point = 1;
         case H264_NAL_SLICE:
             h->has_slice = 1;
 
+            if (slice_run <= 0)
+                slice_run--;
+            if (slice_run > 0) {
+                av_log(h, AV_LOG_WARNING, "encountered non-IDR slice after IDR slice before PPS (is PPS missing?)\n");
+            }
+
             if ((err = ff_h264_queue_decode_slice(h, nal))) {
                 H264SliceContext *sl = h->slice_ctx + h->nb_slice_ctx_queued;
                 sl->ref_count[0] = sl->ref_count[1] = 0;
@@ -732,6 +736,7 @@  static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
             break;
         }
         case H264_NAL_PPS:
+            slice_run = 0;
             if (avctx->hwaccel && avctx->hwaccel->decode_params) {
                 ret = avctx->hwaccel->decode_params(avctx,
                                                     nal->type,