[FFmpeg-devel] [PATCH] Rework VP8 decoder frame skipping code.

[Reimar Döffinger]

This reworks the frame skipping code such that the reference
buffers are still updated according to the header.
However it also ensures that the current frame will not end
up in any reference buffer.
Also fixes a hang with frame-multithreading, probably because
get_buffer was already called and would have reset the progress,
however the frame could remain in framep due to the missing update
(or it could be assigned to next_framep and a skip_frame skip would
then write it into framep - there might be even more failure modes).
Sample might become available at samples/nsv/vp8.nsv

Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
---
 libavcodec/vp8.c |   53 ++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 8cf67ba..d68a91e 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -1561,6 +1561,18 @@ static void release_queued_segmaps(VP8Context *s, int is_close)
     s->maps_are_invalid = 0;
 }
 
+/**
+ * Sets things up for skipping the current frame.
+ * In particular, removes it from the reference buffers.
+ */
+static void skipframe_clear(VP8Context *s)
+{
+    s->invisible = 1;
+    s->next_framep[VP56_FRAME_CURRENT] = NULL;
+    if (s->update_last)
+        s->next_framep[VP56_FRAME_PREVIOUS] = NULL;
+}
+
 static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
                             AVPacket *avpkt)
 {
@@ -1582,10 +1594,6 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     skip_thresh = !referenced ? AVDISCARD_NONREF :
                     !s->keyframe ? AVDISCARD_NONKEY : AVDISCARD_ALL;
 
-    if (avctx->skip_frame >= skip_thresh) {
-        s->invisible = 1;
-        goto skip_decode;
-    }
     s->deblock_filter = s->filter.level && avctx->skip_loop_filter < skip_thresh;
 
     // release no longer referenced frames
@@ -1610,16 +1618,6 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
         av_log(avctx, AV_LOG_FATAL, "Ran out of free frames!\n");
         abort();
     }
-    if (curframe->data[0])
-        vp8_release_frame(s, curframe, 1, 0);
-
-    curframe->key_frame = s->keyframe;
-    curframe->pict_type = s->keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
-    curframe->reference = referenced ? 3 : 0;
-    if ((ret = vp8_alloc_frame(s, curframe))) {
-        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed!\n");
-        return ret;
-    }
 
     // check if golden and altref are swapped
     if (s->update_altref != VP56_FRAME_NONE) {
@@ -1639,7 +1637,11 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     }
     s->next_framep[VP56_FRAME_CURRENT]      = curframe;
 
-    ff_thread_finish_setup(avctx);
+    if (avctx->skip_frame >= skip_thresh) {
+        skipframe_clear(s);
+        ret = avpkt->size;
+        goto skip_decode;
+    }
 
     // Given that arithmetic probabilities are updated every frame, it's quite likely
     // that the values we have on a random interframe are complete junk if we didn't
@@ -1648,9 +1650,25 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
                          !s->framep[VP56_FRAME_GOLDEN] ||
                          !s->framep[VP56_FRAME_GOLDEN2])) {
         av_log(avctx, AV_LOG_WARNING, "Discarding interframe without a prior keyframe!\n");
-        return AVERROR_INVALIDDATA;
+        skipframe_clear(s);
+        ret = AVERROR_INVALIDDATA;
+        goto skip_decode;
+    }
+
+    if (curframe->data[0])
+        vp8_release_frame(s, curframe, 1, 0);
+
+    curframe->key_frame = s->keyframe;
+    curframe->pict_type = s->keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
+    curframe->reference = referenced ? 3 : 0;
+    if ((ret = vp8_alloc_frame(s, curframe))) {
+        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed!\n");
+        skipframe_clear(s);
+        goto skip_decode;
     }
 
+    ff_thread_finish_setup(avctx);
+
     s->linesize   = curframe->linesize[0];
     s->uvlinesize = curframe->linesize[1];
 
@@ -1760,6 +1778,7 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     }
 
     ff_thread_report_progress(curframe, INT_MAX, 0);
+    ret = avpkt->size;
 skip_decode:
     // if future frames don't use the updated probabilities,
     // reset them to the values we saved
@@ -1773,7 +1792,7 @@ skip_decode:
         *data_size = sizeof(AVFrame);
     }
 
-    return avpkt->size;
+    return ret;
 }
 
 static av_cold int vp8_decode_init(AVCodecContext *avctx)
-- 
1.7.7.3