Invalid RTP Timestamp in first RTCP SR when sent before first media packet is sent
Categories
(Core :: WebRTC: Audio/Video, defect)
Tracking
()
People
(Reporter: ebunyan, Assigned: dminor)
Details
Attachments
(1 file)
187.19 KB,
application/cap
|
Details |
Updated•9 years ago
|
Reporter | ||
Comment 2•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Comment 3•9 years ago
|
||
Updated•9 years ago
|
Reporter | ||
Comment 4•9 years ago
|
||
Updated•8 years ago
|
Updated•8 years ago
|
Comment 6•6 years ago
|
||
This is likely the result of the following computation using 0 as |last_rtp_timestamp_| when no RTP has been sent yet:
However, it seems that there's a deeper flaw here. |last_rtp_timestamp_| is updated when an RTP packet is sent. But RFC 3550 says the following:
"Note that
in most cases this timestamp will not be equal to the RTP
timestamp in any adjacent data packet. Rather, it MUST be
calculated from the corresponding NTP timestamp using the
relationship between the RTP timestamp counter and real time as
maintained by periodically checking the wallclock time at a
sampling instant.
"
So, it sounds like the number we should put here is the RTP timestamp we would get right now if we sent an RTP packet right now, not the RTP timestamp of the most recently sent RTP packet.
Nico, Dan, does this sound accurate to you?
Comment 7•6 years ago
|
||
Ok, I see that we're adding some extra to that timestamp based on current time vs when we last encoded, so maybe it works. Still, we shouldn't be using 0 as an input.
Comment 8•6 years ago
|
||
Byron, that seems right to me. I checked the upstream repo and this appears to be there as well.
"Ok, I see that we're adding some extra to that timestamp based on current time vs when we last encoded, so maybe it works. Still, we shouldn't be using 0 as an input."
By my reading of https://tools.ietf.org/html/rfc3550#section-5.1 RTP timestamps SHOULD start at a random value (which doesn't preclude zero). We should probably record whether or not the initial timestamp has been recorded, or stick it in an Optional type, or etc.
Whatever we do, it should be up streamed.
Assignee | ||
Comment 9•6 years ago
|
||
That is my understanding as well, that times should be estimated based on what the timestamp would be if an RTP packet were sent at the current time. The rtp sender code does create a random offset, I can't remember offhand where, but I have hardcoded it to be zero before to make debugging easier. I'll take this bug but I'm not sure I'll be able to start on it right away. Feel free to steal it if you are interested :)
Assignee | ||
Comment 10•6 years ago
•
|
||
At the time this bug was reported, December 2016, we were running branch 49. The branch 57 update, which landed June 2017, added this assertion [1] and this check [2] to prevent sending SRs before media has been sent. A quick browse of the branch 49 code [3] shows nothing similar, so I believe this bug was fixed by Bug 1341285.
[1] https://searchfox.org/mozilla-central/rev/2a10f4812f3f7c7645d253a4fe52f26bf58e20e8/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc#473
[2] https://searchfox.org/mozilla-central/rev/2a10f4812f3f7c7645d253a4fe52f26bf58e20e8/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc#748
[3] https://searchfox.org/mozilla-central/rev/794e00db15cf64b69f3807fd69eb8daa8c927d99/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc#863
Description
•