vp8: fix up handling of segmentation_maps in reference frames.

Associate segmentation_map[] with reference frame, rather than
decoding instance. This fixes cases where the map would be free()'ed
on e.g. a size change in one thread, whereas the other thread was
still accessing it. Also, it fixes cases where threads overwrite data
that is still being referenced by the previous thread, who thinks that
it's part of the frame previously decoded by the next thread.

2 files changed, 65 insertions, 14 deletions

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 7442b99252..d5cdaba486 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -41,24 +41,57 @@ static void free_buffers(VP8Context *s)
     av_freep(&s->top_nnz);
     av_freep(&s->edge_emu_buffer);
     av_freep(&s->top_border);
-    av_freep(&s->segmentation_map);
 
     s->macroblocks = NULL;
 }
 
-static void vp8_decode_flush(AVCodecContext *avctx)
+static int vp8_alloc_frame(VP8Context *s, AVFrame *f)
+{
+    int ret;
+    if ((ret = ff_thread_get_buffer(s->avctx, f)) < 0)
+        return ret;
+    if (!s->maps_are_invalid && s->num_maps_to_be_freed) {
+        f->ref_index[0] = s->segmentation_maps[--s->num_maps_to_be_freed];
+    } else if (!(f->ref_index[0] = av_mallocz(s->mb_width * s->mb_height))) {
+        ff_thread_release_buffer(s->avctx, f);
+        return AVERROR(ENOMEM);
+    }
+    return 0;
+}
+
+static void vp8_release_frame(VP8Context *s, AVFrame *f, int is_close)
+{
+    if (!is_close) {
+        if (f->ref_index[0]) {
+            assert(s->num_maps_to_be_freed < FF_ARRAY_ELEMS(s->segmentation_maps));
+            s->segmentation_maps[s->num_maps_to_be_freed++] = f->ref_index[0];
+            f->ref_index[0] = NULL;
+        }
+    } else {
+        av_freep(&f->ref_index[0]);
+    }
+    ff_thread_release_buffer(s->avctx, f);
+}
+
+static void vp8_decode_flush_impl(AVCodecContext *avctx, int force, int is_close)
 {
     VP8Context *s = avctx->priv_data;
     int i;
 
-    if (!avctx->is_copy) {
+    if (!avctx->is_copy || force) {
         for (i = 0; i < 5; i++)
             if (s->frames[i].data[0])
-                ff_thread_release_buffer(avctx, &s->frames[i]);
+                vp8_release_frame(s, &s->frames[i], is_close);
     }
     memset(s->framep, 0, sizeof(s->framep));
 
     free_buffers(s);
+    s->maps_are_invalid = 1;
+}
+
+static void vp8_decode_flush(AVCodecContext *avctx)
+{
+    vp8_decode_flush_impl(avctx, 0, 0);
 }
 
 static int update_dimensions(VP8Context *s, int width, int height)
@@ -68,7 +101,7 @@ static int update_dimensions(VP8Context *s, int width, int height)
         if (av_image_check_size(width, height, 0, s->avctx))
             return AVERROR_INVALIDDATA;
 
-        vp8_decode_flush(s->avctx);
+        vp8_decode_flush_impl(s->avctx, 1, 0);
 
         avcodec_set_dimensions(s->avctx, width, height);
     }
@@ -81,10 +114,9 @@ static int update_dimensions(VP8Context *s, int width, int height)
     s->intra4x4_pred_mode_top  = av_mallocz(s->mb_width*4);
     s->top_nnz                 = av_mallocz(s->mb_width*sizeof(*s->top_nnz));
     s->top_border              = av_mallocz((s->mb_width+1)*sizeof(*s->top_border));
-    s->segmentation_map        = av_mallocz(s->mb_width*s->mb_height);
 
     if (!s->macroblocks_base || !s->filter_strength || !s->intra4x4_pred_mode_top ||
-        !s->top_nnz || !s->top_border || !s->segmentation_map)
+        !s->top_nnz || !s->top_border)
         return AVERROR(ENOMEM);
 
     s->macroblocks        = s->macroblocks_base + 1;
@@ -1508,6 +1540,14 @@ static void filter_mb_row_simple(VP8Context *s, AVFrame *curframe, int mb_y)
     }
 }
 
