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
This commit is contained in:
Sebastian Dröge 2017-05-15 09:13:38 +02:00 committed by Thibault Saunier
parent 4a906e412e
commit be67574245
4 changed files with 28 additions and 13 deletions

View file

@ -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", GST_ERROR ("Asset could not be created for uri %s, error: %s",
ges_asset_get_id (asset), error->message); ges_asset_get_id (asset), error->message);
} else { } else {
GESProject *project = udata->layer->timeline ? GESProject *project = udata->layer->timeline ?
GES_PROJECT (ges_extractable_get_asset (GES_EXTRACTABLE 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_extractable_set_asset (GES_EXTRACTABLE (udata->clip), asset);
ges_project_add_asset (project, 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); ges_layer_add_clip (udata->layer, udata->clip);
} }
gst_object_unref (asset); gst_object_unref (asset);
gst_object_unref (udata->clip);
g_slice_free (NewAssetUData, udata); g_slice_free (NewAssetUData, udata);
} }
@ -525,7 +529,7 @@ ges_layer_is_empty (GESLayer * layer)
/** /**
* ges_layer_add_clip: * ges_layer_add_clip:
* @layer: a #GESLayer * @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 * Adds the given clip to the layer. Sets the clip's parent, and thus
* takes ownership of the clip. * takes ownership of the clip.
@ -556,6 +560,7 @@ ges_layer_add_clip (GESLayer * layer, GESClip * clip)
current_layer = ges_clip_get_layer (clip); current_layer = ges_clip_get_layer (clip);
if (G_UNLIKELY (current_layer)) { if (G_UNLIKELY (current_layer)) {
GST_WARNING ("Clip %p already belongs to another layer", clip); GST_WARNING ("Clip %p already belongs to another layer", clip);
gst_object_ref_sink (clip);
gst_object_unref (current_layer); gst_object_unref (current_layer);
return FALSE; return FALSE;
@ -566,7 +571,7 @@ ges_layer_add_clip (GESLayer * layer, GESClip * clip)
gchar *id; gchar *id;
NewAssetUData *mudata = g_slice_new (NewAssetUData); NewAssetUData *mudata = g_slice_new (NewAssetUData);
mudata->clip = clip; mudata->clip = gst_object_ref_sink (clip);
mudata->layer = layer; mudata->layer = layer;
GST_DEBUG_OBJECT (layer, "%" GST_PTR_FORMAT " as no reference to any " 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); ges_extractable_set_asset (GES_EXTRACTABLE (clip), asset);
g_slice_free (NewAssetUData, mudata); 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 */ /* Take a reference to the clip and store it stored by start/priority */
priv->clips_start = g_list_insert_sorted (priv->clips_start, clip, priv->clips_start = g_list_insert_sorted (priv->clips_start, clip,
(GCompareFunc) element_start_compare); (GCompareFunc) element_start_compare);
@ -683,8 +687,6 @@ ges_layer_add_asset (GESLayer * layer,
} }
if (!ges_layer_add_clip (layer, clip)) { if (!ges_layer_add_clip (layer, clip)) {
gst_object_unref (clip);
return NULL; return NULL;
} }

View file

@ -508,8 +508,8 @@ _set_name (GESTimelineElement * self, const gchar * wanted_name)
* @self: a #GESTimelineElement * @self: a #GESTimelineElement
* @parent: new parent of self * @parent: new parent of self
* *
* Sets the parent of @self to @parent. The object's reference count will * Sets the parent of @self to @parent. The parents needs to already
* be incremented, and any floating reference will be removed (see gst_object_ref_sink()). * own a hard reference on @self.
* *
* Returns: %TRUE if @parent could be set or %FALSE when @self * Returns: %TRUE if @parent could be set or %FALSE when @self
* already had a parent or @self and @parent are the same. * 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) { if (self == parent) {
GST_INFO_OBJECT (self, "Trying to add %p in itself, not a good idea!", GST_INFO_OBJECT (self, "Trying to add %p in itself, not a good idea!",
self); self);
gst_object_ref_sink (self);
gst_object_unref (self);
return FALSE; return FALSE;
} }
@ -548,6 +549,8 @@ ges_timeline_element_set_parent (GESTimelineElement * self,
had_parent: had_parent:
{ {
GST_WARNING_OBJECT (self, "set parent failed, object already had a parent"); GST_WARNING_OBJECT (self, "set parent failed, object already had a parent");
gst_object_ref_sink (self);
gst_object_unref (self);
return FALSE; return FALSE;
} }
} }

View file

@ -2948,7 +2948,7 @@ ges_timeline_append_layer (GESTimeline * timeline)
/** /**
* ges_timeline_add_layer: * ges_timeline_add_layer:
* @timeline: a #GESTimeline * @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 * Add the layer to the timeline. The reference to the @layer will be stolen
* by the @timeline. * 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 */ /* We can only add a layer that doesn't already belong to another timeline */
if (G_UNLIKELY (layer->timeline)) { if (G_UNLIKELY (layer->timeline)) {
GST_WARNING ("Layer belongs to another timeline, can't add it"); GST_WARNING ("Layer belongs to another timeline, can't add it");
gst_object_ref_sink (layer);
gst_object_unref (layer);
return FALSE; return FALSE;
} }
/* Add to the list of layers, make sure we don't already control it */ /* Add to the list of layers, make sure we don't already control it */
if (G_UNLIKELY (g_list_find (timeline->layers, (gconstpointer) layer))) { if (G_UNLIKELY (g_list_find (timeline->layers, (gconstpointer) layer))) {
GST_WARNING ("Layer is already controlled by this timeline"); GST_WARNING ("Layer is already controlled by this timeline");
gst_object_ref_sink (layer);
gst_object_unref (layer);
return FALSE; return FALSE;
} }

View file

@ -912,7 +912,7 @@ ges_track_set_mixing (GESTrack * track, gboolean mixing)
/** /**
* ges_track_add_element: * ges_track_add_element:
* @track: a #GESTrack * @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, * Adds the given object to the track. Sets the object's controlling track,
* and thus takes ownership of the @object. * 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)) { if (G_UNLIKELY (ges_track_element_get_track (object) != NULL)) {
GST_WARNING ("Object already belongs to another track"); GST_WARNING ("Object already belongs to another track");
gst_object_ref_sink (object);
gst_object_unref (object);
return FALSE; return FALSE;
} }
if (G_UNLIKELY (!ges_track_element_set_track (object, track))) { if (G_UNLIKELY (!ges_track_element_set_track (object, track))) {
GST_ERROR ("Couldn't properly add the object to the Track"); GST_ERROR ("Couldn't properly add the object to the Track");
gst_object_ref_sink (object);
gst_object_unref (object);
return FALSE; 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, if (G_UNLIKELY (!ges_nle_composition_add_object (track->priv->composition,
ges_track_element_get_nleobject (object)))) { ges_track_element_get_nleobject (object)))) {
GST_WARNING ("Couldn't add object to the NleComposition"); GST_WARNING ("Couldn't add object to the NleComposition");
gst_object_ref_sink (object);
gst_object_unref (object);
return FALSE; return FALSE;
} }