> On Aug 15, 2024, at 22:02, Gnattu OC via ffmpeg-devel 
> <ffmpeg-devel@ffmpeg.org> wrote:
> 
> 
> 
>> On Aug 15, 2024, at 19:59, Zhao Zhili <quinkbl...@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 <gnatt...@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.

I see. It’s more appropriate to use a local variable if supported_props is
always released at the end of this function. On the other hand, although
supported_props is write only variable for now, it’s meant to be used
outside of this function is the future. So check and release supported_props
should be done just before being overwritten.

> 
>> 
>>> 
>>>   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-requ...@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-requ...@ffmpeg.org <mailto:ffmpeg-devel-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to