After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726814 - avvidenc: Fix leak of AVBufferRef
avvidenc: Fix leak of AVBufferRef
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 729165 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-03-21 10:58 UTC by Stian Selnes (stianse)
Modified: 2014-04-29 07:14 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch to fix the leak (2.17 KB, patch)
2014-03-21 12:50 UTC, Stian Selnes (stianse) 		committed Details | Review

Description Stian Selnes (stianse) 2014-03-21 10:58:42 UTC 
Attached is a patch to fix the following leak (using avenc_h263p):

==24185== 64 (24 direct, 40 indirect) bytes in 1 blocks are definitely lost in loss record 2,201 of 3,197
==24185==    at 0x4C2C930: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24185==    by 0x4C2CA57: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24185==    by 0x942C5AD: av_malloc (mem.c:81)
==24185==    by 0x942C6FD: av_mallocz (mem.c:207)
==24185==    by 0x9426143: av_buffer_create (buffer.c:47)
==24185==    by 0x9426483: av_buffer_realloc (buffer.c:158)
==24185==    by 0x9315219: av_new_packet (avpacket.c:71)
==24185==    by 0x9331A7E: ff_MPV_encode_picture (mpegvideo_enc.c:1501)
==24185==    by 0x933FBCC: avcodec_encode_video2 (utils.c:1314)
==24185==    by 0x92E5817: gst_ffmpegvidenc_handle_frame (gstavvidenc.c:584)
==24185==    by 0x579C690: gst_video_encoder_chain (gstvideoencoder.c:1433)
==24185==    by 0x5A3C895: gst_pad_chain_data_unchecked (gstpad.c:3809)
==24185==    by 0x5A3D547: gst_pad_push_data (gstpad.c:4042)
...
Comment 1 Thiago Sousa Santos 2014-03-21 12:19:33 UTC 
I guess you forgot to attach the patch.
Comment 2 Stian Selnes (stianse) 2014-03-21 12:50:59 UTC 
Created attachment 272559 [details] [review]
Patch to fix the leak
Comment 3 Haakon Sporsheim (ieei) 2014-04-29 06:33:36 UTC 
*** Bug 729165 has been marked as a duplicate of this bug. ***
Comment 4 Sebastian Dröge (slomo) 2014-04-29 06:38:47 UTC 
Review of attachment 272559 [details] [review]:

Looks good in general but

::: ext/libav/gstavvidenc.c
@@ +596,3 @@
 
+  if (ret < 0 || !have_data)
+    g_slice_free (AVPacket, pkt);

Shouldn't this call gst_ffmpegvidenc_free_avpacket() too, i.e. also av_packet_unref()?
Comment 5 Stian Selnes (stianse) 2014-04-29 07:04:53 UTC 
Before returning from avcodec_encode_video2() there is the following code:

    if (ret < 0 || !*got_packet_ptr)
        av_free_packet(avpkt);

which is logically the same as that test in the element. We should not call av_packet_unref() after this and it looks like the memory is freed correctly.

I agree it's slightly confusing to assume such behavior, but should be OK since it's also described in the documentation of avcodec_encode_video2().
Comment 6 Sebastian Dröge (slomo) 2014-04-29 07:14:24 UTC 
commit 6d92f18d1b3ea9dd40a4feb0f953e938895a8a83
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Apr 29 09:13:44 2014 +0200

    avaudenc: Fix leak of AVBufferRef
    
    AVPacket contains AVBufferRef which may leak unless unreffed properly.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726814

commit 245be5651003110fb954766cd74de1630fbed288
Author: Stian Selnes <stian@pexip.com>
Date:   Fri Mar 21 10:10:14 2014 +0100

    avvidenc: Fix leak of AVBufferRef
    
    AVPacket contains AVBufferRef which may leak unless unreffed properly.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726814