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 782652 - ges: Correctly handling floating references
ges: Correctly handling floating references
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-editing-services
unspecified
Other All
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-05-15 09:36 UTC by Sebastian Dröge (slomo)
Modified: 2018-11-03 12:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ges: Correctly handling floating references (6.50 KB, patch)
2017-05-15 09:36 UTC, Sebastian Dröge (slomo)
none Details | Review
ges: Correctly handling floating references (7.05 KB, patch)
2017-05-15 11:11 UTC, Sebastian Dröge (slomo)
none Details | Review

Description Sebastian Dröge (slomo) 2017-05-15 09:36:22 UTC
ges_timeline_element_set_parent() needs a decision about how it should exactly
work.
Comment 1 Sebastian Dröge (slomo) 2017-05-15 09:36:26 UTC
Created attachment 351873 [details] [review]
ges: Correctly handling floating references

If we ref_sink() a parameter, it must be marked as (transfer floating)
and it also has to be handled consistently between error and normal cases.

See https://bugzilla.gnome.org/show_bug.cgi?id=782499
Comment 2 Sebastian Dröge (slomo) 2017-05-15 11:11:39 UTC
Created attachment 351879 [details] [review]
ges: Correctly handling floating references

If we ref_sink() a parameter, it must be marked as (transfer floating)
and it also has to be handled consistently between error and normal cases.

See https://bugzilla.gnome.org/show_bug.cgi?id=782499
Comment 3 Sebastian Dröge (slomo) 2017-05-15 11:16:23 UTC
Review of attachment 351879 [details] [review]:

::: ges/ges-timeline-element.c
@@ +543,2 @@
   self->parent = parent;
+  gst_object_ref_sink (self);

This breaks the unit tests (and causes leaks), because at least sometimes something else already removed the floating reference flag and then this now increases the reference count.

What to do?
Comment 4 Thibault Saunier 2017-05-20 14:10:13 UTC
(In reply to Sebastian Dröge (slomo) from comment #3)
> Review of attachment 351879 [details] [review] [review]:
> 
> ::: ges/ges-timeline-element.c
> @@ +543,2 @@
>    self->parent = parent;
> +  gst_object_ref_sink (self);
> 
> This breaks the unit tests (and causes leaks), because at least sometimes
> something else already removed the floating reference flag and then this now
> increases the reference count.
> 
> What to do?

I changed the doc as it was wrong, ownership for GESTimelineElement is either the timeline (for clips or groups), or the Track for GESTrackElement.

The parent owns a hard ref.
Comment 5 Thibault Saunier 2017-06-01 21:25:04 UTC
commit be6757424509e1c072d6429cf93715e3311f43fb
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Mon May 15 09:13:38 2017 +0200

    ges: Correctly handling floating references
    
    If we ref_sink() a parameter, it must be marked as (transfer floating)
    and it also has to be handled consistently between error and normal cases.
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=782499
    
    https://bugzilla.gnome.org/show_bug.cgi?id=782652

Thanks, your patch was correct, the doc needed minor fixing.
Comment 6 GStreamer system administrator 2018-11-03 12:53:58 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-editing-services/issues/34.