diff mbox series

[FFmpeg-devel] pthread_frame: uninit the hwaccel of each frame thread

Message ID NXmWSIQ--3-9@lynne.ee
State New
Headers show
Series [FFmpeg-devel] pthread_frame: uninit the hwaccel of each frame thread | expand

Checks

Context Check Description
yinshiyou/configure_loongarch64 warning Failed to apply patch
andriy/configure_x86 warning Failed to apply patch

Commit Message

Lynne June 13, 2023, 2:11 a.m. UTC
The issue is that with a threadsafe hwaccel and multiple enabled
frame threads, hwaccel->uninit() is never called.
Previously, the function was guaranteed to never have any threads
with hwaccel contexts, so it never bothered to uninit it.

Patch attached.

---
--

Comments

Lynne June 14, 2023, 4:39 p.m. UTC | #1
Jun 13, 2023, 04:11 by dev@lynne.ee:

> The issue is that with a threadsafe hwaccel and multiple enabled
> frame threads, hwaccel->uninit() is never called.
> Previously, the function was guaranteed to never have any threads
> with hwaccel contexts, so it never bothered to uninit it.
>
> Patch attached.
>
> ---
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 28335231fd..bdc1718ab3 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -751,6 +751,8 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
>              if (codec->close && p->thread_init != UNINITIALIZED)
>                  codec->close(ctx);
>  
> +            ff_hwaccel_uninit(ctx);
> +
>              if (ctx->priv_data) {
>                  if (codec->p.priv_class)
>                      av_opt_free(ctx->priv_data);
> -- 
>

Ping. Tested it, haven't had issues with this.
Anton Khirnov June 15, 2023, 6:26 p.m. UTC | #2
Quoting Lynne (2023-06-13 04:11:35)
> The issue is that with a threadsafe hwaccel and multiple enabled
> frame threads, hwaccel->uninit() is never called.
> Previously, the function was guaranteed to never have any threads
> with hwaccel contexts, so it never bothered to uninit it.
> 
> Patch attached.

Maybe add a note that this only does something for threadsafe hwaccels,
since otherwise hwaccel state is always cleared here.

Otherwise looks good assuming it was tested with a leak checker.
Lynne June 15, 2023, 8:03 p.m. UTC | #3
Jun 15, 2023, 20:26 by anton@khirnov.net:

> Quoting Lynne (2023-06-13 04:11:35)
>
>> The issue is that with a threadsafe hwaccel and multiple enabled
>> frame threads, hwaccel->uninit() is never called.
>> Previously, the function was guaranteed to never have any threads
>> with hwaccel contexts, so it never bothered to uninit it.
>>
>> Patch attached.
>>
>
> Maybe add a note that this only does something for threadsafe hwaccels,
> since otherwise hwaccel state is always cleared here.
>
> Otherwise looks good assuming it was tested with a leak checker.
>

A leak checker is how I actually found this :)
Thanks, added a note and pushed.
diff mbox series

Patch

From 64efcc4c78a6ef388b038955147946eb0fe8bd3c Mon Sep 17 00:00:00 2001
From: Lynne <dev@lynne.ee>
Date: Tue, 13 Jun 2023 04:04:28 +0200
Subject: [PATCH] pthread_frame: uninit the hwaccel of each frame thread

The issue is that with a threadsafe hwaccel and multiple enabled
frame threads, hwaccel->uninit() is never called.
Previously, the function was guaranteed to never have any threads
with hwaccel contexts, so it never bothered to uninit it.
---
 libavcodec/pthread_frame.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 28335231fd..bdc1718ab3 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -751,6 +751,8 @@  void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
             if (codec->close && p->thread_init != UNINITIALIZED)
                 codec->close(ctx);
 
+            ff_hwaccel_uninit(ctx);
+
             if (ctx->priv_data) {
                 if (codec->p.priv_class)
                     av_opt_free(ctx->priv_data);
-- 
2.40.1