[Bug 762910] onvifparse: extract and apply onvif timestamp

[GStreamer (GNOME Bugzilla)]

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

--- Comment #2 from Nicola <lists at svrinformatica.it> ---
(In reply to Sebastian Dröge (slomo) from comment #1)
> Review of attachment 322727 [details] [review]:

thanks for the review

> 
> This shouldn't be the default, it will break playback unless a) basetime of
> the pipeline is 0 and b) the pipeline uses a clock the returns UNIX time
> (e.g. the real-time system clock).
> 

is a property such as apply-onvif-timestamp as enum:

0 - no
1 - ntp time
2 - unix time

good enough?

the first option will preserve the timestamp generated in rtspsrc

> ::: gst/onvif/gstrtponvifparse.c
> @@ +122,3 @@
> +  self->last_onvif_timestamp =
> +      timestamp_seconds * GST_SECOND + timestamp_nseconds * GST_NSECOND -
> +      (2208988800LL * GST_SECOND);
> 

I think this could be refactored this way

ntptime = GST_READ_UINT64_BE (data);
unixtime = ntptime - (G_GUINT64_CONSTANT (2208988800) << 32);
self->last_onvif_timestamp = gst_util_uint64_scale (unixtime, GST_SECOND,
(G_GINT64_CONSTANT (1) << 32));

the result is the same but this is the way gstreamer already use in other
elements

> Question is also: why convert to UNIX time? 

in my use case I need to extract a recording, having the absolute time for each
buffer allow to accept/drop buffers as needed, unix time is convenient to me
since I can simply build a datetime object from it

>You don't know what your
> pipeline clock is using, it might very well use NTP times.
> 
> @@ +148,3 @@
> +    }
> +    if (GST_CLOCK_TIME_IS_VALID (buf->pts)) {
> +      buf->pts = self->last_onvif_timestamp;
> 
> GST_BUFFER_PTS (buf) = ...

ok

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