After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 762326 - gstrtpjitterbuffer GST_QUERY_POSITION reports position and not stream_time
gstrtpjitterbuffer GST_QUERY_POSITION reports position and not stream_time
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.6.3
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-19 14:06 UTC by Sergio Torres Soldado (zenx)
Modified: 2018-11-03 15:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Generating segment event with ntp time from RTCP (6.29 KB, patch)
2016-02-19 15:29 UTC, Sergio Torres Soldado (zenx)
needs-work Details | Review
Generating segment event with ntp time from RTCP2 (5.23 KB, patch)
2016-02-19 16:29 UTC, Sergio Torres Soldado (zenx)
reviewed Details | Review
Same as before but doing nothing when overflow (6.15 KB, patch)
2016-02-22 16:58 UTC, Sergio Torres Soldado (zenx)
none Details | Review

Description Sergio Torres Soldado (zenx) 2016-02-19 14:06:03 UTC
rtpjitterbuffer reports the segment position instead of stream time for a position query.

in pop_and_push_next() we see that:

1) pts is set with gst_segment_to_position()
2) last_out_time is set with the pts
3) gst_rtp_jitter_buffer_src_query responds to a GST_QUERY_POSITION using npt_start and last_out_time.
Comment 1 Sergio Torres Soldado (zenx) 2016-02-19 15:29:52 UTC
Created attachment 321664 [details] [review]
Generating segment event with ntp time from RTCP

Added a variable in gstrtpjitterbuffer to store the stream_time.

Added a boolean property in gstrtpbin to generate segment events with the ntp time. The gstrtpjitterbuffer then catches the segment.
Comment 2 Sergio Torres Soldado (zenx) 2016-02-19 15:33:55 UTC
The patch only works when creating a pipeline with a rtspsrc source and querying this element for the position. Querying any other element seems to return the segment position and not the stream time.
Comment 3 Sebastian Dröge (slomo) 2016-02-19 15:53:18 UTC
Review of attachment 321664 [details] [review]:

::: gst/rtpmanager/gstrtpjitterbuffer.c
@@ +3809,2 @@
       JBUF_LOCK (priv);
+      stream_time_ = priv->stream_time;

I didn't review the other parts yet, but what you really want here is to return gst_segment_to_stream_time() with the last position. That should give you the same value unless the segment is wrong, in which case the segment must be fixed anyway.
Comment 4 Sergio Torres Soldado (zenx) 2016-02-19 16:00:47 UTC
That is what I am doing, by using a proxy variable priv->stream_time and setting it in pop_and_push_next() because it seems that gst_segment_to_stream_time's last parameter comes from a RTPJitterBufferItem.

Thanks for the help
Comment 5 Sebastian Dröge (slomo) 2016-02-19 16:05:43 UTC
No I mean instead of adding a new variable, just pass the value we had previously there to gst_segment_to_stream_time() before putting it as a result into the position query.
Comment 6 Sergio Torres Soldado (zenx) 2016-02-19 16:29:24 UTC
Created attachment 321671 [details] [review]
Generating segment event with ntp time from RTCP2

Doing gst_segment_to_stream_time() inside the query function.
Not sure what impact keeping "start = priv->npt_start;" has, it seems to be always 0.
Comment 7 Sebastian Dröge (slomo) 2016-02-19 16:32:37 UTC
It's non-0 when using RTSP and having seeked to a non-0 position.
Comment 8 Sebastian Dröge (slomo) 2016-02-20 08:49:20 UTC
Review of attachment 321671 [details] [review]:

::: gst/rtpmanager/gstrtpbin.c
@@ +1464,3 @@
+                                             (G_GINT64_CONSTANT (1) << 32));
+          /* convert from 1900 based time to 1970 based time */
+          ntpnstime -= 2208988800000000000LL;

Why do you do this conversion here? This seems to defeat the purpose a bit, you are going to report "time since 1970" as stream time instead of "time since 1900", the latter actually being the NTP time.

@@ +1474,3 @@
+           * time values and get the equivalent NTP time.
+           */
+          segment->time = ntpnstime - local_running_time;

This is unsigned, but ntpnstime can be smaller than the local running time. Not sure what to do about that.


It should also be relative, i.e. segment->time += ...
Comment 9 Sergio Torres Soldado (zenx) 2016-02-22 10:27:47 UTC
Hi Sebastian,

 It seems that gstreamer uses the unix epoch format and I am doing the conversion to use it in the segment event.
 Regarding the edge cases, wouldn't that only happen if the reported ntp time was close to it's epoch (1900) and/or the local_running_time (which from what I understand starts from 0) is big.
Comment 10 Sebastian Dröge (slomo) 2016-02-22 10:37:50 UTC
GStreamer does not define any epoch for the times, why do you think it's the UNIX epoch?


The running time can start from a high value too, e.g. if you manually set the base time of a pipeline to 0 it will start at the current clock time.
Comment 11 Sergio Torres Soldado (zenx) 2016-02-22 13:40:29 UTC
I got the wrong impression by reading get_current_times().

It there any way to change the clock's base timestamp? I am trying to use gst_segment_set_running_time() but to no avail..
Comment 12 Sebastian Dröge (slomo) 2016-02-22 13:55:34 UTC
What are you trying to do, what is the result you'd like to have? :) Just the NTP timestamp in a 64 bit integer returned from the POSITION query? If so, no adjustments have to be made and I think it would make sense like that.


Question is just what to do if the subtraction there does not work because overflow.
Comment 13 Sergio Torres Soldado (zenx) 2016-02-22 14:40:08 UTC
The objective is the same as originally, to find an appropriate method to expose the ntp timestamps from the RTCP SR through gstreamer. From what I've understood it should be exposed through the stream_time which is what should be returned from using gst_element_query_position() although at least for rtspjitterbuffer gst_element_query_position() returns the actual stream position instead of the stream_time.

Now there is the problem of representing the ntp offset because the running time value might be bigger than the ntp timestamp and therefore the difference cannot be represented in segment->time. We could always make time be a signed value.. but I don't know what issues might arise from that.

Other issues might arise with stream operations (seek/trickmode?)..
Comment 14 Sergio Torres Soldado (zenx) 2016-02-22 16:58:30 UTC
Created attachment 321863 [details] [review]
Same as before but doing nothing when overflow
Comment 15 Sebastian Dröge (slomo) 2016-02-23 09:06:21 UTC
You can get around this overflow though. Instead of moving the stream time, you could move the running time and then also offset the buffer timestamps. Not nice though :)
Comment 16 GStreamer system administrator 2018-11-03 15:07:54 UTC
-- 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-good/issues/258.