From a8da4872b19f4d526d67f41c0d25080bc4c3d01d Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Mon, 14 Oct 2024 17:21:42 +0530 Subject: [PATCH] Drop quality a bit faster on score trending lower to be more responsive. (#3093) Also, logging a bit more about quality changes to understand why high(ish) loss does not drop quality. Will remove the loss thresholded logging after collecting some data. --- go.mod | 10 ++--- go.sum | 20 ++++----- pkg/sfu/connectionquality/scorer.go | 68 +++++++++++++++++------------ 3 files changed, 55 insertions(+), 43 deletions(-) diff --git a/go.mod b/go.mod index 8f9f080a4..c09e21f2b 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/jxskiss/base62 v1.1.0 github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1 github.com/livekit/mediatransportutil v0.0.0-20240730083616-559fa5ece598 - github.com/livekit/protocol v1.23.1-0.20241008082340-082848150f8f + github.com/livekit/protocol v1.24.1-0.20241014105216-a3ae28b3c5e1 github.com/livekit/psrpc v0.6.1-0.20240924010758-9f0a4268a3b9 github.com/mackerelio/go-osstat v0.2.5 github.com/magefile/mage v1.15.0 @@ -50,9 +50,9 @@ require ( go.uber.org/atomic v1.11.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 - golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6 + golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c golang.org/x/sync v0.8.0 - google.golang.org/protobuf v1.34.2 + google.golang.org/protobuf v1.35.1 gopkg.in/yaml.v3 v3.0.1 ) @@ -90,7 +90,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.7 // indirect github.com/hashicorp/golang-lru v0.5.4 // indirect github.com/josharian/native v1.1.0 // indirect - github.com/klauspost/compress v1.17.10 // indirect + github.com/klauspost/compress v1.17.11 // indirect github.com/klauspost/cpuid/v2 v2.2.6 // indirect github.com/lithammer/shortuuid/v4 v4.0.0 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect @@ -134,7 +134,7 @@ require ( golang.org/x/text v0.19.0 // indirect golang.org/x/tools v0.26.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240930140551-af27646dc61f // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20241007155032-5fefd90f89a9 // indirect google.golang.org/grpc v1.67.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/go.sum b/go.sum index 198b0c6e1..ab6e1bf0d 100644 --- a/go.sum +++ b/go.sum @@ -142,8 +142,8 @@ github.com/jxskiss/base62 v1.1.0 h1:A5zbF8v8WXx2xixnAKD2w+abC+sIzYJX+nxmhA6HWFw= github.com/jxskiss/base62 v1.1.0/go.mod h1:HhWAlUXvxKThfOlZbcuFzsqwtF5TcqS9ru3y5GfjWAc= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/klauspost/compress v1.17.10 h1:oXAz+Vh0PMUvJczoi+flxpnBEPxoER1IaAnU/NMPtT0= -github.com/klauspost/compress v1.17.10/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0= +github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IXrJmUc= +github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0= github.com/klauspost/cpuid/v2 v2.2.6 h1:ndNyv040zDGIDh8thGkXYjnFtiN02M1PVVF+JE/48xc= github.com/klauspost/cpuid/v2 v2.2.6/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= @@ -165,8 +165,8 @@ github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1 h1:jm09419p0lqTkD github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1/go.mod h1:Rs3MhFwutWhGwmY1VQsygw28z5bWcnEYmS1OG9OxjOQ= github.com/livekit/mediatransportutil v0.0.0-20240730083616-559fa5ece598 h1:yLlkHk2feSLHstD9n4VKg7YEBR4rLODTI4WE8gNBEnQ= github.com/livekit/mediatransportutil v0.0.0-20240730083616-559fa5ece598/go.mod h1:jwKUCmObuiEDH0iiuJHaGMXwRs3RjrB4G6qqgkr/5oE= -github.com/livekit/protocol v1.23.1-0.20241008082340-082848150f8f h1:ZtT9U7Mfcfv+uI1uCZUUeBf/R0uin+KIHanyStuXr68= -github.com/livekit/protocol v1.23.1-0.20241008082340-082848150f8f/go.mod h1:nxRzmQBKSYK64gqr7ABWwt78hvrgiO2wYuCojRYb7Gs= +github.com/livekit/protocol v1.24.1-0.20241014105216-a3ae28b3c5e1 h1:2WBCNRuoVZ1skSHGD4gwMVbz7A2JtQEtYugrGklxJZk= +github.com/livekit/protocol v1.24.1-0.20241014105216-a3ae28b3c5e1/go.mod h1:nxRzmQBKSYK64gqr7ABWwt78hvrgiO2wYuCojRYb7Gs= github.com/livekit/psrpc v0.6.1-0.20240924010758-9f0a4268a3b9 h1:33oBjGpVD9tYkDXQU42tnHl8eCX9G6PVUToBVuCUyOs= github.com/livekit/psrpc v0.6.1-0.20240924010758-9f0a4268a3b9/go.mod h1:CQUBSPfYYAaevg1TNCc6/aYsa8DJH4jSRFdCeSZk5u0= github.com/mackerelio/go-osstat v0.2.5 h1:+MqTbZUhoIt4m8qzkVoXUJg1EuifwlAJSk4Yl2GXh+o= @@ -363,8 +363,8 @@ golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1m golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.28.0 h1:GBDwsMXVQi34v5CCYUm2jkJvu4cbtru2U4TN2PSyQnw= golang.org/x/crypto v0.28.0/go.mod h1:rmgy+3RHxRZMyY0jjAJShp2zgEdOqj2AO7U0pYmeQ7U= -golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6 h1:1wqE9dj9NpSm04INVsJhhEUzhuDVjbcyKH91sVyPATw= -golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6/go.mod h1:NQtJDoLvd6faHhE7m4T/1IY708gDefGGjR/iUW8yQQ8= +golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c h1:7dEasQXItcW1xKJ2+gg5VOiBnqWrJc+rq0DPKyvvdbY= +golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c/go.mod h1:NQtJDoLvd6faHhE7m4T/1IY708gDefGGjR/iUW8yQQ8= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= @@ -483,14 +483,14 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 h1:wKguEg1hsxI2/L3hUYrpo1RVi48K+uTyzKqprwLXsb8= google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142/go.mod h1:d6be+8HhtEtucleCbxpPW9PA9XwISACu8nvpPqF0BVo= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240930140551-af27646dc61f h1:cUMEy+8oS78BWIH9OWazBkzbr090Od9tWBNtZHkOhf0= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240930140551-af27646dc61f/go.mod h1:UqMtugtsSgubUsoxbuAoiCXvqvErP7Gf0so0mK9tHxU= +google.golang.org/genproto/googleapis/rpc v0.0.0-20241007155032-5fefd90f89a9 h1:QCqS/PdaHTSWGvupk2F/ehwHtGc0/GYkT+3GAcR1CCc= +google.golang.org/genproto/googleapis/rpc v0.0.0-20241007155032-5fefd90f89a9/go.mod h1:GX3210XPVPUjJbTUbvwI8f2IpZDMZuPJWDzDuebbviI= google.golang.org/grpc v1.67.1 h1:zWnc1Vrcno+lHZCOofnIMvycFcc0QRGIzm9dhnDX68E= google.golang.org/grpc v1.67.1/go.mod h1:1gLDyUQU7CTLJI90u3nXZ9ekeghjeM7pTDZlqFNg2AA= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= -google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= -google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= +google.golang.org/protobuf v1.35.1 h1:m3LfL6/Ca+fqnjnlqQXNpFPABW1UD7mjh8KO2mKFytA= +google.golang.org/protobuf v1.35.1/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/pkg/sfu/connectionquality/scorer.go b/pkg/sfu/connectionquality/scorer.go index 0ad03c78b..d5afea6f4 100644 --- a/pkg/sfu/connectionquality/scorer.go +++ b/pkg/sfu/connectionquality/scorer.go @@ -33,12 +33,12 @@ const ( cMaxScore = float64(100.0) cMinScore = float64(30.0) - increaseFactor = float64(0.4) // slower increase, i. e. when score is recovering move up slower -> conservative - decreaseFactor = float64(0.7) // faster decrease, i. e. when score is dropping move down faster -> aggressive to be responsive to quality drops + cIncreaseFactor = float64(0.4) // slower increase, i. e. when score is recovering move up slower -> conservative + cDecreaseFactor = float64(0.8) // faster decrease, i. e. when score is dropping move down faster -> aggressive to be responsive to quality drops - distanceWeight = float64(35.0) // each spatial layer missed drops a quality level + cDistanceWeight = float64(35.0) // each spatial layer missed drops a quality level - unmuteTimeThreshold = float64(0.5) + cUnmuteTimeThreshold = float64(0.5) ) var ( @@ -88,8 +88,8 @@ func (w *windowStat) calculatePacketScore(plw float64, includeRTT bool, includeJ // discount out-of-order packets from loss to deal with a scenario like // 1. up stream has loss // 2. down stream forwards with loss/hole in sequence number - // 3. down stream client reports a certain number of loss - // 4. while processing that, up stream could have retransmitted missing packets + // 3. down stream client reports a certain number of loss via RTCP RR + // 4. while processing that RTCP RR, up stream could have retransmitted missing packets // 5. those retransmitted packets are forwarded, // - server's view: it has forwarded those packets // - client's view: it had not seen those packets when sending RTCP RR @@ -397,7 +397,7 @@ func (q *qualityScorer) updateAtLocked(stat *windowStat, at time.Time) { plw := q.getPacketLossWeight(stat) reason := "none" - var score float64 + var score, packetScore, bitrateScore, layerScore float64 if stat.packets+stat.packetsPadding == 0 { if !stat.lastRTCPAt.IsZero() && at.Sub(stat.lastRTCPAt) > stat.duration { reason = "dry" @@ -407,9 +407,9 @@ func (q *qualityScorer) updateAtLocked(stat *windowStat, at time.Time) { score = qualityTransitionScore[livekit.ConnectionQuality_POOR] } } else { - packetScore := stat.calculatePacketScore(plw, q.params.IncludeRTT, q.params.IncludeJitter) - bitrateScore := stat.calculateBitrateScore(expectedBits, q.params.EnableBitrateScore) - layerScore := math.Max(math.Min(cMaxScore, cMaxScore-(expectedDistance*distanceWeight)), 0.0) + packetScore = stat.calculatePacketScore(plw, q.params.IncludeRTT, q.params.IncludeJitter) + bitrateScore = stat.calculateBitrateScore(expectedBits, q.params.EnableBitrateScore) + layerScore = math.Max(math.Min(cMaxScore, cMaxScore-(expectedDistance*cDistanceWeight)), 0.0) minScore := math.Min(packetScore, bitrateScore) minScore = math.Min(minScore, layerScore) @@ -428,9 +428,9 @@ func (q *qualityScorer) updateAtLocked(stat *windowStat, at time.Time) { score = layerScore } - factor := increaseFactor + factor := cIncreaseFactor if score < q.score { - factor = decreaseFactor + factor = cDecreaseFactor } score = factor*score + (1.0-factor)*q.score if score < cMinScore { @@ -440,23 +440,35 @@ func (q *qualityScorer) updateAtLocked(stat *windowStat, at time.Time) { score = cMinScore } } + prevCQ := scoreToConnectionQuality(q.score) currCQ := scoreToConnectionQuality(score) - if utils.IsConnectionQualityLower(prevCQ, currCQ) { - q.params.Logger.Debugw( - "quality drop", - "reason", reason, - "prevScore", q.score, - "prevQuality", prevCQ, - "prevStat", &q.stat, - "score", score, - "quality", currCQ, - "stat", stat, - "packetLossWeight", plw, - "maxPPS", q.maxPPS, - "expectedBits", expectedBits, - "expectedDistance", expectedDistance, - ) + ulgr := q.params.Logger.WithUnlikelyValues( + "reason", reason, + "prevScore", q.score, + "prevQuality", prevCQ, + "prevStat", &q.stat, + "score", score, + "packetScore", packetScore, + "layerScore", layerScore, + "bitrateScore", bitrateScore, + "quality", currCQ, + "stat", stat, + "packetLossWeight", plw, + "maxPPS", q.maxPPS, + "expectedBits", expectedBits, + "expectedDistance", expectedDistance, + ) + switch { + case utils.IsConnectionQualityLower(prevCQ, currCQ): + ulgr.Debugw("quality drop") + case utils.IsConnectionQualityHigher(prevCQ, currCQ): + ulgr.Debugw("quality rise") + default: + packets := stat.packets + stat.packetsPadding + if packets != 0 && (stat.packetsLost*100/packets) > 10 { + ulgr.Debugw("quality hold - high loss") + } } q.score = score @@ -504,7 +516,7 @@ func (q *qualityScorer) isUnmutedEnough(at time.Time) bool { sinceLastUpdate := at.Sub(q.lastUpdateAt) - return validDuration.Seconds()/sinceLastUpdate.Seconds() > unmuteTimeThreshold + return validDuration.Seconds()/sinceLastUpdate.Seconds() > cUnmuteTimeThreshold } func (q *qualityScorer) isLayerMuted() bool {