[Bug 777048] timecodestamper: Post element message with current timecode

[GStreamer (GNOME Bugzilla)]

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

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

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

--- Comment #2 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 343159:
 --> (https://bugzilla.gnome.org/review?bug=777048&attachment=343159)

::: gst/timecode/gsttimecodestamper.c
@@ +446,3 @@
+      gst_util_uint64_scale_int (GST_SECOND, timecodestamper->vinfo.fps_d,
+      timecodestamper->vinfo.fps_n);
+  s = gst_structure_new ("timecodestamper", "running-time", G_TYPE_UINT64,

Maybe stream-time too? What does level/spectrum do?

Also should the name be more generic or is it ok like that? Suggestions?

@@ +447,3 @@
+      timecodestamper->vinfo.fps_n);
+  s = gst_structure_new ("timecodestamper", "running-time", G_TYPE_UINT64,
+      ref_time, "duration", G_TYPE_UINT64, duration, "timecode",
G_TYPE_STRING,

It would make more sense to put the actual GstVideoTimeCode in here instead of
a string

@@ +450,3 @@
+      tc_str, NULL);
+  msg = gst_message_new_element (GST_OBJECT (timecodestamper), s);
+  gst_element_post_message (GST_ELEMENT (timecodestamper), msg);

Might want a property to enable/disable this as that many messages can be
expensive

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.