Andreas Rheinhardt:
> Currently, ff_frame_thread_free() uses the last worker thread
> to updates the first worker thread via update_context_from_thread()
> immediately before freeing all these worker threads. This is
> a remnant of the time in which the first worker was special.
> (E.g. the first worker shared its AVCodecInternal with the public
> AVCodecContext.)
>
> But these times are over (none of the uses of is_copy matter
> for ff_frame_thread_free()); nowadays the only thing that
> update_context_from_thread() does is referencing a few
> buffers/frames and replacing them with other references instead.
> These new references will then be freed immediately thereafter
> when the first worker thread is freed. Ensuring that the code is
> free of double-frees is achieved by using reference-counted structures
> (or in case of AVChannelLayouts: by giving each worker its own copy).
>
> Some archaeology:
> a) Updating the first worker thread from the last one used
> has been done since frame-threading was added in
> 37b00b47cbeecd66bb34c5c7c534d016d6e8da24.
> b) The precursor to ff_mpv_common_end() checked for is_copy
> before freeing pictures (i.e. it only freed them for the first
> worker thread).
> c) Commits c2dfb1e37cc72bf144545c4410a4621cbff5c4b1 and
> e33811bd2686411233cb0eb4a4ee45eb99d7e736 modified the
> update_thread_context function of the H.264 decoder
> so that it could fail before calling ff_mpeg_update_thread_context().
> d) This led to a double free/an assert violation with an H.264
> sample for which ff_mpeg_update_thread_context() is not reached
> for the final update_context_from_thread(). Commit
> a6e4796fbf0aa9b13451a8ef917ecc4e80d1d272 added code to fix this
> sample.
> e) This issue was fixed (even with the last mentioned commit reverted)
> when the H.264 decoder was deMpegEncContextized in commit
> b7fe35c9e50e1701274364adf7280bf4a02b092b (merging commit
> 2c541554076cc8a72e7145d4da30389ca763f32f).
> f) mpegvideo.c stopped using is_copy when it was switched to refcounted
> frames in 759001c534287a96dc96d1e274665feb7059145d.
> g) 1f4cf92cfbd3accbae582ac63126ed5570ddfd37 removed the init_thread_copy
> callbacks; now no FFCodec.close callback checks for is_copy at all
> any more.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
> ---
> libavcodec/pthread_frame.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index c667706206..8faea75a49 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -714,13 +714,6 @@ void ff_frame_thread_free(AVCodecContext *avctx, int
> thread_count)
> }
> }
>
> - if (fctx->prev_thread && fctx->prev_thread != fctx->threads)
> - if (update_context_from_thread(fctx->threads->avctx,
> fctx->prev_thread->avctx, 0) < 0) {
> - av_log(avctx, AV_LOG_ERROR, "Final thread update failed\n");
> - fctx->prev_thread->avctx->internal->is_copy =
> fctx->threads->avctx->internal->is_copy;
> - fctx->threads->avctx->internal->is_copy = 1;
> - }
> -
> for (i = 0; i < thread_count; i++) {
> PerThreadContext *p = &fctx->threads[i];
> AVCodecContext *ctx = p->avctx;
Will apply this patch tomorrow unless there are objections.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
