Commit Graph

852 Commits

Author SHA1 Message Date
Raja Subramanian
9702d3b541 A couple of more opportunities in stream allocator. (#1906)
1. When re-allocating for a track in DEFICIENT state, try to use
   available headroom to accommodate change before trying to steal
   bits from other tracks.
2. If the changing track gives back bits (because of muting or
   moving to a lower layer subscription), use the returned bits
   to try and boost deficient track(s).
2023-07-26 15:35:07 +05:30
Paul Wells
3980d049c9 close disconnected participants when signal channel fails (#1895)
* close disconnected participants when signal channel fails

* fix typefake

* update reason
2023-07-20 19:23:35 -07:00
Raja Subramanian
469f1cd073 Minor changes to publisher bool. (#1880)
* Minor changes to publisher bool.

* address feedback
2023-07-15 12:43:05 +05:30
Raja Subramanian
4c02a6d717 Time stamp adjustments v2 (I think) (#1875)
* WIP commit

* WIP commit

* WIP commit

* Some clean up
- Removed a chatty debug log
- some spelling, punctuation correction in comments
- missed an `Abs` in check, add it.
2023-07-14 11:47:07 +05:30
David Zhao
557fe7c9d3 Mark room as dirty after track published changes (#1878)
Ensure that we are recomputing NumPublished when needed
2023-07-13 16:33:04 -07:00
Raja Subramanian
ed867fafe5 Log unexpected ICE connection states (#1870) 2023-07-12 10:28:36 +05:30
Raja Subramanian
1cb74b9e1b Check for desired before clean up. (#1865)
Fix a potential race between needsCleanup checking and a re-subscribe
setting desired back to true.
2023-07-10 13:20:57 +05:30
David Zhao
3e71ea3d77 Fixed hidden participant update (#1857) 2023-07-07 13:36:15 -07:00
David Zhao
919355c873 Log additional details when updating participant permissions (#1855)
To help track down sporadic updateParticipant failures
2023-07-06 23:38:01 -07:00
lukasIO
7e96c98dc3 Select highest layer of equal dimensions (#1841)
* Select highest layer of equal dimensions

* clean up test
2023-07-03 17:32:28 +02:00
Raja Subramanian
869f23a054 Close subscriptions promptly (#1845)
* Close subscriptions promptly

Two things:
-----------
1. Because the desired is not changed, the notifiers are not notified
that the subscription is not observing any more. So, that holds
a refernce to the subscription manager.

Address the above by setting `setDesired` to false on all subscriptions
when subscription manager closes. That will remove observer from the
notifiers.

2. When subscription manager is closed, the down track close
is invoked which flows back (with onClose callback of downtrack) to
subscription manager "handleSubscribedTrackClose". That callback
handler sets the subscribed track to nil for that subscription.

A couple of scenarios here
a. Without the above change, desired could have been true and it would
have looked that the track needs to try subscription again because
`needsSubscribe == true` (desired == true && subscribedTrack == nil)

b. Even with the change above, there is a new condition of
`desired == false && subscribedTrack == nil` and there was no handler
for that condition in the reconciler.

Address this by adding a `needsCleanup` function and delete subscription
from the map. Note that the reconciler may not be running to execute
this action as subscription manager would have closed the `closeCh`, but
doing the code in the interest of proper clean up.

* clean up
2023-07-01 12:31:51 +05:30
Raja Subramanian
06f9b574cb Delete down track from receiver in close always. (#1842)
* Delete down track from receiver in close always.

I think with the parallel close in goroutines, it so happens that
peer connection can get closed first and unbind the track.

The delete down track and RTCP reader close was inside if `bound` block.
So, they were not running leaving a dangling down track in the receiver.

* fix tests

* fix test
2023-06-30 20:44:57 +05:30
David Zhao
7be9e2258d Upgrade to Pion 3.0.11, disable active TCP (#1836) 2023-06-28 16:53:58 -07:00
Raja Subramanian
eaf70d5549 Pacer in down stream path. (#1835)
* Pacer interface to send packets

* notify outside lock

* use select

* use pass through pacer

* add error to OnSent

* Remove log which could get noisy

* Starting TWCC work (#1727)

* add packet time

* WIP commit

* WIP commit

* WIP commit

* minor comments

* Some measurements (#1736)

* WIP commit

* some notes

* WIP commit

* variable name change and do not post to closed channel

* unlock

* clean up

* comment

* Hooking up some more bits for TWCC (#1752)

* wake under lock

* Pacer in down stream path.

Splitting out only the pacer from a feature branch to
introduce the concept of pacer.

Currently, there should be no difference in functionality
as a pass through pacer is used.

Another implementation exists which is just put it in a queue and send
it from one goroutine.

A potential implementation to try would be data paced by bandwidth
estimate. That could include priority queues and such.

But, the main goal here is to introduce notion of pacer in the down
stream path and prepare for more congestion control possibilities down
the line.

* Don't need peak detector

* remove throttling of write IO errors
2023-06-28 13:22:44 +05:30
Raja Subramanian
2896aeb126 Set potential codecs for tracks without simulcast codecs. (#1828)
When migrating muted track, need to set potential codecs.
For audio, there may not be `simulcast_codecs` in `AddTrack`.
Hence when migrating a muted track, the potential codecs are not set.
That results in no receivers in relay up track (because all this
could happen before the audio track is unmuted).

So, look at MimeType in TrackInfo (this will be set in OnTrack) and
use that as potential codec.
2023-06-27 04:34:41 +05:30
Raja Subramanian
352bb1d204 Add GetClientInfo interface, to be used to decide migration vs full-reconenct (#1827) 2023-06-26 23:15:53 +05:30
Raja Subramanian
95f360bbce Do not process events after participant close. (#1824)
* Do not process events after participant close.

Avoid processing transport events after participant/transport close.
It causes error logs which are not really errors, but distracting noise.

* correct comment
2023-06-25 09:26:14 +05:30
Raja Subramanian
81f41aca20 Full reconnect on publication mismatch on resume. (#1823)
* Full reconnect on publication mismatch on resume.

It is possible that publications mismatch on resume. An example sequence
- Client sends `AddTrack` for `trackA`
- Server never receives it due to signalling connection breakage.
- Client could do a resume (reconnect=1) noticing signalling connection
  breakage.
- Client's view thinks that `trackA` is known to server, but server does
  not know about it.
- A subsequence offer containing `trackA` triggers `trackInfo not
  available before track publish` and the track does not get published.

Detect the case of missing track and issue a full reconnect.

* UpdateSubscriptions from sync state a la cloud

* add missing shouldReconnect
2023-06-24 19:18:05 +05:30
Raja Subramanian
00558dee5c Close participant on full reconnect. (#1818)
* Close participant on full reconnect.

A full reconnect == irrecoverable error. Participant cannot continue.
So, close the participant when issuing a full reconnect.
That should prevent subscription manager reconcile till the participant
is finally closed down when participant is stale.

* format
2023-06-22 10:09:10 +05:30
Raja Subramanian
2438058474 Drop error logs due to pipe close (#1813) 2023-06-21 14:11:17 +05:30
Raja Subramanian
583648a1ed Avoid closure to reduce life span of objects. (#1809)
A subscription in subscription manager could live till the source
track goes away even though the participant with that subscription
is long gone due to closure on source track removal. Handle it by using
trackID to look up on source track removal.

Also, logging SDPs when a negotiation failure happens to check
if there are any mismatches.
2023-06-20 19:06:01 +05:30
Paul Wells
a6d091a810 update protocol (#1803) 2023-06-18 18:13:34 -07:00
Raja Subramanian
cadf3bf649 Simulate muted audio track publish on migration. (#1799)
Till now only video was using simulated publish when migrating on mute.
But, with `pauseUpstream() + replaceTrack(null)`, it is possible that
client does not send any data when muted.

I do not think there is a problem to do this (even when cleint is
actually using mute which sends silence frames).
2023-06-16 22:00:38 +05:30
Raja Subramanian
908b7a9bb1 Promote some migration logs to Infow (#1798) 2023-06-16 19:00:17 +05:30
David Zhao
f71544e27a Do not send ParticipantJoined webhook if connection was resumed (#1795)
* Do not send ParticipantJoined webhook if connection was resumed

* isResume -> isMigration
2023-06-15 15:39:04 -07:00
Raja Subramanian
12db469297 Better tracking of signalling connection. (#1794)
* Better tracking of signalling connection.

- Reason for closing signaling channel.
- ConnectionID attached to request source/response sink

* Tests
2023-06-15 12:53:34 +05:30
cnderrauber
c91889edfd Add dependency descriptor stream tracker for svc codecs (#1788)
* Add dependency descriptor stream tracker for svc codecs

* Solve comments
2023-06-12 15:07:47 +08:00
Raja Subramanian
7ed3af193a No proof that this helps (#1772) 2023-06-06 11:28:13 +05:30
David Zhao
b5c8fe5294 Perform unsubscribe in parallel to avoid blocking (#1760)
* Perform unsubscribe in parallel to avoid blocking

When unsubscribing from tracks, we flush a blank frame in order to prepare
the transceivers for re-use. This process is blocking for ~200ms. If
the unsubscribes are performed serially, it would prevent other subscribe
operation from continuing.

This PR parallelizes that operation, and ensures subsequent subscribe
operations could reuse the existing transceivers.

* also perform in parallel when uptrack close

* fix a few log fields
2023-06-02 00:13:18 -07:00
cnderrauber
c1842cb54f Avoid reconnect loop for unsupported downtrack (#1754)
* Avoid reconnect loop for unsupported downtrack

If the client subscribes to a track which codec is unsupported by the
client, sfu will trigger negotiation failed and issue a full reconnect
after received client answer. If the client try to subscribe that track
then it will got full reconnect again. That will cause a infinite
reconnect loop until the client don't subscribe that track. This PR
will unsubscribe the error track for the client and send a
SubscriptionResponse that contain the reason to indicates the track's
codec is not supported to avoid the reconnect loop.
2023-05-31 11:41:22 +08:00
Raja Subramanian
1c920812d3 Return max spatial layer from selectors. (#1743)
* Return max spatial layer from selectors.

With differing requirements of SVC and allowing overshoot in Simulcast,
selectors are best placed to indicate what is the max spatial layer when
they indicate a switch to max spatial layer.

* fix test

* prevent race
2023-05-26 12:49:31 +05:30
Raja Subramanian
0354626bfc Adjust sender report time stamp for slow publishers. (#1740)
It is possible that publisher paces the media.
So, RTCP sender report from publisher could be ahead of
what is being fowarded by a good amount (have seen up to 2 seconds
ahead). Using the forwarded time stamp for RTCP sender report
in the down stream leads to jumps back and forth in the down track
RTCP sender report.

So, look at the publisher's RTCP sender report to check for it being
ahead and use the publisher rate as a guide.
2023-05-25 21:55:54 +05:30
David Zhao
61d393e709 Disable active TCP by rolling back to ICE v2.3.3 (#1735)
* Revert "Disable active TCP (#1726)"

This reverts commit 5260907ffe.

* Disable active TCP by rolling back to ICE v2.3.3
2023-05-23 21:27:03 -07:00
Raja Subramanian
bbbe815260 Init min to max MOS (#1734)
* Init min to max MOS

Could have been contributing to low p50 score in prom stats.

* don't need to reset on no tracks as default is that
2023-05-23 12:55:24 +05:30
David Zhao
12c6f1e12c Added Xiaomi 2201117TI to devices that does not support H.264 (#1728) 2023-05-22 21:38:56 -07:00
David Zhao
5260907ffe Disable active TCP (#1726)
Active TCP was added in pion/ice v2.3.4. This is causing a couple of issues for us.

Active TCP does not make sense for an SFU. Clients are expected to be behind NAT and we should not be dialing them. Instead, LiveKit exposes a TCP port so clients could dial in
Active TCP is causing all iOS clients to become disconnected immediately. This is impacting all version of libwebrtc-based iOS clients (tested from M104 to M111)
2023-05-19 23:00:06 -07:00
cnderrauber
1c88a03366 Don't add nack if it is already present in track codec (#1714) 2023-05-17 18:48:54 +08:00
Raja Subramanian
a085afc6ee Send quality stats to prometheus. (#1708) 2023-05-12 09:44:03 +05:30
Benjamin Pracht
4244542840 Adopt WebRTCConfig from mediatransportutil (#1707)
This also adds support for inline fields in ToCLIFlagNames
2023-05-10 20:00:34 -07:00
Raja Subramanian
f543e3f8d0 Send left over RTCP packets. (#1699) 2023-05-09 18:46:30 +05:30
Raja Subramanian
14a2d06bcd RTCP sender reports every three seconds. (#1692)
* RTCP sender reports every three seconds.

Ideally, we should be sending this based on data rate.
But, increasing frequency a little as a lost sender report
means the client may not have sender report for 10 seconds
and that could affect sync. We do receiver reports once a second.
Thought of setting this to that level too, but not making a big change
from existing rate.

Also, simplifying the RTCP send loop. Don't need to hold and
do the processing after collecting all reports.

* consistent use of GetSubscribedTracks
2023-05-07 10:09:30 +05:30
Raja Subramanian
3fb93135f5 Experimental flag to try time stamp adjustment to control drift. (#1687)
* Experimental flag to try time stamp adjustment to control drift.

There is a config to enable this.

Using a PID controller to try and keep the sample rate at expected
value. Need to be seen if this works well. Adjustment are limited
to 25 ms max at a time to ensure there are no large jumps.
And it is applied when doing RTCP sender report which happens
once in 5 seconds currently for both audio and video tracks.

A nice introduction to PID controllers - https://alphaville.github.io/qub/pid-101/#/
Implementation borrowed from - https://github.com/pms67/PID

A few things TODO
1. PID controller tuning is a process. Have picked values from test from
   that implementation above. May not be the best. Need to try.
2. Can potentially run this more often. Rather than running it only when
   running RTCP sender report (which is once in 5 seconds now), can
   potentially run it every second and limit the amount of change to
   something like 10 ms max.

* remove unused variable

* debug log a bit more
2023-05-06 11:52:57 +05:30
David Zhao
0586009e0d Do not send hidden participants after resume (#1689) 2023-05-05 22:38:17 -07:00
Raja Subramanian
25d6fd751f Cleaning up smoothed OWD calculation for sender report. (#1684)
* 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

* anticipatory clean up

* further clean up

* add received sender report logging
2023-05-05 13:14:12 +05:30
cnderrauber
298ebaee78 Suppress error log of setPrefferedCodec for simulcast codec track (#1682) 2023-05-04 15:14:20 +08:00
David Zhao
5fcd682fb0 Refactor participant metadata updates to avoid duplication (#1679)
* Refactor participant metadata updates to avoid duplication

* generated fakes
2023-05-03 13:50:45 -07:00
Paul Wells
87e2b2366e reduce log level of signal close errors (#1675)
* reduce log level of signal close errors

* update psrpc

* cleanup

* cleanup
2023-05-02 08:31:12 -07:00
Raja Subramanian
35b8319b08 Remove disallowed subscriptions on close. (#1668)
With subscription manager, there is no need to tell a publisher
about a subscriber going away. Before subscription manager,
the up track manager of a participant (i. e. the publisher side)
was holding a list of pending subscriptions for its published tracks
and that had to be cleaned up if one of the subscriber goes away.
That is not the case any more.

Also set publisherID early so that subscription permission update has
the right publisherID. In fact, saw an empty ID in the logs and saw
that we still have the disallowed subscription handling which is not
necessary any more.
2023-04-29 09:18:07 +05:30
Benjamin Pracht
a08cd23b6d Adopt pion logging initialization moving to protocol (#1667) 2023-04-28 10:51:41 -07:00
Raja Subramanian
1148d38978 hopefully more stable tests (#1665)
* hopefully more stable tests

* do eventual checks as some callbacks happen in go routines.

Needs a bit more work to ensure that some conditions do not happen.
But, with goroutines, the amount of wait is always tricky.``
2023-04-28 17:02:31 +05:30