From 1d3faefc5eb91caab0524e10ea1584b48a31a7a2 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Thu, 18 May 2023 20:16:43 +0530 Subject: [PATCH] More scoring tweaks (#1719) 1. Completely removing RTT and jitter from score calculation. Need to do more work there. a. Jitter is slow moving (RFC 3550 formula is designed that way). But, we still get high values at times. Ideally, that should penalise the score, but due to jitter buffer, effect may not be too bad. b. Need to smooth RTT. It is based on receiver report and if one sample causes a high number, score could be penalised (this was being used in down track direction only). One option is to smooth it like the jitter formula above and try using it. But, for now, disabling that also. 2. When receiving lesser number of packets (for example DTX), reduce the weight of packet loss with a quadratic relationship to packet loss ratio. Previously using a square root and it was potentially weighting it too high. For example, if only 5 packets were received due to DTX instead of 50, we were still giving 30% weight (sqrt(0.1)). Now, it gets 1% weight. So, if one of those 5 packets were lost (20% packet loss ratio), it still does not get much weight as the number of packets is low., 3. Slightly slower decrease in score (in EWMA) 4. When using RED, increase packet loss weight thresholds to be able to take more loss before penalizing score. --- pkg/sfu/buffer/rtpstats.go | 26 +++-- pkg/sfu/connectionquality/connectionstats.go | 18 +-- .../connectionquality/connectionstats_test.go | 108 +++++++++--------- pkg/sfu/connectionquality/scorer.go | 23 ++-- pkg/sfu/downtrack.go | 1 - pkg/sfu/receiver.go | 9 +- 6 files changed, 93 insertions(+), 92 deletions(-) diff --git a/pkg/sfu/buffer/rtpstats.go b/pkg/sfu/buffer/rtpstats.go index 0667f8464..873c8d59d 100644 --- a/pkg/sfu/buffer/rtpstats.go +++ b/pkg/sfu/buffer/rtpstats.go @@ -1896,18 +1896,20 @@ func (p *PIDController) Update(setpoint, measurement float64, at time.Time) floa p.prevError = errorTerm p.prevMeasurement = measurement p.prevMeasurementTime = at - p.logger.Debugw( - "pid controller", - "setpoint", setpoint, - "measurement", measurement, - "errorTerm", errorTerm, - "proportional", proportional, - "integral", iVal, - "integralLimited", boundIVal, - "derivative", p.dVal, - "output", output, - "outputLimited", boundOutput, - ) + /* + p.logger.Debugw( + "pid controller", + "setpoint", setpoint, + "measurement", measurement, + "errorTerm", errorTerm, + "proportional", proportional, + "integral", iVal, + "integralLimited", boundIVal, + "derivative", p.dVal, + "output", output, + "outputLimited", boundOutput, + ) + */ return boundOutput } diff --git a/pkg/sfu/connectionquality/connectionstats.go b/pkg/sfu/connectionquality/connectionstats.go index 0da29996a..9cb5c716c 100644 --- a/pkg/sfu/connectionquality/connectionstats.go +++ b/pkg/sfu/connectionquality/connectionstats.go @@ -25,8 +25,8 @@ type ConnectionStatsParams struct { UpdateInterval time.Duration MimeType string IsFECEnabled bool - IsDependentRTT bool - IsDependentJitter bool + IncludeRTT bool + IncludeJitter bool GetDeltaStats func() map[uint32]*buffer.StreamStatsWithLayers GetDeltaStatsOverridden func() map[uint32]*buffer.StreamStatsWithLayers GetLastReceiverReportTime func() time.Time @@ -53,10 +53,10 @@ func NewConnectionStats(params ConnectionStatsParams) *ConnectionStats { return &ConnectionStats{ params: params, scorer: newQualityScorer(qualityScorerParams{ - PacketLossWeight: getPacketLossWeight(params.MimeType, params.IsFECEnabled), // LK-TODO: have to notify codec change? - IsDependentRTT: params.IsDependentRTT, - IsDependentJitter: params.IsDependentJitter, - Logger: params.Logger, + PacketLossWeight: getPacketLossWeight(params.MimeType, params.IsFECEnabled), // LK-TODO: have to notify codec change? + IncludeRTT: params.IncludeRTT, + IncludeJitter: params.IncludeJitter, + Logger: params.Logger, }), done: core.NewFuse(), } @@ -293,10 +293,10 @@ func getPacketLossWeight(mimeType string, isFecEnabled bool) float64 { } case strings.EqualFold(mimeType, "audio/red"): - // 6.66%: fall to GOOD, 20.0%: fall to POOR - plw = 3.0 + // 10%: fall to GOOD, 30.0%: fall to POOR + plw = 2.0 if isFecEnabled { - // 10%: fall to GOOD, 30.0%: fall to POOR + // 15%: fall to GOOD, 45.0%: fall to POOR plw /= 1.5 } diff --git a/pkg/sfu/connectionquality/connectionstats_test.go b/pkg/sfu/connectionquality/connectionstats_test.go index 2ed5f3789..57793ef85 100644 --- a/pkg/sfu/connectionquality/connectionstats_test.go +++ b/pkg/sfu/connectionquality/connectionstats_test.go @@ -12,19 +12,19 @@ import ( "github.com/livekit/protocol/logger" ) -func newConnectionStats(mimeType string, isFECEnabled bool, isDependentRTT bool, isDependentJitter bool) *ConnectionStats { +func newConnectionStats(mimeType string, isFECEnabled bool, includeRTT bool, includeJitter bool) *ConnectionStats { return NewConnectionStats(ConnectionStatsParams{ - MimeType: mimeType, - IsFECEnabled: isFECEnabled, - IsDependentRTT: isDependentRTT, - IsDependentJitter: isDependentJitter, - Logger: logger.GetLogger(), + MimeType: mimeType, + IsFECEnabled: isFECEnabled, + IncludeRTT: includeRTT, + IncludeJitter: includeJitter, + Logger: logger.GetLogger(), }) } func TestConnectionQuality(t *testing.T) { t.Run("quality scorer state machine", func(t *testing.T) { - cs := newConnectionStats("audio/opus", false, false, false) + cs := newConnectionStats("audio/opus", false, true, true) duration := 5 * time.Second now := time.Now() @@ -184,7 +184,7 @@ func TestConnectionQuality(t *testing.T) { StartTime: now, Duration: duration, Packets: 250, - PacketsLost: 25, + PacketsLost: 30, }, }, } @@ -239,7 +239,7 @@ func TestConnectionQuality(t *testing.T) { cs.UpdateMute(false, now.Add(2*time.Second)) // with lesser number of packet (simulating DTX). - // even higher loss (like 10%) should only knock down quality to GOOD, typically would be POOR at that loss rate + // even higher loss (like 10%) should not knock down quality due to quadratic weighting of packet loss ratio streams = map[uint32]*buffer.StreamStatsWithLayers{ 1: { RTPStats: &buffer.RTPDeltaInfo{ @@ -252,8 +252,8 @@ func TestConnectionQuality(t *testing.T) { } cs.updateScore(streams, now.Add(duration)) mos, quality = cs.GetScoreAndQuality() - require.Greater(t, float32(4.1), mos) - require.Equal(t, livekit.ConnectionQuality_GOOD, quality) + require.Greater(t, float32(4.6), mos) + require.Equal(t, livekit.ConnectionQuality_EXCELLENT, quality) // mute/unmute to bring quality back up now = now.Add(duration) @@ -269,7 +269,7 @@ func TestConnectionQuality(t *testing.T) { Duration: duration, Packets: 250, PacketsLost: 5, - RttMax: 300, + RttMax: 400, JitterMax: 30000, }, }, @@ -294,7 +294,7 @@ func TestConnectionQuality(t *testing.T) { StartTime: now, Duration: duration, Packets: 250, - Bytes: 8_000_000 / 8 / 4, + Bytes: 8_000_000 / 8 / 5, }, }, } @@ -320,7 +320,7 @@ func TestConnectionQuality(t *testing.T) { StartTime: now, Duration: duration, Packets: 250, - Bytes: 8_000_000 / 8 / 4, + Bytes: 8_000_000 / 8 / 5, }, }, } @@ -345,7 +345,7 @@ func TestConnectionQuality(t *testing.T) { StartTime: now, Duration: duration, Packets: 250, - Bytes: 8_000_000 / 8 / 4, + Bytes: 8_000_000 / 8 / 5, }, }, } @@ -356,7 +356,7 @@ func TestConnectionQuality(t *testing.T) { }) t.Run("quality scorer dependent rtt", func(t *testing.T) { - cs := newConnectionStats("audio/opus", false, true, false) + cs := newConnectionStats("audio/opus", false, false, true) duration := 5 * time.Second now := time.Now() @@ -384,7 +384,7 @@ func TestConnectionQuality(t *testing.T) { }) t.Run("quality scorer dependent jitter", func(t *testing.T) { - cs := newConnectionStats("audio/opus", false, false, true) + cs := newConnectionStats("audio/opus", false, true, false) duration := 5 * time.Second now := time.Now() @@ -462,47 +462,23 @@ func TestConnectionQuality(t *testing.T) { expectedQuality: livekit.ConnectionQuality_EXCELLENT, }, { - packetLossPercentage: 4.1, + packetLossPercentage: 4.4, expectedMOS: 4.1, expectedQuality: livekit.ConnectionQuality_GOOD, }, { - packetLossPercentage: 13.2, + packetLossPercentage: 15.0, expectedMOS: 2.1, expectedQuality: livekit.ConnectionQuality_POOR, }, }, }, - // "audio/red" - no fec - 0 <= loss < 6.66%: EXCELLENT, 6.66% <= loss < 20%: GOOD, >= 20%: POOR + // "audio/red" - no fec - 0 <= loss < 10%: EXCELLENT, 10% <= loss < 30%: GOOD, >= 30%: POOR { name: "audio/red - no fec", mimeType: "audio/red", isFECEnabled: false, packetsExpected: 200, - expectedQualities: []expectedQuality{ - { - packetLossPercentage: 6.0, - expectedMOS: 4.6, - expectedQuality: livekit.ConnectionQuality_EXCELLENT, - }, - { - packetLossPercentage: 10.0, - expectedMOS: 4.1, - expectedQuality: livekit.ConnectionQuality_GOOD, - }, - { - packetLossPercentage: 23.0, - expectedMOS: 2.1, - expectedQuality: livekit.ConnectionQuality_POOR, - }, - }, - }, - // "audio/red" - fec - 0 <= loss < 10%: EXCELLENT, 10% <= loss < 30%: GOOD, >= 30%: POOR - { - name: "audio/red - fec", - mimeType: "audio/red", - isFECEnabled: true, - packetsExpected: 200, expectedQualities: []expectedQuality{ { packetLossPercentage: 8.0, @@ -510,12 +486,36 @@ func TestConnectionQuality(t *testing.T) { expectedQuality: livekit.ConnectionQuality_EXCELLENT, }, { - packetLossPercentage: 18.0, + packetLossPercentage: 12.0, expectedMOS: 4.1, expectedQuality: livekit.ConnectionQuality_GOOD, }, { - packetLossPercentage: 36.0, + packetLossPercentage: 39.0, + expectedMOS: 2.1, + expectedQuality: livekit.ConnectionQuality_POOR, + }, + }, + }, + // "audio/red" - fec - 0 <= loss < 15%: EXCELLENT, 15% <= loss < 45%: GOOD, >= 45%: POOR + { + name: "audio/red - fec", + mimeType: "audio/red", + isFECEnabled: true, + packetsExpected: 200, + expectedQualities: []expectedQuality{ + { + packetLossPercentage: 12.0, + expectedMOS: 4.6, + expectedQuality: livekit.ConnectionQuality_EXCELLENT, + }, + { + packetLossPercentage: 20.0, + expectedMOS: 4.1, + expectedQuality: livekit.ConnectionQuality_GOOD, + }, + { + packetLossPercentage: 60.0, expectedMOS: 2.1, expectedQuality: livekit.ConnectionQuality_POOR, }, @@ -534,12 +534,12 @@ func TestConnectionQuality(t *testing.T) { expectedQuality: livekit.ConnectionQuality_EXCELLENT, }, { - packetLossPercentage: 2.5, + packetLossPercentage: 3.5, expectedMOS: 4.1, expectedQuality: livekit.ConnectionQuality_GOOD, }, { - packetLossPercentage: 7.0, + packetLossPercentage: 8.0, expectedMOS: 2.1, expectedQuality: livekit.ConnectionQuality_POOR, }, @@ -549,7 +549,7 @@ func TestConnectionQuality(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - cs := newConnectionStats(tc.mimeType, tc.isFECEnabled, false, false) + cs := newConnectionStats(tc.mimeType, tc.isFECEnabled, true, true) duration := 5 * time.Second now := time.Now() @@ -619,7 +619,7 @@ func TestConnectionQuality(t *testing.T) { offset: 3 * time.Second, }, }, - bytes: uint64(math.Ceil(7_000_000.0 / 8.0 / 3.5)), + bytes: uint64(math.Ceil(7_000_000.0 / 8.0 / 4.2)), expectedMOS: 4.1, expectedQuality: livekit.ConnectionQuality_GOOD, }, @@ -634,7 +634,7 @@ func TestConnectionQuality(t *testing.T) { offset: 3 * time.Second, }, }, - bytes: uint64(math.Ceil(8_000_000.0 / 8.0 / 43.0)), + bytes: uint64(math.Ceil(8_000_000.0 / 8.0 / 75.0)), expectedMOS: 2.1, expectedQuality: livekit.ConnectionQuality_POOR, }, @@ -642,7 +642,7 @@ func TestConnectionQuality(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - cs := newConnectionStats("video/vp8", false, false, false) + cs := newConnectionStats("video/vp8", false, true, true) duration := 5 * time.Second now := time.Now() @@ -718,7 +718,7 @@ func TestConnectionQuality(t *testing.T) { distance: 2.0, }, { - distance: 2.2, + distance: 2.6, offset: 1 * time.Second, }, }, @@ -729,7 +729,7 @@ func TestConnectionQuality(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - cs := newConnectionStats("video/vp8", false, false, false) + cs := newConnectionStats("video/vp8", false, true, true) duration := 5 * time.Second now := time.Now() diff --git a/pkg/sfu/connectionquality/scorer.go b/pkg/sfu/connectionquality/scorer.go index 0c0a40e49..56e008a10 100644 --- a/pkg/sfu/connectionquality/scorer.go +++ b/pkg/sfu/connectionquality/scorer.go @@ -18,8 +18,8 @@ const ( poorScore = float64(30.0) minScore = float64(20.0) - increaseFactor = float64(0.4) // slow increase - decreaseFactor = float64(0.8) // fast decrease + 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 distanceWeight = float64(35.0) // each spatial layer missed drops a quality level @@ -39,7 +39,7 @@ type windowStat struct { jitterMax float64 } -func (w *windowStat) calculatePacketScore(plw float64, isDependentRTT bool, isDependentJitter bool) float64 { +func (w *windowStat) calculatePacketScore(plw float64, includeRTT bool, includeJitter bool) float64 { // this is based on simplified E-model based on packet loss, rtt, jitter as // outlined at https://www.pingman.com/kb/article/how-is-mos-calculated-in-pingplotter-pro-50.html. effectiveDelay := 0.0 @@ -48,10 +48,10 @@ func (w *windowStat) calculatePacketScore(plw float64, isDependentRTT bool, isDe // 1. in the up stream, RTT cannot be measured without RTCP-XR, it is using down stream RTT. // 2. in the down stream, up stream jitter affects it. although jitter can be adjusted to account for up stream // jitter, this lever can be used to discount jitter in scoring. - if !isDependentRTT { + if includeRTT { effectiveDelay += float64(w.rttMax) / 2.0 } - if !isDependentJitter { + if includeJitter { effectiveDelay += (w.jitterMax * 2.0) / 1000.0 } delayEffect := effectiveDelay / 40.0 @@ -127,10 +127,10 @@ type layerTransition struct { } type qualityScorerParams struct { - PacketLossWeight float64 - IsDependentRTT bool - IsDependentJitter bool - Logger logger.Logger + PacketLossWeight float64 + IncludeRTT bool + IncludeJitter bool + Logger logger.Logger } type qualityScorer struct { @@ -261,7 +261,7 @@ func (q *qualityScorer) Update(stat *windowStat, at time.Time) { reason = "dry" score = poorScore } else { - packetScore := stat.calculatePacketScore(plw, q.params.IsDependentRTT, q.params.IsDependentJitter) + packetScore := stat.calculatePacketScore(plw, q.params.IncludeRTT, q.params.IncludeJitter) bitrateScore := stat.calculateBitrateScore(expectedBitrate) layerScore := math.Max(math.Min(maxScore, maxScore-(expectedDistance*distanceWeight)), 0.0) @@ -370,7 +370,8 @@ func (q *qualityScorer) getPacketLossWeight(stat *windowStat) float64 { return q.params.PacketLossWeight } - return math.Sqrt(pps/q.maxPPS) * q.params.PacketLossWeight + packetRatio := pps / q.maxPPS + return packetRatio * packetRatio * q.params.PacketLossWeight } func (q *qualityScorer) getExpectedBitsAndUpdateTransitions(at time.Time) int64 { diff --git a/pkg/sfu/downtrack.go b/pkg/sfu/downtrack.go index 6769dec9e..c072645c6 100644 --- a/pkg/sfu/downtrack.go +++ b/pkg/sfu/downtrack.go @@ -303,7 +303,6 @@ func NewDownTrack( d.connectionStats = connectionquality.NewConnectionStats(connectionquality.ConnectionStatsParams{ MimeType: codecs[0].MimeType, // LK-TODO have to notify on codec change IsFECEnabled: strings.EqualFold(codecs[0].MimeType, webrtc.MimeTypeOpus) && strings.Contains(strings.ToLower(codecs[0].SDPFmtpLine), "fec"), - IsDependentJitter: true, GetDeltaStats: d.getDeltaStats, GetDeltaStatsOverridden: d.getDeltaStatsOverridden, GetLastReceiverReportTime: func() time.Time { return d.rtpStats.LastReceiverReport() }, diff --git a/pkg/sfu/receiver.go b/pkg/sfu/receiver.go index 5fd083bed..496e90ad0 100644 --- a/pkg/sfu/receiver.go +++ b/pkg/sfu/receiver.go @@ -203,11 +203,10 @@ func NewWebRTCReceiver( }) w.connectionStats = connectionquality.NewConnectionStats(connectionquality.ConnectionStatsParams{ - MimeType: w.codec.MimeType, - IsFECEnabled: strings.EqualFold(w.codec.MimeType, webrtc.MimeTypeOpus) && strings.Contains(strings.ToLower(w.codec.SDPFmtpLine), "fec"), - IsDependentRTT: true, - GetDeltaStats: w.getDeltaStats, - Logger: w.logger.WithValues("direction", "up"), + MimeType: w.codec.MimeType, + IsFECEnabled: strings.EqualFold(w.codec.MimeType, webrtc.MimeTypeOpus) && strings.Contains(strings.ToLower(w.codec.SDPFmtpLine), "fec"), + GetDeltaStats: w.getDeltaStats, + Logger: w.logger.WithValues("direction", "up"), }) w.connectionStats.OnStatsUpdate(func(_cs *connectionquality.ConnectionStats, stat *livekit.AnalyticsStat) { if w.onStatsUpdate != nil {