Skip to content

F
ffmpeg.wasm-core
Commit 2e59210e authored by Ronald S. Bultje's avatar Ronald S. Bultje Committed by Clément Bœsch
Browse files
Options

lavc/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 and Helgrind.

Original patch by Ronald, adapted with a local_ref_count by Clément,
following the suggestion of Michael Niedermayer.
Signed-off-by: 's avatarClément Bœsch <clement.boesch@smartjog.com>
parent 5cdd3b99
Hide whitespace changes
Inline Side-by-side
Showing with 28 additions and 40 deletions
libavcodec/h264_cabac.c
View file @ 2e59210e
...@@ -1864,6 +1864,7 @@ int ff_h264_decode_mb_cabac(H264Context *h) { ...@@ -1864,6 +1864,7 @@ int ff_h264_decode_mb_cabac(H264Context *h) {
int dct8x8_allowed= h->pps.transform_8x8_mode; int dct8x8_allowed= h->pps.transform_8x8_mode;
int decode_chroma = h->sps.chroma_format_idc == 1 || h->sps.chroma_format_idc == 2; int decode_chroma = h->sps.chroma_format_idc == 1 || h->sps.chroma_format_idc == 2;
const int pixel_shift = h->pixel_shift; const int pixel_shift = h->pixel_shift;
unsigned local_ref_count[2];
mb_xy = h->mb_xy = s->mb_x + s->mb_y*s->mb_stride; mb_xy = h->mb_xy = s->mb_x + s->mb_y*s->mb_stride;
...@@ -2004,10 +2005,8 @@ decode_intra_mb: ...@@ -2004,10 +2005,8 @@ decode_intra_mb:
return 0; return 0;
} }
if(MB_MBAFF){ local_ref_count[0] = h->ref_count[0] << MB_MBAFF;
h->ref_count[0] <<= 1; local_ref_count[1] = h->ref_count[1] << MB_MBAFF;
h->ref_count[1] <<= 1;
}
fill_decode_caches(h, mb_type); fill_decode_caches(h, mb_type);
...@@ -2077,10 +2076,10 @@ decode_intra_mb: ...@@ -2077,10 +2076,10 @@ decode_intra_mb:
for( i = 0; i < 4; i++ ) { for( i = 0; i < 4; i++ ) {
if(IS_DIRECT(h->sub_mb_type[i])) continue; if(IS_DIRECT(h->sub_mb_type[i])) continue;
if(IS_DIR(h->sub_mb_type[i], 0, list)){ if(IS_DIR(h->sub_mb_type[i], 0, list)){
if( h->ref_count[list] > 1 ){ if (local_ref_count[list] > 1) {
ref[list][i] = decode_cabac_mb_ref( h, list, 4*i ); ref[list][i] = decode_cabac_mb_ref( h, list, 4*i );
if(ref[list][i] >= (unsigned)h->ref_count[list]){ if (ref[list][i] >= (unsigned)local_ref_count[list]) {
av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref[list][i], h->ref_count[list]); av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref[list][i], local_ref_count[list]);
return -1; return -1;
} }
}else }else
...@@ -2163,10 +2162,10 @@ decode_intra_mb: ...@@ -2163,10 +2162,10 @@ decode_intra_mb:
for(list=0; list<h->list_count; list++){ for(list=0; list<h->list_count; list++){
if(IS_DIR(mb_type, 0, list)){ if(IS_DIR(mb_type, 0, list)){
int ref; int ref;
if(h->ref_count[list] > 1){ if (local_ref_count[list] > 1) {
ref= decode_cabac_mb_ref(h, list, 0); ref= decode_cabac_mb_ref(h, list, 0);
if(ref >= (unsigned)h->ref_count[list]){ if (ref >= (unsigned)local_ref_count[list]) {
av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, h->ref_count[list]); av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, local_ref_count[list]);
return -1; return -1;
} }
}else }else
...@@ -2191,10 +2190,10 @@ decode_intra_mb: ...@@ -2191,10 +2190,10 @@ decode_intra_mb:
for(i=0; i<2; i++){ for(i=0; i<2; i++){
if(IS_DIR(mb_type, i, list)){ if(IS_DIR(mb_type, i, list)){
int ref; int ref;
if(h->ref_count[list] > 1){ if (local_ref_count[list] > 1) {
ref= decode_cabac_mb_ref( h, list, 8*i ); ref= decode_cabac_mb_ref( h, list, 8*i );
if(ref >= (unsigned)h->ref_count[list]){ if (ref >= (unsigned)local_ref_count[list]) {
av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, h->ref_count[list]); av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, local_ref_count[list]);
return -1; return -1;
} }
}else }else
...@@ -2226,10 +2225,10 @@ decode_intra_mb: ...@@ -2226,10 +2225,10 @@ decode_intra_mb:
for(i=0; i<2; i++){ for(i=0; i<2; i++){
if(IS_DIR(mb_type, i, list)){ //FIXME optimize if(IS_DIR(mb_type, i, list)){ //FIXME optimize
int ref; int ref;
if(h->ref_count[list] > 1){ if (local_ref_count[list] > 1) {
ref= decode_cabac_mb_ref( h, list, 4*i ); ref= decode_cabac_mb_ref( h, list, 4*i );
if(ref >= (unsigned)h->ref_count[list]){ if (ref >= (unsigned)local_ref_count[list]) {
av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, h->ref_count[list]); av_log(s->avctx, AV_LOG_ERROR, "Reference %d >= %d\n", ref, local_ref_count[list]);
return -1; return -1;
} }
}else }else
...@@ -2402,10 +2401,5 @@ decode_intra_mb: ...@@ -2402,10 +2401,5 @@ decode_intra_mb:
s->current_picture.f.qscale_table[mb_xy] = s->qscale; s->current_picture.f.qscale_table[mb_xy] = s->qscale;
write_back_non_zero_count(h); write_back_non_zero_count(h);
if(MB_MBAFF){
h->ref_count[0] >>= 1;
h->ref_count[1] >>= 1;
}
return 0; return 0;
} }
libavcodec/h264_cavlc.c
View file @ 2e59210e
...@@ -700,6 +700,7 @@ int ff_h264_decode_mb_cavlc(H264Context *h){ ...@@ -700,6 +700,7 @@ int ff_h264_decode_mb_cavlc(H264Context *h){
int dct8x8_allowed= h->pps.transform_8x8_mode; int dct8x8_allowed= h->pps.transform_8x8_mode;
int decode_chroma = h->sps.chroma_format_idc == 1 || h->sps.chroma_format_idc == 2; int decode_chroma = h->sps.chroma_format_idc == 1 || h->sps.chroma_format_idc == 2;
const int pixel_shift = h->pixel_shift; const int pixel_shift = h->pixel_shift;
unsigned local_ref_count[2];
mb_xy = h->mb_xy = s->mb_x + s->mb_y*s->mb_stride; mb_xy = h->mb_xy = s->mb_x + s->mb_y*s->mb_stride;
...@@ -785,10 +786,8 @@ decode_intra_mb: ...@@ -785,10 +786,8 @@ decode_intra_mb:
return 0; return 0;
} }
if(MB_MBAFF){ local_ref_count[0] = h->ref_count[0] << MB_MBAFF;
h->ref_count[0] <<= 1; local_ref_count[1] = h->ref_count[1] << MB_MBAFF;
h->ref_count[1] <<= 1;
}
fill_decode_neighbors(h, mb_type); fill_decode_neighbors(h, mb_type);
fill_decode_caches(h, mb_type); fill_decode_caches(h, mb_type);
...@@ -869,7 +868,7 @@ decode_intra_mb: ...@@ -869,7 +868,7 @@ decode_intra_mb:
} }
for(list=0; list<h->list_count; list++){ 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 : local_ref_count[list];
for(i=0; i<4; i++){ for(i=0; i<4; i++){
if(IS_DIRECT(h->sub_mb_type[i])) continue; if(IS_DIRECT(h->sub_mb_type[i])) continue;
if(IS_DIR(h->sub_mb_type[i], 0, list)){ if(IS_DIR(h->sub_mb_type[i], 0, list)){
...@@ -949,13 +948,13 @@ decode_intra_mb: ...@@ -949,13 +948,13 @@ decode_intra_mb:
for(list=0; list<h->list_count; list++){ for(list=0; list<h->list_count; list++){
unsigned int val; unsigned int val;
if(IS_DIR(mb_type, 0, list)){ if(IS_DIR(mb_type, 0, list)){
if(h->ref_count[list]==1){ if(local_ref_count[list]==1){
val= 0; val= 0;
}else if(h->ref_count[list]==2){ }else if(local_ref_count[list]==2){
val= get_bits1(&s->gb)^1; val= get_bits1(&s->gb)^1;
}else{ }else{
val= get_ue_golomb_31(&s->gb); val= get_ue_golomb_31(&s->gb);
if(val >= h->ref_count[list]){ if(val >= local_ref_count[list]){
av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val); av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
return -1; return -1;
} }
...@@ -979,13 +978,13 @@ decode_intra_mb: ...@@ -979,13 +978,13 @@ decode_intra_mb:
for(i=0; i<2; i++){ for(i=0; i<2; i++){
unsigned int val; unsigned int val;
if(IS_DIR(mb_type, i, list)){ if(IS_DIR(mb_type, i, list)){
if(h->ref_count[list] == 1){ if(local_ref_count[list] == 1){
val= 0; val= 0;
}else if(h->ref_count[list] == 2){ }else if(local_ref_count[list] == 2){
val= get_bits1(&s->gb)^1; val= get_bits1(&s->gb)^1;
}else{ }else{
val= get_ue_golomb_31(&s->gb); val= get_ue_golomb_31(&s->gb);
if(val >= h->ref_count[list]){ if(val >= local_ref_count[list]){
av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val); av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
return -1; return -1;
} }
...@@ -1016,13 +1015,13 @@ decode_intra_mb: ...@@ -1016,13 +1015,13 @@ decode_intra_mb:
for(i=0; i<2; i++){ for(i=0; i<2; i++){
unsigned int val; unsigned int val;
if(IS_DIR(mb_type, i, list)){ //FIXME optimize if(IS_DIR(mb_type, i, list)){ //FIXME optimize
if(h->ref_count[list]==1){ if(local_ref_count[list]==1){
val= 0; val= 0;
}else if(h->ref_count[list]==2){ }else if(local_ref_count[list]==2){
val= get_bits1(&s->gb)^1; val= get_bits1(&s->gb)^1;
}else{ }else{
val= get_ue_golomb_31(&s->gb); val= get_ue_golomb_31(&s->gb);
if(val >= h->ref_count[list]){ if(val >= local_ref_count[list]){
av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val); av_log(h->s.avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
return -1; return -1;
} }
...@@ -1162,10 +1161,5 @@ decode_intra_mb: ...@@ -1162,10 +1161,5 @@ decode_intra_mb:
s->current_picture.f.qscale_table[mb_xy] = s->qscale; s->current_picture.f.qscale_table[mb_xy] = s->qscale;
write_back_non_zero_count(h); write_back_non_zero_count(h);
if(MB_MBAFF){
h->ref_count[0] >>= 1;
h->ref_count[1] >>= 1;
}
return 0; return 0;
} }
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