Commit 9a54c6f2 authored by Ronald S. Bultje's avatar Ronald S. Bultje

vp8: make wait/thread_mb_pos atomic.

Fixes tsan warnings like this in fate-vp8-test-vector-007:

WARNING: ThreadSanitizer: data race (pid=3590)
  Write of size 4 at 0x7d8c0000e07c by thread T2:
    #0 decode_mb_row_no_filter src/libavcodec/vp8.c:2330 (ffmpeg+0x000000ffb59e)
[..]
  Previous write of size 4 at 0x7d8c0000e07c by thread T1:
    #0 decode_mb_row_no_filter src/libavcodec/vp8.c:2330 (ffmpeg+0x000000ffb59e)
parent 83ae7e63
...@@ -2247,15 +2247,15 @@ static void vp8_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *cur_frame, ...@@ -2247,15 +2247,15 @@ static void vp8_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *cur_frame,
#define check_thread_pos(td, otd, mb_x_check, mb_y_check) \ #define check_thread_pos(td, otd, mb_x_check, mb_y_check) \
do { \ do { \
int tmp = (mb_y_check << 16) | (mb_x_check & 0xFFFF); \ int tmp = (mb_y_check << 16) | (mb_x_check & 0xFFFF); \
if (otd->thread_mb_pos < tmp) { \ if (atomic_load(&otd->thread_mb_pos) < tmp) { \
pthread_mutex_lock(&otd->lock); \ pthread_mutex_lock(&otd->lock); \
td->wait_mb_pos = tmp; \ atomic_store(&td->wait_mb_pos, tmp); \
do { \ do { \
if (otd->thread_mb_pos >= tmp) \ if (atomic_load(&otd->thread_mb_pos) >= tmp) \
break; \ break; \
pthread_cond_wait(&otd->cond, &otd->lock); \ pthread_cond_wait(&otd->cond, &otd->lock); \
} while (1); \ } while (1); \
td->wait_mb_pos = INT_MAX; \ atomic_store(&td->wait_mb_pos, INT_MAX); \
pthread_mutex_unlock(&otd->lock); \ pthread_mutex_unlock(&otd->lock); \
} \ } \
} while (0) } while (0)
...@@ -2266,12 +2266,10 @@ static void vp8_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *cur_frame, ...@@ -2266,12 +2266,10 @@ static void vp8_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *cur_frame,
int sliced_threading = (avctx->active_thread_type == FF_THREAD_SLICE) && \ int sliced_threading = (avctx->active_thread_type == FF_THREAD_SLICE) && \
(num_jobs > 1); \ (num_jobs > 1); \
int is_null = !next_td || !prev_td; \ int is_null = !next_td || !prev_td; \
int pos_check = (is_null) ? 1 \ int pos_check = (is_null) ? 1 : \
: (next_td != td && \ (next_td != td && pos >= atomic_load(&next_td->wait_mb_pos)) || \
pos >= next_td->wait_mb_pos) || \ (prev_td != td && pos >= atomic_load(&prev_td->wait_mb_pos)); \
(prev_td != td && \ atomic_store(&td->thread_mb_pos, pos); \
pos >= prev_td->wait_mb_pos); \
td->thread_mb_pos = pos; \
if (sliced_threading && pos_check) { \ if (sliced_threading && pos_check) { \
pthread_mutex_lock(&td->lock); \ pthread_mutex_lock(&td->lock); \
pthread_cond_broadcast(&td->cond); \ pthread_cond_broadcast(&td->cond); \
...@@ -2288,7 +2286,7 @@ static av_always_inline int decode_mb_row_no_filter(AVCodecContext *avctx, void ...@@ -2288,7 +2286,7 @@ static av_always_inline int decode_mb_row_no_filter(AVCodecContext *avctx, void
{ {
VP8Context *s = avctx->priv_data; VP8Context *s = avctx->priv_data;
VP8ThreadData *prev_td, *next_td, *td = &s->thread_data[threadnr]; VP8ThreadData *prev_td, *next_td, *td = &s->thread_data[threadnr];
int mb_y = td->thread_mb_pos >> 16; int mb_y = atomic_load(&td->thread_mb_pos) >> 16;
int mb_x, mb_xy = mb_y * s->mb_width; int mb_x, mb_xy = mb_y * s->mb_width;
int num_jobs = s->num_jobs; int num_jobs = s->num_jobs;
VP8Frame *curframe = s->curframe, *prev_frame = s->prev_frame; VP8Frame *curframe = s->curframe, *prev_frame = s->prev_frame;
...@@ -2428,7 +2426,7 @@ static av_always_inline void filter_mb_row(AVCodecContext *avctx, void *tdata, ...@@ -2428,7 +2426,7 @@ static av_always_inline void filter_mb_row(AVCodecContext *avctx, void *tdata,
{ {
VP8Context *s = avctx->priv_data; VP8Context *s = avctx->priv_data;
VP8ThreadData *td = &s->thread_data[threadnr]; VP8ThreadData *td = &s->thread_data[threadnr];
int mb_x, mb_y = td->thread_mb_pos >> 16, num_jobs = s->num_jobs; int mb_x, mb_y = atomic_load(&td->thread_mb_pos) >> 16, num_jobs = s->num_jobs;
AVFrame *curframe = s->curframe->tf.f; AVFrame *curframe = s->curframe->tf.f;
VP8Macroblock *mb; VP8Macroblock *mb;
VP8ThreadData *prev_td, *next_td; VP8ThreadData *prev_td, *next_td;
...@@ -2507,7 +2505,7 @@ int vp78_decode_mb_row_sliced(AVCodecContext *avctx, void *tdata, int jobnr, ...@@ -2507,7 +2505,7 @@ int vp78_decode_mb_row_sliced(AVCodecContext *avctx, void *tdata, int jobnr,
td->thread_nr = threadnr; td->thread_nr = threadnr;
for (mb_y = jobnr; mb_y < s->mb_height; mb_y += num_jobs) { for (mb_y = jobnr; mb_y < s->mb_height; mb_y += num_jobs) {
td->thread_mb_pos = mb_y << 16; atomic_store(&td->thread_mb_pos, mb_y << 16);
ret = s->decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr); ret = s->decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr);
if (ret < 0) { if (ret < 0) {
update_pos(td, s->mb_height, INT_MAX & 0xFFFF); update_pos(td, s->mb_height, INT_MAX & 0xFFFF);
...@@ -2667,8 +2665,9 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, ...@@ -2667,8 +2665,9 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
s->mv_min.y = -MARGIN; s->mv_min.y = -MARGIN;
s->mv_max.y = ((s->mb_height - 1) << 6) + MARGIN; s->mv_max.y = ((s->mb_height - 1) << 6) + MARGIN;
for (i = 0; i < MAX_THREADS; i++) { for (i = 0; i < MAX_THREADS; i++) {
s->thread_data[i].thread_mb_pos = 0; VP8ThreadData *td = &s->thread_data[i];
s->thread_data[i].wait_mb_pos = INT_MAX; atomic_init(&td->thread_mb_pos, 0);
atomic_init(&td->wait_mb_pos, INT_MAX);
} }
if (is_vp7) if (is_vp7)
avctx->execute2(avctx, vp7_decode_mb_row_sliced, s->thread_data, NULL, avctx->execute2(avctx, vp7_decode_mb_row_sliced, s->thread_data, NULL,
......
...@@ -26,6 +26,8 @@ ...@@ -26,6 +26,8 @@
#ifndef AVCODEC_VP8_H #ifndef AVCODEC_VP8_H
#define AVCODEC_VP8_H #define AVCODEC_VP8_H
#include <stdatomic.h>
#include "libavutil/buffer.h" #include "libavutil/buffer.h"
#include "libavutil/thread.h" #include "libavutil/thread.h"
...@@ -114,8 +116,8 @@ typedef struct VP8ThreadData { ...@@ -114,8 +116,8 @@ typedef struct VP8ThreadData {
pthread_mutex_t lock; pthread_mutex_t lock;
pthread_cond_t cond; pthread_cond_t cond;
#endif #endif
int thread_mb_pos; // (mb_y << 16) | (mb_x & 0xFFFF) atomic_int thread_mb_pos; // (mb_y << 16) | (mb_x & 0xFFFF)
int wait_mb_pos; // What the current thread is waiting on. atomic_int wait_mb_pos; // What the current thread is waiting on.
#define EDGE_EMU_LINESIZE 32 #define EDGE_EMU_LINESIZE 32
DECLARE_ALIGNED(16, uint8_t, edge_emu_buffer)[21 * EDGE_EMU_LINESIZE]; DECLARE_ALIGNED(16, uint8_t, edge_emu_buffer)[21 * EDGE_EMU_LINESIZE];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment