Keep track of expected RTP time stamp and control drift. #1681
Conversation
- Use monotonic clock in RTCP Sender Report and packet times - Keep the time stamp close to expected time stamp on layer/SSRC switches
| ) | ||
|
|
||
| type pendingPacket struct { | ||
| arrivalTime int64 | ||
| arrivalTime time.Time |
There was a problem hiding this comment.
Switching to golang time.Time so that we have a monotonic clock available for elapsed time calculations.
| // submit to TWCC even if it is a padding only packet. Clients use padding only packets as probes | ||
| // for bandwidth estimation | ||
| if b.twcc != nil && b.twccExt != 0 { | ||
| if ext := p.GetExtension(b.twccExt); ext != nil { | ||
| b.twcc.Push(binary.BigEndian.Uint16(ext[0:2]), arrivalTime, p.Marker) | ||
| b.twcc.Push(binary.BigEndian.Uint16(ext[0:2]), arrivalTime.UnixNano(), p.Marker) |
There was a problem hiding this comment.
I would like to change this also to use time.Time so that we have monotonic clock in TWCC also. But, did not want to introduce that dependency in this PR. Will look at it later.
| "highestTS", r.highestTS, | ||
| "highestTime", r.highestTime.String(), | ||
| ) | ||
| return uint32(expectedExtRTP), nil |
There was a problem hiding this comment.
Idea here is to check for time elapsed since first packet was sent and calculate expected time stamp based on clock rate. This is used to decide next time stamp on layer switch.
There was a problem hiding this comment.
Another idea here (if necessary) is to use something like a PID controller to push/pull the difference between expected and actual when doing sender report so that the timestamp does not drift.
| now := time.Now() | ||
| // construct current time based on monotonic clock | ||
| timeSinceFirst := time.Since(r.firstTime) | ||
| now := r.firstTime.Add(timeSinceFirst) |
There was a problem hiding this comment.
Done this way to ensure that monotonic clock is used compared to the first packet, i. e. first packet time acts as the base and then everything else is an offset.
| @@ -638,7 +643,7 @@ func (d *DownTrack) WriteRTP(extPkt *buffer.ExtPacket, layer int32) error { | |||
| } | |||
| } | |||
|
|
|||
| d.rtpStats.Update(hdr, len(payload), 0, time.Now().UnixNano()) | |||
| d.rtpStats.Update(hdr, len(payload), 0, extPkt.Arrival) | |||
There was a problem hiding this comment.
Just using the arrival time stamp in the forwarding path so that it hugs the publish side stamping on SFU.
| normalizedTS := srLayer.SenderReportData.RTPTimestamp + uint32(ntpDiff*float64(s.clockRate)) | ||
| ntpDiff := srRef.SenderReportData.NTPTimestamp.Time().Sub(srLayer.SenderReportData.NTPTimestamp.Time()) | ||
| rtpDiff := ntpDiff.Nanoseconds() * int64(s.clockRate) / 1e9 | ||
| normalizedTS := srLayer.SenderReportData.RTPTimestamp + uint32(rtpDiff) |
There was a problem hiding this comment.
changing this to time.Time based code to make it a bit easier to read.
| switch { | ||
| case rl && el && er: // lastTS < refTS < expectedTS | ||
| nextTS = uint32(float64(refTS) + 0.95*float64(expectedTS-refTS)) | ||
| explain = fmt.Sprintf("l < r < e, %d, %d", refTS-lastTS, expectedTS-refTS) |
There was a problem hiding this comment.
This is an interesting case. Using 95% towards expected because of time warp seen. When muting/unmuting audio, instead of applying mute/unmute on the track, if the track is paused and replaced with an empty track, there is time warp. There are time windows where wall clock moves by 5 seconds, but RTP time stamp moves only by 3 seconds worth or so. So, on an unmute, using the publisher's time stamp as is mean there is
significant drift. Not exactly sure why that happens when replacing with an empty track, but that drift could be significant.
In this example, the three time stamps could be something like
lastTS = t
refTS = t + mutedDuration - 2 seconds
expectedTS = t + mutedDuration
* Keep track of expected RTP time stamp and control drift. - Use monotonic clock in RTCP Sender Report and packet times - Keep the time stamp close to expected time stamp on layer/SSRC switches * clean up * fix test compile * more test compile failures
A few things TODO if this looks good