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 729382 - Add a name property to GESTimelineElement
Add a name property to GESTimelineElement
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-editing-services
unspecified
Other All
: Normal enhancement
: 1.3.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-05-02 12:33 UTC by Thibault Saunier
Modified: 2014-09-23 09:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tools: Position printing is now done at the gst-validate level (2.79 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
Add a notion of 'name' in GESTimelineElement (41.36 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools:validate: Add an action to allow editing clips (3.64 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
scenario: Add a scenario that edits a clip while the pipeline is paused (2.74 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
ges-validate: Add a scenario to set children properties (2.17 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools:validate: Always seek after editing a clip (5.18 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools:validate: Add an action to scerialize the project (1.82 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
ges: Rename remaning tlobj to clip (1.16 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools: Disable --set-scenario if not compiled against gst-validate (969 bytes, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools: Handle times as doubles + concider duration=0 as TIME_NONE (1.64 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools: Always activate gst-validate to have position printing (3.10 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
project: Enhance debugging when badly updating a URI (1.18 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools: Add an option to disable mixing (4.22 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review
tools: Add a way to look for moved media sample recursively (4.80 KB, patch)
2014-05-02 12:34 UTC, Thibault Saunier
committed Details | Review

Description Thibault Saunier 2014-05-02 12:33:58 UTC
This way it is simpler to find the in a timeline.

This allows us to implement easily more actionf for GstValidate.

Attaching patches that also add new actions
Comment 1 Thibault Saunier 2014-05-02 12:34:01 UTC
Created attachment 275634 [details] [review]
tools: Position printing is now done at the gst-validate level
Comment 2 Thibault Saunier 2014-05-02 12:34:06 UTC
Created attachment 275635 [details] [review]
Add a notion of 'name' in GESTimelineElement
Comment 3 Thibault Saunier 2014-05-02 12:34:10 UTC
Created attachment 275636 [details] [review]
tools:validate: Add an action to allow editing clips
Comment 4 Thibault Saunier 2014-05-02 12:34:13 UTC
Created attachment 275637 [details] [review]
scenario: Add a scenario that edits a clip while the pipeline is paused
Comment 5 Thibault Saunier 2014-05-02 12:34:17 UTC
Created attachment 275638 [details] [review]
ges-validate: Add a scenario to set children properties
Comment 6 Thibault Saunier 2014-05-02 12:34:21 UTC
Created attachment 275639 [details] [review]
tools:validate: Always seek after editing a clip

Otherwize the displayed frame will not be updated when paused.
Comment 7 Thibault Saunier 2014-05-02 12:34:25 UTC
Created attachment 275640 [details] [review]
tools:validate: Add an action to scerialize the project
Comment 8 Thibault Saunier 2014-05-02 12:34:29 UTC
Created attachment 275641 [details] [review]
ges: Rename remaning tlobj to clip
Comment 9 Thibault Saunier 2014-05-02 12:34:32 UTC
Created attachment 275642 [details] [review]
tools: Disable --set-scenario if not compiled against gst-validate
Comment 10 Thibault Saunier 2014-05-02 12:34:36 UTC
Created attachment 275643 [details] [review]
tools: Handle times as doubles + concider duration=0 as TIME_NONE
Comment 11 Thibault Saunier 2014-05-02 12:34:39 UTC
Created attachment 275644 [details] [review]
tools: Always activate gst-validate to have position printing
Comment 12 Thibault Saunier 2014-05-02 12:34:43 UTC
Created attachment 275645 [details] [review]
project: Enhance debugging when badly updating a URI
Comment 13 Thibault Saunier 2014-05-02 12:34:47 UTC
Created attachment 275646 [details] [review]
tools: Add an option to disable mixing
Comment 14 Thibault Saunier 2014-05-02 12:34:51 UTC
Created attachment 275647 [details] [review]
tools: Add a way to look for moved media sample recursively

In ges-launch let the user set a folder where the media sample that
move can be found recursing into that specified folder.
Comment 15 Mathieu Duponchelle 2014-05-02 12:45:11 UTC
Review of attachment 275634 [details] [review]:

OK
Comment 16 Mathieu Duponchelle 2014-05-02 12:50:36 UTC
Review of attachment 275635 [details] [review]:

OK

::: ges/ges-timeline-element.c
@@ +296,3 @@
+    goto had_parent;
+
+  g_free (self->name);

You could get that before the check for parent and remove the g_free in had_parent.
Comment 17 Mathieu Duponchelle 2014-05-02 12:55:51 UTC
Review of attachment 275636 [details] [review]:

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

@@ +65,3 @@
+  g_return_val_if_fail (timeline, FALSE);
+
+  clip = ges_timeline_get_element (timeline, clip_name);

get_element is (transfer-full) right ?

@@ +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
Comment 18 Mathieu Duponchelle 2014-05-02 12:56:18 UTC
Review of attachment 275637 [details] [review]:

OK
Comment 19 Mathieu Duponchelle 2014-05-02 12:58:42 UTC
Review of attachment 275638 [details] [review]:

The name of the commit is a bit off I think, it's rather about adding an action. Not OK

::: tools/ges-validate.c
@@ +60,3 @@
+  g_return_val_if_fail (GES_IS_TRACK_ELEMENT (element), FALSE);
+
+  property_name = gst_structure_get_string (action->structure, "property");

Same comment, mandatory blabla

@@ -123,2 +150,4 @@
         move_clip_mandatory_fields, "Allows to seek into the files", FALSE);
 
+    gst_validate_add_action_type ("set-child-property", _set_child_property,
+        move_clip_mandatory_fields,

move_clip_mandatory_fields ? shouldn't it be a new array ?
Comment 20 Mathieu Duponchelle 2014-05-02 13:01:11 UTC
Review of attachment 275639 [details] [review]:

Split that commit please :)
Comment 21 Mathieu Duponchelle 2014-05-02 13:02:02 UTC
Review of attachment 275640 [details] [review]:

OK

::: tools/ges-validate.c
@@ +84,3 @@
+_serialize_project (GstValidateScenario * scenario, GstValidateAction * action)
+{
+  const gchar *uri = gst_structure_get_string (action->structure, "uri");

Same here, let's make sure validate enforces the mandatory fields
Comment 22 Mathieu Duponchelle 2014-05-02 13:02:29 UTC
Review of attachment 275641 [details] [review]:

OK but the commit name is playing tricks on my brain :)
Comment 23 Mathieu Duponchelle 2014-05-02 13:02:45 UTC
Review of attachment 275642 [details] [review]:

OK
Comment 24 Mathieu Duponchelle 2014-05-02 13:03:41 UTC
Review of attachment 275643 [details] [review]:

OK
Comment 25 Mathieu Duponchelle 2014-05-02 13:04:29 UTC
Review of attachment 275644 [details] [review]:

OK
Comment 26 Mathieu Duponchelle 2014-05-02 13:05:09 UTC
Review of attachment 275645 [details] [review]:

OK, maybe make the commit name a bit more explicit ?
Comment 27 Mathieu Duponchelle 2014-05-02 13:05:39 UTC
Review of attachment 275646 [details] [review]:

OK
Comment 28 Mathieu Duponchelle 2014-05-02 13:07:06 UTC
Review of attachment 275647 [details] [review]:

OK, did you try having media with the same name at different levels of the folder structure when recusring though ?
Comment 29 Mathieu Duponchelle 2014-05-02 13:07:18 UTC
Review of attachment 275639 [details] [review]:

Split that commit please :)
Comment 30 Thibault Saunier 2014-05-02 13:08:36 UTC
(In reply to comment #17)
> Review of attachment 275636 [details] [review]:
> 
> 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" :)
Comment 31 Thibault Saunier 2014-05-02 13:14:36 UTC
(In reply to comment #29)
> Review of attachment 275639 [details] [review]:
> 
> Split that commit please :)

Will just fixup the part on .scenario to the commit it was introduced in
Comment 32 Thibault Saunier 2014-05-02 14:31:03 UTC
Review of attachment 275636 [details] [review]:

::: tools/ges-validate.c
@@ -44,0 +46,15 @@
+static gboolean
+_edit_clip (GstValidateScenario * scenario, GstValidateAction * action)
+{
... 12 more ...

Removed

@@ +65,3 @@
+  g_return_val_if_fail (timeline, FALSE);
+
+  clip = ges_timeline_get_element (timeline, clip_name);

Fixed

@@ +81,3 @@
+    gst_validate_utils_enum_from_str (GES_TYPE_EDGE, edge_str, &edge);
+
+  gst_structure_get_int (action->structure, "new-layer-priority",

Not an issue
Comment 33 Thibault Saunier 2014-05-02 14:31:06 UTC
Review of attachment 275636 [details] [review]:

::: tools/ges-validate.c
@@ -44,0 +46,15 @@
+static gboolean
+_edit_clip (GstValidateScenario * scenario, GstValidateAction * action)
+{
... 12 more ...

Removed

@@ +65,3 @@
+  g_return_val_if_fail (timeline, FALSE);
+
+  clip = ges_timeline_get_element (timeline, clip_name);

Fixed

@@ +81,3 @@
+    gst_validate_utils_enum_from_str (GES_TYPE_EDGE, edge_str, &edge);
+
+  gst_structure_get_int (action->structure, "new-layer-priority",

Not an issue
Comment 34 Thibault Saunier 2014-05-02 14:38:11 UTC
Review of attachment 275638 [details] [review]:

Changed the commit name to  ges-validate: Add a GstValidate action to set children properties

::: tools/ges-validate.c
@@ -123,2 +150,4 @@
         move_clip_mandatory_fields, "Allows to seek into the files", FALSE);
 
+    gst_validate_add_action_type ("set-child-property", _set_child_property,
+        move_clip_mandatory_fields,

Added an array like:

    const gchar *set_child_property_mandatory_fields[] =
        { "element-name", "property", "value", NULL };

They all are mandatory
Comment 35 Thibault Saunier 2014-05-02 14:43:07 UTC
Review of attachment 275639 [details] [review]:

Splitted
Comment 36 Thibault Saunier 2014-05-02 14:43:08 UTC
Review of attachment 275639 [details] [review]:

Splitted
Comment 37 Thibault Saunier 2014-05-02 14:53:33 UTC
Review of attachment 275645 [details] [review]:

Changed to "project: Enhance debugging when updating URI with a bad URI"
Comment 38 Thibault Saunier 2014-05-02 15:14:26 UTC
commit 5785178e521c87d5ec58cf3351bb5e818fe55327
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Thu May 1 10:13:39 2014 +0200

    tools: Add a way to look for moved media sample recursively
    
    In ges-launch let the user set a folder where the media sample that
    move can be found recursing into that specified folder.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit 2dd59ab53a485bd00b372497fc9d602235ed4317
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Wed Apr 30 20:58:42 2014 +0200

    tools: Add an option to disable mixing
    
    + Add a a GObject property so that the info is seralized
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit cba79c09acda63d9e06b1fd076e773ed79f91961
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Wed Apr 30 16:26:03 2014 +0200

    project: Enhance debugging when updating URI with an invalid one
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit 92b115d3e0d3fdb6e448ae9e1fe5643300be6173
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Fri May 2 16:49:10 2014 +0200

    tools: Always activate gst-validate to have position printing
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382


commit 36cc9f13ec89f2ec2ff63f29f1a7f22e893f0306
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Tue Apr 29 21:29:54 2014 +0200

    tools: Handle times as doubles + concider duration=0 as TIME_NONE
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit 44cbc27cb831575f81dc9684ebf05be647f232c2
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Sat Apr 26 09:51:37 2014 +0200

    tools: Disable --set-scenario if not compiled against gst-validate
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit ff1446c2b28a8b896a45a3a17a669fac39975b9b
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Sat Apr 26 08:55:31 2014 +0200

    ges: Rename remaning tlobj to clip
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit 8601666a5aca1ed318fc6a91e56c8e37f01c4b95
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Fri May 2 16:43:42 2014 +0200

    ges-validate: Add an action to serialize the project
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382
    
    Conflicts:
        tools/ges-validate.c

commit 4ce52d8a62b18c0cd9d0bb6c5ff5eea194ccf251
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Fri Apr 25 18:23:06 2014 +0200

    tools:validate: Always seek after editing a clip
    
    Otherwize the displayed frame will not be updated when paused.
    
    + Add a get_timeline internal helper method in ges-validate.c
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit 9cb7d8e3e7892e477e0ddcacf9bef3694dcfd9f9
Author: Thibault Saunier <thibault.saunier@collabora.com>
Date:   Fri Mar 14 12:10:53 2014 +0100

    ges-validate: Add a GstValidate action to set children properties
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382

commit 0b0bbdddd0f1c05fb9a88b38ebce9696fd346c40
Author: Thibault Saunier <thibault.saunier@collabora.com>
Date:   Tue Feb 18 18:52:38 2014 +0100

    scenario: Add a scenario that edits a clip while the pipeline is paused
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729382