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 608564 - [ffdec_cook] memory leak
[ffdec_cook] memory leak
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal normal
: 0.10.14
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 608533
Blocks:
 
 
Reported: 2010-01-30 23:22 UTC by Tim-Philipp Müller
Modified: 2010-02-01 18:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Valgrind log with rtpsrc leak. (95.17 KB, text/plain)
2010-02-01 14:07 UTC, Massimiliano
  Details
possible patch (1.95 KB, patch)
2010-02-01 15:51 UTC, Wim Taymans
committed Details | Review

Description Tim-Philipp Müller 2010-01-30 23:22:46 UTC
+++ This bug was initially created as a clone of Bug #608533 +++

This seems to leak memory in gst-ffmpeg:

  gst-launch uridecodebin uri=rtsp://live.media.rai.it/broadcast/radiodue.rm ! fakesink


The following patch seems to fix it, but I've got a feeling it's not the right fix. I don't quite understand the comment - maybe these buffers (or GST_BUFFER_DATA) are supposed to be put in a free list somewhere and freed on state change to READY later?

diff --git a/ext/ffmpeg/gstffmpegdec.c b/ext/ffmpeg/gstffmpegdec.c
index 0b6d621..1e612f9 100644
--- a/ext/ffmpeg/gstffmpegdec.c
+++ b/ext/ffmpeg/gstffmpegdec.c
@@ -2070,6 +2070,7 @@ gst_ffmpegdec_audio_frame (GstFFMpegDec * ffmpegdec,
       goto clipped;
 
   } else if (len > 0 && have_data == 0) {
+    gst_buffer_unref (*outbuf);
     /* cache output, because it may be used for caching (in-place) */
     *outbuf = NULL;
   } else {
Comment 1 Wim Taymans 2010-02-01 12:11:49 UTC
This seems to leak as well, suggesting a leak in rtspsrc or the RDT manager:

gst-launch rtspsrc location=rtsp://live.media.rai.it/broadcast/radiodue.rm ! fakesink
Comment 2 Tim-Philipp Müller 2010-02-01 12:15:46 UTC
> This seems to leak as well, suggesting a leak in rtspsrc or the RDT manager:
> 
> gst-launch rtspsrc location=rtsp://live.media.rai.it/broadcast/radiodue.rm !
> fakesink

Even with my recent fixes in -ugly?
Comment 3 Massimiliano 2010-02-01 14:06:09 UTC
Tim: it seems so. I've run the chain under valgrind for a couple of minutes and I'm going to attach relevant valgrind log. The command used for testing was:

G_SLICE=always-malloc G_DEBUG=gc-friendly GLIBCPP_FORCE_NEW=1 GLIBCXX_FORCE_NEW=1 valgrind --tool=memcheck --leak-check=full --leak-resolution=high    --trace-children=yes --num-callers=20 --log-file=testrun.txt -v  gst-launch rtspsrc location=rtsp://live.media.rai.it/broadcast/radiodue.rm ! fakesink
Comment 4 Massimiliano 2010-02-01 14:07:06 UTC
Created attachment 152730 [details]
Valgrind log with rtpsrc leak.
Comment 5 Tim-Philipp Müller 2010-02-01 14:17:22 UTC
> Created an attachment (id=152730) [details]
> Valgrind log with rtpsrc leak.

99% of that is noise (GObject one-time allocations or somesuch). You may be able to filter it out using the very latest gst.supp from the common module:

http://cgit.freedesktop.org/gstreamer/common/tree/gst.supp?id=HEAD
Comment 6 Wim Taymans 2010-02-01 14:26:07 UTC
With the recent -ugly fixes it seems to only leak the first buffer in gst-ffmpeg (for which your patch makes sense). It seems to leak a little more somewhere...
Comment 7 Wim Taymans 2010-02-01 14:27:46 UTC
commit 8312a8f89d067e065c22ad80d7efbac96d37b372
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Feb 1 15:25:11 2010 +0100

    ffdec: free audio buffer when not decoded
    
    When we don't decode an audio frame (for audio codecs that need a previous audio
    frame) free the buffer we allocated.
    
    See #608564
Comment 8 Massimiliano 2010-02-01 14:32:15 UTC
Tim: Even after filter as you suggested, there remains those 8 blocks lost. Those are at the moment the most I was able to reproduce here. Relevant part for another valgrind run:

==5763== 8 bytes in 2 blocks are definitely lost in loss record 631 of 2,285
==5763==    at 0x4024C1C: malloc (vg_replace_malloc.c:195)
==5763==    by 0x4024CA6: realloc (vg_replace_malloc.c:476)
==5763==    by 0x41901DE: g_realloc (gmem.c:170)
==5763==    by 0x41AB1DE: g_string_maybe_expand (gstring.c:361)
==5763==    by 0x41AC2DC: g_string_sized_new (gstring.c:386)
==5763==    by 0x4C0EB1A: rtsp_ext_real_get_transports (rtspreal.c:69)
==5763==    by 0x4B35548: gst_rtsp_extension_get_transports (gstrtspextension.c:180)
==5763==    by 0x4B0BD8C: gst_rtsp_ext_list_get_transports (gstrtspext.c:219)
==5763==    by 0x4B00A19: gst_rtspsrc_setup_streams (gstrtspsrc.c:4088)
==5763==    by 0x4B05D6C: gst_rtspsrc_open (gstrtspsrc.c:4787)
==5763==    by 0x4B067CC: gst_rtspsrc_change_state (gstrtspsrc.c:5418)
==5763==    by 0x4079304: gst_element_change_state (gstelement.c:2548)
==5763==    by 0x407CB83: gst_element_set_state_func (gstelement.c:2504)
==5763==    by 0x407855F: gst_element_set_state (gstelement.c:2405)
==5763==    by 0x406725E: gst_bin_change_state_func (gstbin.c:2118)
==5763==    by 0x409E0BA: gst_pipeline_change_state (gstpipeline.c:462)
==5763==    by 0x4079304: gst_element_change_state (gstelement.c:2548)
==5763==    by 0x4079091: gst_element_continue_state (gstelement.c:2222)
==5763==    by 0x407938E: gst_element_change_state (gstelement.c:2585)
==5763==    by 0x407CB83: gst_element_set_state_func (gstelement.c:2504)
Comment 9 Wim Taymans 2010-02-01 15:51:04 UTC
Created attachment 152735 [details] [review]
possible patch

This patch removes the last 'definitly lost' leak found by valgrind.

After this valgrind seems to only report cached stuff.
Comment 10 Wim Taymans 2010-02-01 18:36:24 UTC
commit c35a9848014559ec13d7dbe6af41d216367039b9
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Feb 1 16:46:36 2010 +0100

    rtspsrc: free transports on errors
    
    See #608564
Comment 11 Wim Taymans 2010-02-01 18:38:50 UTC
with the fixes in ffmpeg, rdtdepay and rtspsrc, this should be fixed now.  Moving to -ugly as that is where the biggest leak was.