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 755247 - {track,videotrack,smart-video-mixer}: leaks their internal objects
{track,videotrack,smart-video-mixer}: leaks their internal objects
Product: GStreamer
Classification: Platform
Component: gst-editing-services
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2015-09-19 07:51 UTC by Justin Kim
Modified: 2015-09-30 10:02 UTC
See Also:
GNOME target: ---
GNOME version: ---

{track,videotrack,smart-video-mixer}: don't leak internal object (4.98 KB, patch)
2015-09-19 07:52 UTC, Justin Kim
reviewed Details | Review
timeline-element,track,framepositionner: don't leaks internal object (2.55 KB, patch)
2015-09-21 16:08 UTC, Justin Kim
committed Details | Review

Description Justin Kim 2015-09-19 07:51:55 UTC
Here's one of report from valgrind which my proposed patch solved.
(When I run valgrind, too many points reported so I add this partially)

==29654== 8,932 (464 direct, 8,468 indirect) bytes in 1 blocks are definitely lost in loss record 3,164 of 3,172
==29654==    at 0x6B6FD8A: g_type_create_instance (gtype.c:1849)
==29654==    by 0x6B53246: g_object_new_internal (gobject.c:1774)
==29654==    by 0x6B54B9C: g_object_newv (gobject.c:1922)
==29654==    by 0x6B552EB: g_object_new (gobject.c:1614)
==29654==    by 0x4E89F91: ges_smart_mixer_new (ges-smart-video-mixer.c:273)
==29654==    by 0x4E681BF: ges_video_track_get_mixing_element (ges-video-track.c:134)
==29654==    by 0x4E66430: ges_track_constructed (ges-track.c:514)
==29654==    by 0x6B533E3: g_object_new_internal (gobject.c:1814)
==29654==    by 0x6B54FC4: g_object_new_valist (gobject.c:2034)
==29654==    by 0x6B552D3: g_object_new (gobject.c:1617)
==29654==    by 0x4E682CC: ges_video_track_new (ges-video-track.c:172)
==29654==    by 0x401463: test_ges_scenario (basic.c:69)
==29654==    by 0x6315018: tcase_run_tfun_fork (check_run.c:450)
==29654==    by 0x6315018: srunner_iterate_tcase_tfuns (check_run.c:222)
==29654==    by 0x6315018: srunner_run_tcase (check_run.c:362)
==29654==    by 0x6315018: srunner_iterate_suites (check_run.c:195)
==29654==    by 0x6315018: srunner_run (check_run.c:706)
==29654==    by 0x630A3CD: gst_check_run_suite (gstcheck.c:824)
==29654==    by 0x4011BC: main (basic.c:798)
Comment 1 Justin Kim 2015-09-19 07:52:28 UTC
Created attachment 311663 [details] [review]
{track,videotrack,smart-video-mixer}: don't leak internal object
Comment 2 Thibault Saunier 2015-09-19 08:36:46 UTC
Review of attachment 311663 [details] [review]:

::: ges/ges-smart-video-mixer.c
@@ -205,2 +205,3 @@
+  if (!gst_element_remove_pad (GST_ELEMENT (self), self->srcpad)) {

That should not happen, and we should anyway have the infor from gst_element_remove_pad  anyway.

::: ges/ges-track.c
@@ +465,3 @@
+    if (priv->nleobject) {
+      if (priv->mixer) {
+        gst_bin_remove (GST_BIN (priv->nleobject), priv->mixer);

That should be done disposing ourself, then recursing down to all the object contained in it I think, why do you think it is needed here?

@@ -520,2 +534,3 @@
+    self->priv->mixer = gst_object_ref (mixer);

We should not need that, adding the mixer to the nleobject guarantees us we can use the mixer until we destroy the nleobject.
Comment 3 Justin Kim 2015-09-21 15:32:43 UTC
Comment on attachment 311663 [details] [review]
{track,videotrack,smart-video-mixer}: don't leak internal object

Yes, you are right. My proposed patch is totally wrong I just thought about how to decrease ref count. GstBin already does nested unref for its children. I missed it.

However, it's also true that nleobject and mixer are not disposed even after calling gst_object_unref (track). I'll try to find the root cause of this leakage again. Thank you for review.
Comment 4 Justin Kim 2015-09-21 16:08:38 UTC
Created attachment 311775 [details] [review]
timeline-element,track,framepositionner: don't leaks internal object
Comment 5 Thibault Saunier 2015-09-21 17:09:32 UTC
Review of attachment 311775 [details] [review]:

Looks right :)
Comment 6 Justin Kim 2015-09-28 05:28:36 UTC
ping? please merge this if there's wrong. :)
Comment 7 Justin Kim 2015-09-28 07:20:36 UTC
(In reply to Justin J. Kim from comment #6)
> ping? please merge this if there's wrong. :)

sorry .. wrong ==> right.
Comment 8 Thibault Saunier 2015-09-30 10:02:16 UTC
commit 731bda15926418bedd9a7e1343efdd49ae3dc108
Author: Justin Kim <>
Date:   Tue Sep 22 01:06:00 2015 +0900

    timeline-element,track,framepositionner: don't leak internal object