Commit Graph

1531 Commits

Author SHA1 Message Date
boks1971 0bf93042ed Use min packets threshold for NACK based congestion signal. 2023-07-13 00:18:01 +05:30
Raja Subramanian 8dc2c005c3 Add ability to roll back video layer selection. (#1871)
* Add ability to roll back video layer selection.

Not currently useful, but it is possible to do things like not
applying a layer switch if the switch point time stamp is too far back.

Add ability to roll back a layer switch and invoke rollback if
a packet was selected for forwarding, but a subsequent error or decision
to drop the packet can rollback layer switch if that was the switching
packet.

In current code, the paths where a packet can be dropped after selection
does not happen at switch points. So, it was okay to apply the selection
unconditionally. But, adding the call to rollback in the current code
also in all paths where packet is dropped after selection for consistent
code flow.

* separate switch for temporal layer
2023-07-12 14:12:00 +05:30
Raja Subramanian ed867fafe5 Log unexpected ICE connection states (#1870) 2023-07-12 10:28:36 +05:30
Raja Subramanian 5459bd2931 Push track quality to poor on a bandwidth constrained pause. (#1867)
* Push track quality to poor on a bandwidth constrained pause.

* add tests

* scale distance by divisor

* fix test distance to desired

* wait longer for subscription manager to reconcile
2023-07-11 15:29:35 +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
Raja Subramanian e6f5f2f344 Prevent anachronous sample reading. (#1863)
* Prevenet anachronous sample reading.

Not so pretty way of solving this. Please let me know if you have
thoughts.

Passing in time allows testing easier. But, that also leads to
time reversal problems. Example scenario
1. Connection stats worker gets a time and initiates quality
   calculation.
2. A layer transition is recorded after that.
3. By the time, scorer is called to calculate score with time from Step
   1, there is time reversal and results in anachronous sample.

One option is to use a scorer lock in connection stats module and wrap
all calls to scorer in that lock, but that does not prevent the passed
in time stamps themselves getting out of order. Also, stand alond use
of scorer in some other context will be problematic.

Doing the hybrid thing of taking current time in scorer if passed in
time is zero so that scorer lock domain controls it.

* use zero time everywhere in normal flow

* make APIs with and without time passed in as Paul suggested
2023-07-10 08:39:52 +05:30
David Zhao 42a7d52272 Return 404 with DeleteRoom/RemoveParticipant when deleting non-existent resources (#1860)
Fixes #1587
2023-07-08 22:30:49 -07:00
David Zhao cbec68ae44 Do not use cancellable context for Redis operations (#1859) 2023-07-08 12:06:31 -07:00
Raja Subramanian bf3732b898 Remove noisy debug logs. (#1858) 2023-07-08 11:58:56 +05:30
David Zhao 3e71ea3d77 Fixed hidden participant update (#1857) 2023-07-07 13:36:15 -07:00
cnderrauber 873c87f24b Fix nack issue for svc codecs (#1856)
* Fix nack issue for svc codecs

* Fix test
2023-07-07 15:46:18 +08: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
Raja Subramanian 6198aac7a8 Make a default config variable (#1848)
* Make a default config variable

* decoder.Decode needs pointer to config
2023-07-03 23:49:03 +05:30
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 e3954d1d64 Use timed aggregator. (#1843)
* Use timed aggregator.

For aggregate bitrate and average distance from desired.

Also, clean up debug added to track leak.

* update deps
2023-07-01 10:21:15 +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
Raja Subramanian 496656627e Logging more to understand layer transition leak better. (#1840) 2023-06-30 11:59:53 +05:30
Raja Subramanian 69a1e572be Attempt to reduce disruption due to probe. (#1839)
* Make congestion controller probe config

* Wait for enough estimate samples

* fixes

* format

* limit number of times a packet is ACKed

* ramp up probe duration

* go format

* correct comment

* restore default

* add float64 type to generated CLI
2023-06-30 11:09:46 +05:30
David Zhao 7be9e2258d Upgrade to Pion 3.0.11, disable active TCP (#1836) 2023-06-28 16:53:58 -07:00
Juan Navarro 2668073c29 Honor bind address passed as --bind also for RTC ports (#1815)
* Use net.JoinHostPort to build "host:port" strings for `net.Listen`

net.JoinHostPort provides a unified way of building strings of the form
"Host:Port", abstracting the particular syntax requirements of some
methods in the `net` package (namely, that IPv4 addresses can be given
as-is to `net.Listen`, but IPv6 addresses must be given enclosed in
square brackets).

This change makes sense because an address such as `[::1]` is *not* a
valid IPv6 address; the square brackets are just a detail particular to
the Go `net` library. As such, this syntax shouldn't be exposed to the
user, and configuration should just accept valid IPv6 addresses and
convert them as needed for usage within the code.

* Use '--bind' CLI flag to also filter RTC bind address

The local address passed to a command such as

    livekit-server --dev --bind 127.0.0.1

was being used as binding address for the TCP WebSocket port, but was
being ignored for RTC connections.

With `--dev`, the conf.RTC.UDPPort config is set to 7882, which enables
"UDP muxing" mechanism. Without interface or address filtering, Pion
would try to bind to port 7882 on *all* interfaces.

This was failing on a system with IPv6 enabled, when trying to bind to
an IPv6 address of the `docker0` interface. It seems to make sense that
the user-passed bind addresses are also honored for the RTC port
bindings.
2023-06-28 16:52:43 -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 2b0a470474 Less flapping in probe. (#1834)
- Increase max interval between probes to 2 minutes.
- Use a minimum probe rate of 200 kbps. This is to ensure that
the probe rate is decent and can produce a stronger signal.
2023-06-28 12:48:38 +05:30
Raja Subramanian cea41e4189 Discount out-of-order packets in downstream score. (#1831)
* Discount out-of-order packets in downstream score.

More notes inline.

* correct comment

* clean up comment
2023-06-27 17:44:53 +05:30
cnderrauber 5b975af55f Refine dependency descriptor based selection forwarder (#1808)
* Don't update dependency info if unordered packet received

* Trace all active svc chains for downtrack

* Try to keep lower decode target decodable

* remove comments

* Test case

* clean code

* solve comments
2023-06-27 15:11:06 +08:00
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 8ac394c5bb Removing commented out short cut path, don't need more debug data. (#1822) 2023-06-23 14:18:55 +05:30
Benjamin Pracht 1f6efedd31 Send updated events on state updates (#1819) 2023-06-22 09:20:58 -07:00
Paul Wells c38791ff0a stop retrying signal connection if the request context is closed (#1820) 2023-06-22 07:09:34 -07:00
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 84994b39ab Make the samples string more readable. (#1810) 2023-06-21 11:35:38 +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
Raja Subramanian 27051e9999 It is possible that pipe is closed before blank frame send, do not warn (#1807) 2023-06-20 11:58:01 +05:30
Raja Subramanian f11a7a229f Remove unnecessary check (#1806) 2023-06-19 16:40:05 +05:30
Paul Wells a6d091a810 update protocol (#1803) 2023-06-18 18:13:34 -07:00
Raja Subramanian 40f5902d36 Consistently use connID as log tag (#1801) 2023-06-17 21:02:02 +05:30
Raja Subramanian 2383234f6e Simplify sliding window collapse. (#1802)
* Simplify sliding window collapse.

Keep the same value collapsing simple.
Add it to sliding window as long as same value is received for longer
than collapse threshold.
But, add a prune with three conditions to process the siliding window
to ensure only valid samples are kept.

* flip the order of validity window and same value pruning

* increase collapse threshold to 0.5 seconds during non-probe
2023-06-17 18:56:38 +05:30
Raja Subramanian 395f403132 Small stream allocator tweaks. (#1800)
1. Probe end time needs to include the probe cluster running time also.
2. Apply collapse window only within the sliding window. This is to
   prevent cases of some old data declaring congestion. For example,
   an estimate could have fallen 15 seconds ago and there might have
   been a bunch of estimates at that fallen value. And the whole
   sliding window could have that value at some point. But, a further
   drop may trigger congestion detection. But, that might be acting too
   fast, i. e. on one instance of value fall. Change it so that we
   detect if there is a fall within the sliding window and apply
   collapse based on that.
2023-06-17 12:35:29 +05:30
Benjamin Pracht 552e3758d5 Add IngressUpdated event (#1775) 2023-06-16 10:58:49 -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
Raja Subramanian 6946d0a3a1 Do not mute forwarder when paused to bandwidth congestion. (#1796)
* Do not mute forwarder when paused to bandwidth congestion.

Detailed notes in code.

* remove word
2023-06-16 12:08:01 +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
paulwe 0dab55556d add drain function to rtc service 2023-06-15 11:56:41 -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
shishirng 2dd4e1365b Send EgressUpdated event (#1792)
Signed-off-by: shishir gowda <shishir@livekit.io>
2023-06-14 18:56:07 -04:00