-
Notifications
You must be signed in to change notification settings - Fork 773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce DISCONNECTED
connection quality.
#2265
Conversation
Currently, this state happens when any up stream track does not send any packets in an analysis window when it is expected to send packets. This can be used by participants to know the quality of a potentially disconnected participant. Previously, it took 20 - 30 seconds for the stale timeout to kick in and disconnect the limbo participant which triggered a participant update through which other participants knew about it. Previously, `POOR` quality was also overloaded to denote that the up stream is not sending any packets. With this change, that is a separate indicator, i. e. `DISCONNECTED`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
pkg/sfu/connectionquality/scorer.go
Outdated
@@ -219,7 +220,10 @@ func (q *qualityScorer) Start() { | |||
func (q *qualityScorer) updateMuteAtLocked(isMuted bool, at time.Time) { | |||
if isMuted { | |||
q.mutedAt = at | |||
q.score = maxScore | |||
// mting when DISCONNECTED should not push quality to EXCELLENT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
muting?
@@ -531,7 +536,11 @@ func scoreToConnectionQuality(score float64) livekit.ConnectionQuality { | |||
return livekit.ConnectionQuality_GOOD | |||
} | |||
|
|||
return livekit.ConnectionQuality_POOR | |||
if score > 20.0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: better to use const cMinScore
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was just using numbers there. Will change it. Actually, can use 'cDisconnectedScore' which might be easier to read. Will make constants for others too in a separate PR.
hi, when do you plan to release this? |
Currently, this state happens when any up stream track does not send any packets in an analysis window when it is expected to send packets.
This can be used by participants to know the quality of a potentially disconnected participant. Previously, it took 20 - 30 seconds for the stale timeout to kick in and disconnect the limbo participant which triggered a participant update through which other participants knew about it.
Previously,
POOR
quality was also overloaded to denote that the up stream is not sending any packets. With this change, that is a separate indicator, i. e.DISCONNECTED
.For #2048