+static void release_queued_segmaps(VP8Context *s, int is_close)
+{
+    int leave_behind = is_close ? 0 : !s->maps_are_invalid;
+    while (s->num_maps_to_be_freed > leave_behind)
+        av_freep(&s->segmentation_maps[--s->num_maps_to_be_freed]);
+    s->maps_are_invalid = 0;
+}
+
 static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
                             AVPacket *avpkt)
 {
@@ -1516,6 +1556,8 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     enum AVDiscard skip_thresh;
     AVFrame *av_uninit(curframe), *prev_frame = s->framep[VP56_FRAME_CURRENT];
 
+    release_queued_segmaps(s, 0);
+
     if ((ret = decode_frame_header(s, avpkt->data, avpkt->size)) < 0)
         return ret;
 
@@ -1538,7 +1580,7 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
             &s->frames[i] != s->framep[VP56_FRAME_PREVIOUS] &&
             &s->frames[i] != s->framep[VP56_FRAME_GOLDEN] &&
             &s->frames[i] != s->framep[VP56_FRAME_GOLDEN2])
-            ff_thread_release_buffer(avctx, &s->frames[i]);
+            vp8_release_frame(s, &s->frames[i], 0);
 
     // find a free buffer
     for (i = 0; i < 5; i++)
@@ -1559,8 +1601,7 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
     curframe->key_frame = s->keyframe;
     curframe->pict_type = s->keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
     curframe->reference = referenced ? 3 : 0;
-    curframe->ref_index[0] = s->segmentation_map;
-    if ((ret = ff_thread_get_buffer(avctx, curframe))) {
+    if ((ret = vp8_alloc_frame(s, curframe))) {
         av_log(avctx, AV_LOG_ERROR, "get_buffer() failed!\n");
         return ret;
     }
@@ -1652,8 +1693,8 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
             s->dsp.prefetch(dst[0] + (mb_x&3)*4*s->linesize + 64, s->linesize, 4);
             s->dsp.prefetch(dst[1] + (mb_x&7)*s->uvlinesize + 64, dst[2] - dst[1], 2);
 
-            decode_mb_mode(s, mb, mb_x, mb_y, s->segmentation_map + mb_xy,
-                           prev_frame ? prev_frame->ref_index[0] + mb_xy : NULL);
+            decode_mb_mode(s, mb, mb_x, mb_y, curframe->ref_index[0] + mb_xy,
+                           prev_frame && prev_frame->ref_index[0] ? prev_frame->ref_index[0] + mb_xy : NULL);
 
             prefetch_motion(s, mb, mb_x, mb_y, mb_xy, VP56_FRAME_PREVIOUS);
 
@@ -1736,7 +1777,8 @@ static av_cold int vp8_decode_init(AVCodecContext *avctx)
 
 static av_cold int vp8_decode_free(AVCodecContext *avctx)
 {
-    vp8_decode_flush(avctx);
+    vp8_decode_flush_impl(avctx, 0, 1);
+    release_queued_segmaps(avctx->priv_data, 1);
     return 0;
 }
 
diff --git a/libavcodec/vp8.h b/libavcodec/vp8.h
index 5a96cd436c..6cbdca2d88 100644
--- a/libavcodec/vp8.h
+++ b/libavcodec/vp8.h
@@ -130,7 +130,6 @@ typedef struct {
 
     uint8_t *intra4x4_pred_mode_top;
     uint8_t intra4x4_pred_mode_left[4];
-    uint8_t *segmentation_map;
 
     /**
      * Macroblocks can have one of 4 different quants in a frame when
@@ -237,6 +236,16 @@ typedef struct {
     H264PredContext hpc;
     vp8_mc_func put_pixels_tab[3][3][3];
     AVFrame frames[5];
+
+    /**
+     * A list of segmentation_map buffers that are to be free()'ed in
+     * the next decoding iteration. We can't free() them right away
+     * because the map may still be used by subsequent decoding threads.
+     * Unused if frame threading is off.
+     */
+    uint8_t *segmentation_maps[5];
+    int num_maps_to_be_freed;
+    int maps_are_invalid;
 } VP8Context;
 
 #endif /* AVCODEC_VP8_H */