From: Sebastian Dröge <sebastian@centricular.com>
Date: Mon, 15 May 2017 07:13:38 +0000 (+0200)
Subject: ges: Correctly handling floating references
X-Git-Tag: 1.19.3~493^2~812
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=be6757424509e1c072d6429cf93715e3311f43fb;p=platform%2Fupstream%2Fgstreamer.git

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
---

diff --git a/ges/ges-layer.c b/ges/ges-layer.c
index 86eaaef..3e4c187 100644
--- a/ges/ges-layer.c
+++ b/ges/ges-layer.c
@@ -308,7 +308,6 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, NewAssetUData * udata)
 
     GST_ERROR ("Asset could not be created for uri %s, error: %s",
         ges_asset_get_id (asset), error->message);
-
   } else {
     GESProject *project = udata->layer->timeline ?
         GES_PROJECT (ges_extractable_get_asset (GES_EXTRACTABLE
@@ -316,10 +315,15 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, NewAssetUData * udata)
     ges_extractable_set_asset (GES_EXTRACTABLE (udata->clip), asset);
 
     ges_project_add_asset (project, asset);
+
+    /* clip was already ref-sinked when creating udata,
+     * gst_layer_add_clip() creates a new ref as such and
+     * below we unref the ref from udata */
     ges_layer_add_clip (udata->layer, udata->clip);
   }
 
   gst_object_unref (asset);
+  gst_object_unref (udata->clip);
   g_slice_free (NewAssetUData, udata);
 }
 
@@ -525,7 +529,7 @@ ges_layer_is_empty (GESLayer * layer)
 /**
  * ges_layer_add_clip:
  * @layer: a #GESLayer
- * @clip: (transfer full): the #GESClip to add.
+ * @clip: (transfer floating): the #GESClip to add.
  *
  * Adds the given clip to the layer. Sets the clip's parent, and thus
  * takes ownership of the clip.
@@ -556,6 +560,7 @@ ges_layer_add_clip (GESLayer * layer, GESClip * clip)
   current_layer = ges_clip_get_layer (clip);
   if (G_UNLIKELY (current_layer)) {
     GST_WARNING ("Clip %p already belongs to another layer", clip);
+    gst_object_ref_sink (clip);
     gst_object_unref (current_layer);
 
     return FALSE;
@@ -566,7 +571,7 @@ ges_layer_add_clip (GESLayer * layer, GESClip * clip)
     gchar *id;
     NewAssetUData *mudata = g_slice_new (NewAssetUData);
 
-    mudata->clip = clip;
+    mudata->clip = gst_object_ref_sink (clip);
     mudata->layer = layer;
 
     GST_DEBUG_OBJECT (layer, "%" GST_PTR_FORMAT " as no reference to any "
@@ -594,11 +599,10 @@ ges_layer_add_clip (GESLayer * layer, GESClip * clip)
     ges_extractable_set_asset (GES_EXTRACTABLE (clip), asset);
 
     g_slice_free (NewAssetUData, mudata);
+  } else {
+    gst_object_ref_sink (clip);
   }
 
-
-  gst_object_ref_sink (clip);
-
   /* Take a reference to the clip and store it stored by start/priority */
   priv->clips_start = g_list_insert_sorted (priv->clips_start, clip,
       (GCompareFunc) element_start_compare);
@@ -683,8 +687,6 @@ ges_layer_add_asset (GESLayer * layer,
   }
 
   if (!ges_layer_add_clip (layer, clip)) {
-    gst_object_unref (clip);
-
     return NULL;
   }
 
diff --git a/ges/ges-timeline-element.c b/ges/ges-timeline-element.c
index 6531f6b..0211cd0 100644
--- a/ges/ges-timeline-element.c
+++ b/ges/ges-timeline-element.c
@@ -508,8 +508,8 @@ _set_name (GESTimelineElement * self, const gchar * wanted_name)
  * @self: a #GESTimelineElement
  * @parent: new parent of self
  *
