From 667928204a124062e2fef1819e67caecfd46a7b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 14 Feb 2025 14:30:22 +0100 Subject: [PATCH 1/2] test: Add unit tests for adding and distributing stats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../analyzers/PeerConnectionAnalyzer.spec.js | 214 ++++++++++++++++++ 1 file changed, 214 insertions(+) diff --git a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js index d69844eaee8..e153602b82c 100644 --- a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js +++ b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js @@ -3361,4 +3361,218 @@ describe('PeerConnectionAnalyzer', () => { expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.GOOD) }) }) + + describe('add stats', () => { + test.each([ + ['initial stats', 'audio'], + ['initial stats', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + + expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5]) + expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN]) + expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2]) + }) + + test.each([ + ['packet count not repeated', 'audio'], + ['packet count not repeated', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 200, 50, 11250, 0.3) + + expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 50]) + expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 10]) + expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 0.2]) + expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0, 1250]) + expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 1250]) + expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, 40]) + expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.3]) + }) + + test.each([ + ['packet count repeated one time', 'audio'], + ['packet count repeated one time', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + + expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5]) + expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0]) + expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN]) + expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2]) + + expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([150]) + expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([40]) + expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([10000]) + expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([0.2]) + }) + + test.each([ + ['packet count repeated one time then changed', 'audio'], + ['packet count repeated one time then changed', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 250, 60, 12500, 0.3) + + expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 50, 50]) + expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 10, 10]) + expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 0.2, 0.2]) + expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([1250, 1250]) + expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 1250, 1250]) + expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, 40, 40]) + expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.2, 0.3]) + + expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([]) + }) + + describe('distribute staged stats', () => { + + const expectRelativeStagedStats = (kind, index, expectedPackets, expectedPacketsLost, expectedTimestamps, expectedRoundTripTime) => { + expect(peerConnectionAnalyzer._stagedPackets[kind][index]).toBe(expectedPackets) + expect(peerConnectionAnalyzer._stagedPacketsLost[kind][index]).toBe(expectedPacketsLost) + expect(peerConnectionAnalyzer._stagedTimestamps[kind][index]).toBe(expectedTimestamps) + expect(peerConnectionAnalyzer._stagedRoundTripTime[kind][index]).toBe(expectedRoundTripTime) + } + + test.each([ + ['two sets of different values with repeated timestamps', 'audio'], + ['two sets of different values with repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 250, 60, 12500, 0.3) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 200, 50, 11250, 0.2) + expectRelativeStagedStats(kind, 1, 250, 60, 12500, 0.3) + }) + + test.each([ + ['two sets of different values without repeated timestamps', 'audio'], + ['two sets of different values without repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 250, 60, 14000, 0.3) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 175, 45, 11000, 0.2) + expectRelativeStagedStats(kind, 1, 250, 60, 14000, 0.3) + }) + + test.each([ + ['two sets of repeated values with repeated timestamps', 'audio'], + ['two sets of repeated values with repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 12500, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 150, 40, 11250, 0.2) + expectRelativeStagedStats(kind, 1, 150, 40, 12500, 0.2) + }) + + test.each([ + ['two sets of repeated values without repeated timestamps', 'audio'], + ['two sets of repeated values without repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 14000, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 150, 40, 11000, 0.2) + expectRelativeStagedStats(kind, 1, 150, 40, 14000, 0.2) + }) + + test.each([ + ['several sets of different values with repeated timestamps', 'audio'], + ['several sets of different values with repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.3) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.4) + peerConnectionAnalyzer._stageStats(kind, 350, 80, 14000, 0.1) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 200, 50, 11000, 0.2) + expectRelativeStagedStats(kind, 1, 250, 60, 12000, 0.3) + expectRelativeStagedStats(kind, 2, 300, 70, 13000, 0.4) + expectRelativeStagedStats(kind, 3, 350, 80, 14000, 0.1) + }) + + test.each([ + ['several sets of different values without repeated timestamps', 'audio'], + ['several sets of different values without repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 15000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 18000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 350, 80, 20000, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 170, 44, 11000, 0.2) + expectRelativeStagedStats(kind, 1, 250, 60, 15000, 0.2) + expectRelativeStagedStats(kind, 2, 310, 72, 18000, 0.2) + expectRelativeStagedStats(kind, 3, 350, 80, 20000, 0.2) + }) + + test.each([ + ['several sets of repeated values with repeated timestamps', 'audio'], + ['several sets of repeated values with repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 14000, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 150, 40, 11000, 0.2) + expectRelativeStagedStats(kind, 1, 150, 40, 12000, 0.2) + expectRelativeStagedStats(kind, 2, 150, 40, 13000, 0.2) + expectRelativeStagedStats(kind, 3, 150, 40, 14000, 0.2) + }) + + test.each([ + ['several sets of repeated values without repeated timestamps', 'audio'], + ['several sets of repeated values without repeated timestamps', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 15000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 17500, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 20000, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 150, 40, 11000, 0.2) + expectRelativeStagedStats(kind, 1, 150, 40, 15000, 0.2) + expectRelativeStagedStats(kind, 2, 150, 40, 17500, 0.2) + expectRelativeStagedStats(kind, 3, 150, 40, 20000, 0.2) + }) + }) + }) }) From b972e9ff13c86aa9c795be6e0ebe632f2977c354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 14 Feb 2025 14:33:29 +0100 Subject: [PATCH 2/2] fix: Fix distributing staged stats when not updated three times in a row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stats were supposed to be distributed once they had changed, but in practice they were always distributed, as the packet count is absolute rather than relative. Nevertheless, if the packet count did not change distributing them would have no effect. The problem could appear in the (rare) case of the timestamps not being updated three times in a row, as the distribution algorithm failed if the final timestamp was the same as the initial timestamp (causing NaN to be set for the packets and packets lost). Due to all that now the staged stats are always distributed before being commited (which is still done when the stats stalled for two seconds), although the distribution exits early if the timestamps did not change. Signed-off-by: Daniel Calviño Sánchez --- .../analyzers/PeerConnectionAnalyzer.js | 15 ++-- .../analyzers/PeerConnectionAnalyzer.spec.js | 77 +++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js index d0695a5d55b..95281da8058 100644 --- a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js +++ b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js @@ -506,11 +506,10 @@ PeerConnectionAnalyzer.prototype = { this._stageStats(kind, packets, packetsLost, timestamp, roundTripTime) - // If the packets have changed now it is assumed that the previous stats - // were stalled. - if (packets > 0) { - this._distributeStagedStats(kind) - } + // Distributing the stats has no effect if the stats were not stalled + // (that is, if the values are still unchanged, so it is probably an + // actual connection problem rather than a stalled report). + this._distributeStagedStats(kind) while (this._stagedPackets[kind].length > 0) { const stagedPackets = this._stagedPackets[kind].shift() @@ -550,6 +549,12 @@ PeerConnectionAnalyzer.prototype = { let packetsLostTotal = 0 let timestampsTotal = 0 + // If the last timestamp is still stalled there is nothing to + // distribute. + if (this._stagedTimestamps[kind][this._stagedTimestamps[kind].length - 1] === timestampsBase) { + return + } + // If the first timestamp stalled it is assumed that all of them // stalled and are thus evenly distributed based on the new timestamp. if (this._stagedTimestamps[kind][0] === timestampsBase) { diff --git a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js index e153602b82c..5ec0efe1972 100644 --- a/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js +++ b/src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js @@ -3437,6 +3437,51 @@ describe('PeerConnectionAnalyzer', () => { expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([]) }) + test.each([ + ['packet count repeated two times', 'audio'], + ['packet count repeated two times', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + + expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 0, 0]) + expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 0, 0]) + expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 1.5, 1.5]) + expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0, 0]) + expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 0, 0]) + expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, NaN, NaN]) + expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.2, 0.2]) + + expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([]) + }) + + test.each([ + ['packet count repeated two times then changed', 'audio'], + ['packet count repeated two times then changed', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._addStats(kind, 300, 70, 13750, 0.3) + + expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 0, 0, 150]) + expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 0, 0, 30]) + expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 1.5, 1.5, 0.2]) + expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0, 3750]) + expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 0, 0, 3750]) + expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, NaN, NaN, 40]) + expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.2, 0.2, 0.3]) + + expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([]) + expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([]) + }) + describe('distribute staged stats', () => { const expectRelativeStagedStats = (kind, index, expectedPackets, expectedPacketsLost, expectedTimestamps, expectedRoundTripTime) => { @@ -3502,6 +3547,20 @@ describe('PeerConnectionAnalyzer', () => { expectRelativeStagedStats(kind, 1, 150, 40, 14000, 0.2) }) + test.each([ + ['two sets of fully repeated values', 'audio'], + ['two sets of fully repeated values', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 150, 40, 10000, 0.2) + expectRelativeStagedStats(kind, 1, 150, 40, 10000, 0.2) + }) + test.each([ ['several sets of different values with repeated timestamps', 'audio'], ['several sets of different values with repeated timestamps', 'video'], @@ -3573,6 +3632,24 @@ describe('PeerConnectionAnalyzer', () => { expectRelativeStagedStats(kind, 2, 150, 40, 17500, 0.2) expectRelativeStagedStats(kind, 3, 150, 40, 20000, 0.2) }) + + test.each([ + ['several sets of fully repeated values', 'audio'], + ['several sets of fully repeated values', 'video'], + ])('%s, %s', (name, kind) => { + peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2) + + peerConnectionAnalyzer._distributeStagedStats(kind) + + expectRelativeStagedStats(kind, 0, 150, 40, 10000, 0.2) + expectRelativeStagedStats(kind, 1, 150, 40, 10000, 0.2) + expectRelativeStagedStats(kind, 2, 150, 40, 10000, 0.2) + expectRelativeStagedStats(kind, 3, 150, 40, 10000, 0.2) + }) }) }) })