Skip to content

Commit 53e61db

Browse files
authored
Merge pull request #14450 from nextcloud/backport/14419/stable31
[stable31] Fix distributing staged stats when not updated three times in a row
2 parents c103608 + b972e9f commit 53e61db

File tree

2 files changed

+301
-5
lines changed

2 files changed

+301
-5
lines changed

src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js

+10-5
Original file line numberDiff line numberDiff line change
@@ -506,11 +506,10 @@ PeerConnectionAnalyzer.prototype = {
506506

507507
this._stageStats(kind, packets, packetsLost, timestamp, roundTripTime)
508508

509-
// If the packets have changed now it is assumed that the previous stats
510-
// were stalled.
511-
if (packets > 0) {
512-
this._distributeStagedStats(kind)
513-
}
509+
// Distributing the stats has no effect if the stats were not stalled
510+
// (that is, if the values are still unchanged, so it is probably an
511+
// actual connection problem rather than a stalled report).
512+
this._distributeStagedStats(kind)
514513

515514
while (this._stagedPackets[kind].length > 0) {
516515
const stagedPackets = this._stagedPackets[kind].shift()
@@ -550,6 +549,12 @@ PeerConnectionAnalyzer.prototype = {
550549
let packetsLostTotal = 0
551550
let timestampsTotal = 0
552551

552+
// If the last timestamp is still stalled there is nothing to
553+
// distribute.
554+
if (this._stagedTimestamps[kind][this._stagedTimestamps[kind].length - 1] === timestampsBase) {
555+
return
556+
}
557+
553558
// If the first timestamp stalled it is assumed that all of them
554559
// stalled and are thus evenly distributed based on the new timestamp.
555560
if (this._stagedTimestamps[kind][0] === timestampsBase) {

src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js

+291
Original file line numberDiff line numberDiff line change
@@ -3361,4 +3361,295 @@ describe('PeerConnectionAnalyzer', () => {
33613361
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.GOOD)
33623362
})
33633363
})
3364+
3365+
describe('add stats', () => {
3366+
test.each([
3367+
['initial stats', 'audio'],
3368+
['initial stats', 'video'],
3369+
])('%s, %s', (name, kind) => {
3370+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3371+
3372+
expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0])
3373+
expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0])
3374+
expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5])
3375+
expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0])
3376+
expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0])
3377+
expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN])
3378+
expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2])
3379+
})
3380+
3381+
test.each([
3382+
['packet count not repeated', 'audio'],
3383+
['packet count not repeated', 'video'],
3384+
])('%s, %s', (name, kind) => {
3385+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3386+
peerConnectionAnalyzer._addStats(kind, 200, 50, 11250, 0.3)
3387+
3388+
expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 50])
3389+
expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 10])
3390+
expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 0.2])
3391+
expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0, 1250])
3392+
expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 1250])
3393+
expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, 40])
3394+
expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.3])
3395+
})
3396+
3397+
test.each([
3398+
['packet count repeated one time', 'audio'],
3399+
['packet count repeated one time', 'video'],
3400+
])('%s, %s', (name, kind) => {
3401+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3402+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3403+
3404+
expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0])
3405+
expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0])
3406+
expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5])
3407+
expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0])
3408+
expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0])
3409+
expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN])
3410+
expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2])
3411+
3412+
expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([150])
3413+
expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([40])
3414+
expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([10000])
3415+
expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([0.2])
3416+
})
3417+
3418+
test.each([
3419+
['packet count repeated one time then changed', 'audio'],
3420+
['packet count repeated one time then changed', 'video'],
3421+
])('%s, %s', (name, kind) => {
3422+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3423+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3424+
peerConnectionAnalyzer._addStats(kind, 250, 60, 12500, 0.3)
3425+
3426+
expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 50, 50])
3427+
expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 10, 10])
3428+
expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 0.2, 0.2])
3429+
expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([1250, 1250])
3430+
expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 1250, 1250])
3431+
expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, 40, 40])
3432+
expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.2, 0.3])
3433+
3434+
expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([])
3435+
expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([])
3436+
expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([])
3437+
expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([])
3438+
})
3439+
3440+
test.each([
3441+
['packet count repeated two times', 'audio'],
3442+
['packet count repeated two times', 'video'],
3443+
])('%s, %s', (name, kind) => {
3444+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3445+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3446+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3447+
3448+
expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 0, 0])
3449+
expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 0, 0])
3450+
expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 1.5, 1.5])
3451+
expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0, 0])
3452+
expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 0, 0])
3453+
expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, NaN, NaN])
3454+
expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.2, 0.2])
3455+
3456+
expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([])
3457+
expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([])
3458+
expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([])
3459+
expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([])
3460+
})
3461+
3462+
test.each([
3463+
['packet count repeated two times then changed', 'audio'],
3464+
['packet count repeated two times then changed', 'video'],
3465+
])('%s, %s', (name, kind) => {
3466+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3467+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3468+
peerConnectionAnalyzer._addStats(kind, 150, 40, 10000, 0.2)
3469+
peerConnectionAnalyzer._addStats(kind, 300, 70, 13750, 0.3)
3470+
3471+
expect(peerConnectionAnalyzer._packets[kind]._relativeValues).toEqual([0, 0, 0, 150])
3472+
expect(peerConnectionAnalyzer._packetsLost[kind]._relativeValues).toEqual([0, 0, 0, 30])
3473+
expect(peerConnectionAnalyzer._packetsLostRatio[kind]._relativeValues).toEqual([1.5, 1.5, 1.5, 0.2])
3474+
expect(peerConnectionAnalyzer._timestamps[kind]._relativeValues).toEqual([0, 3750])
3475+
expect(peerConnectionAnalyzer._timestampsForLogs[kind]._relativeValues).toEqual([0, 0, 0, 3750])
3476+
expect(peerConnectionAnalyzer._packetsPerSecond[kind]._relativeValues).toEqual([NaN, NaN, NaN, 40])
3477+
expect(peerConnectionAnalyzer._roundTripTime[kind]._relativeValues).toEqual([0.2, 0.2, 0.2, 0.3])
3478+
3479+
expect(peerConnectionAnalyzer._stagedPackets[kind]).toEqual([])
3480+
expect(peerConnectionAnalyzer._stagedPacketsLost[kind]).toEqual([])
3481+
expect(peerConnectionAnalyzer._stagedTimestamps[kind]).toEqual([])
3482+
expect(peerConnectionAnalyzer._stagedRoundTripTime[kind]).toEqual([])
3483+
})
3484+
3485+
describe('distribute staged stats', () => {
3486+
3487+
const expectRelativeStagedStats = (kind, index, expectedPackets, expectedPacketsLost, expectedTimestamps, expectedRoundTripTime) => {
3488+
expect(peerConnectionAnalyzer._stagedPackets[kind][index]).toBe(expectedPackets)
3489+
expect(peerConnectionAnalyzer._stagedPacketsLost[kind][index]).toBe(expectedPacketsLost)
3490+
expect(peerConnectionAnalyzer._stagedTimestamps[kind][index]).toBe(expectedTimestamps)
3491+
expect(peerConnectionAnalyzer._stagedRoundTripTime[kind][index]).toBe(expectedRoundTripTime)
3492+
}
3493+
3494+
test.each([
3495+
['two sets of different values with repeated timestamps', 'audio'],
3496+
['two sets of different values with repeated timestamps', 'video'],
3497+
])('%s, %s', (name, kind) => {
3498+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3499+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3500+
peerConnectionAnalyzer._stageStats(kind, 250, 60, 12500, 0.3)
3501+
3502+
peerConnectionAnalyzer._distributeStagedStats(kind)
3503+
3504+
expectRelativeStagedStats(kind, 0, 200, 50, 11250, 0.2)
3505+
expectRelativeStagedStats(kind, 1, 250, 60, 12500, 0.3)
3506+
})
3507+
3508+
test.each([
3509+
['two sets of different values without repeated timestamps', 'audio'],
3510+
['two sets of different values without repeated timestamps', 'video'],
3511+
])('%s, %s', (name, kind) => {
3512+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3513+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2)
3514+
peerConnectionAnalyzer._stageStats(kind, 250, 60, 14000, 0.3)
3515+
3516+
peerConnectionAnalyzer._distributeStagedStats(kind)
3517+
3518+
expectRelativeStagedStats(kind, 0, 175, 45, 11000, 0.2)
3519+
expectRelativeStagedStats(kind, 1, 250, 60, 14000, 0.3)
3520+
})
3521+
3522+
test.each([
3523+
['two sets of repeated values with repeated timestamps', 'audio'],
3524+
['two sets of repeated values with repeated timestamps', 'video'],
3525+
])('%s, %s', (name, kind) => {
3526+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3527+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3528+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 12500, 0.2)
3529+
3530+
peerConnectionAnalyzer._distributeStagedStats(kind)
3531+
3532+
expectRelativeStagedStats(kind, 0, 150, 40, 11250, 0.2)
3533+
expectRelativeStagedStats(kind, 1, 150, 40, 12500, 0.2)
3534+
})
3535+
3536+
test.each([
3537+
['two sets of repeated values without repeated timestamps', 'audio'],
3538+
['two sets of repeated values without repeated timestamps', 'video'],
3539+
])('%s, %s', (name, kind) => {
3540+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3541+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2)
3542+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 14000, 0.2)
3543+
3544+
peerConnectionAnalyzer._distributeStagedStats(kind)
3545+
3546+
expectRelativeStagedStats(kind, 0, 150, 40, 11000, 0.2)
3547+
expectRelativeStagedStats(kind, 1, 150, 40, 14000, 0.2)
3548+
})
3549+
3550+
test.each([
3551+
['two sets of fully repeated values', 'audio'],
3552+
['two sets of fully repeated values', 'video'],
3553+
])('%s, %s', (name, kind) => {
3554+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3555+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3556+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3557+
3558+
peerConnectionAnalyzer._distributeStagedStats(kind)
3559+
3560+
expectRelativeStagedStats(kind, 0, 150, 40, 10000, 0.2)
3561+
expectRelativeStagedStats(kind, 1, 150, 40, 10000, 0.2)
3562+
})
3563+
3564+
test.each([
3565+
['several sets of different values with repeated timestamps', 'audio'],
3566+
['several sets of different values with repeated timestamps', 'video'],
3567+
])('%s, %s', (name, kind) => {
3568+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3569+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3570+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.3)
3571+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.4)
3572+
peerConnectionAnalyzer._stageStats(kind, 350, 80, 14000, 0.1)
3573+
3574+
peerConnectionAnalyzer._distributeStagedStats(kind)
3575+
3576+
expectRelativeStagedStats(kind, 0, 200, 50, 11000, 0.2)
3577+
expectRelativeStagedStats(kind, 1, 250, 60, 12000, 0.3)
3578+
expectRelativeStagedStats(kind, 2, 300, 70, 13000, 0.4)
3579+
expectRelativeStagedStats(kind, 3, 350, 80, 14000, 0.1)
3580+
})
3581+
3582+
test.each([
3583+
['several sets of different values without repeated timestamps', 'audio'],
3584+
['several sets of different values without repeated timestamps', 'video'],
3585+
])('%s, %s', (name, kind) => {
3586+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3587+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2)
3588+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 15000, 0.2)
3589+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 18000, 0.2)
3590+
peerConnectionAnalyzer._stageStats(kind, 350, 80, 20000, 0.2)
3591+
3592+
peerConnectionAnalyzer._distributeStagedStats(kind)
3593+
3594+
expectRelativeStagedStats(kind, 0, 170, 44, 11000, 0.2)
3595+
expectRelativeStagedStats(kind, 1, 250, 60, 15000, 0.2)
3596+
expectRelativeStagedStats(kind, 2, 310, 72, 18000, 0.2)
3597+
expectRelativeStagedStats(kind, 3, 350, 80, 20000, 0.2)
3598+
})
3599+
3600+
test.each([
3601+
['several sets of repeated values with repeated timestamps', 'audio'],
3602+
['several sets of repeated values with repeated timestamps', 'video'],
3603+
])('%s, %s', (name, kind) => {
3604+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3605+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3606+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3607+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3608+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 14000, 0.2)
3609+
3610+
peerConnectionAnalyzer._distributeStagedStats(kind)
3611+
3612+
expectRelativeStagedStats(kind, 0, 150, 40, 11000, 0.2)
3613+
expectRelativeStagedStats(kind, 1, 150, 40, 12000, 0.2)
3614+
expectRelativeStagedStats(kind, 2, 150, 40, 13000, 0.2)
3615+
expectRelativeStagedStats(kind, 3, 150, 40, 14000, 0.2)
3616+
})
3617+
3618+
test.each([
3619+
['several sets of repeated values without repeated timestamps', 'audio'],
3620+
['several sets of repeated values without repeated timestamps', 'video'],
3621+
])('%s, %s', (name, kind) => {
3622+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3623+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 11000, 0.2)
3624+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 15000, 0.2)
3625+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 17500, 0.2)
3626+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 20000, 0.2)
3627+
3628+
peerConnectionAnalyzer._distributeStagedStats(kind)
3629+
3630+
expectRelativeStagedStats(kind, 0, 150, 40, 11000, 0.2)
3631+
expectRelativeStagedStats(kind, 1, 150, 40, 15000, 0.2)
3632+
expectRelativeStagedStats(kind, 2, 150, 40, 17500, 0.2)
3633+
expectRelativeStagedStats(kind, 3, 150, 40, 20000, 0.2)
3634+
})
3635+
3636+
test.each([
3637+
['several sets of fully repeated values', 'audio'],
3638+
['several sets of fully repeated values', 'video'],
3639+
])('%s, %s', (name, kind) => {
3640+
peerConnectionAnalyzer._commitStats(kind, 150, 40, 10000, 0.2)
3641+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3642+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3643+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3644+
peerConnectionAnalyzer._stageStats(kind, 150, 40, 10000, 0.2)
3645+
3646+
peerConnectionAnalyzer._distributeStagedStats(kind)
3647+
3648+
expectRelativeStagedStats(kind, 0, 150, 40, 10000, 0.2)
3649+
expectRelativeStagedStats(kind, 1, 150, 40, 10000, 0.2)
3650+
expectRelativeStagedStats(kind, 2, 150, 40, 10000, 0.2)
3651+
expectRelativeStagedStats(kind, 3, 150, 40, 10000, 0.2)
3652+
})
3653+
})
3654+
})
33643655
})

0 commit comments

Comments
 (0)