From 839155324e008c582d916ce108513bd469e5bb0b Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Tue, 29 Apr 2025 23:20:49 +0200 Subject: [PATCH] avcodec/mpegvideo_dec: Move memcpy'ing ctx to mpeg4videodec.c When the destination MpegEncContext in ff_mpeg_update_thread_context() is not initialized, the source MpegEncContext is simply copied over it before (potentially) calling ff_mpv_common_init(). This leads to data races when this code is executed which is why it should be replaced with only copying the necessary fields (this is for future commits). Given that the RV30 and RV40 decoders always call said function with an already initialized MpegEncContext (they use context_reinit in case of frame size changes), they don't need this ugly initialization (and are therefore race-free). This means that this code can be moved to the only decoder that actually needs it: MPEG-4. This commit does so. Signed-off-by: Andreas Rheinhardt --- libavcodec/mpeg4videodec.c | 27 ++++++++++++++++++++++++++- libavcodec/mpegvideo_dec.c | 19 ------------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c index ac049b95fc..60b06ad750 100644 --- a/libavcodec/mpeg4videodec.c +++ b/libavcodec/mpeg4videodec.c @@ -3774,15 +3774,40 @@ int ff_mpeg4_frame_end(AVCodecContext *avctx, const AVPacket *pkt) #if CONFIG_MPEG4_DECODER #if HAVE_THREADS +static int update_mpvctx(MpegEncContext *s, const MpegEncContext *s1) +{ + AVCodecContext *avctx = s->avctx; + // FIXME the following leads to a data race; instead copy only + // the necessary fields. + memcpy(s, s1, sizeof(*s)); + + s->context_initialized = 0; + s->context_reinit = 0; + s->avctx = avctx; + + if (s1->context_initialized) { + int err = ff_mpv_common_init(s); + if (err < 0) + return err; + } + return 0; +} + static int mpeg4_update_thread_context(AVCodecContext *dst, const AVCodecContext *src) { Mpeg4DecContext *s = dst->priv_data; const Mpeg4DecContext *s1 = src->priv_data; int init = s->m.context_initialized; + int ret; - int ret = ff_mpeg_update_thread_context(dst, src); + if (!init) { + ret = update_mpvctx(&s->m, &s1->m); + if (ret < 0) + return ret; + } + ret = ff_mpeg_update_thread_context(dst, src); if (ret < 0) return ret; diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c index 75c765218c..b8b84ffd8d 100644 --- a/libavcodec/mpegvideo_dec.c +++ b/libavcodec/mpegvideo_dec.c @@ -87,25 +87,6 @@ int ff_mpeg_update_thread_context(AVCodecContext *dst, av_assert0(s != s1); - // FIXME can parameters change on I-frames? - // in that case dst may need a reinit - if (!s->context_initialized) { - void *private_ctx = s->private_ctx; - int err; - memcpy(s, s1, sizeof(*s)); - - s->context_initialized = 0; - s->context_reinit = 0; - s->avctx = dst; - s->private_ctx = private_ctx; - - if (s1->context_initialized) { - if ((err = ff_mpv_common_init(s)) < 0) - return err; - ret = 1; - } - } - if (s->height != s1->height || s->width != s1->width || s->context_reinit) { s->height = s1->height; s->width = s1->width;