diff options
author | Andreas Rheinhardt <[email protected]> | 2025-06-13 10:21:13 +0200 |
---|---|---|
committer | Andreas Rheinhardt <[email protected]> | 2025-06-21 22:08:52 +0200 |
commit | c84a03d7520093074509d48b986e5940220680ce (patch) | |
tree | bfbb444cd52438f6c5dd0721ec6ab76228804b82 | |
parent | eb597d9a1e0e9aec8148a5f3c07e5665ba2029fe (diff) |
avcodec/mpegvideo: Allocate dc_val for each encoder slice
This fixes data races (which are UB) in the MPEG-4 and H.263+
encoder when predicting DC values; these encoders unconditionally
read values from the line above the current line and only check
lateron (via first_slice_line) whether said prediction can be used
at all. It will also allow to remove said checks (by setting the
entries to 1024 upon opening a new slice).
The vsynth{1,2,3,_lena}-mpeg4-thread FATE tests were affected by this:
https://fate.ffmpeg.org/report.cgi?slot=x86_64-archlinux-gcc-tsan-slices&time=20250613002615
Signed-off-by: Andreas Rheinhardt <[email protected]>
-rw-r--r-- | libavcodec/mpegvideo.c | 15 | ||||
-rw-r--r-- | libavcodec/mpegvideo_enc.c | 1 |
2 files changed, 13 insertions, 3 deletions
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c index 6c76a382cc..4701267d81 100644 --- a/libavcodec/mpegvideo.c +++ b/libavcodec/mpegvideo.c @@ -179,6 +179,7 @@ static void backup_duplicate_context(MpegEncContext *bak, MpegEncContext *src) COPY(block); COPY(start_mb_y); COPY(end_mb_y); + COPY(dc_val); COPY(ac_val); #undef COPY } @@ -235,7 +236,7 @@ av_cold int ff_mpv_init_context_frame(MpegEncContext *s) s->avctx->active_thread_type & FF_THREAD_SLICE) ? s->avctx->thread_count : 1; BufferPoolContext *const pools = &s->buffer_pools; - int y_size, c_size, yc_size, i, mb_array_size, mv_table_size, x, y; + int y_size, c_size, yc_size, mb_array_size, mv_table_size, x, y; int mb_height; if (s->encoding && s->avctx->slices) @@ -337,6 +338,9 @@ av_cold int ff_mpv_init_context_frame(MpegEncContext *s) } if (s->h263_pred || s->h263_aic || !s->encoding) { + // When encoding, each slice (and therefore each thread) + // gets its own ac_val and dc_val buffers in order to avoid + // races. size_t allslice_yc_size = yc_size * (s->encoding ? nb_slices : 1); if (s->out_format == FMT_H263) { /* ac values */ @@ -349,10 +353,15 @@ av_cold int ff_mpv_init_context_frame(MpegEncContext *s) // MN: we need these for error resilience of intra-frames // Allocating them unconditionally for decoders also means // that we don't need to reinitialize when e.g. h263_aic changes. - if (!FF_ALLOC_TYPED_ARRAY(s->dc_val_base, yc_size)) + + // y_size and therefore yc_size is always odd; allocate one element + // more for each encoder slice in order to be able to align each slice's + // dc_val to four in order to use aligned stores when cleaning dc_val. + allslice_yc_size += s->encoding * nb_slices; + if (!FF_ALLOC_TYPED_ARRAY(s->dc_val_base, allslice_yc_size)) return AVERROR(ENOMEM); s->dc_val = s->dc_val_base + s->b8_stride + 1; - for (i = 0; i < yc_size; i++) + for (size_t i = 0; i < allslice_yc_size; ++i) s->dc_val_base[i] = 1024; } diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c index b4e0099567..1f581ec500 100644 --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -545,6 +545,7 @@ static av_cold int init_slice_buffers(MPVMainEncContext *const m) } if (s2->c.ac_val) { + s2->c.dc_val += offset + i; s2->c.ac_val += offset; offset += yc_size; } |