@@ -2389,11 +2389,11 @@ static int vp8_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *cur_frame,
#endif
static av_always_inline int decode_mb_row_no_filter(AVCodecContext *avctx, void *tdata,
- int jobnr, int threadnr, int is_vp7)
+ int jobnr, int threadnr, int mb_y,
+ int is_vp7)
{
VP8Context *s = avctx->priv_data;
VP8ThreadData *prev_td, *next_td, *td = &s->thread_data[threadnr];
- int mb_y = atomic_load(&td->thread_mb_pos) >> 16;
int mb_x, mb_xy = mb_y * s->mb_width;
int num_jobs = s->num_jobs;
const VP8Frame *prev_frame = s->prev_frame;
@@ -2518,23 +2518,24 @@ static av_always_inline int decode_mb_row_no_filter(AVCodecContext *avctx, void
}
static int vp7_decode_mb_row_no_filter(AVCodecContext *avctx, void *tdata,
- int jobnr, int threadnr)
+ int jobnr, int threadnr, int mb_y)
{
- return decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr, 1);
+ return decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr, mb_y, 1);
}
static int vp8_decode_mb_row_no_filter(AVCodecContext *avctx, void *tdata,
- int jobnr, int threadnr)
+ int jobnr, int threadnr, int mb_y)
{
- return decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr, 0);
+ return decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr, mb_y, 0);
}
static av_always_inline void filter_mb_row(AVCodecContext *avctx, void *tdata,
- int jobnr, int threadnr, int is_vp7)
+ int jobnr, int threadnr, int mb_y,
+ int is_vp7)
{
VP8Context *s = avctx->priv_data;
VP8ThreadData *td = &s->thread_data[threadnr];
- int mb_x, mb_y = atomic_load(&td->thread_mb_pos) >> 16, num_jobs = s->num_jobs;
+ int mb_x, num_jobs = s->num_jobs;
AVFrame *curframe = s->curframe->tf.f;
VP8Macroblock *mb;
VP8ThreadData *prev_td, *next_td;
@@ -2589,15 +2590,15 @@ static av_always_inline void filter_mb_row(AVCodecContext *avctx, void *tdata,
}
static void vp7_filter_mb_row(AVCodecContext *avctx, void *tdata,
- int jobnr, int threadnr)
+ int jobnr, int threadnr, int mb_y)
{
- filter_mb_row(avctx, tdata, jobnr, threadnr, 1);
+ filter_mb_row(avctx, tdata, jobnr, threadnr, mb_y, 1);
}
static void vp8_filter_mb_row(AVCodecContext *avctx, void *tdata,
- int jobnr, int threadnr)
+ int jobnr, int threadnr, int mb_y)
{
- filter_mb_row(avctx, tdata, jobnr, threadnr, 0);
+ filter_mb_row(avctx, tdata, jobnr, threadnr, mb_y, 0);
}
static av_always_inline
@@ -2615,14 +2616,13 @@ int vp78_decode_mb_row_sliced(AVCodecContext *avctx, void *tdata, int jobnr,
td->mv_bounds.mv_min.y = -MARGIN - 64 * threadnr;
td->mv_bounds.mv_max.y = ((s->mb_height - 1) << 6) + MARGIN - 64 * threadnr;
for (mb_y = jobnr; mb_y < s->mb_height; mb_y += num_jobs) {
- 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, mb_y);
if (ret < 0) {
update_pos(td, s->mb_height, INT_MAX & 0xFFFF);
return ret;
}
if (s->deblock_filter)
- s->filter_mb_row(avctx, tdata, jobnr, threadnr);
+ s->filter_mb_row(avctx, tdata, jobnr, threadnr, mb_y);
update_pos(td, mb_y, INT_MAX & 0xFFFF);
td->mv_bounds.mv_min.y -= 64 * num_jobs;
@@ -330,8 +330,8 @@ typedef struct VP8Context {
*/
int mb_layout;
- int (*decode_mb_row_no_filter)(AVCodecContext *avctx, void *tdata, int jobnr, int threadnr);
- void (*filter_mb_row)(AVCodecContext *avctx, void *tdata, int jobnr, int threadnr);
+ int (*decode_mb_row_no_filter)(AVCodecContext *avctx, void *tdata, int jobnr, int threadnr, int mb_y);
+ void (*filter_mb_row)(AVCodecContext *avctx, void *tdata, int jobnr, int threadnr, int mb_y);
int vp7;
Avoids atomic stores and loads and is a prerequisite for removing all atomic synchronizations for VP7. Notice that removing the explicit atomic_store() in vp78_decode_mb_row_sliced() does not negatively affect parallelism during slice-threading, because no check_thread_pos() ever waits for an (mb_x, mb_y) pair with mb_x == 0 (which this atomic store signalled). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Btw: The code in update_pos looks fishy to me; namely the part that tries to avoid the broadcast. Consider the scenario in which the other threads (prev and next, A and B) are not waiting when the current thread C checks their wait_mb_pos. Then one of the other threads reads C's thread_mb_pos and notices that it needs to wait for an update from C. It therefore locks C's mutex, stores its wait_mb_pos, checks C's thread_mb_pos again (still reading the old value in this scenario) and waits via pthread_cond_wait(). Then C updates its thread_mb_pos, but because C uses outdated values for A and B's wait_mb_pos, it never signals a broadcast. Who will then wake up the waiting thread? This should be fixable by moving the loads after C's update of thread_mb_pos: In case C's read of A's wait_mb_pos value happens before A updates it, then C's update of its thread_mb_pos happens before A updates its wait_mb_pos and A will therefore read C's updated value of thread_mb_pos its atomic_load while holding C's lock (and will therefore never call pthread_cond_wait()). In case C's read of A's wait_mb_pos value happens after A updates it, C will emit its broadcast, waking A which reads the updated value and stops. libavcodec/vp8.c | 30 +++++++++++++++--------------- libavcodec/vp8.h | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-)