[Bug 784132] rtpmanager: Add support for in-band synchronization (RFC 6051).

[GStreamer (GNOME Bugzilla)]

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

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

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

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

Also not a real review yet, just some comments/questions.

In rtpjitterbuffer this could probably also be used for some of the sync modes
instead of interpolating timestamps. There already is code that calculates
timestamps based on the NTP timestamp, which could directly use this then.

::: gst/rtpmanager/gstrtpbin.c
@@ +1633,3 @@
+
+  if (gst_structure_has_field (s, "ntp-time")) {
+    /* perform in-band synchronization for this stream */

Now you would do sync based on this, and based on RTCP. Is that a problem?
Also this now would only work if the first RTP packet has the header extension,
or not? Or will this be called for every single RTP packet (that would be a lot
of overhead)?

::: gst/rtpmanager/rtpsession.c
@@ +1964,3 @@
+    if (G_LIKELY (gst_rtp_hdrext_get_ntp_64 (data, size, &ntptime)))
+      return ntptime;
+  } else if (gst_rtp_buffer_get_extension_onebyte_header (rtp, 0xB, 0, &data,

0xA / 0xB are not really reserved for this but could also be used for other
header extensions, or not? Isn't this usually negotiated over the SDP?

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