[FFmpeg-devel] [PATCH] h264: don't touch H264Context->ref_count[] during MB decoding.
Ronald S. Bultje
rsbultje at gmail.com
Thu Oct 4 01:25:14 CEST 2012
From: "Ronald S. Bultje" <rsbultje at gmail.com>
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.
---
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 7e8947b..97e8128 100644
--- a/libavcodec/h264_cabac.c
+++ b/libavcodec/h264_cabac.c
@@ -2003,11 +2003,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 ) ) {
@@ -2076,10 +2071,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
@@ -2161,11 +2157,11 @@ decode_intra_mb:
if(IS_16X16(mb_type)){
for(list=0; list<h->list_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
@@ -2189,11 +2185,11 @@ decode_intra_mb:
for(list=0; list<h->list_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
@@ -2224,11 +2220,11 @@ decode_intra_mb:
for(list=0; list<h->list_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
@@ -2401,10 +2397,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 06e1ba7..cf3ce8f 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; list<h->list_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; list<h->list_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;
}
@@ -1161,10 +1159,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;
}
--
1.7.9.6 (Apple Git-31.1)
More information about the ffmpeg-devel
mailing list