Skip to content

Commit

Permalink
fix: Do not report very bad quality with a low packets count
Browse files Browse the repository at this point in the history 
In the past it was observed that, even for small videos, 10 packets per
second was a reasonable threshold to detect connection issues even if
there were no lost packets. However, nowadays it seems that it can
sometimes trigger a false positive (typically when the background blur
is enabled and the video quality is reduced due to being in a call with
several participants), so for now the connection problem is no longer
reported to the user but just logged.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
  • Loading branch information
@danxuliu
danxuliu committed Jan 29, 2025
1 parent a5707cd commit 70530ac
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 14 deletions.
7 changes: 5 additions & 2 deletions src/utils/webrtc/analyzers/PeerConnectionAnalyzer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,10 +680,13 @@ PeerConnectionAnalyzer.prototype = {
// quality to keep a smooth video, albeit on a lower resolution. Thus
// with a threshold of 10 packets issues can be detected too for videos,
// although only once they can not be further downscaled.
// Despite all of the above it has been observed that less than 10
// packets are sometimes sent without any connection problem (for
// example, when the background is blurred and the video quality is
// reduced due to being in a call with several participants), so for now
// it is only logged but not reported.
if (packetsPerSecond.getWeightedAverage() < 10) {
this._logStats(kind, 'Low packets per second: ' + packetsPerSecond.getWeightedAverage())

return CONNECTION_QUALITY.VERY_BAD
}

if (packetsLostRatioWeightedAverage > 0.3) {
Expand Down
158 changes: 146 additions & 12 deletions src/utils/webrtc/analyzers/PeerConnectionAnalyzer.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,142 @@ describe('PeerConnectionAnalyzer', () => {
})

test.each([
['very bad quality due to low packets', 'audio'],
['very bad quality due to low packets', 'video'],
['very bad quality with low packets and packet loss', 'audio'],
['very bad quality with low packets and packet loss', 'video'],
])('%s, %s', async (name, kind) => {
peerConnection.getStats
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 5, timestamp: 10000 },
{ type: 'remote-inbound-rtp', kind, packetsReceived: 3, timestamp: 10000, packetsLost: 2, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 10, timestamp: 11000 },
{ type: 'remote-inbound-rtp', kind, packetsReceived: 6, timestamp: 11000, packetsLost: 4, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 15, timestamp: 11950 },
{ type: 'remote-inbound-rtp', kind, packetsReceived: 9, timestamp: 11950, packetsLost: 6, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 20, timestamp: 13020 },
{ type: 'remote-inbound-rtp', kind, packetsReceived: 12, timestamp: 13020, packetsLost: 8, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 25, timestamp: 14010 },
{ type: 'remote-inbound-rtp', kind, packetsReceived: 15, timestamp: 14010, packetsLost: 10, roundTripTime: 0.1 },
]))
// A sixth report is needed for the initial calculation due to
// the first stats report being used as the base to calculate
// relative values of cumulative stats.
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 30, timestamp: 14985 },
{ type: 'remote-inbound-rtp', kind, packetsReceived: 18, timestamp: 14985, packetsLost: 12, roundTripTime: 0.1 },
]))

peerConnectionAnalyzer.setPeerConnection(peerConnection, PEER_DIRECTION.SENDER)

jest.advanceTimersByTime(5000)
// Force the promises returning the stats to be executed.
await null

expect(peerConnection.getStats).toHaveBeenCalledTimes(5)

expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(0)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(0)

jest.advanceTimersByTime(1000)
// Force the promises returning the stats to be executed.
await null

expect(peerConnection.getStats).toHaveBeenCalledTimes(6)

if (kind === 'audio') {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(0)
} else {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(0)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
}
})

test.each([
['good quality even with low packets if no packet loss, missing remote packet count', 'audio'],
['good quality even with low packets if no packet loss, missing remote packet count', 'video'],
])('%s, %s', async (name, kind) => {
peerConnection.getStats
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 5, timestamp: 10000 },
{ type: 'remote-inbound-rtp', kind, timestamp: 10000, packetsLost: 2, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 10, timestamp: 11000 },
{ type: 'remote-inbound-rtp', kind, timestamp: 11000, packetsLost: 4, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 15, timestamp: 11950 },
{ type: 'remote-inbound-rtp', kind, timestamp: 11950, packetsLost: 6, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 20, timestamp: 13020 },
{ type: 'remote-inbound-rtp', kind, timestamp: 13020, packetsLost: 8, roundTripTime: 0.1 },
]))
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 25, timestamp: 14010 },
{ type: 'remote-inbound-rtp', kind, timestamp: 14010, packetsLost: 10, roundTripTime: 0.1 },
]))
// A sixth report is needed for the initial calculation due to
// the first stats report being used as the base to calculate
// relative values of cumulative stats.
.mockResolvedValueOnce(newRTCStatsReport([
{ type: 'outbound-rtp', kind, packetsSent: 30, timestamp: 14985 },
{ type: 'remote-inbound-rtp', kind, timestamp: 14985, packetsLost: 12, roundTripTime: 0.1 },
]))

peerConnectionAnalyzer.setPeerConnection(peerConnection, PEER_DIRECTION.SENDER)

jest.advanceTimersByTime(5000)
// Force the promises returning the stats to be executed.
await null

expect(peerConnection.getStats).toHaveBeenCalledTimes(5)

expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(0)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(0)

jest.advanceTimersByTime(1000)
// Force the promises returning the stats to be executed.
await null

expect(peerConnection.getStats).toHaveBeenCalledTimes(6)

if (kind === 'audio') {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(0)
} else {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(0)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
}
})

test.each([
['good quality even with low packets if no packet loss', 'audio'],
['good quality even with low packets if no packet loss', 'video'],
])('%s, %s', async (name, kind) => {
peerConnection.getStats
.mockResolvedValueOnce(newRTCStatsReport([
Expand Down Expand Up @@ -715,23 +849,23 @@ describe('PeerConnectionAnalyzer', () => {
expect(peerConnection.getStats).toHaveBeenCalledTimes(6)

if (kind === 'audio') {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.GOOD)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.GOOD)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(0)
} else {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.GOOD)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(0)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.GOOD)
}
})

test.each([
['very bad quality due to low packets, missing remote packet count', 'audio'],
['very bad quality due to low packets, missing remote packet count', 'video'],
['good quality even with low packets if no packet loss, missing remote packet count', 'audio'],
['good quality even with low packets if no packet loss, missing remote packet count', 'video'],
])('%s, %s', async (name, kind) => {
peerConnection.getStats
.mockResolvedValueOnce(newRTCStatsReport([
Expand Down Expand Up @@ -782,17 +916,17 @@ describe('PeerConnectionAnalyzer', () => {
expect(peerConnection.getStats).toHaveBeenCalledTimes(6)

if (kind === 'audio') {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.GOOD)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.GOOD)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(0)
} else {
expect(peerConnectionAnalyzer.getConnectionQualityAudio()).toBe(CONNECTION_QUALITY.UNKNOWN)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.VERY_BAD)
expect(peerConnectionAnalyzer.getConnectionQualityVideo()).toBe(CONNECTION_QUALITY.GOOD)
expect(changeConnectionQualityAudioHandler).toHaveBeenCalledTimes(0)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledTimes(1)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.VERY_BAD)
expect(changeConnectionQualityVideoHandler).toHaveBeenCalledWith(peerConnectionAnalyzer, CONNECTION_QUALITY.GOOD)
}
})

Expand Down

0 comments on commit 70530ac

Please sign in to comment.