- * Sets the parent of @self to @parent. The object's reference count will
- * be incremented, and any floating reference will be removed (see gst_object_ref_sink()).
+ * Sets the parent of @self to @parent. The parents needs to already
+ * own a hard reference on @self.
  *
  * Returns: %TRUE if @parent could be set or %FALSE when @self
  * already had a parent or @self and @parent are the same.
@@ -525,7 +525,8 @@ ges_timeline_element_set_parent (GESTimelineElement * self,
   if (self == parent) {
     GST_INFO_OBJECT (self, "Trying to add %p in itself, not a good idea!",
         self);
-
+    gst_object_ref_sink (self);
+    gst_object_unref (self);
     return FALSE;
   }
 
@@ -548,6 +549,8 @@ ges_timeline_element_set_parent (GESTimelineElement * self,
 had_parent:
   {
     GST_WARNING_OBJECT (self, "set parent failed, object already had a parent");
+    gst_object_ref_sink (self);
+    gst_object_unref (self);
     return FALSE;
   }
 }
diff --git a/ges/ges-timeline.c b/ges/ges-timeline.c
index 3b62226..9ded7be 100644
--- a/ges/ges-timeline.c
+++ b/ges/ges-timeline.c
@@ -2948,7 +2948,7 @@ ges_timeline_append_layer (GESTimeline * timeline)
 /**
  * ges_timeline_add_layer:
  * @timeline: a #GESTimeline
- * @layer: (transfer full): the #GESLayer to add
+ * @layer: (transfer floating): the #GESLayer to add
  *
  * Add the layer to the timeline. The reference to the @layer will be stolen
  * by the @timeline.
@@ -2966,12 +2966,16 @@ ges_timeline_add_layer (GESTimeline * timeline, GESLayer * layer)
   /* We can only add a layer that doesn't already belong to another timeline */
   if (G_UNLIKELY (layer->timeline)) {
     GST_WARNING ("Layer belongs to another timeline, can't add it");
+    gst_object_ref_sink (layer);
+    gst_object_unref (layer);
     return FALSE;
   }
 
   /* Add to the list of layers, make sure we don't already control it */
   if (G_UNLIKELY (g_list_find (timeline->layers, (gconstpointer) layer))) {
     GST_WARNING ("Layer is already controlled by this timeline");
+    gst_object_ref_sink (layer);
+    gst_object_unref (layer);
     return FALSE;
   }
 
diff --git a/ges/ges-track.c b/ges/ges-track.c
index 879b66f..1b86ec4 100644
--- a/ges/ges-track.c
+++ b/ges/ges-track.c
@@ -912,7 +912,7 @@ ges_track_set_mixing (GESTrack * track, gboolean mixing)
 /**
  * ges_track_add_element:
  * @track: a #GESTrack
- * @object: (transfer full): the #GESTrackElement to add
+ * @object: (transfer floating): the #GESTrackElement to add
  *
  * Adds the given object to the track. Sets the object's controlling track,
  * and thus takes ownership of the @object.
@@ -932,11 +932,15 @@ ges_track_add_element (GESTrack * track, GESTrackElement * object)
 
   if (G_UNLIKELY (ges_track_element_get_track (object) != NULL)) {
     GST_WARNING ("Object already belongs to another track");
+    gst_object_ref_sink (object);
+    gst_object_unref (object);
     return FALSE;
   }
 
   if (G_UNLIKELY (!ges_track_element_set_track (object, track))) {
     GST_ERROR ("Couldn't properly add the object to the Track");
+    gst_object_ref_sink (object);
+    gst_object_unref (object);
     return FALSE;
   }
 
@@ -947,6 +951,8 @@ ges_track_add_element (GESTrack * track, GESTrackElement * object)
   if (G_UNLIKELY (!ges_nle_composition_add_object (track->priv->composition,
               ges_track_element_get_nleobject (object)))) {
     GST_WARNING ("Couldn't add object to the NleComposition");
+    gst_object_ref_sink (object);
+    gst_object_unref (object);
     return FALSE;
   }