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 {