| Message ID | 20220903183930.9223-2-anton@khirnov.net |
|---|---|
| State | Accepted |
| Commit | cc867f2c09d2b69cee8a0eccd62aff002cbbfe11 |
| Headers | show |
| Series | [FFmpeg-devel,1/2] lavc: fix and extend AVCodecContext.hwaccel_context doxy | 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 |
> > > av_packet_unref(p->avpkt); > @@ -655,6 +670,14 @@ void ff_thread_finish_setup(AVCodecContext *avctx) { > async_lock(p->parent); > } > > + /* save hwaccel state for passing to the next thread; > + * this is done here so that this worker thread can wipe its own > hwaccel > + * state after decoding, without requiring synchronization */ > + av_assert0(!p->parent->stash_hwaccel); > + p->parent->stash_hwaccel = avctx->hwaccel; > + p->parent->stash_hwaccel_context = avctx->hwaccel_context; > + p->parent->stash_hwaccel_priv = avctx->internal->hwaccel_priv_data; > Assertion failure when seeking. Step to reproduce: ./ffmpeg -stream_loop -1 -an -hwaccel vaapi -i test.mp4 -f null >/dev/null Regards
Wang Bin <wbsecg1@gmail.com> 于2022年9月12日周一 10:02写道： > >> av_packet_unref(p->avpkt); >> @@ -655,6 +670,14 @@ void ff_thread_finish_setup(AVCodecContext *avctx) { >> async_lock(p->parent); >> } >> >> + /* save hwaccel state for passing to the next thread; >> + * this is done here so that this worker thread can wipe its own >> hwaccel >> + * state after decoding, without requiring synchronization */ >> + av_assert0(!p->parent->stash_hwaccel); >> + p->parent->stash_hwaccel = avctx->hwaccel; >> + p->parent->stash_hwaccel_context = avctx->hwaccel_context; >> + p->parent->stash_hwaccel_priv = >> avctx->internal->hwaccel_priv_data; >> > > Assertion failure when seeking. Step to reproduce: > ./ffmpeg -stream_loop -1 -an -hwaccel vaapi -i test.mp4 -f null >/dev/null > > Regards > > The change breaks hwaccel on all platforms. videotoolbox has another crash. Regards
On 12 Sep 2022, at 8:59, Wang Bin wrote: > Wang Bin <wbsecg1@gmail.com> 于2022年9月12日周一 10:02写道： > >> >>> av_packet_unref(p->avpkt); >>> @@ -655,6 +670,14 @@ void ff_thread_finish_setup(AVCodecContext *avctx) { >>> async_lock(p->parent); >>> } >>> >>> + /* save hwaccel state for passing to the next thread; >>> + * this is done here so that this worker thread can wipe its own >>> hwaccel >>> + * state after decoding, without requiring synchronization */ >>> + av_assert0(!p->parent->stash_hwaccel); >>> + p->parent->stash_hwaccel = avctx->hwaccel; >>> + p->parent->stash_hwaccel_context = avctx->hwaccel_context; >>> + p->parent->stash_hwaccel_priv = >>> avctx->internal->hwaccel_priv_data; >>> >> >> Assertion failure when seeking. Step to reproduce: >> ./ffmpeg -stream_loop -1 -an -hwaccel vaapi -i test.mp4 -f null >/dev/null >> >> Regards >> >> > > The change breaks hwaccel on all platforms. videotoolbox has another crash. > Can confirm this breaks videotoolbox, to reproduce simply do: ./ffmpeg -v debug -hwaccel videotoolbox -i ~/Downloads/HDR\ Mix\ 8k\ Dolby\ Vision-sLs6yYYDpTw.webm -f null /dev/null Trace: * thread #30, name = 'com.apple.coremedia.rootQueue.47', queue = 'vtdecoder-callback-queue-0x10331f520', stop reason = EXC_BAD_ACCESS (code=1, address=0x10) frame #0: 0x0000000100d376b8 ffmpeg_g`videotoolbox_decoder_callback(opaque=0x0000000108c079c0, sourceFrameRefCon=0x0000000000000000, status=0, flags=0, image_buffer=0x0000600003300000, pts=CMTime @ 0x0000700005db5a00, duration=0 seconds) at videotoolbox.c:696:16 [opt] 693 AVCodecContext *avctx = opaque; 694 VTContext *vtctx = avctx->internal->hwaccel_priv_data; 695 -> 696 if (vtctx->frame) { 697 CVPixelBufferRelease(vtctx->frame); 698 vtctx->frame = NULL; 699 } Target 0: (ffmpeg_g) stopped. The hwaccel_priv_data is NULL here. It works fine when using -threads 1. > Regards > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 08a6f98898..066269621d 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -148,6 +148,12 @@ typedef struct FrameThreadContext { * Set for the first N packets, where N is the number of threads. * While it is set, ff_thread_en/decode_frame won't return any results. */ + + /* hwaccel state is temporarily stored here in order to transfer its ownership + * to the next decoding thread without the need for extra synchronization */ + const AVHWAccel *stash_hwaccel; + void *stash_hwaccel_context; + void *stash_hwaccel_priv; } FrameThreadContext; #if FF_API_THREAD_SAFE_CALLBACKS @@ -228,9 +234,17 @@ FF_ENABLE_DEPRECATION_WARNINGS ff_thread_finish_setup(avctx); if (p->hwaccel_serializing) { + /* wipe hwaccel state to avoid stale pointers lying around; + * the state was transferred to FrameThreadContext in + * ff_thread_finish_setup(), so nothing is leaked */ + avctx->hwaccel = NULL; + avctx->hwaccel_context = NULL; + avctx->internal->hwaccel_priv_data = NULL; + p->hwaccel_serializing = 0; pthread_mutex_unlock(&p->parent->hwaccel_mutex); } + av_assert0(!avctx->hwaccel); if (p->async_serializing) { p->async_serializing = 0; @@ -294,9 +308,6 @@ static int update_context_from_thread(AVCodecContext *dst, AVCodecContext *src, dst->color_range = src->color_range; dst->chroma_sample_location = src->chroma_sample_location; - dst->hwaccel = src->hwaccel; - dst->hwaccel_context = src->hwaccel_context; - dst->sample_rate = src->sample_rate; dst->sample_fmt = src->sample_fmt; #if FF_API_OLD_CHANNEL_LAYOUT @@ -309,8 +320,6 @@ FF_ENABLE_DEPRECATION_WARNINGS if (err < 0) return err; - dst->internal->hwaccel_priv_data = src->internal->hwaccel_priv_data; - if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx || (dst->hw_frames_ctx && dst->hw_frames_ctx->data != src->hw_frames_ctx->data)) { av_buffer_unref(&dst->hw_frames_ctx); @@ -450,6 +459,12 @@ static int submit_packet(PerThreadContext *p, AVCodecContext *user_avctx, pthread_mutex_unlock(&p->mutex); return err; } + + /* transfer hwaccel state stashed from previous thread, if any */ + av_assert0(!p->avctx->hwaccel); + FFSWAP(const AVHWAccel*, p->avctx->hwaccel, fctx->stash_hwaccel); + FFSWAP(void*, p->avctx->hwaccel_context, fctx->stash_hwaccel_context); + FFSWAP(void*, p->avctx->internal->hwaccel_priv_data, fctx->stash_hwaccel_priv); } av_packet_unref(p->avpkt); @@ -655,6 +670,14 @@ void ff_thread_finish_setup(AVCodecContext *avctx) { async_lock(p->parent); } + /* save hwaccel state for passing to the next thread; + * this is done here so that this worker thread can wipe its own hwaccel + * state after decoding, without requiring synchronization */ + av_assert0(!p->parent->stash_hwaccel); + p->parent->stash_hwaccel = avctx->hwaccel; + p->parent->stash_hwaccel_context = avctx->hwaccel_context; + p->parent->stash_hwaccel_priv = avctx->internal->hwaccel_priv_data; + pthread_mutex_lock(&p->progress_mutex); if(atomic_load(&p->state) == STATE_SETUP_FINISHED){ av_log(avctx, AV_LOG_WARNING, "Multiple ff_thread_finish_setup() calls\n"); @@ -708,13 +731,6 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) park_frame_worker_threads(fctx, thread_count); - if (fctx->prev_thread && avctx->internal->hwaccel_priv_data != - fctx->prev_thread->avctx->internal->hwaccel_priv_data) { - if (update_context_from_thread(avctx, fctx->prev_thread->avctx, 1) < 0) { - av_log(avctx, AV_LOG_ERROR, "Failed to update user thread.\n"); - } - } - for (i = 0; i < thread_count; i++) { PerThreadContext *p = &fctx->threads[i]; AVCodecContext *ctx = p->avctx; @@ -761,6 +777,13 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) av_freep(&fctx->threads); ff_pthread_free(fctx, thread_ctx_offsets); + /* if we have stashed hwaccel state, move it to the user-facing context, + * so it will be freed in avcodec_close() */ + av_assert0(!avctx->hwaccel); + FFSWAP(const AVHWAccel*, avctx->hwaccel, fctx->stash_hwaccel); + FFSWAP(void*, avctx->hwaccel_context, fctx->stash_hwaccel_context); + FFSWAP(void*, avctx->internal->hwaccel_priv_data, fctx->stash_hwaccel_priv); + av_freep(&avctx->internal->thread_ctx); }