Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: always release supported_props
@ 2024-08-10  6:06 gnattu via ffmpeg-devel
  2024-08-15 11:59 ` Zhao Zhili
  0 siblings, 1 reply; 3+ messages in thread
From: gnattu via ffmpeg-devel @ 2024-08-10  6:06 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: gnattu

In vtenc_populate_extradata, supported_props should always be released
to avoid memory leak.

Regression from cd2f8a22e94700c68b1de7968df11e8bebfd315b

Signed-off-by: gnattu <gnattuoc@me.com>
---
 libavcodec/videotoolboxenc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 78ef474d7ae..6cc45db4a96 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -2787,9 +2787,16 @@ pe_cleanup:
 
     if (status) {
         vtenc_reset(vtctx);
-    } else if (vtctx->session) {
-        CFRelease(vtctx->session);
-        vtctx->session = NULL;
+    } else {
+        if (vtctx->session) {
+            CFRelease(vtctx->session);
+            vtctx->session = NULL;
+        }
+
+        if (vtctx->supported_props) {
+            CFRelease(vtctx->supported_props);
+            vtctx->supported_props = NULL;
+        }
     }
 
     vtctx->frame_ct_out = 0;
-- 
2.39.3 (Apple Git-146)

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: always release supported_props
  2024-08-10  6:06 [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: always release supported_props gnattu via ffmpeg-devel
@ 2024-08-15 11:59 ` Zhao Zhili
  2024-08-15 14:02   ` Gnattu OC via ffmpeg-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Zhao Zhili @ 2024-08-15 11:59 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: gnattu



> On Aug 10, 2024, at 14:06, gnattu via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
> 
> In vtenc_populate_extradata, supported_props should always be released
> to avoid memory leak.
> 
> Regression from cd2f8a22e94700c68b1de7968df11e8bebfd315b
> 
> Signed-off-by: gnattu <gnattuoc@me.com>
> ---
> libavcodec/videotoolboxenc.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 78ef474d7ae..6cc45db4a96 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -2787,9 +2787,16 @@ pe_cleanup:
> 
>     if (status) {
>         vtenc_reset(vtctx);
> -    } else if (vtctx->session) {
> -        CFRelease(vtctx->session);
> -        vtctx->session = NULL;
> +    } else {
> +        if (vtctx->session) {
> +            CFRelease(vtctx->session);
> +            vtctx->session = NULL;
> +        }
> +
> +        if (vtctx->supported_props) {
> +            CFRelease(vtctx->supported_props);
> +            vtctx->supported_props = NULL;
> +        }
>     }

Could you elaborate on how supported_props is leaked? Isn’t it cleaned by
vtenc_close -> vtenc_reset?

> 
>     vtctx->frame_ct_out = 0;
> -- 
> 2.39.3 (Apple Git-146)
> 
> _______________________________________________
> 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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: always release supported_props
  2024-08-15 11:59 ` Zhao Zhili
@ 2024-08-15 14:02   ` Gnattu OC via ffmpeg-devel
  0 siblings, 0 replies; 3+ messages in thread
From: Gnattu OC via ffmpeg-devel @ 2024-08-15 14:02 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Gnattu OC



> On Aug 15, 2024, at 19:59, Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
> 
> 
>> On Aug 10, 2024, at 14:06, gnattu via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>> 
>> In vtenc_populate_extradata, supported_props should always be released
>> to avoid memory leak.
>> 
>> Regression from cd2f8a22e94700c68b1de7968df11e8bebfd315b
>> 
>> Signed-off-by: gnattu <gnattuoc@me.com>
>> ---
>> libavcodec/videotoolboxenc.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>> 
>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> index 78ef474d7ae..6cc45db4a96 100644
>> --- a/libavcodec/videotoolboxenc.c
>> +++ b/libavcodec/videotoolboxenc.c
>> @@ -2787,9 +2787,16 @@ pe_cleanup:
>> 
>>    if (status) {
>>        vtenc_reset(vtctx);
>> -    } else if (vtctx->session) {
>> -        CFRelease(vtctx->session);
>> -        vtctx->session = NULL;
>> +    } else {
>> +        if (vtctx->session) {
>> +            CFRelease(vtctx->session);
>> +            vtctx->session = NULL;
>> +        }
>> +
>> +        if (vtctx->supported_props) {
>> +            CFRelease(vtctx->supported_props);
>> +            vtctx->supported_props = NULL;
>> +        }
>>    }
> 
> Could you elaborate on how supported_props is leaked? Isn’t it cleaned by
> vtenc_close -> vtenc_reset?

In the function `vtenc_populate_extradata`, a call is made to `vtenc_create_encoder` to create a `VTCompressionSession`. During this process, a `supported_props` dictionary is created. However, if the status is 0, only the compression session is released, while `supported_props` remains allocated. 

Later, in `vtenc_configure_encoder`, `vtenc_create_encoder` is called again after `vtenc_populate_extradata` returns. This call replaces the reference to `supported_props` with a new dictionary, causing the reference to the old one (created in `vtenc_populate_extradata`) to be lost. As a result, when `vtenc_close -> vtenc_reset` is called, it will only release the most recent `supported_props`, leading to a memory leak as all previous allocations are not properly released.

> 
>> 
>>    vtctx->frame_ct_out = 0;
>> -- 
>> 2.39.3 (Apple Git-146)
>> 
>> _______________________________________________
>> 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".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-08-15 14:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-10  6:06 [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: always release supported_props gnattu via ffmpeg-devel
2024-08-15 11:59 ` Zhao Zhili
2024-08-15 14:02   ` Gnattu OC via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git