GNOME Bugzilla – Bug 797117
debugutilsbad: Add timestamper element
Last modified: 2018-11-03 14:31:25 UTC
This is a simple element that simply set timestamp based on fixed duration. The timestamp method could be enhanced later base on needs. This was written initially to verify if broken or bad timestamp was directly causing some issues we where observing. It's also handy in some test when pulling some raw frames from the disk (e.g. multifilesrc). Currently, there is one property "duration".
Created attachment 373593 [details] [review] debugutilsbad: Add timestamper element This is a simple element that sets PTS on GstBuffer. For this first version, the timestamp are simply derived from a configured duration.
Review of attachment 373593 [details] [review]: ::: 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
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 373593 [details] [review] [review]: > > ::: 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. Sure, I thought about these, and I think it can be added as needed later. You simply add a timestamping-mode enum, and depending on that enum, different properties / method can be used. > > @@ +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() :) I didn't see it, and didn't see a g_warning either. I want to drop upstream segment completely, I can recheck the code, I thought it was handled. > > @@ +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? I'm completely replacing the timestamp, there is no point in using upstream segments. > > @@ +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 Will fix.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/783.