[Bug 772764] New: GstVideoTimeCode lacks GstValue functions

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Bug ID: 772764
Summary: GstVideoTimeCode lacks GstValue functions
Classification: Platform
Product: GStreamer
Version: git master
OS: All
Status: NEW
Severity: normal
Priority: Normal
Component: gst-plugins-base
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@gmail.com
QA Contact: gstreamer-***@lists.freedesktop.org
GNOME version: ---

GstVideoTimeCode has no registered string transform, compare or
serialization/deserialization functions. The first would enable easier binding
while the last would make the timecode-string property on timecodewait
unnecessary.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@zen.co.uk
Severity|normal |enhancement

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@coaxion.net
Assignee|gstreamer-***@lists.freede |***@ahiru.eu
|sktop.org |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

--- Comment #1 from Tim-Philipp Müller <***@zen.co.uk> ---
Do you plan on making a patch for this?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

--- Comment #2 from Jan Alexander Steffens (heftig) <***@gmail.com> ---
No, I do not. It's just something I noticed while looking at the GVTC code
during the conference.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

--- Comment #3 from Vivia Nikolaidou <***@ahiru.eu> ---
Created attachment 342633
--> https://bugzilla.gnome.org/attachment.cgi?id=342633&action=edit
0001-videotimecode-Add-GstValue-functions.patch

Please see attached patch.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342633|none |reviewed
status| |

--- Comment #4 from Tim-Philipp Müller <***@zen.co.uk> ---
Comment on attachment 342633
--> https://bugzilla.gnome.org/attachment.cgi?id=342633
0001-videotimecode-Add-GstValue-functions.patch

Nice, just one question: is all this GOnce stuff needed? I believe
G_DEFINE_BOXED_TYPE does that internally already, and we can rely on the extra
code being within the GOnce protected section.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Vivia Nikolaidou <***@ahiru.eu> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342633|reviewed |none
status| |
Attachment #342633|0 |1
is obsolete| |

--- Comment #5 from Vivia Nikolaidou <***@ahiru.eu> ---
Created attachment 342634
--> https://bugzilla.gnome.org/attachment.cgi?id=342634&action=edit
0001-videotimecode-Add-GstValue-functions.patch

Oops, you're right. :)

Also fixed more issues.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Vivia Nikolaidou <***@ahiru.eu> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342634|0 |1
is obsolete| |

--- Comment #6 from Vivia Nikolaidou <***@ahiru.eu> ---
Created attachment 342635
--> https://bugzilla.gnome.org/attachment.cgi?id=342635&action=edit
0001-videotimecode-Add-GstValue-functions.patch

(sorry for the noise, should be OK now)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

--- Comment #7 from Vivia Nikolaidou <***@ahiru.eu> ---
Created attachment 342639
--> https://bugzilla.gnome.org/attachment.cgi?id=342639&action=edit
0001-videotimecode-Added-GstValue-functions-unit-test.patch

Added unit test.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342635|none |needs-work
status| |

--- Comment #8 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Review of attachment 342635:
--> (https://bugzilla.gnome.org/review?bug=772764&attachment=342635)

::: gst-libs/gst/video/gstvideotimecode.c
@@ +550,3 @@
+ tc = gst_video_time_code_new_from_string (tc_str);
+ g_value_set_boxed (tc_val, tc);
+ gst_video_time_code_free (tc);

g_value_take_boxed(), then you don't have a useless copy here and don't need to
free

@@ +560,3 @@
+
+ tc_str = gst_video_time_code_to_string (tc);
+ g_value_set_string (str_val, tc_str);

g_value_take_string() (see above)

@@ +579,3 @@
+ return FALSE;
+
+ g_value_set_boxed (dest, tc);

g_value_take_boxed()

@@ +601,3 @@
+ gchar **parts;
+
+ parts = g_strsplit (tc_str, ":", 4);

g_strsplit_set(tc_str, ":;,", 4) so that it also works for all the other
variants. But that needs validation that the string is not e.g. "12;32,23" or
something like that.

You could do
if (sscanf (tc_str, "%02u:%02u:%02u:%02u", &hours, &minutes, &seconds, &frames)
== 4 ||
sscanf (tc_str, "%02u:%02u:%02u;%02u", &hours, &minutes, &seconds, &frames)
== 4 ||
sscanf (tc_str, "%02u:%02u:%02u.%02u", &hours, &minutes, &seconds, &frames)
== 4 ||
sscanf (tc_str, "%02u:%02u:%02u,%02u", &hours, &minutes, &seconds, &frames)
== 4) {
// good case
} else {
// error
}

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342639|none |accepted-commit_now
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Vivia Nikolaidou <***@ahiru.eu> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342635|needs-work |none
status| |
Attachment #342635|0 |1
is obsolete| |

--- Comment #9 from Vivia Nikolaidou <***@ahiru.eu> ---
Created attachment 343168
--> https://bugzilla.gnome.org/attachment.cgi?id=343168&action=edit
0001-videotimecode-Add-GstValue-functions.patch

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED

--- Comment #10 from Sebastian Dröge (slomo) <***@coaxion.net> ---
commit 5cbb52285c40c74bcdfcbd283d2d1b2b1634ed8f
Author: Vivia Nikolaidou <***@ahiru.eu>
Date: Fri Dec 30 20:27:48 2016 +0200

videotimecode: Add GstValue functions unit test

https://bugzilla.gnome.org/show_bug.cgi?id=772764

commit 06ff74e51dd83299f62dfc176804c57bd2f3a704
Author: Vivia Nikolaidou <***@toolsonair.com>
Date: Fri Dec 30 19:08:16 2016 +0200

videotimecode: Add GstValue functions

Add compare, serialization and deserialization functions

https://bugzilla.gnome.org/show_bug.cgi?id=772764

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #343168|none |committed
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|git master |1.11.1

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772764

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #342639|accepted-commit_now |committed
status| |