Closed lminiero closed 1 year ago
It doesn't look like this is of interest to anyone, since we got no feedback.
@lionelnicolas @danjenkins just pinging you as you added some reactions to the original post: is this something you tested and/or are interested in? Otherwise I'll just close it, as we personally don't have any use for it at the moment.
Interested in yes. Forgot to test due to busy-ness and the fact I'm still running on 0.x branch... will make an effort to upgrade and test over the next two weeks!
@lminiero We recently had to deal (especially @tmatth) with A/V sync issues when pulling a stream from janus to (pion|gortsplib)-based clients, especially when the webrtc publisher was degraded.
We are also still running 0.x and we haven't tested this PR, but this looks very interesting to implement A/V sync detection/correction.
Interested in this as basis for abs-capture-time support in streaming plugin.
I think you need to add abs-capture-time to janus_rtp_extension_id
or it never gets offered.
Oh I see, probably not needed for echotest.
FWIW the generic parts of this are working for me. The only place I've found to probe is, in Chrome, an undocumented captureTimestamp
property of RTCRtpContributingSource
(or RTCRtpSynchronizationSource
). It has the timestamp as milliseconds from the NTP 1900 epoch.
Added PR related to this one.
@mvollrath it can be interesting for you as it adds support to streaming plugin and forwards abs-capture-time
from RTP source
https://github.com/meetecho/janus-gateway/pull/3258
Merging.
The abs-capture-time RTP extension was added recently to Chrome (~M107). While I'm still not exactly clear on how/when it could help, apparently the purpose of this experimental extension is to evaluate and increase audio/video synchronization, since the sender basically puts the NTP value of when the packet they're sending was actually captured, independently on when it gets anywhere.
At the moment, this patch simply copies the value across if an incoming packet with such an extension is relayed to one ore more destination, meaning the Janus core doesn't do any adaptation of the value. The current version of the document, speaking of intermediate systems, says:
which means we may have to rewrite it sometime in the future, especially considering the value is based on the sender's NTP clock, which may very well not be the same as the instance Janus is running on. As such, I'm not exactly sure if our current implementation has any real value or not, but I'm publishing it anyway so that interested parties can experiment with it.
It's also worth pointing out that, although we can parse, and send, 16-bit extensions including both clock value and offset, we currently only parse and send the clock value, ignoring the offset entirely: this is an arbitrary decision I made to simplify this first patch by looking at an existing capture coming from Chrome, where the offset was always 0, and so that's what I'm setting it to as well. Not sure if the offset part is not implemented at all in browsers, or if I was in a situation where 0 was indeed the right value: this is a draft PR anyway, so in case you know it's a value that actually makes sense to involve in the processing, it should be relatively simple to extend the support to the offset to.
This patch only adds support for this extension to the EchoTest plugin, which will automatically negotiate the extension if you send an SDP that offers it: notice that if you want to test it, you'll probably have to munge the SDP to add the extension, since at least in my versions of Chrome it's not offered by default. EchoTest support should be enough to start testing incoming and outgoing packets containing this extension: depending on how well it goes, I'll add it to other plugins too, e.g., the VideoRoom.
Feedback welcome!