diff mbox

[FFmpeg-devel] fic: set pict_type/key_frame after (instead of during) slice decoding.

Message ID 1490745134-60882-1-git-send-email-rsbultje@gmail.com
State Accepted
Commit 73f863d751df84db7a0ca1bd83cdff1b95dc94dd
Headers show

Commit Message

Ronald S. Bultje March 28, 2017, 11:52 p.m. UTC 
This fixes a race condition that was already documented in the source
code, and is also reported by tsan in fate-fic-avi.
---
 libavcodec/fic.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Comments

Paul B Mahol March 29, 2017, 9:35 p.m. UTC | #1 
On 3/29/17, Ronald S. Bultje <rsbultje@gmail.com> wrote:
> This fixes a race condition that was already documented in the source
> code, and is also reported by tsan in fate-fic-avi.
> ---
>  libavcodec/fic.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>

LGTM
diff mbox

Patch

diff --git a/libavcodec/fic.c b/libavcodec/fic.c
index d3952a4..2bec3d7 100644
--- a/libavcodec/fic.c
+++ b/libavcodec/fic.c
@@ -34,6 +34,7 @@  typedef struct FICThreadContext {
     int slice_h;
     int src_size;
     int y_off;
+    int p_frame;
 } FICThreadContext;
 
 typedef struct FICContext {
@@ -133,16 +134,13 @@  static void fic_idct_put(uint8_t *dst, int stride, int16_t *block)
     }
 }
 static int fic_decode_block(FICContext *ctx, GetBitContext *gb,
-                            uint8_t *dst, int stride, int16_t *block)
+                            uint8_t *dst, int stride, int16_t *block, int *is_p)
 {
     int i, num_coeff;
 
     /* Is it a skip block? */
     if (get_bits1(gb)) {
-        /* This is a P-frame. */
-        ctx->frame->key_frame = 0;
-        ctx->frame->pict_type = AV_PICTURE_TYPE_P;
-
+        *is_p = 1;
         return 0;
     }
 
@@ -182,7 +180,8 @@  static int fic_decode_slice(AVCodecContext *avctx, void *tdata)
             for (x = 0; x < (ctx->aligned_width >> !!p); x += 8) {
                 int ret;
 
-                if ((ret = fic_decode_block(ctx, &gb, dst + x, stride, tctx->block)) != 0)
+                if ((ret = fic_decode_block(ctx, &gb, dst + x, stride,
+                                            tctx->block, &tctx->p_frame)) != 0)
                     return ret;
             }
 
@@ -348,15 +347,6 @@  static int fic_decode_frame(AVCodecContext *avctx, void *data,
         return AVERROR_INVALIDDATA;
     }
 
-    /*
-     * Set the frametype to I initially. It will be set to P if the frame
-     * has any dependencies (skip blocks). There will be a race condition
-     * inside the slice decode function to set these, but we do not care.
-     * since they will only ever be set to 0/P.
-     */
-    ctx->frame->key_frame = 1;
-    ctx->frame->pict_type = AV_PICTURE_TYPE_I;
-
     /* Allocate slice data. */
     av_fast_malloc(&ctx->slice_data, &ctx->slice_data_size,
                    nslices * sizeof(ctx->slice_data[0]));
@@ -398,6 +388,15 @@  static int fic_decode_frame(AVCodecContext *avctx, void *data,
                               NULL, nslices, sizeof(ctx->slice_data[0]))) < 0)
         return ret;
 
+    ctx->frame->key_frame = 1;
+    ctx->frame->pict_type = AV_PICTURE_TYPE_I;
+    for (slice = 0; slice < nslices; slice++) {
+        if (ctx->slice_data[slice].p_frame) {
+            ctx->frame->key_frame = 0;
+            ctx->frame->pict_type = AV_PICTURE_TYPE_P;
+            break;
+        }
+    }
     av_frame_free(&ctx->final_frame);
     ctx->final_frame = av_frame_clone(ctx->frame);
     if (!ctx->final_frame) {