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