GNOME Bugzilla – Bug 776447
videotimecode: New GstVideoTimeCodeDiff type, ability to add to a GstVideoTimeCode
Last modified: 2017-01-11 00:35:01 UTC
Sometimes there is a human-oriented timecode that represents a difference between two other timecodes. It corresponds to the human perception of "add X hours" or "add X seconds" to a specific timecode, taking drop-frame oddities into account. This difference-representing timecode is now a GstVideoTimeCodeDiff. Also added function to add it to a GstVideoTimeCode.
Created attachment 342422 [details] [review] 0001-videotimecode-New-GstVideoTimeCodeDiff-type-ability-.patch
What's the usual term to refer to this then? Is it time code difference? Or maybe it should be GstVideoTime[Code]Interval ?
Created attachment 342423 [details] [review] 0001-videotimecode-New-GstVideoTimeCodeInterval-type-abil.patch Thanks, interval sounds much better! :) Also clarified a bit the documentation for gst_video_time_code_add_interval
Review of attachment 342423 [details] [review]: One typo, otherwise looks fine to me. ::: gst-libs/gst/video/gstvideotimecode.c @@ +713,3 @@ + */ +GstVideoTimeCodeInterval * +gst_video_time_code_interval_new_from_string (const gchar * tc_str) tc_str -> tc_diff_str @@ +719,3 @@ + gchar **parts; + + parts = g_strsplit (tc_diff_str, ":", 4); The parameter name in the function definition is wrong, so this line doesn't compile.
Created attachment 342573 [details] [review] 0001-videotimecode-New-GstVideoTimeCodeInterval-type-abil.patch Thanks, fixed the typo.
Created attachment 342631 [details] [review] 0001-videotimecode-New-GstVideoTimeCodeInterval-type-abil.patch Another typo fixed!
Created attachment 342636 [details] [review] 0001-videotimecode-New-GstVideoTimeCodeInterval-type-abil.patch (sorry for the noise, should be OK now)
Review of attachment 342636 [details] [review]: ::: gst-libs/gst/video/gstvideotimecode.c @@ +719,3 @@ + gchar **parts; + + parts = g_strsplit (tc_inter_str, ":", 4); Same comment here as for https://bugzilla.gnome.org/show_bug.cgi?id=772764#c8 Should this also get all the GValue/GstValue functions?
Created attachment 343169 [details] [review] 0001-videotimecode-New-GstVideoTimeCodeInterval-type-abil.patch
Created attachment 343248 [details] [review] 0001-videotimecode-Added-unit-test-for-GstVideoTimeCodeIn.patch
Thanks, pushed: commit eefa0d8cf5f843f3af72bf07ad57bfbbc06ca54d Author: Vivia Nikolaidou <vivia@ahiru.eu> Date: Tue Jan 10 16:36:08 2017 +0200 videotimecode: Added unit test for GstVideoTimeCodeInterval https://bugzilla.gnome.org/show_bug.cgi?id=776447 commit 629e8229a7b9de1632222f92cd69a9245b5f0494 Author: Vivia Nikolaidou <vivia@toolsonair.com> Date: Thu Dec 29 14:42:52 2016 +0200 videotimecode: New GstVideoTimeCodeInterval type, ability to add to a GstVideoTimeCode Sometimes there is a human-oriented timecode that represents an interval between two other timecodes. It corresponds to the human perception of "add X hours" or "add X seconds" to a specific timecode, taking drop-frame oddities into account. This interval-representing timecode is now a GstVideoTimeCodeInterval. Also added function to add it to a GstVideoTimeCode. https://bugzilla.gnome.org/show_bug.cgi?id=776447