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 730133 - motioncells:fix memleak
motioncells:fix memleak
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.x
Other All
: Normal normal
: 1.3.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-05-14 14:16 UTC by Nicola
Modified: 2014-05-14 23:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix memleak (1.04 KB, patch)
2014-05-14 14:16 UTC, Nicola
none Details | Review
fix memleak (725 bytes, patch)
2014-05-14 14:35 UTC, Nicola
none Details | Review
improved patch (1.93 KB, patch)
2014-05-14 17:15 UTC, Nicola
needs-work Details | Review
fix memleak (19.61 KB, patch)
2014-05-14 19:36 UTC, Nicola
committed Details | Review

Description Nicola 2014-05-14 14:16:43 UTC
Created attachment 276536 [details] [review]
fix memleak

please see the attached patch
Comment 1 Nicola 2014-05-14 14:35:47 UTC
Created attachment 276537 [details] [review]
fix memleak

previous patch break alpha blending
Comment 2 Nicola 2014-05-14 17:15:59 UTC
Created attachment 276548 [details] [review]
improved patch
Comment 3 Thiago Sousa Santos 2014-05-14 18:30:54 UTC
Review of attachment 276548 [details] [review]:

Just a minor improvement below.

::: ext/opencv/gstmotioncells.c
@@ +864,3 @@
   GST_OBJECT_LOCK (filter);
+  if (!gst_buffer_is_writable (buf))
+    buf = gst_buffer_make_writable (buf);

You are making the buffer writable even when the 'calculate_motion' is set to false. It would be better to only do it if the buffer is actually going to be used.
Comment 4 Tim-Philipp Müller 2014-05-14 18:42:06 UTC
And please don't do  if (!is_writable) make_writable, just call make_writable() unconditionally - it will return the input buffer if the buffer is already writable.
Comment 5 Nicola 2014-05-14 19:36:49 UTC
Created attachment 276556 [details] [review]
fix memleak

the patch seems big now but I simply replaced:

gst_buffer_map (buf, &info, GST_MAP_WRITE)

with

if (gst_buffer_map (buf, &info, GST_MAP_WRITE)) {
...
}

and added the else cause, other then this the only change are the two gst_buffer_unmap needed to avoid the memleaks
Comment 6 Thiago Sousa Santos 2014-05-14 23:33:37 UTC
Thanks for the patch

commit bd34e628722c8916b023e776677b272df8d77eac
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Wed May 14 21:32:10 2014 +0200

    motioncells: fix memleak
    
    Check gst_buffer_map return and remember to unmap and free memory before
    returning
    
    https://bugzilla.gnome.org/show_bug.cgi?id=730133