The receiver should not change, but code wise, the option of replacing
receiver object makes more sense, i.e. otherwise, it could look like we
are leaving the stale object in there without replacing with new
receiver of same type.
* Tweak adaptation to increase in propagation delay.
A couple of issues
- RTCP Sender Reports rate will vary based on underying track bitrate.
(at least in theory, not all entities will do it though, for example
SFU does standard rate of one per three seconds irrespective of track
bit rate). So, adapt the long term estimate of propagation delay delta
based on spacing of reports.
- Re-init of propagation delay to adapt to path change was taking the
last value before the switch. But, that one value could have been an
outlier and accepting it is not great. So, adapt spike time
propagation delay in a smoother fashion to ensure that all values
during spike contribute to the final value.
* clean up
Another case of duplciate tracks in SDP.
During migration (if both publisher and subscriber migrate), subscriber
could attach the remote track of the publisher. But, while that is
happening, publisher could migrate into the node and close the remote
media track. This was causing subscriber to switch from attaching to
remote media track -> attaching to local media track.
But, as remote media track was closed while add subscription was
happening, the subscriber is removed without subscription manager being
aware of it.
So, the subscription manager's reconcile and the remove subscriber is
racing and when subscription manager re-subscribes, caching has not run
yet and that creates a duplicate.
Delay removing subscribed track till after caching is done. That means,
even if the reconciler runs, it will get an `errAlreadySubscribed` error
and it will force it to reconcile again. By the time the subscribed
track is deleted from the subscriptions map, caching is done.
* Notify initial permissions
NOTE: This does add an initial subscription permission notification
which should be fine, but something to watch for.
A stress test combining
- mute/unmute on publisher side.
- allowing/revoking permission for subscriber from publisher side.
- subscribing/unsubscribing from subscriber side.
results in a scenario where a subscription permission update of
`not_allowed` being sent and on a re-subscribe, an `allowed` update does
not happen.
It happens like so
- Subscription revoke cloes the down track of subscriber.
- The subscription is still desired.
- So, a subscription reconcile runs and sees `permission: false`. This
sends subscription permission of `not_allowed`.
- Unsubscribe request comes in and sets `desired: false`.
- Reconsiler runs again and sees `desired: false` and `subscribedTrack:
nil`. This cleans up the subscription.
- Publisher grants permission for the subscriber.
- Subscriber subscribes to the track again. A new subscription is
created.
- Reconciler runs and sees `permission: true`, but there is no
permission change as it is a new subscription object. So, `allowed`
subscription permission update is not sent and the client is stuck at
`not_allowed`.
Fix, maintain if permission has been initialized. Has the effect of
sending an initial update which should be fine.
* clean up comment
* no default
On migration, when subscription moved from remote -> local,
transceiver caching was racing. Although a very small possibility,
it could happen like so
1. down track close
2. down track close callback fires go routine to close subscribed track
3. subscribed track close handler in subscription manager tries to
reconcile
4. reconcile adds subscribed track again
5. cannot find cached transceiver as caching happens after down track
close finishes in stap 1 above. Although there are a couple of
gortouine jumps (step 2 fires a goroutine to close subscribed track
and step 4 will reconcile in a goroutine too), it is theoretically
possible that the step 1 has not finished and hence transceiver is
not cached.
Fix is to move caching to before closing subscribed track.
- When audio is muted, server injects silence frames which moves the
time stamp forward and adjusts offset. That cannot be used against
publisher side sender report. Use a pinned version.
- Ignore small changes to propagation delay even while checking for
sharp increase. That is spamming a lot for small changes, i.e.
existing delta is 100 micro seconds or so and the new one is 300 micro
seconds. Also rename to `longTerm` from `smoothed` as it is a slow
varying long term estimate of propagation delay delta. And slow down
that adaptation more.
* Forward publisher sender report.
Publisher side RTCP sernfer report is rebased to SFU time base
and used to send sender rerport to subscriber.
Will wait to merge till previous versions are out as this will require a
bunch of testing.
* - Add rebased report drift
- update protocol dep
- fix path change check, it has to check against delta of propagation
delay and not propagation delay as the two side clocks could be way
off.
* Use start time stamp to calculate down stream sender report.
With first packet time adjustment, using the first time stamp is more
accurate.
This still suffers if the up stream clock rate changes (happens in cases
like noise suppression which is not well understood). Will be looking at
pass through of sender report from publisher to subscriber.
* similar log strings
* avoid early sender reports
* log messages
* Reduce first packet adjustment threshold to 15 seconds
It is possible that migration could trigger without migrating out node
knowing about it. So, when a migration started notification comes in,
set up migration timer if not already set.
* Support XR request/response for rtt calculation
* Update pkg/sfu/downtrack.go
Co-authored-by: David Zhao <dz@livekit.io>
---------
Co-authored-by: David Zhao <dz@livekit.io>
* Remove subscriber if track closed while adding subscriber.
It is possible that the track is closed when subscriber add is
processed. That subscriber would have been dangling off a closed track.
Check again after adding subscriber if track is closed.
If it is, remove the subscriber and return error so that subscription
manager re-resolves.
* oops, wrong unlock
Raja Subramanian