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 731248 - Enhance and rename ges_clip_set_priority and add tests to it
Enhance and rename ges_clip_set_priority and add tests to it
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-editing-services
unspecified
Other All
: Normal normal
: 1.3.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-06-05 02:26 UTC by Thibault Saunier
Modified: 2014-09-23 09:16 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
tests: Check ges_clip_set_position behaviour (3.03 KB, patch)
2014-06-05 02:26 UTC, Thibault Saunier 		committed Details | Review
clip: Rename set_top_effect_priority to set_top_effect_position (4.82 KB, patch)
2014-06-05 02:26 UTC, Thibault Saunier 		committed Details | Review
clip: Fix the ges_clip_set_position function (5.18 KB, patch)
2014-06-05 02:26 UTC, Thibault Saunier 		none Details | Review
clip: Add test for effects priorities (2.83 KB, patch)
2014-06-05 02:29 UTC, Thibault Saunier 		committed Details | Review
clip: Fix the ges_clip_set_position function (5.15 KB, patch)
2014-06-06 11:26 UTC, Thibault Saunier 		committed Details | Review

Description Thibault Saunier 2014-06-05 02:26:13 UTC 
This method is porly named and has currently now unit tests
Comment 1 Thibault Saunier 2014-06-05 02:26:16 UTC 
Created attachment 277916 [details] [review]
tests: Check ges_clip_set_position behaviour

+ Minor fix to handle properly the feature when clip is not in any layer
Comment 2 Thibault Saunier 2014-06-05 02:26:20 UTC 
Created attachment 277917 [details] [review]
clip: Rename set_top_effect_priority to set_top_effect_position

Keeping the old method to not break the API but removing it from the
documentation as users should use the new method (which is the exact
same with a better naming)
Comment 3 Thibault Saunier 2014-06-05 02:26:25 UTC 
Created attachment 277918 [details] [review]
clip: Fix the ges_clip_set_position function

And enhance the new test
Comment 4 Thibault Saunier 2014-06-05 02:29:18 UTC 
Created attachment 277919 [details] [review]
clip: Add test for effects priorities
Comment 5 Thibault Saunier 2014-06-06 11:26:46 UTC 
Created attachment 278017 [details] [review]
clip: Fix the ges_clip_set_position function

And enhance the new test
Comment 6 Mathieu Duponchelle 2014-06-06 14:45:22 UTC 
Review of attachment 277916 [details] [review]:

::: tests/check/ges/clip.c
@@ +461,3 @@
           GES_TIMELINE_ELEMENT (effect2)));
 
+  fail_unless_equals_int (MIN_GNL_PRIO + 0, _PRIORITY (effect));

Adding 0 seems completely necessary :P
Comment 7 Mathieu Duponchelle 2014-06-06 14:46:37 UTC 
Review of attachment 277917 [details] [review]:

Not sure I like the "position" naming, seems a little vague to me ?
Comment 8 Mathieu Duponchelle 2014-06-06 14:50:06 UTC 
Review of attachment 277919 [details] [review]:

OK
Comment 9 Mathieu Duponchelle 2014-06-06 14:53:10 UTC 
Review of attachment 278017 [details] [review]:

Commit now when set_position -> set_index is done
Comment 10 Thibault Saunier 2014-06-09 14:13:08 UTC 
Attachment 277916 [details] pushed as 2db5368 - tests: Check ges_clip_set_position behaviour
Attachment 277919 [details] pushed as 6c717b2 - clip: Add test for effects priorities
Attachment 278017 [details] pushed as 03c87db - clip: Fix the ges_clip_set_position function