From f6f7d1504134683c435e2c7d804279d982e52bb4 Mon Sep 17 00:00:00 2001 From: "Ronald S. Bultje" Date: Wed, 3 Oct 2012 16:25:14 -0700 Subject: [PATCH] h264: don't touch H264Context->ref_count[] during MB decoding The variable is copied to subsequent threads at the same time, so this may cause wrong ref_count[] values to be copied to subsequent threads. This bug was found using TSAN. Signed-off-by: Luca Barbato --- libavcodec/h264_cabac.c | 41 ++++++++++++++++------------------------- libavcodec/h264_cavlc.c | 33 +++++++++++++-------------------- 2 files changed, 29 insertions(+), 45 deletions(-) diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c index f2fea5d3f2..92c1c03740 100644 --- a/libavcodec/h264_cabac.c +++ b/libavcodec/h264_cabac.c @@ -2005,11 +2005,6 @@ decode_intra_mb: return 0; } - if(MB_MBAFF){ - h->ref_count[0] <<= 1; - h->ref_count[1] <<= 1; - } - fill_decode_caches(h, mb_type); if( IS_INTRA( mb_type ) ) { @@ -2078,10 +2073,11 @@ decode_intra_mb: for( i = 0; i < 4; i++ ) { if(IS_DIRECT(h->sub_mb_type[i])) continue; if(IS_DIR(h->sub_mb_type[i], 0, list)){ - if( h->ref_count[list] > 1 ){ + int rc = h->ref_count[list] << MB_MBAFF; + if (rc > 1) { ref[list][i] = decode_cabac_mb_ref( h, list, 4*i ); - if(ref[list][i] >= (unsigned)h->ref_count[list]){ - av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref[list][i], h->ref_count[list]); + if (ref[list][i] >= (unsigned) rc) { + av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref[list][i], rc); return -1; } }else @@ -2163,11 +2159,11 @@ decode_intra_mb: if(IS_16X16(mb_type)){ for(list=0; listlist_count; list++){ if(IS_DIR(mb_type, 0, list)){ - int ref; - if(h->ref_count[list] > 1){ + int ref, rc = h->ref_count[list] << MB_MBAFF; + if (rc > 1) { ref= decode_cabac_mb_ref(h, list, 0); - if(ref >= (unsigned)h->ref_count[list]){ - av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, h->ref_count[list]); + if (ref >= (unsigned) rc) { + av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, rc); return -1; } }else @@ -2191,11 +2187,11 @@ decode_intra_mb: for(list=0; listlist_count; list++){ for(i=0; i<2; i++){ if(IS_DIR(mb_type, i, list)){ - int ref; - if(h->ref_count[list] > 1){ + int ref, rc = h->ref_count[list] << MB_MBAFF; + if (rc > 1) { ref= decode_cabac_mb_ref( h, list, 8*i ); - if(ref >= (unsigned)h->ref_count[list]){ - av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, h->ref_count[list]); + if (ref >= (unsigned) rc) { + av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, rc); return -1; } }else @@ -2226,11 +2222,11 @@ decode_intra_mb: for(list=0; listlist_count; list++){ for(i=0; i<2; i++){ if(IS_DIR(mb_type, i, list)){ //FIXME optimize - int ref; - if(h->ref_count[list] > 1){ + int ref, rc = h->ref_count[list] << MB_MBAFF; + if (rc > 1) { ref= decode_cabac_mb_ref( h, list, 4*i ); - if(ref >= (unsigned)h->ref_count[list]){ - av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, h->ref_count[list]); + if (ref >= (unsigned) rc) { + av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, rc); return -1; } }else @@ -2403,10 +2399,5 @@ decode_intra_mb: s->current_picture.f.qscale_table[mb_xy] = s->qscale; write_back_non_zero_count(h); - if(MB_MBAFF){ - h->ref_count[0] >>= 1; - h->ref_count[1] >>= 1; - } - return 0; } diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c index c4159e241c..8996057c31 100644 --- a/libavcodec/h264_cavlc.c +++ b/libavcodec/h264_cavlc.c @@ -784,11 +784,6 @@ decode_intra_mb: return 0; } - if(MB_MBAFF){ - h->ref_count[0] <<= 1; - h->ref_count[1] <<= 1; - } - fill_decode_neighbors(h, mb_type); fill_decode_caches(h, mb_type); @@ -868,7 +863,7 @@ decode_intra_mb: } for(list=0; listlist_count; list++){ - int ref_count= IS_REF0(mb_type) ? 1 : h->ref_count[list]; + int ref_count = IS_REF0(mb_type) ? 1 : h->ref_count[list] << MB_MBAFF; for(i=0; i<4; i++){ if(IS_DIRECT(h->sub_mb_type[i])) continue; if(IS_DIR(h->sub_mb_type[i], 0, list)){ @@ -948,13 +943,14 @@ decode_intra_mb: for(list=0; listlist_count; list++){ unsigned int val; if(IS_DIR(mb_type, 0, list)){ - if(h->ref_count[list]==1){ + int rc = h->ref_count[list] << MB_MBAFF; + if (rc == 1) { val= 0; - }else if(h->ref_count[list]==2){ + } else if (rc == 2) { val= get_bits1(&s->gb)^1; }else{ val= get_ue_golomb_31(&s->gb); - if(val >= h->ref_count[list]){ + if (val >= rc) { av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val); return -1; } @@ -978,13 +974,14 @@ decode_intra_mb: for(i=0; i<2; i++){ unsigned int val; if(IS_DIR(mb_type, i, list)){ - if(h->ref_count[list] == 1){ + int rc = h->ref_count[list] << MB_MBAFF; + if (rc == 1) { val= 0; - }else if(h->ref_count[list] == 2){ + } else if (rc == 2) { val= get_bits1(&s->gb)^1; }else{ val= get_ue_golomb_31(&s->gb); - if(val >= h->ref_count[list]){ + if (val >= rc) { av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val); return -1; } @@ -1015,13 +1012,14 @@ decode_intra_mb: for(i=0; i<2; i++){ unsigned int val; if(IS_DIR(mb_type, i, list)){ //FIXME optimize - if(h->ref_count[list]==1){ + int rc = h->ref_count[list] << MB_MBAFF; + if (rc == 1) { val= 0; - }else if(h->ref_count[list]==2){ + } else if (rc == 2) { val= get_bits1(&s->gb)^1; }else{ val= get_ue_golomb_31(&s->gb); - if(val >= h->ref_count[list]){ + if (val >= rc) { av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val); return -1; } @@ -1180,10 +1178,5 @@ decode_intra_mb: s->current_picture.f.qscale_table[mb_xy] = s->qscale; write_back_non_zero_count(h); - if(MB_MBAFF){ - h->ref_count[0] >>= 1; - h->ref_count[1] >>= 1; - } - return 0; }