[Bug 797117] debugutilsbad: Add timestamper element

[GStreamer (GNOME Bugzilla)]

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

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

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

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

::: gst/debugutils/gsttimestamper.c
@@ +53,3 @@
+{
+  PROP_0,
+  PROP_DURATION,

A start timestamp property could also be useful, and for odd framerates
(30000/1001) or sample rates (44.1kHz) the duration as a fraction could be
useful (to prevent accumulating rounding errors)

Also one could think of counting buffers as you do now, or having a byte-rate
and switch between both with a property.

@@ +80,3 @@
+    gst_segment_init (&segment, GST_FORMAT_TIME);
+    event = gst_event_new_segment (&segment);
+    gst_pad_push_event (trans->srcpad, event);

Isn't basetransform doing that already? Also how can there be no upstream
segment? That would cause a g_warning() :)

@@ +158,3 @@
+      gst_segment_init (&segment, GST_FORMAT_TIME);
+      event = gst_event_new_segment (&segment);
+      gst_event_set_seqnum (event, seqnum);

You probably want to set need_segment=FALSE here? And why do you always replace
the segment instead of passing it through, what's the idea behind this?

@@ +208,3 @@
+  g_object_class_install_property (gobject_class, PROP_DURATION,
+      g_param_spec_uint64 ("duration", "Duration",
+          "The duratuion in nanosecond of one buffer. Timestamps will be "

Typo: duration

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