diff mbox series

[FFmpeg-devel] fix memory leak in qsvenc.c

Message ID CAAE7vKc=Kosha2FR4wMO-tFU6Rxgqa-7=nGX_KQHtvOvkaLB-g@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] fix memory leak in qsvenc.c | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Alex Pokotilo Aug. 13, 2020, 2:14 a.m. UTC 
Subject: [PATCH] fix memory leak in qsvenc.c

"ff_qsv_enc_close" function called at the end of qsv encoding session and it frees q->async_fifo content content. "bs" item has two linked pointers and they should be freed the same way as in ff_qsv_encode.
To reproduce just close h264 encoding session with pending frames
---
 libavcodec/qsvenc.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Max Dmitrichenko Aug. 13, 2020, 9:26 a.m. UTC | #1 
make sense.

regards
Max

On Thu, Aug 13, 2020 at 4:15 AM Alex Pokotilo <support@wmspanel.com> wrote:

>
> _______________________________________________
> 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".
Linjie Fu Aug. 14, 2020, 9:02 a.m. UTC | #2 
On Thu, Aug 13, 2020 at 10:15 AM Alex Pokotilo <support@wmspanel.com> wrote:
>
>
> _______________________________________________
> 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".

Seems reasonable, any special reason to reorder av_freep(&sync)?

- Linjie
Xiang, Haihao Aug. 21, 2020, 1:59 a.m. UTC | #3 
Thanks for fixing the memory leak in qsv, it looks good to me. 

Regards
Haihao


> _______________________________________________
> 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".
Alex Pokotilo Aug. 21, 2020, 6:44 a.m. UTC | #4 
>  Seems reasonable, any special reason to reorder av_freep(&sync)?
I want to combine "bs" and it's children structures disposal in the same
order as in

ff_qsv_encode where we have

#if QSV_VERSION_ATLEAST(1, 26)
        if (avctx->codec_id == AV_CODEC_ID_H264) {
            enc_buf = bs->ExtParam;
            enc_info = (mfxExtAVCEncodedFrameInfo *)(*bs->ExtParam);
            ff_side_data_set_encoder_stats(&new_pkt,
                enc_info->QP * FF_QP2LAMBDA, NULL, 0, pict_type);
            av_freep(&enc_info);
            av_freep(&enc_buf);
        }
#endif
        av_freep(&bs);
        av_freep(&sync);


On Fri, Aug 21, 2020 at 11:59 AM Xiang, Haihao <haihao.xiang@intel.com>
wrote:

>
> Thanks for fixing the memory leak in qsv, it looks good to me.
>
> Thanks for review
diff mbox series

Patch

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 1ed8f5d973..9ea2c7b1b9 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -1612,6 +1612,10 @@  FF_ENABLE_DEPRECATION_WARNINGS
 int ff_qsv_enc_close(AVCodecContext *avctx, QSVEncContext *q)
 {
     QSVFrame *cur;
+#if QSV_VERSION_ATLEAST(1, 26)
+        mfxExtAVCEncodedFrameInfo *enc_info;
+        mfxExtBuffer **enc_buf;
+#endif
 
     if (q->session)
         MFXVideoENCODE_Close(q->session);
@@ -1640,8 +1644,16 @@  int ff_qsv_enc_close(AVCodecContext *avctx, QSVEncContext *q)
         av_fifo_generic_read(q->async_fifo, &sync, sizeof(sync), NULL);
         av_fifo_generic_read(q->async_fifo, &bs,   sizeof(bs),   NULL);
 
-        av_freep(&sync);
+#if QSV_VERSION_ATLEAST(1, 26)
+        if (avctx->codec_id == AV_CODEC_ID_H264) {
+            enc_buf = bs->ExtParam;
+            enc_info = (mfxExtAVCEncodedFrameInfo *)(*bs->ExtParam);
+            av_freep(&enc_info);
+            av_freep(&enc_buf);
+        }
+#endif        
         av_freep(&bs);
+        av_freep(&sync);
         av_packet_unref(&pkt);
     }
     av_fifo_free(q->async_fifo);