[Bug 729382] Add a name property to GESTimelineElement

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=729382
  GStreamer | gst-editing-services | unspecified

--- Comment #30 from Thibault Saunier <tsaunier at gnome.org> 2014-05-02 13:08:36 UTC ---
(In reply to comment #17)
> Review of attachment 275636 [details]:
> 
> Not OK, I think some of these comments need to be addressed.
> 
> ::: tools/ges-validate.c
> @@ -44,0 +46,15 @@
> +static gboolean
> +_edit_clip (GstValidateScenario * scenario, GstValidateAction * action)
> +{
> ... 12 more ...
> 
> Unneeded new line :)
> 
> @@ -44,0 +46,17 @@
> +static gboolean
> +_edit_clip (GstValidateScenario * scenario, GstValidateAction * action)
> +{
> ... 14 more ...
> 
> You need to either check that it was correctly obtained or make validate
> enforce mandatory fields I think. Even though passing NULL to get_element
> should also return NULL

Mandatory should be mandatory and in action implementation it is right to
concider you will find them I think.

> @@ +65,3 @@
> +  g_return_val_if_fail (timeline, FALSE);
> +
> +  clip = ges_timeline_get_element (timeline, clip_name);
> 
> get_element is (transfer-full) right ?

Indeed.

> @@ +81,3 @@
> +    gst_validate_utils_enum_from_str (GES_TYPE_EDGE, edge_str, &edge);
> +
> +  gst_structure_get_int (action->structure, "new-layer-priority",
> 
> If new-layer-priority is not provided we will end up editing to a prio of -1 I
> think

And -1 means "No move" :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.