RepoMirrors
/
ffmpeg
mirror of https://git.ffmpeg.org/ffmpeg.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

avcodec/mpeg4videodec: Move use_intra_dc_vlc to stack, fix data race 

use_intra_dc_vlc is currently kept in sync between frame threads
in mpeg4_update_thread_context(), yet it is set when decoding
blocks, i.e. after ff_thread_finish_setup(). This is a data race
and therefore undefined behaviour.

This race can be fixed easily by moving the variable from the context
to the stack: use_intra_dc_vlc is only read in
mpeg4_decode_block() and only if one is decoding an intra block.
There are three callsites for this function: One in
mpeg4_decode_partitioned_mb() which always sets use_intra_dc_vlc
before the call and two in mpeg4_decode_mb(). One of these callsites
is for intra blocks and use_intra_dc_vlc is set before it;
the last callsite is for non-intra blocks, where use_intra_dc_vlc
is ignored. So if it is used, it always uses a new value and can
therefore be moved to the stack.

The above also explains why this data race did not lead to
FATE-test failures.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This commit is contained in:
Andreas Rheinhardt 2022-01-26 21:29:42 +01:00
parent 24a654c6c9
commit 577ba4a1af
2 changed files with 14 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
libavcodec/mpeg4video.h
View File

@ -94,7 +94,6 @@ typedef struct Mpeg4DecContext {
int new_pred;
int enhancement_type;
int scalability;
int use_intra_dc_vlc;
/// QP above which the ac VLC should be used for intra dc
int intra_dc_threshold;

24
libavcodec/mpeg4videodec.c
View File

@ -1105,7 +1105,8 @@ int ff_mpeg4_decode_partitions(Mpeg4DecContext *ctx)
* @return <0 if an error occurred
*/
static inline int mpeg4_decode_block(Mpeg4DecContext *ctx, int16_t *block,
int n, int coded, int intra, int rvlc)
int n, int coded, int intra,
int use_intra_dc_vlc, int rvlc)
{
MpegEncContext *s = &ctx->m;
int level, i, last, run, qmul, qadd;
@ -1117,7 +1118,7 @@ static inline int mpeg4_decode_block(Mpeg4DecContext *ctx, int16_t *block,
// Note intra & rvlc should be optimized away if this is inlined
if (intra) {
if (ctx->use_intra_dc_vlc) {
if (use_intra_dc_vlc) {
/* DC coef */
if (s->partitioned_frame) {
level = s->dc_val[0][s->block_index[n]];
@ -1357,7 +1358,7 @@ static inline int mpeg4_decode_block(Mpeg4DecContext *ctx, int16_t *block,
not_coded:
if (intra) {
if (!ctx->use_intra_dc_vlc) {
if (!use_intra_dc_vlc) {
block[0] = ff_mpeg4_pred_dc(s, n, block[0], &dc_pred_dir, 0);
i -= i >> 31; // if (i == -1) i = 0;
@ -1378,7 +1379,7 @@ not_coded:
static int mpeg4_decode_partitioned_mb(MpegEncContext *s, int16_t block[6][64])
{
Mpeg4DecContext *ctx = s->avctx->priv_data;
int cbp, mb_type;
int cbp, mb_type, use_intra_dc_vlc;
const int xy = s->mb_x + s->mb_y * s->mb_stride;
av_assert2(s == (void*)ctx);
@ -1386,7 +1387,7 @@ static int mpeg4_decode_partitioned_mb(MpegEncContext *s, int16_t block[6][64])
mb_type = s->current_picture.mb_type[xy];
cbp = s->cbp_table[xy];
ctx->use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold;
use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold;
if (s->current_picture.qscale_table[xy] != s->qscale)
ff_set_qscale(s, s->current_picture.qscale_table[xy]);
@ -1436,7 +1437,8 @@ static int mpeg4_decode_partitioned_mb(MpegEncContext *s, int16_t block[6][64])
s->bdsp.clear_blocks(s->block[0]);
/* decode each block */
for (i = 0; i < 6; i++) {
if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, s->mb_intra, ctx->rvlc) < 0) {
if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, s->mb_intra,
use_intra_dc_vlc, ctx->rvlc) < 0) {
av_log(s->avctx, AV_LOG_ERROR,
"texture corrupted at %d %d %d\n",
s->mb_x, s->mb_y, s->mb_intra);
@ -1763,6 +1765,8 @@ static int mpeg4_decode_mb(MpegEncContext *s, int16_t block[6][64])
}
s->current_picture.mb_type[xy] = mb_type;
} else { /* I-Frame */
int use_intra_dc_vlc;
do {
cbpc = get_vlc2(&s->gb, ff_h263_intra_MCBPC_vlc.table, INTRA_MCBPC_VLC_BITS, 2);
if (cbpc < 0) {
@ -1790,7 +1794,7 @@ intra:
}
cbp = (cbpc & 3) | (cbpy << 2);
ctx->use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold;
use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold;
if (dquant)
ff_set_qscale(s, s->qscale + quant_tab[get_bits(&s->gb, 2)]);
@ -1801,7 +1805,8 @@ intra:
s->bdsp.clear_blocks(s->block[0]);
/* decode each block */
for (i = 0; i < 6; i++) {
if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, 1, 0) < 0)
if (mpeg4_decode_block(ctx, block[i], i, cbp & 32,
1, use_intra_dc_vlc, 0) < 0)
return AVERROR_INVALIDDATA;
cbp += cbp;
}
@ -1810,7 +1815,7 @@ intra:
/* decode each block */
for (i = 0; i < 6; i++) {
if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, 0, 0) < 0)
if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, 0, 0, 0) < 0)
return AVERROR_INVALIDDATA;
cbp += cbp;
}
@ -3529,7 +3534,6 @@ static int mpeg4_update_thread_context(AVCodecContext *dst,
s->new_pred = s1->new_pred;
s->enhancement_type = s1->enhancement_type;
s->scalability = s1->scalability;
s->use_intra_dc_vlc = s1->use_intra_dc_vlc;
s->intra_dc_threshold = s1->intra_dc_threshold;
s->divx_version = s1->divx_version;
s->divx_build = s1->divx_build;