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 789403 - avwait: Added end-timecode property
avwait: Added end-timecode property
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal enhancement
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-24 13:20 UTC by Vivia Nikolaidou
Modified: 2017-10-25 11:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
avwait: Added end-timecode property (10.98 KB, patch)
2017-10-24 13:20 UTC, Vivia Nikolaidou
none Details | Review
avwait: Added end-timecode property (10.88 KB, patch)
2017-10-25 11:28 UTC, Vivia Nikolaidou
committed Details | Review

Description Vivia Nikolaidou 2017-10-24 13:20:18 UTC
See commit message
Comment 1 Vivia Nikolaidou 2017-10-24 13:20:23 UTC
Created attachment 362180 [details] [review]
avwait: Added end-timecode property

avwait can now be configured to stop when a given timecode has been
reached. It will start at the timecode indicated with start-timecode and
end at the timecode indicated with end-timecode. If end-timecode is
NULL (default), the previous functionality is preserved: keep going and
not end.
Comment 2 Mathieu Duponchelle 2017-10-25 10:58:20 UTC
Review of attachment 362180 [details] [review]:

Looks good to me otherwise, please push once this is fixed :)

::: gst/timecode/gstavwait.c
@@ -385,0 +417,22 @@
+      if (self->end_tc
+          && gst_video_time_code_compare (self->tc, self->end_tc) != -1) {
+        gchar *start_tc, *end_tc;
... 19 more ...

copy paste issue ? :)
Comment 3 Vivia Nikolaidou 2017-10-25 11:03:42 UTC
Thanks for the review! :)

(In reply to Mathieu Duponchelle from comment #2)
>
> ::: gst/timecode/gstavwait.c
> @@ -385,0 +417,22 @@
> +      if (self->end_tc
> +          && gst_video_time_code_compare (self->tc, self->end_tc) != -1) {
> +        gchar *start_tc, *end_tc;
> ... 19 more ...
> 
> copy paste issue ? :)

Not really :) There is the same check but slightly different actions that have to be taken in three different places:

1) we provide start-timecode as string and it turns out to be after the end-timecode: don't bother stringifying the start timecode, reject it (I'd say, not bother creating it at the first place either - will update the patch)

2) we provide start-timecode as an object and it turns out to be after the end-timecode: stringify both, reject start-timecode

3) we provide end-timecode as an object and it turns out to be after the start-timecode: stringify both, reject end-timecode
Comment 4 Vivia Nikolaidou 2017-10-25 11:04:29 UTC
(In reply to Vivia Nikolaidou from comment #3)
> 
> 1) we provide start-timecode as string and it turns out to be after the
> end-timecode: don't bother stringifying the start timecode, reject it (I'd
> say, not bother creating it at the first place either - will update the
> patch)
> 

Oops, we actually do have to create it in order to compare it! :) So it's OK as it is.
Comment 5 Vivia Nikolaidou 2017-10-25 11:28:20 UTC
Created attachment 362250 [details] [review]
avwait: Added end-timecode property

avwait can now be configured to stop when a given timecode has been
reached. It will start at the timecode indicated with start-timecode and
end at the timecode indicated with end-timecode. If end-timecode is
NULL (default), the previous functionality is preserved: keep going and
not end.
Comment 6 Mathieu Duponchelle 2017-10-25 11:39:55 UTC
Attachment 362250 [details] pushed as a160b85 - avwait: Added end-timecode property