Previously, the bit rate interval config was checked first. That would
have returned `!ok` for invalid layers. A recent change to prevent
duplicate tracker addition re-arranged the code and the tracker array
was accessed out-of-bounds.
Unclear why an invalid layer is passed in. Need to investigate that.
There are cases where the RTP time stamp does not increment acros
mute/unmute. Seems to happen fairly consistently with React Native
clients.
Something like the following happens
- Track is progressing
- Mute at `t = x`, assume RTP time stamp at the point is `y` and RTP clock
rate is `z`.
- Through mute, more RTCP sender reports come in from publisher and the
RTP time stamp in those reports are progressing at expected rate of
`z` RTP clock ticks per second.
- Forwarding path uses those sender reports from publisher to build the
sender report for subscribers.
- Unmute happens at `t = x + a` seconds.
- Ideally packets coming in after that, should have a time stamp of `y +
(a * z)`, but they tend to have something a little bit more than `y`.
- RTCP sender reports also have a time stamp that goes back. SFU ignores
these.
- Mean while the forwarding path has adjusted to the new RTP time stamp
base and it has calculated a TS offset (from publisher -> subscriber).
Effectively, that offset comes out close to `(a * z)`, i. e. jump
corresponding to the mute interval.
- When it is time to send a RTCP sender report to subscriber, the old
sender report from publisher is used (as intervening ones from
publisher were rejected because time stamp is moving backwards).
The problem is that the old report is used with new offset.
So, it looks like time stamp jumped ahead by `a` seconds.
Address it by storing time stamp offset at the time of receiving the
publisher side sender report. And use that while sending subscriber side
sender report. There are very edge cases where this can
get mismatched, but should be rare. Hopefully, this should prevent
unnecessary jumps in time stamp in RTCP sender report to subscribers.
Was hitting the edge case mentioned in the (now deleted in this PR)
comments. It is fine to reset and let it declare available again.
Available layer handler will ignore repeats.
When relaying buffers are stopped and restarted. On a restart,
the buffer adds a tracker. But, the tracker is not destroyed till the
end. So, the old tracker and new tracker for the same layer stomp on
each other and declare layer unavailable (the old tracker is not getting
any packets).
Fix by not creating a new tracker if one exists already.
With the move to forwarding NTP timestamp as is, we get a bunch more of
this error logged as the remote is basing it off of previous report and
local (i. e. server-side) bases it off of a more recent report.
Anyhow, this code has been around for a long time and there is nothing
new to learn from those errors. Just log it at Debugw in case we can
learn something from it for specific projects or environments where
Debugw is okay.
With Read and ReadExtended waiting (they are two different goroutines),
use Broadcast always. In theory, they both should not be waiting at the
same time, but just being safe.
* Do not use LastTS for dummy offset.
LastTS could be random when using dummy start. That should not be used
in calculating offsets.
Also, do not push padding into sequence before init. Could have heppened
with dummy start.
* apply dummy offset before comparing to last
* refresh ref TS
* initialize codec munger on catch up forwarding
* Simplify time stamp calculation on switches.
Trying to simplify time stamp calculation on restarts.
The additional checks take effect rarely and it not worth the extra
complication.
Also, doing the reference time stamp in extended range.
The challenge with that is when publisher migrates the extended
timestamp could change post migration (i. e. post migration would not
know about rollovers). To address that, maintain an offset that is
updated on resync.
* WIP
* Revert to resume threshold
* typo
* clean up
Raja Subramanian