From 74782dc4f2cbb1481bf73deb4341c71022ba834f Mon Sep 17 00:00:00 2001 From: Hampus Date: Fri, 11 Sep 2026 12:53:29 +0200 Subject: [PATCH] fix(voice): confirm a decode stall before withdrawing a codec (#2661) --- ...creenShareDecodeStallRenegotiation.test.ts | 138 +++++++++++++++++- .../voice/engine/VoiceRoomEventBinder.ts | 20 +-- .../utils/ScreenShareCodecDiagnostics.test.ts | 86 +++++++++++ .../utils/ScreenShareCodecDiagnostics.ts | 64 ++++++-- 4 files changed, 281 insertions(+), 27 deletions(-) diff --git a/fluxer_app/src/features/voice/engine/ScreenShareDecodeStallRenegotiation.test.ts b/fluxer_app/src/features/voice/engine/ScreenShareDecodeStallRenegotiation.test.ts index 774d9326b..ce8446477 100644 --- a/fluxer_app/src/features/voice/engine/ScreenShareDecodeStallRenegotiation.test.ts +++ b/fluxer_app/src/features/voice/engine/ScreenShareDecodeStallRenegotiation.test.ts @@ -2,7 +2,7 @@ import type {FluxerCodecAdvertisement} from '@app/features/voice/engine/ScreenShareCodecNegotiation'; import type {HardwareEncodeReport} from '@app/features/voice/utils/GpuEncoderCapabilities'; -import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; const gpuReport: HardwareEncodeReport = { av1: 'hardware', @@ -77,13 +77,15 @@ const { buildLocalCodecAdvertisements, computeNegotiatedVideoCodec, } = await import('./ScreenShareCodecNegotiation'); -const {findStalledVideoDecoder} = await import('@app/features/voice/utils/ScreenShareCodecDiagnostics'); +const {findStalledVideoDecoder, scheduleScreenShareDecoderVerification} = await import( + '@app/features/voice/utils/ScreenShareCodecDiagnostics' +); const {getVideoDecoderExclusionsSync, markScreenShareDecodeFailure, resetVideoDecoderExclusions} = await import( '@app/features/voice/utils/VideoDecoderCapabilities' ); const {resetCachedCodecCapabilities} = await import('@app/features/voice/utils/CodecCapabilityDetector'); -function stalledScreenShareStats(mimeType: string): RTCStatsReport { +function screenShareStats(mimeType: string, frames: {framesReceived: number; framesDecoded: number}): RTCStatsReport { const entries: Array> = [ {id: 'codec-1', type: 'codec', mimeType}, { @@ -93,13 +95,23 @@ function stalledScreenShareStats(mimeType: string): RTCStatsReport { codecId: 'codec-1', packetsReceived: 4200, bytesReceived: 3_500_000, - framesReceived: 180, - framesDecoded: 0, + decoderImplementation: 'D3D11VideoDecoder', + powerEfficientDecoder: true, + ...frames, }, ]; return new Map(entries.map((entry) => [entry.id as string, entry])) as unknown as RTCStatsReport; } +function stalledScreenShareStats(mimeType: string): RTCStatsReport { + return screenShareStats(mimeType, {framesReceived: 180, framesDecoded: 0}); +} + +function statsSequence(...reports: Array): () => Promise { + const queue = [...reports]; + return () => Promise.resolve(queue.shift()); +} + function decodeAdvertisedFor(name: 'H264' | 'VP8' | 'VP9'): boolean | undefined { return buildLocalCodecAdvertisements().find((codec) => codec.name === name)?.decode; } @@ -164,3 +176,119 @@ describe('a stalled H.264 screen share decode', () => { expect(decodeAdvertisedFor('VP8')).toBe(true); }); }); + +describe('confirming a screen share decode stall before a codec is withdrawn', () => { + beforeEach(() => { + vi.useFakeTimers(); + resetVideoDecoderExclusions(); + resetCachedCodecCapabilities(); + ScreenShareCodecNegotiation.dispose(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('withdraws nothing from a single stalled sample', async () => { + const onDecodeFailure = vi.fn(); + scheduleScreenShareDecoderVerification( + statsSequence( + screenShareStats('video/H264', {framesReceived: 180, framesDecoded: 0}), + screenShareStats('video/H264', {framesReceived: 240, framesDecoded: 0}), + ), + undefined, + onDecodeFailure, + ); + await vi.advanceTimersByTimeAsync(5000); + expect(onDecodeFailure).not.toHaveBeenCalled(); + expect(decodeAdvertisedFor('H264')).toBe(true); + }); + + it('withdraws nothing when packets arrive but no whole frames do', async () => { + const onDecodeFailure = vi.fn(); + scheduleScreenShareDecoderVerification( + statsSequence( + screenShareStats('video/H264', {framesReceived: 0, framesDecoded: 0}), + screenShareStats('video/H264', {framesReceived: 0, framesDecoded: 0}), + ), + undefined, + onDecodeFailure, + ); + await vi.advanceTimersByTimeAsync(5000); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(5000); + expect(onDecodeFailure).not.toHaveBeenCalled(); + expect(decodeAdvertisedFor('H264')).toBe(true); + }); + + it('withdraws the codec exactly once when a second sample confirms the stall', async () => { + const withdrawn: Array = []; + scheduleScreenShareDecoderVerification( + statsSequence( + screenShareStats('video/H264', {framesReceived: 180, framesDecoded: 0}), + screenShareStats('video/H264', {framesReceived: 240, framesDecoded: 0}), + ), + undefined, + (failure) => { + if (markScreenShareDecodeFailure(failure.codec, 'screen-share-decode-stalled')) { + withdrawn.push(failure.codec); + } + }, + ); + await vi.advanceTimersByTimeAsync(5000); + expect(withdrawn).toEqual([]); + expect(decodeAdvertisedFor('H264')).toBe(true); + await vi.advanceTimersByTimeAsync(5000); + expect(withdrawn).toEqual(['h264']); + expect(decodeAdvertisedFor('H264')).toBe(false); + }); + + it('withdraws nothing when the second sample decoded a frame', async () => { + const onDecodeFailure = vi.fn(); + scheduleScreenShareDecoderVerification( + statsSequence( + screenShareStats('video/H264', {framesReceived: 180, framesDecoded: 0}), + screenShareStats('video/H264', {framesReceived: 240, framesDecoded: 12}), + ), + undefined, + onDecodeFailure, + ); + await vi.advanceTimersByTimeAsync(10000); + expect(onDecodeFailure).not.toHaveBeenCalled(); + expect(decodeAdvertisedFor('H264')).toBe(true); + }); + + it('withdraws nothing when no new frames arrived between the samples', async () => { + const onDecodeFailure = vi.fn(); + scheduleScreenShareDecoderVerification( + statsSequence( + screenShareStats('video/H264', {framesReceived: 180, framesDecoded: 0}), + screenShareStats('video/H264', {framesReceived: 180, framesDecoded: 0}), + ), + undefined, + onDecodeFailure, + ); + await vi.advanceTimersByTimeAsync(10000); + expect(onDecodeFailure).not.toHaveBeenCalled(); + expect(decodeAdvertisedFor('H264')).toBe(true); + }); + + it('withdraws nothing when the track goes away between the samples', async () => { + const onDecodeFailure = vi.fn(); + const cancel = scheduleScreenShareDecoderVerification( + statsSequence( + screenShareStats('video/H264', {framesReceived: 180, framesDecoded: 0}), + screenShareStats('video/H264', {framesReceived: 240, framesDecoded: 0}), + ), + undefined, + onDecodeFailure, + ); + await vi.advanceTimersByTimeAsync(5000); + expect(vi.getTimerCount()).toBe(1); + cancel(); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(5000); + expect(onDecodeFailure).not.toHaveBeenCalled(); + expect(decodeAdvertisedFor('H264')).toBe(true); + }); +}); diff --git a/fluxer_app/src/features/voice/engine/VoiceRoomEventBinder.ts b/fluxer_app/src/features/voice/engine/VoiceRoomEventBinder.ts index 75f9404c9..b527ccedb 100644 --- a/fluxer_app/src/features/voice/engine/VoiceRoomEventBinder.ts +++ b/fluxer_app/src/features/voice/engine/VoiceRoomEventBinder.ts @@ -141,7 +141,7 @@ export function bindRoomEvents( ): void { const guard = dependencies.connection.createGuardedHandler; const participantSpeakingDisposers = new Map(); - const screenShareDecoderVerificationTimers = new Map(); + const screenShareDecoderVerificationCancels = new Map void>(); const remoteTrackLifecycleDisposers = new Map void>(); let codecNegotiationDisposer: (() => void) | null = null; let screenShareMigrationDisposer: (() => void) | null = null; @@ -227,16 +227,16 @@ export function bindRoomEvents( }; const clearScreenShareDecoderVerification = (trackSid: string | undefined): void => { if (!trackSid) return; - const timer = screenShareDecoderVerificationTimers.get(trackSid); - if (!timer) return; - clearTimeout(timer); - screenShareDecoderVerificationTimers.delete(trackSid); + const cancel = screenShareDecoderVerificationCancels.get(trackSid); + if (!cancel) return; + cancel(); + screenShareDecoderVerificationCancels.delete(trackSid); }; const clearAllScreenShareDecoderVerifications = (): void => { - for (const timer of screenShareDecoderVerificationTimers.values()) { - clearTimeout(timer); + for (const cancel of screenShareDecoderVerificationCancels.values()) { + cancel(); } - screenShareDecoderVerificationTimers.clear(); + screenShareDecoderVerificationCancels.clear(); }; const bindCodecNegotiation = (): void => { codecNegotiationDisposer?.(); @@ -258,12 +258,12 @@ export function bindRoomEvents( if (pub.source !== Track.Source.ScreenShare || pub.kind !== Track.Kind.Video) return; clearScreenShareDecoderVerification(pub.trackSid); const trackSid = pub.trackSid; - screenShareDecoderVerificationTimers.set( + screenShareDecoderVerificationCancels.set( trackSid, scheduleScreenShareDecoderVerification( () => track.getRTCStatsReport(), () => { - screenShareDecoderVerificationTimers.delete(trackSid); + screenShareDecoderVerificationCancels.delete(trackSid); }, (failure) => { if (!markScreenShareDecodeFailure(failure.codec, 'screen-share-decode-stalled')) return; diff --git a/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.test.ts b/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.test.ts index 25f90177a..4c9adc370 100644 --- a/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.test.ts +++ b/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.test.ts @@ -2,15 +2,29 @@ import {describe, expect, it} from 'vitest'; import { + confirmDecodeStall, findSoftwareVideoDecoder, findStalledVideoDecoder, isSoftwareVideoImplementation, + type StalledVideoDecoderInfo, } from './ScreenShareCodecDiagnostics'; function createStatsReport(entries: Array>): RTCStatsReport { return new Map(entries.map((entry) => [entry.id as string, entry])) as unknown as RTCStatsReport; } +function createStallInfo(overrides: Partial = {}): StalledVideoDecoderInfo { + return { + codec: 'h264', + mimeType: 'video/H264', + packetsReceived: 400, + bytesReceived: 250_000, + framesDecoded: 0, + framesReceived: 20, + ...overrides, + }; +} + describe('isSoftwareVideoImplementation', () => { it('detects common software encoder and decoder implementations', () => { expect(isSoftwareVideoImplementation('libvpx')).toBe(true); @@ -133,4 +147,76 @@ describe('findStalledVideoDecoder', () => { ]); expect(findStalledVideoDecoder(stats)).toBeNull(); }); + it('does not treat a first keyframe that is still arriving as a decoder stall', () => { + const stats = createStatsReport([ + {id: 'codec-1', type: 'codec', mimeType: 'video/H264'}, + { + id: 'inbound-1', + type: 'inbound-rtp', + kind: 'video', + codecId: 'codec-1', + packetsReceived: 40, + bytesReceived: 60_000, + framesReceived: 0, + framesDecoded: 0, + }, + ]); + expect(findStalledVideoDecoder(stats)).toBeNull(); + }); + it('does not treat packets that carry no video frames as a decoder stall', () => { + const stats = createStatsReport([ + {id: 'codec-1', type: 'codec', mimeType: 'video/H264'}, + { + id: 'inbound-1', + type: 'inbound-rtp', + kind: 'video', + codecId: 'codec-1', + packetsReceived: 50, + bytesReceived: 0, + framesReceived: 0, + framesDecoded: 0, + }, + ]); + expect(findStalledVideoDecoder(stats)).toBeNull(); + }); + it('treats whole frames that never decode as a decoder stall', () => { + const stats = createStatsReport([ + {id: 'codec-1', type: 'codec', mimeType: 'video/H264'}, + { + id: 'inbound-1', + type: 'inbound-rtp', + kind: 'video', + codecId: 'codec-1', + packetsReceived: 40, + bytesReceived: 60_000, + framesReceived: 40, + framesDecoded: 0, + }, + ]); + expect(findStalledVideoDecoder(stats)).toMatchObject({codec: 'h264', framesReceived: 40, framesDecoded: 0}); + }); +}); + +describe('confirmDecodeStall', () => { + it('confirms a stall when more frames arrived and none of them decoded', () => { + const second = createStallInfo({framesReceived: 45}); + expect(confirmDecodeStall(createStallInfo({framesReceived: 20}), second)).toBe(second); + }); + it('does not confirm a stall when the second sample decoded a frame', () => { + expect(confirmDecodeStall(createStallInfo(), createStallInfo({framesReceived: 45, framesDecoded: 3}))).toBeNull(); + }); + it('does not confirm a stall when the second sample is missing', () => { + expect(confirmDecodeStall(createStallInfo(), null)).toBeNull(); + }); + it('does not confirm a stall when no new frames arrived between the samples', () => { + expect(confirmDecodeStall(createStallInfo({framesReceived: 20}), createStallInfo({framesReceived: 20}))).toBeNull(); + }); + it('does not confirm a stall across two different codecs', () => { + expect( + confirmDecodeStall( + createStallInfo({framesReceived: 20}), + createStallInfo({codec: 'vp9', mimeType: 'video/VP9', framesReceived: 45}), + ), + ).toBeNull(); + }); }); diff --git a/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.ts b/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.ts index e5f180915..2ddf3e020 100644 --- a/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.ts +++ b/fluxer_app/src/features/voice/utils/ScreenShareCodecDiagnostics.ts @@ -46,7 +46,7 @@ export interface StalledVideoDecoderInfo { packetsReceived: number; bytesReceived: number; framesDecoded: number; - framesReceived: number | null; + framesReceived: number; } function isSoftwareVideoStats(implementation: string | null, powerEfficient: boolean | null): boolean { @@ -127,11 +127,10 @@ export function findStalledVideoDecoder(stats: RTCStatsReport): StalledVideoDeco if (getStatsKind(report, codecs) !== 'video') continue; const framesDecoded = finiteNumber(report.framesDecoded); if (framesDecoded === null || framesDecoded > 0) continue; + const framesReceived = finiteNumber(report.framesReceived); + if (framesReceived === null || framesReceived < 1) continue; const packetsReceived = finiteNumber(report.packetsReceived) ?? 0; const bytesReceived = finiteNumber(report.bytesReceived) ?? 0; - const framesReceived = finiteNumber(report.framesReceived); - const hasVideoPayload = packetsReceived >= 10 || bytesReceived >= 8192 || (framesReceived ?? 0) >= 2; - if (!hasVideoPayload) continue; const mimeType = report.codecId ? codecs.get(report.codecId)?.mimeType : undefined; const codec = getVideoCodecFromMimeType(mimeType); if (!codec) continue; @@ -147,20 +146,49 @@ export function findStalledVideoDecoder(stats: RTCStatsReport): StalledVideoDeco return null; } +export function confirmDecodeStall( + first: StalledVideoDecoderInfo | null, + second: StalledVideoDecoderInfo | null, +): StalledVideoDecoderInfo | null { + if (!first || !second) return null; + if (first.codec !== second.codec || first.mimeType !== second.mimeType) return null; + if (first.framesDecoded !== 0 || second.framesDecoded !== 0) return null; + if (second.framesReceived <= first.framesReceived) return null; + return second; +} + export function scheduleScreenShareDecoderVerification( getStats: () => Promise, onComplete?: () => void, onDecodeFailure?: (failure: StalledVideoDecoderInfo) => void, -): NodeJS.Timeout { - return setTimeout(async () => { +): () => void { + let cancelled = false; + let timer: ReturnType | null = null; + const scheduleConfirmation = (first: StalledVideoDecoderInfo): void => { + timer = setTimeout(async () => { + timer = null; + try { + const stats = await getStats(); + const confirmed = confirmDecodeStall(first, stats ? findStalledVideoDecoder(stats) : null); + if (cancelled || !confirmed) return; + logger.warn('Screen share video decode is stalled', confirmed); + onDecodeFailure?.(confirmed); + } catch (error) { + logger.debug('Failed to confirm the screen share decode stall', {error}); + } finally { + if (!cancelled) { + onComplete?.(); + } + } + }, DECODER_VERIFICATION_DELAY_MS); + }; + timer = setTimeout(async () => { + timer = null; + let firstStall: StalledVideoDecoderInfo | null = null; try { const stats = await getStats(); if (!stats) return; - const stalledDecoder = findStalledVideoDecoder(stats); - if (stalledDecoder) { - logger.warn('Screen share video decode is stalled', stalledDecoder); - onDecodeFailure?.(stalledDecoder); - } + firstStall = findStalledVideoDecoder(stats); const decoder = findSoftwareVideoDecoder(stats); if (!decoder) return; logger.warn('Screen share is using a software decoder', decoder); @@ -168,7 +196,19 @@ export function scheduleScreenShareDecoderVerification( } catch (error) { logger.debug('Failed to verify screen share decoder', {error}); } finally { - onComplete?.(); + if (!cancelled) { + if (firstStall) { + scheduleConfirmation(firstStall); + } else { + onComplete?.(); + } + } } }, DECODER_VERIFICATION_DELAY_MS); + return () => { + cancelled = true; + if (!timer) return; + clearTimeout(timer); + timer = null; + }; }