| Message ID | 20240322202841.31730-8-anton@khirnov.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/12] tests/fate/ffmpeg: evaluate thread count in fate-run.sh rather than make | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Anton Khirnov: > It is highly unsafe, as AVCodecContext contains many allocated fields. > Copying information via AVCodecParameters and with av_opt_copy() should > handle everything needed by thread workers. > --- > libavcodec/frame_thread_encoder.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c > index cda5158117..744062b776 100644 > --- a/libavcodec/frame_thread_encoder.c > +++ b/libavcodec/frame_thread_encoder.c > @@ -28,6 +28,7 @@ > #include "libavutil/thread.h" > #include "avcodec.h" > #include "avcodec_internal.h" > +#include "codec_par.h" > #include "encode.h" > #include "internal.h" > #include "pthread_internal.h" > @@ -121,6 +122,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx) > int i=0; > ThreadContext *c; > AVCodecContext *thread_avctx = NULL; > + AVCodecParameters *par = NULL; > int ret; > > if( !(avctx->thread_type & FF_THREAD_FRAME) > @@ -194,18 +196,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx) > } > } > > + par = avcodec_parameters_alloc(); > + if (!par) { > + ret = AVERROR(ENOMEM); > + goto fail; > + } > + > + ret = avcodec_parameters_from_context(par, avctx); > + if (ret < 0) > + goto fail; > + > for(i=0; i<avctx->thread_count ; i++){ > - void *tmpv; > thread_avctx = avcodec_alloc_context3(avctx->codec); > if (!thread_avctx) { > ret = AVERROR(ENOMEM); > goto fail; > } > - tmpv = thread_avctx->priv_data; > - *thread_avctx = *avctx; > - thread_avctx->priv_data = tmpv; > - thread_avctx->internal = NULL; > - thread_avctx->hw_frames_ctx = NULL; > + > + ret = avcodec_parameters_to_context(thread_avctx, par); > + if (ret < 0) > + goto fail; > + > ret = av_opt_copy(thread_avctx, avctx); > if (ret < 0) > goto fail; > @@ -227,10 +238,13 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx) > } > } > > + avcodec_parameters_free(&par); > + > avctx->active_thread_type = FF_THREAD_FRAME; > > return 0; > fail: > + avcodec_parameters_free(&par); > ff_codec_close(thread_avctx); > av_freep(&thread_avctx); > avctx->thread_count = i; IIRC the mjpeg encoders use intra_matrix and this will now no longer be copied to the worker threads. - Andreas
On Fri, Mar 22, 2024 at 09:28:37PM +0100, Anton Khirnov wrote: > It is highly unsafe, as AVCodecContext contains many allocated fields. > Copying information via AVCodecParameters and with av_opt_copy() should > handle everything needed by thread workers. > --- > libavcodec/frame_thread_encoder.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) this breaks huffyuv with 2 pass [...]
diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c index cda5158117..744062b776 100644 --- a/libavcodec/frame_thread_encoder.c +++ b/libavcodec/frame_thread_encoder.c @@ -28,6 +28,7 @@ #include "libavutil/thread.h" #include "avcodec.h" #include "avcodec_internal.h" +#include "codec_par.h" #include "encode.h" #include "internal.h" #include "pthread_internal.h" @@ -121,6 +122,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx) int i=0; ThreadContext *c; AVCodecContext *thread_avctx = NULL; + AVCodecParameters *par = NULL; int ret; if( !(avctx->thread_type & FF_THREAD_FRAME) @@ -194,18 +196,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx) } } + par = avcodec_parameters_alloc(); + if (!par) { + ret = AVERROR(ENOMEM); + goto fail; + } + + ret = avcodec_parameters_from_context(par, avctx); + if (ret < 0) + goto fail; + for(i=0; i<avctx->thread_count ; i++){ - void *tmpv; thread_avctx = avcodec_alloc_context3(avctx->codec); if (!thread_avctx) { ret = AVERROR(ENOMEM); goto fail; } - tmpv = thread_avctx->priv_data; - *thread_avctx = *avctx; - thread_avctx->priv_data = tmpv; - thread_avctx->internal = NULL; - thread_avctx->hw_frames_ctx = NULL; + + ret = avcodec_parameters_to_context(thread_avctx, par); + if (ret < 0) + goto fail; + ret = av_opt_copy(thread_avctx, avctx); if (ret < 0) goto fail; @@ -227,10 +238,13 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx) } } + avcodec_parameters_free(&par); + avctx->active_thread_type = FF_THREAD_FRAME; return 0; fail: + avcodec_parameters_free(&par); ff_codec_close(thread_avctx); av_freep(&thread_avctx); avctx->thread_count = i;