fix(voice): confirm a decode stall before withdrawing a codec (#2661)

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<Record<string, unknown>> = [
 		{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<RTCStatsReport>): () => Promise<RTCStatsReport | undefined> {
+	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<string> = [];
+		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);
+	});
+});

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<string, ParticipantSpeakingDisposer>();
-	const screenShareDecoderVerificationTimers = new Map<string, NodeJS.Timeout>();
+	const screenShareDecoderVerificationCancels = new Map<string, () => void>();
 	const remoteTrackLifecycleDisposers = new Map<string, () => 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;

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<Record<string, unknown>>): RTCStatsReport {
 	return new Map(entries.map((entry) => [entry.id as string, entry])) as unknown as RTCStatsReport;
 }
 
+function createStallInfo(overrides: Partial<StalledVideoDecoderInfo> = {}): 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();
+	});
 });

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<RTCStatsReport | undefined>,
 	onComplete?: () => void,
 	onDecodeFailure?: (failure: StalledVideoDecoderInfo) => void,
-): NodeJS.Timeout {
-	return setTimeout(async () => {
+): () => void {
+	let cancelled = false;
+	let timer: ReturnType<typeof setTimeout> | 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;
+	};
 }