[FFmpeg-cvslog] avcodec/pthread_frame: Fix cleanup during init
Andreas Rheinhardt
git at videolan.org
Thu Apr 8 13:53:19 EEST 2021
ffmpeg | branch: release/4.4 | Andreas Rheinhardt <andreas.rheinhardt at gmail.com> | Thu Feb 11 13:45:31 2021 +0100| [aa8f8748caa15d99a77537cbd5bbec014a6630b8] | committer: Andreas Rheinhardt
avcodec/pthread_frame: Fix cleanup during init
In case an error happened when setting up the child threads,
ff_frame_thread_init() would up until now call ff_frame_thread_free()
to clean up all threads set up so far, including the current, not
properly initialized one.
But a half-allocated context needs special handling which
ff_frame_thread_frame_free() doesn't provide.
Notably, if allocating the AVCodecInternal, the codec's private data
or setting the options fails, the codec's close function will be
called (if there is one); it will also be called if the codec's init
function fails, regardless of whether the FF_CODEC_CAP_INIT_CLEANUP
is set. This is not supported by all codecs; in ticket #9099 it led
to a crash.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
(cherry picked from commit e9b66175793e5c2af19beefe8e143f6e4901b5df)
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=aa8f8748caa15d99a77537cbd5bbec014a6630b8
---
libavcodec/pthread_frame.c | 137 ++++++++++++++++++++++-----------------------
1 file changed, 67 insertions(+), 70 deletions(-)
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 311d6ed771..db2f0cb3d2 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -64,6 +64,12 @@ enum {
STATE_SETUP_FINISHED,
};
+enum {
+ UNINITIALIZED, ///< Thread has not been created, AVCodec->close mustn't be called
+ NEEDS_CLOSE, ///< AVCodec->close needs to be called
+ INITIALIZED, ///< Thread has been properly set up
+};
+
/**
* Context used by codec threads and stored in their AVCodecInternal thread_ctx.
*/
@@ -698,27 +704,40 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
for (i = 0; i < thread_count; i++) {
PerThreadContext *p = &fctx->threads[i];
+ AVCodecContext *ctx = p->avctx;
- pthread_mutex_lock(&p->mutex);
- p->die = 1;
- pthread_cond_signal(&p->input_cond);
- pthread_mutex_unlock(&p->mutex);
-
- if (p->thread_init)
- pthread_join(p->thread, NULL);
- p->thread_init=0;
+ if (ctx->internal) {
+ if (p->thread_init == INITIALIZED) {
+ pthread_mutex_lock(&p->mutex);
+ p->die = 1;
+ pthread_cond_signal(&p->input_cond);
+ pthread_mutex_unlock(&p->mutex);
- if (codec->close && p->avctx)
- codec->close(p->avctx);
+ pthread_join(p->thread, NULL);
+ }
+ if (codec->close && p->thread_init != UNINITIALIZED)
+ codec->close(ctx);
#if FF_API_THREAD_SAFE_CALLBACKS
- release_delayed_buffers(p);
+ release_delayed_buffers(p);
+ for (int j = 0; j < p->released_buffers_allocated; j++)
+ av_frame_free(&p->released_buffers[j]);
+ av_freep(&p->released_buffers);
#endif
- av_frame_free(&p->frame);
- }
+ if (ctx->priv_data) {
+ if (codec->priv_class)
+ av_opt_free(ctx->priv_data);
+ av_freep(&ctx->priv_data);
+ }
- for (i = 0; i < thread_count; i++) {
- PerThreadContext *p = &fctx->threads[i];
+ av_freep(&ctx->slice_offset);
+
+ av_buffer_unref(&ctx->internal->pool);
+ av_freep(&ctx->internal);
+ av_buffer_unref(&ctx->hw_frames_ctx);
+ }
+
+ av_frame_free(&p->frame);
pthread_mutex_destroy(&p->mutex);
pthread_mutex_destroy(&p->progress_mutex);
@@ -727,26 +746,6 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
pthread_cond_destroy(&p->output_cond);
av_packet_free(&p->avpkt);
-#if FF_API_THREAD_SAFE_CALLBACKS
- for (int j = 0; j < p->released_buffers_allocated; j++)
- av_frame_free(&p->released_buffers[j]);
- av_freep(&p->released_buffers);
-#endif
-
- if (p->avctx) {
- if (codec->priv_class)
- av_opt_free(p->avctx->priv_data);
- av_freep(&p->avctx->priv_data);
-
- av_freep(&p->avctx->slice_offset);
- }
-
- if (p->avctx) {
- av_buffer_unref(&p->avctx->internal->pool);
- av_freep(&p->avctx->internal);
- av_buffer_unref(&p->avctx->hw_frames_ctx);
- }
-
av_freep(&p->avctx);
}
@@ -763,47 +762,37 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
avctx->codec = NULL;
}
-static av_cold int init_thread(PerThreadContext *p,
+static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
FrameThreadContext *fctx, AVCodecContext *avctx,
AVCodecContext *src, const AVCodec *codec, int first)
{
- AVCodecContext *copy = av_malloc(sizeof(AVCodecContext));
+ AVCodecContext *copy;
int err;
+ atomic_init(&p->state, STATE_INPUT_READY);
+
+ copy = av_memdup(src, sizeof(*src));
+ if (!copy)
+ return AVERROR(ENOMEM);
+ copy->priv_data = NULL;
+
+ /* From now on, this PerThreadContext will be cleaned up by
+ * ff_frame_thread_free in case of errors. */
+ (*threads_to_free)++;
+
pthread_mutex_init(&p->mutex, NULL);
pthread_mutex_init(&p->progress_mutex, NULL);
pthread_cond_init(&p->input_cond, NULL);
pthread_cond_init(&p->progress_cond, NULL);
pthread_cond_init(&p->output_cond, NULL);
- p->frame = av_frame_alloc();
- if (!p->frame) {
- av_freep(©);
- return AVERROR(ENOMEM);
- }
- p->avpkt = av_packet_alloc();
- if (!p->avpkt) {
- av_freep(©);
- return AVERROR(ENOMEM);
- }
-
- p->parent = fctx;
- p->avctx = copy;
+ p->parent = fctx;
+ p->avctx = copy;
- if (!copy) {
- return AVERROR(ENOMEM);
- }
-
- *copy = *src;
-
- copy->internal = av_malloc(sizeof(AVCodecInternal));
- if (!copy->internal) {
- copy->priv_data = NULL;
- return AVERROR(ENOMEM);
- }
- *copy->internal = *src->internal;
+ copy->internal = av_memdup(src->internal, sizeof(*src->internal));
+ if (!copy->internal)
+ return AVERROR(ENOMEM);
copy->internal->thread_ctx = p;
- copy->internal->last_pkt_props = p->avpkt;
copy->delay = avctx->delay;
@@ -821,14 +810,22 @@ static av_cold int init_thread(PerThreadContext *p,
}
}
+ if (!(p->frame = av_frame_alloc()) ||
+ !(p->avpkt = av_packet_alloc()))
+ return AVERROR(ENOMEM);
+ copy->internal->last_pkt_props = p->avpkt;
+
if (!first)
copy->internal->is_copy = 1;
if (codec->init)
err = codec->init(copy);
if (err < 0) {
+ if (codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP)
+ p->thread_init = NEEDS_CLOSE;
return err;
}
+ p->thread_init = NEEDS_CLOSE;
if (first)
update_context_from_thread(avctx, copy, 1);
@@ -836,9 +833,10 @@ static av_cold int init_thread(PerThreadContext *p,
atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0);
err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p));
- p->thread_init= !err;
- if(!p->thread_init)
- return err;
+ if (err < 0)
+ return err;
+ p->thread_init = INITIALIZED;
+
return 0;
}
@@ -885,11 +883,11 @@ int ff_frame_thread_init(AVCodecContext *avctx)
if (codec->type == AVMEDIA_TYPE_VIDEO)
avctx->delay = src->thread_count - 1;
- for (i = 0; i < thread_count; i++) {
+ for (i = 0; i < thread_count; ) {
PerThreadContext *p = &fctx->threads[i];
int first = !i;
- err = init_thread(p, fctx, avctx, src, codec, first);
+ err = init_thread(p, &i, fctx, avctx, src, codec, first);
if (err < 0)
goto error;
}
@@ -897,8 +895,7 @@ int ff_frame_thread_init(AVCodecContext *avctx)
return 0;
error:
- ff_frame_thread_free(avctx, i+1);
-
+ ff_frame_thread_free(avctx, i);
return err;
}
More information about the ffmpeg-cvslog
mailing list