From 924eec5cc32269137646ed20d9ef9df24f925a40 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 29 Aug 2026 11:33:59 +0000 Subject: [PATCH] fix(webrtc): serialize screen-share publication and capture restarts Fixes #70. On macOS the app terminated in ScreenCaptureKit/ReplayKit with "Collection <__NSArrayM> was mutated while being enumerated" after repeatedly publishing the same screen_share track while the room recovered. The publish path decided replace-vs-publish by reading trackPublications and then awaited publishTrack. livekit only inserts the publication once that await resolves, so two overlapping callers both read "nothing published yet" and both published: "publishing a second track with the same source" and "TrackInvalidError: a track with the same ID has already been published". Surfacing that rejection drove another retry, and the retries restarted capture underneath the running stream. Overlap was easy to reach: CapturePreview's publish effect re-runs on five dependencies, and isHosting alone flaps false/true on every reconnect, while a publish ending in "publication of local track timed out" stays in flight for seconds. - Serialize publishStream/unpublishStream onto one promise chain so the replace-vs-publish decision and the publish are atomic, and so a stop/start straddling a reconnect cannot unpublish the track the restart just aired. - Treat "already been published" as success instead of an error, so a duplicate that slips through cannot start a retry loop. - Give publishStream an isStale callback, checked once it reaches the front of the queue; CapturePreview's effect uses it to abandon a publish whose capture session has been superseded. - Guard the three capture entry points with a ref instead of isCapturing state, which is not visible to another handler in the same tick and so let two concurrent ScreenCaptureKit sessions start. - Snapshot trackPublications in stopHosting before unpublishing; unpublishing deletes from the Map being walked. - Log publish/replace/unpublish with source and track id. apps/web's SFU host never received the replace-or-publish fix from #80, so it published unconditionally on every call. Brought it to parity with the same serialization, dedupe and already-published handling. Regression tests cover the overlap, the publish/unpublish interleave, the already-published error and the stale-publish skip; the first two fail without the queue. The existing mock resolved publishes synchronously, which is why it never caught this. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MfEYUb5EtniuCgViazw7Vm --- .../components/capture/CapturePreview.tsx | 13 +- .../hooks/useWebRTCHostSFUAPI.test.ts | 157 +++++++ .../src/renderer/hooks/useWebRTCHostSFUAPI.ts | 236 +++++++--- apps/desktop/src/renderer/routes/home.tsx | 424 +++++++++--------- apps/web/src/hooks/useWebRTCHostSFU.ts | 185 ++++++-- 5 files changed, 699 insertions(+), 316 deletions(-) diff --git a/apps/desktop/src/renderer/components/capture/CapturePreview.tsx b/apps/desktop/src/renderer/components/capture/CapturePreview.tsx index 128cf59c..4cfa314d 100644 --- a/apps/desktop/src/renderer/components/capture/CapturePreview.tsx +++ b/apps/desktop/src/renderer/components/capture/CapturePreview.tsx @@ -704,9 +704,20 @@ export function CapturePreview({ videoTracks: publishStream.getVideoTracks().length, audioTracks: publishStream.getAudioTracks().length, audioSource: hostMicStream ? 'host-mic' : 'capture-stream', + videoTrackId: presentationVideoTrack.id, + micTrackId: micTrack?.id, }); - void hostPublishStream(publishStream); + // This effect re-runs on five dependencies, and `isHosting` alone flaps + // false/true on every reconnect — so a publish for capture session N is + // routinely still queued when the effect fires for N+1. `superseded` lets + // the host drop the stale one instead of replacing the live track with a + // track whose capture has already been stopped. + let superseded = false; + void hostPublishStream(publishStream, () => superseded); + return () => { + superseded = true; + }; }, [presentationVideoTrack, hostMicStream, isHosting, hostPublishStream, stream]); // The stream handed to the RTMP broadcast. Like the recording — and unlike diff --git a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts index 945e3f00..32d88d4c 100644 --- a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts +++ b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts @@ -694,4 +694,161 @@ describe('useWebRTCHostSFUAPI', () => { expect(compositeTrack.contentHint).toBe('detail'); }); }); + + // Issue #70: on macOS the app terminated in ScreenCaptureKit/ReplayKit after + // repeatedly publishing the same screen_share track while the room recovered. + // The publish path decides replace-vs-publish *before* it awaits, so two + // overlapping callers both decided "publish". + describe('concurrent publication of the screen share', () => { + const mockReplaceTrack = vi.fn().mockResolvedValue(undefined); + + /** + * Mirror livekit with a publish that stays in flight: the publication only + * appears in `trackPublications` once the returned promise resolves. That + * window is the entire bug, and the existing helper above cannot express it + * because it resolves synchronously. + */ + function slowPublishingRoom() { + mockReplaceTrack.mockClear(); + const pending: (() => void)[] = []; + + const publicationFor = (track: { id: string }, options: { source: string }) => { + const publication = { + source: options.source, + track: { mediaStreamTrack: track, replaceTrack: mockReplaceTrack }, + }; + mockTrackPublications.set(options.source, publication); + return publication; + }; + + mockPublishTrack.mockImplementation((track: { id: string }, options: { source: string }) => { + // Only the screen share is held open. startHosting awaits the host mic + // publish inline, so deferring that one would hang the test before it + // ever reaches the code under test. + if (options.source !== 'screen_share') { + return Promise.resolve(publicationFor(track, options)); + } + return new Promise((resolve) => { + pending.push(() => { + resolve(publicationFor(track, options)); + }); + }); + }); + + return { + inFlight: () => pending.length, + settleAll: () => { + while (pending.length > 0) pending.shift()?.(); + }, + }; + } + + /** Let queued microtasks run without letting a hung promise stall the test. */ + const flush = async () => { + for (let i = 0; i < 20; i++) await Promise.resolve(); + }; + + async function hostingRoom() { + const { result } = renderHook(() => + useWebRTCHostSFUAPI({ sessionId: 'session-1', hostId: 'host-1', localStream: null }) + ); + await act(async () => { + await result.current.startHosting(); + await Promise.resolve(); + }); + return result; + } + + const videoPublishes = () => + mockPublishTrack.mock.calls.filter( + (call) => (call[1] as { source: string }).source === 'screen_share' + ); + + it('publishes the screen track once when two publishes overlap', async () => { + const room = slowPublishingRoom(); + const result = await hostingRoom(); + + const screenTrack = { kind: 'video', id: 'screen-track' }; + + await act(async () => { + // Both calls are made before either can finish — the shape produced by + // the publish effect re-running while a publish is still awaiting. + const first = result.current.publishStream( + new MockMediaStream([screenTrack]) as unknown as MediaStream + ); + const second = result.current.publishStream( + new MockMediaStream([screenTrack]) as unknown as MediaStream + ); + + await flush(); + room.settleAll(); + await flush(); + // Settle again so an unserialised second publish resolves too, and this + // test fails on the assertion rather than timing out. + room.settleAll(); + await Promise.all([first, second]); + }); + + expect(videoPublishes()).toHaveLength(1); + }); + + it('does not let an unpublish interleave with an in-flight publish', async () => { + const room = slowPublishingRoom(); + const result = await hostingRoom(); + + const screenTrack = { kind: 'video', id: 'screen-track' }; + + await act(async () => { + const publishing = result.current.publishStream( + new MockMediaStream([screenTrack]) as unknown as MediaStream + ); + await flush(); + + // The unpublish must wait its turn: running it now would tear down the + // publication the in-flight publish is still creating, leaving viewers + // on a black frame with the host still marked live. + const unpublishing = result.current.unpublishStream(); + await flush(); + expect(mockUnpublishTrack).not.toHaveBeenCalled(); + + room.settleAll(); + await Promise.all([publishing, unpublishing]); + }); + + expect(mockUnpublishTrack).toHaveBeenCalledTimes(1); + }); + + it('treats an already-published track as success, not an error', async () => { + const result = await hostingRoom(); + + // livekit's rejection when a duplicate slips through anyway. Surfacing it + // is what drove the caller into another retry, and the retries are what + // restarted capture underneath the running ScreenCaptureKit stream. + mockPublishTrack.mockRejectedValue( + new Error('a track with the same ID has already been published') + ); + + await act(async () => { + await expect( + result.current.publishStream( + new MockMediaStream([{ kind: 'video', id: 'screen-track' }]) as unknown as MediaStream + ) + ).resolves.toBeUndefined(); + }); + }); + + it('skips a publish whose capture session has already been superseded', async () => { + slowPublishingRoom(); + const result = await hostingRoom(); + + await act(async () => { + await result.current.publishStream( + new MockMediaStream([{ kind: 'video', id: 'stale-track' }]) as unknown as MediaStream, + () => true + ); + }); + + expect(videoPublishes()).toHaveLength(0); + }); + }); }); diff --git a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts index 45e8196d..1a0a7f26 100644 --- a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts +++ b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts @@ -35,6 +35,19 @@ import { amplifyRemoteAudio, type AmplifiedAudioTrack } from '@/lib/remoteAudioG const LIVEKIT_URL = process.env.NEXT_PUBLIC_LIVEKIT_URL ?? ''; +/** + * Did this publish fail only because the track is already on the air? + * + * livekit signals it as `TrackInvalidError: a track with the same ID has + * already been published`. There is no error code to match on, so the message + * is the only handle — matched loosely enough to survive rewording, and + * deliberately narrow enough not to swallow a genuine publish failure. + */ +function isAlreadyPublished(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return /already been published|already publish/i.test(message); +} + const encoder = new TextEncoder(); const decoder = new TextDecoder(); const REJECTED_INPUT_LOG_INTERVAL_MS = 5_000; @@ -81,7 +94,12 @@ interface UseWebRTCHostSFUAPIReturn { error: string | null; startHosting: () => Promise; stopHosting: () => void; - publishStream: (stream: MediaStream) => Promise; + /** + * Publish the presentation stream. `isStale` is polled once the call reaches + * the front of the publish queue: return true to abandon a publication whose + * capture session has already been replaced or torn down. + */ + publishStream: (stream: MediaStream, isStale?: () => boolean) => Promise; unpublishStream: () => Promise; grantControl: (viewerId: string) => void; sendTailnetHello: (viewerId: string, ips: string[], reply: boolean) => void; @@ -118,6 +136,8 @@ export function useWebRTCHostSFUAPI({ const roomRef = useRef(null); const startingRef = useRef(false); + // Serialises every mutation of the screen-share publication — see publishStream. + const publishQueueRef = useRef>(Promise.resolve()); const viewersRef = useRef>(new Map()); const authTokenRef = useRef(null); const hostMicStreamRef = useRef(null); @@ -571,6 +591,35 @@ export function useWebRTCHostSFUAPI({ } }, [sessionId, hostId, addViewer, attachViewerAudio, removeViewer, handleDataReceived]); + /** + * Run `op` with exclusive access to the screen-share publication. + * + * Publishing is a read-then-await-then-write: `publicationFor()` decides + * replace-vs-publish, then the very next line yields at an `await`. Two + * overlapping callers therefore both read "nothing published yet" and both + * call `publishTrack` — livekit logs `publishing a second track with the same + * source: screen_share` and rejects the loser with `TrackInvalidError`. That + * rejection drives the caller into another retry, and on macOS the retries + * restart ScreenCaptureKit underneath the running stream, which terminates the + * app with `Collection <__NSArrayM> was mutated while being enumerated`. + * + * Overlap is easy to hit: the publish effect in CapturePreview re-runs on any + * of five dependencies, and a slow publish that ends in "publication of local + * track timed out" stays in flight for many seconds while it does. + * + * Chaining onto a single promise makes each decision-and-publish atomic. The + * queue itself must never settle rejected or every later op would be dropped, + * so the stored tail always swallows; the caller still sees its own error. + */ + const enqueuePublishOp = useCallback((op: () => Promise): Promise => { + const run = publishQueueRef.current.then(op, op); + publishQueueRef.current = run.then( + () => undefined, + () => undefined + ); + return run; + }, []); + /** * Publish the presentation stream, replacing whatever is already published. * @@ -584,93 +633,142 @@ export function useWebRTCHostSFUAPI({ * Replacing the track inside the existing publication keeps the same * publication SID, so viewers switch over without resubscribing. The P2P * host does exactly this, for exactly this reason — see useWebRTCHostAPI. + * + * Callers never run concurrently: every call is serialised by + * `enqueuePublishOp`, and the room/publication state is read *inside* the + * critical section so a queued call sees what the one ahead of it did. */ - const publishStream = useCallback(async (stream: MediaStream) => { - const room = roomRef.current; - if (room?.state !== LKConnectionState.Connected) { - console.warn('[WebRTCHostSFUAPI] Cannot publish stream: room not connected'); - return; - } + const publishStream = useCallback( + (stream: MediaStream, isStale?: () => boolean): Promise => + enqueuePublishOp(async () => { + // Re-checked here rather than at call time: by the time this reaches the + // front of the queue the capture session may have been replaced or the + // host may have stopped, and republishing a dead track would knock the + // live one off the air. + if (isStale?.()) { + console.log('[WebRTCHostSFUAPI] Skipping stale publish (capture session changed)'); + return; + } - const publicationFor = (source: Track.Source): LocalTrackPublication | undefined => - Array.from(room.localParticipant.trackPublications.values()).find( - (pub: LocalTrackPublication) => pub.source === source - ); + const room = roomRef.current; + if (room?.state !== LKConnectionState.Connected) { + console.warn('[WebRTCHostSFUAPI] Cannot publish stream: room not connected'); + return; + } - for (const track of stream.getTracks()) { - try { - const source = - track.kind === 'video' ? Track.Source.ScreenShare : Track.Source.ScreenShareAudio; + const publicationFor = (source: Track.Source): LocalTrackPublication | undefined => + Array.from(room.localParticipant.trackPublications.values()).find( + (pub: LocalTrackPublication) => pub.source === source + ); - if (track.kind === 'video') { - track.contentHint = 'detail'; - } + for (const track of stream.getTracks()) { + try { + const source = + track.kind === 'video' ? Track.Source.ScreenShare : Track.Source.ScreenShareAudio; - const existing = publicationFor(source); - if (existing?.track) { - // Already publishing this exact track — a re-render, not a new source. - if (existing.track.mediaStreamTrack.id === track.id) continue; + if (track.kind === 'video') { + track.contentHint = 'detail'; + } - await existing.track.replaceTrack(track); - console.log('[WebRTCHostSFUAPI] Replaced published track', { - source, - trackId: track.id, - }); - continue; - } + const existing = publicationFor(source); + if (existing?.track) { + // Already publishing this exact track — a re-render, not a new source. + if (existing.track.mediaStreamTrack.id === track.id) continue; - if (track.kind === 'video') { - await room.localParticipant.publishTrack(track, { - source: Track.Source.ScreenShare, - simulcast: false, - videoEncoding: { - maxBitrate: 8_000_000, - maxFramerate: 60, - }, - }); - } else { - await room.localParticipant.publishTrack(track, { - source: Track.Source.ScreenShareAudio, - }); + await existing.track.replaceTrack(track); + console.log('[WebRTCHostSFUAPI] Replaced published track', { + source, + trackId: track.id, + }); + continue; + } + + console.log('[WebRTCHostSFUAPI] Publishing track', { source, trackId: track.id }); + + if (track.kind === 'video') { + await room.localParticipant.publishTrack(track, { + source: Track.Source.ScreenShare, + simulcast: false, + videoEncoding: { + maxBitrate: 8_000_000, + maxFramerate: 60, + }, + }); + } else { + await room.localParticipant.publishTrack(track, { + source: Track.Source.ScreenShareAudio, + }); + } + } catch (err) { + // Losing a publish race is not a failure: the track this call wanted + // on the air is already on the air. Serialisation should prevent it, + // but a livekit-internal retry can still land one, and treating it as + // an error is what turns a duplicate into a retry loop. + if (isAlreadyPublished(err)) { + console.log('[WebRTCHostSFUAPI] Track already published — treating as success', { + trackId: track.id, + }); + continue; + } + + // A transient ICE/consent blip can reject an in-flight publish ("publication + // of local track timed out") even though livekit reconnects and the track + // ends up published. Only treat it as fatal if the room is actually gone; + // otherwise swallow it so it never bubbles up as an uncaught rejection / + // scary "Streaming error" toast while the stream is still live. The cast + // widens room.state back to the full enum — it mutates across the await, + // so the narrowing from the early-return guard no longer holds here. + if ((room.state as LKConnectionState) === LKConnectionState.Disconnected) throw err; + console.warn('[WebRTCHostSFUAPI] publishTrack hiccup (room recovering):', err); + } } - } catch (err) { - // A transient ICE/consent blip can reject an in-flight publish ("publication - // of local track timed out") even though livekit reconnects and the track - // ends up published. Only treat it as fatal if the room is actually gone; - // otherwise swallow it so it never bubbles up as an uncaught rejection / - // scary "Streaming error" toast while the stream is still live. The cast - // widens room.state back to the full enum — it mutates across the await, - // so the narrowing from the early-return guard no longer holds here. - if ((room.state as LKConnectionState) === LKConnectionState.Disconnected) throw err; - console.warn('[WebRTCHostSFUAPI] publishTrack hiccup (room recovering):', err); - } - } - }, []); + }), + [enqueuePublishOp] + ); // Unpublish screen share tracks (room stays connected, viewers stay connected) - const unpublishStream = useCallback(async () => { - const room = roomRef.current; - if (!room) return; - - const pubs = Array.from(room.localParticipant.trackPublications.values()); - for (const pub of pubs) { - if (pub.source === Track.Source.ScreenShare || pub.source === Track.Source.ScreenShareAudio) { - if (pub.track) { - await room.localParticipant.unpublishTrack(pub.track); + // + // Shares publishStream's queue. Interleaving the two is how a stop/start + // straddling a reconnect ends up unpublishing the track the restart just put + // on the air, leaving viewers on a black frame with the host still "live". + const unpublishStream = useCallback( + (): Promise => + enqueuePublishOp(async () => { + const room = roomRef.current; + if (!room) return; + + const pubs = Array.from(room.localParticipant.trackPublications.values()); + for (const pub of pubs) { + if ( + pub.source === Track.Source.ScreenShare || + pub.source === Track.Source.ScreenShareAudio + ) { + if (pub.track) { + console.log('[WebRTCHostSFUAPI] Unpublishing track', { + source: pub.source, + trackId: pub.track.mediaStreamTrack.id, + }); + await room.localParticipant.unpublishTrack(pub.track); + } + } } - } - } - }, []); + }), + [enqueuePublishOp] + ); // Stop hosting const stopHosting = useCallback(() => { const room = roomRef.current; if (room) { - room.localParticipant.trackPublications.forEach((pub: LocalTrackPublication) => { + // Snapshot first: unpublishTrack deletes from trackPublications, and + // deleting from a Map while forEach walks it is the same mutate-while- + // enumerating shape that crashes the native capture side. unpublishStream + // already snapshots; this path had been missed. + for (const pub of Array.from(room.localParticipant.trackPublications.values())) { if (pub.track) { void room.localParticipant.unpublishTrack(pub.track); } - }); + } void room.disconnect(); roomRef.current = null; } diff --git a/apps/desktop/src/renderer/routes/home.tsx b/apps/desktop/src/renderer/routes/home.tsx index 98d71f99..f8b66d4d 100644 --- a/apps/desktop/src/renderer/routes/home.tsx +++ b/apps/desktop/src/renderer/routes/home.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import { useNavigate, useSearchParams } from 'react-router-dom'; import { Users, Link2, Loader2, Mic, Radio, Calendar, User as UserIcon } from 'lucide-react'; import { SourcePicker } from '@/components/capture/SourcePicker'; @@ -87,6 +87,30 @@ export function HomePage() { initialIsWaylandGuess(isElectron() ? getElectronAPI().platform : null) ); const [isCapturing, setIsCapturing] = useState(false); + // `isCapturing` drives the UI, but it cannot guard the capture calls: a state + // update is not visible to another handler running in the same tick, so two + // clicks — or a click racing a room-recovery restart — both read `false` and + // both open a capture session. On macOS that means two concurrent + // ScreenCaptureKit `contentPickerDidSelectFilter:forStream:` callbacks + // mutating one stream collection, which terminates the app with an uncaught + // NSGenericException. The ref flips synchronously, so it actually excludes. + const capturingRef = useRef(false); + + /** Run a capture start/restart with exclusive access, or skip if one is live. */ + const runExclusiveCapture = useCallback(async (capture: () => Promise) => { + if (capturingRef.current) { + console.warn('[Renderer] Capture already starting — ignoring concurrent request'); + return; + } + capturingRef.current = true; + setIsCapturing(true); + try { + await capture(); + } finally { + capturingRef.current = false; + setIsCapturing(false); + } + }, []); const [showCreateLinkModal, setShowCreateLinkModal] = useState(false); const [showStartMeetingModal, setShowStartMeetingModal] = useState(false); const [preCreatedSession, setPreCreatedSession] = useState(null); @@ -158,180 +182,173 @@ export function HomePage() { }, [searchParams, preCreatedSession, sessionActive, loadingExistingSession]); const handleSourceSelect = async (source: CaptureSource) => { - // Prevent multiple concurrent capture attempts - if (isCapturing) return; - - setSelectedSource(source); - setError(null); - setIsCapturing(true); - - try { - // Stop existing stream - if (stream) { - stream.getTracks().forEach((track) => { - track.stop(); - }); - } - - console.log('[Renderer] Starting capture for source:', source.id); - console.log('[Renderer] Display server:', displayServer); + await runExclusiveCapture(async () => { + setSelectedSource(source); + setError(null); - let mediaStream: MediaStream; + try { + // Stop existing stream + if (stream) { + stream.getTracks().forEach((track) => { + track.stop(); + }); + } - if (isWayland) { - // Wayland: Use getDisplayMedia with PipeWire portal - // This will show the system's screen picker dialog. - // Main's display-media handler cannot tell which source was picked - // from the request alone, so hand it the id first. - console.log('[Renderer] Using getDisplayMedia for Wayland'); - await getElectronAPI().invoke('capture:setPreferredSource', { sourceId: source.id }); - mediaStream = await navigator.mediaDevices.getDisplayMedia({ - video: { - displaySurface: source.type === 'screen' ? 'monitor' : 'window', - width: { ideal: 1920, max: 3840 }, - height: { ideal: 1080, max: 2160 }, - frameRate: { ideal: 30, max: 60 }, - }, - audio: false, - }); - } else { - // X11/Windows/macOS: Use getUserMedia with chromeMediaSource - console.log('[Renderer] Using getUserMedia with chromeMediaSource'); - mediaStream = await navigator.mediaDevices.getUserMedia({ - audio: false, - video: { - // @ts-expect-error Electron-specific constraint - mandatory: { - chromeMediaSource: 'desktop', - chromeMediaSourceId: source.id, - minWidth: 1280, - maxWidth: 3840, - minHeight: 720, - maxHeight: 2160, - minFrameRate: 15, - maxFrameRate: 60, + console.log('[Renderer] Starting capture for source:', source.id); + console.log('[Renderer] Display server:', displayServer); + + let mediaStream: MediaStream; + + if (isWayland) { + // Wayland: Use getDisplayMedia with PipeWire portal + // This will show the system's screen picker dialog. + // Main's display-media handler cannot tell which source was picked + // from the request alone, so hand it the id first. + console.log('[Renderer] Using getDisplayMedia for Wayland'); + await getElectronAPI().invoke('capture:setPreferredSource', { sourceId: source.id }); + mediaStream = await navigator.mediaDevices.getDisplayMedia({ + video: { + displaySurface: source.type === 'screen' ? 'monitor' : 'window', + width: { ideal: 1920, max: 3840 }, + height: { ideal: 1080, max: 2160 }, + frameRate: { ideal: 30, max: 60 }, }, - }, - }); - } - - console.log('[Renderer] Capture started successfully'); - - // Set content hint on video track for screen sharing optimization - // 'detail' tells encoder to prioritize sharpness (good for text) - const videoTrack = mediaStream.getVideoTracks()[0]; - videoTrack.contentHint = 'detail'; - - // Align resolution to 16px boundary to prevent VP9 green bar artifacts - await constrainTrackToQualitySetting(videoTrack); + audio: false, + }); + } else { + // X11/Windows/macOS: Use getUserMedia with chromeMediaSource + console.log('[Renderer] Using getUserMedia with chromeMediaSource'); + mediaStream = await navigator.mediaDevices.getUserMedia({ + audio: false, + video: { + // @ts-expect-error Electron-specific constraint + mandatory: { + chromeMediaSource: 'desktop', + chromeMediaSourceId: source.id, + minWidth: 1280, + maxWidth: 3840, + minHeight: 720, + maxHeight: 2160, + minFrameRate: 15, + maxFrameRate: 60, + }, + }, + }); + } - // Add microphone audio for streaming to viewers - try { - const micStream = await navigator.mediaDevices.getUserMedia({ - audio: VOICE_AUDIO_CONSTRAINTS, - video: false, - }); - micStream.getAudioTracks().forEach((track) => { - mediaStream.addTrack(track); - }); - console.log('[Renderer] Microphone audio added to stream'); - } catch (micErr) { - console.warn('[Renderer] Could not access microphone, streaming without audio:', micErr); - } + console.log('[Renderer] Capture started successfully'); + + // Set content hint on video track for screen sharing optimization + // 'detail' tells encoder to prioritize sharpness (good for text) + const videoTrack = mediaStream.getVideoTracks()[0]; + videoTrack.contentHint = 'detail'; + + // Align resolution to 16px boundary to prevent VP9 green bar artifacts + await constrainTrackToQualitySetting(videoTrack); + + // Add microphone audio for streaming to viewers + try { + const micStream = await navigator.mediaDevices.getUserMedia({ + audio: VOICE_AUDIO_CONSTRAINTS, + video: false, + }); + micStream.getAudioTracks().forEach((track) => { + mediaStream.addTrack(track); + }); + console.log('[Renderer] Microphone audio added to stream'); + } catch (micErr) { + console.warn('[Renderer] Could not access microphone, streaming without audio:', micErr); + } - setStream(mediaStream); - setSessionActive(true); - } catch (err) { - console.error('[Renderer] Failed to start capture:', err); - const message = err instanceof Error ? err.message : 'Unknown error'; - // Provide more user-friendly error messages - if (message.includes('Permission denied') || message.includes('NotAllowedError')) { - setError('Screen capture was canceled or permission denied. Please try again.'); - } else { - setError(`Failed to capture: ${message}`); + setStream(mediaStream); + setSessionActive(true); + } catch (err) { + console.error('[Renderer] Failed to start capture:', err); + const message = err instanceof Error ? err.message : 'Unknown error'; + // Provide more user-friendly error messages + if (message.includes('Permission denied') || message.includes('NotAllowedError')) { + setError('Screen capture was canceled or permission denied. Please try again.'); + } else { + setError(`Failed to capture: ${message}`); + } + setSelectedSource(null); } - setSelectedSource(null); - } finally { - setIsCapturing(false); - } + }); }; // Handle Wayland direct capture (bypasses source picker) const handleWaylandCapture = async () => { - if (isCapturing) return; - - setError(null); - setIsCapturing(true); - - try { - if (stream) { - stream.getTracks().forEach((track) => { - track.stop(); - }); - } + await runExclusiveCapture(async () => { + setError(null); - console.log('[Renderer] Starting Wayland capture with system picker'); + try { + if (stream) { + stream.getTracks().forEach((track) => { + track.stop(); + }); + } - // No in-app pick to honour here — clear any stale preference so the - // portal's own picker decides. - await getElectronAPI().invoke('capture:setPreferredSource', { sourceId: null }); + console.log('[Renderer] Starting Wayland capture with system picker'); - const mediaStream = await navigator.mediaDevices.getDisplayMedia({ - video: { - width: { ideal: 1920, max: 3840 }, - height: { ideal: 1080, max: 2160 }, - frameRate: { ideal: 30, max: 60 }, - }, - audio: false, - }); + // No in-app pick to honour here — clear any stale preference so the + // portal's own picker decides. + await getElectronAPI().invoke('capture:setPreferredSource', { sourceId: null }); - // Create a synthetic source from the stream - const track = mediaStream.getVideoTracks()[0]; + const mediaStream = await navigator.mediaDevices.getDisplayMedia({ + video: { + width: { ideal: 1920, max: 3840 }, + height: { ideal: 1080, max: 2160 }, + frameRate: { ideal: 30, max: 60 }, + }, + audio: false, + }); - // Set content hint for screen sharing optimization - // 'detail' tells encoder to prioritize sharpness (good for text) - track.contentHint = 'detail'; + // Create a synthetic source from the stream + const track = mediaStream.getVideoTracks()[0]; - // Align resolution to 16px boundary to prevent VP9 green bar artifacts - await constrainTrackToQualitySetting(track); + // Set content hint for screen sharing optimization + // 'detail' tells encoder to prioritize sharpness (good for text) + track.contentHint = 'detail'; - const settings = track.getSettings(); + // Align resolution to 16px boundary to prevent VP9 green bar artifacts + await constrainTrackToQualitySetting(track); - setSelectedSource({ - id: track.id, - name: track.label || 'Screen', - type: 'screen', - thumbnail: undefined, - }); + const settings = track.getSettings(); - // Add microphone audio for streaming to viewers - try { - const micStream = await navigator.mediaDevices.getUserMedia({ - audio: VOICE_AUDIO_CONSTRAINTS, - video: false, - }); - micStream.getAudioTracks().forEach((t) => { - mediaStream.addTrack(t); + setSelectedSource({ + id: track.id, + name: track.label || 'Screen', + type: 'screen', + thumbnail: undefined, }); - console.log('[Renderer] Microphone audio added to Wayland stream'); - } catch (micErr) { - console.warn('[Renderer] Could not access microphone, streaming without audio:', micErr); - } - console.log('[Renderer] Wayland capture started:', settings); - setStream(mediaStream); - setSessionActive(true); - } catch (err) { - console.error('[Renderer] Failed to start Wayland capture:', err); - const message = err instanceof Error ? err.message : 'Unknown error'; - if (message.includes('Permission denied') || message.includes('NotAllowedError')) { - setError('Screen capture was canceled or permission denied. Please try again.'); - } else { - setError(`Failed to capture: ${message}`); + // Add microphone audio for streaming to viewers + try { + const micStream = await navigator.mediaDevices.getUserMedia({ + audio: VOICE_AUDIO_CONSTRAINTS, + video: false, + }); + micStream.getAudioTracks().forEach((t) => { + mediaStream.addTrack(t); + }); + console.log('[Renderer] Microphone audio added to Wayland stream'); + } catch (micErr) { + console.warn('[Renderer] Could not access microphone, streaming without audio:', micErr); + } + + console.log('[Renderer] Wayland capture started:', settings); + setStream(mediaStream); + setSessionActive(true); + } catch (err) { + console.error('[Renderer] Failed to start Wayland capture:', err); + const message = err instanceof Error ? err.message : 'Unknown error'; + if (message.includes('Permission denied') || message.includes('NotAllowedError')) { + setError('Screen capture was canceled or permission denied. Please try again.'); + } else { + setError(`Failed to capture: ${message}`); + } } - } finally { - setIsCapturing(false); - } + }); }; // Stop screen sharing only (session continues with voice) @@ -360,65 +377,62 @@ export function HomePage() { // Start/restart screen capture from within an active session const handleStartCaptureInSession = async () => { - if (isCapturing) return; - setIsCapturing(true); - - try { - if (stream) { - stream.getTracks().forEach((track) => { - track.stop(); - }); - } + await runExclusiveCapture(async () => { + try { + if (stream) { + stream.getTracks().forEach((track) => { + track.stop(); + }); + } - // Restarting capture mid-session: the user re-picks in the system / - // portal dialog, so no preference to honour. - await getElectronAPI().invoke('capture:setPreferredSource', { sourceId: null }); - - const mediaStream = await navigator.mediaDevices.getDisplayMedia({ - video: { - width: { ideal: 1920, max: 3840 }, - height: { ideal: 1080, max: 2160 }, - frameRate: { ideal: 30, max: 60 }, - }, - audio: false, - }); + // Restarting capture mid-session: the user re-picks in the system / + // portal dialog, so no preference to honour. + await getElectronAPI().invoke('capture:setPreferredSource', { sourceId: null }); - const videoTrack = mediaStream.getVideoTracks()[0]; - videoTrack.contentHint = 'detail'; - await constrainTrackToQualitySetting(videoTrack); + const mediaStream = await navigator.mediaDevices.getDisplayMedia({ + video: { + width: { ideal: 1920, max: 3840 }, + height: { ideal: 1080, max: 2160 }, + frameRate: { ideal: 30, max: 60 }, + }, + audio: false, + }); - setSelectedSource({ - id: videoTrack.id, - name: videoTrack.label || 'Screen', - type: 'screen', - thumbnail: undefined, - }); + const videoTrack = mediaStream.getVideoTracks()[0]; + videoTrack.contentHint = 'detail'; + await constrainTrackToQualitySetting(videoTrack); - // Add microphone audio - try { - const micStream = await navigator.mediaDevices.getUserMedia({ - audio: VOICE_AUDIO_CONSTRAINTS, - video: false, - }); - micStream.getAudioTracks().forEach((t) => { - mediaStream.addTrack(t); + setSelectedSource({ + id: videoTrack.id, + name: videoTrack.label || 'Screen', + type: 'screen', + thumbnail: undefined, }); - } catch { - // No mic available - } - setStream(mediaStream); - } catch (err) { - console.error('[Home] Failed to start capture in session:', err); - const message = err instanceof Error ? err.message : 'Unknown error'; - if (message.includes('Permission denied') || message.includes('NotAllowedError')) { - setError('Screen capture was canceled or permission denied.'); - } else { - setError(`Failed to capture: ${message}`); + // Add microphone audio + try { + const micStream = await navigator.mediaDevices.getUserMedia({ + audio: VOICE_AUDIO_CONSTRAINTS, + video: false, + }); + micStream.getAudioTracks().forEach((t) => { + mediaStream.addTrack(t); + }); + } catch { + // No mic available + } + + setStream(mediaStream); + } catch (err) { + console.error('[Home] Failed to start capture in session:', err); + const message = err instanceof Error ? err.message : 'Unknown error'; + if (message.includes('Permission denied') || message.includes('NotAllowedError')) { + setError('Screen capture was canceled or permission denied.'); + } else { + setError(`Failed to capture: ${message}`); + } } - } finally { - setIsCapturing(false); - } + }); }; return ( diff --git a/apps/web/src/hooks/useWebRTCHostSFU.ts b/apps/web/src/hooks/useWebRTCHostSFU.ts index a532eb7d..e2bc25ee 100644 --- a/apps/web/src/hooks/useWebRTCHostSFU.ts +++ b/apps/web/src/hooks/useWebRTCHostSFU.ts @@ -20,6 +20,19 @@ import { amplifyRemoteAudio, type AmplifiedAudioTrack } from '@/lib/remoteAudioG const LIVEKIT_URL = process.env.NEXT_PUBLIC_LIVEKIT_URL ?? ''; +/** + * Did this publish fail only because the track is already on the air? + * + * livekit signals it as `TrackInvalidError: a track with the same ID has + * already been published`. There is no error code to match on, so the message + * is the only handle — matched loosely enough to survive rewording, and + * deliberately narrow enough not to swallow a genuine publish failure. + */ +function isAlreadyPublished(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return /already been published|already publish/i.test(message); +} + const encoder = new TextEncoder(); const decoder = new TextDecoder(); @@ -57,7 +70,12 @@ interface UseWebRTCHostSFUReturn { error: string | null; startHosting: () => Promise; stopHosting: () => void; - publishStream: (stream: MediaStream) => Promise; + /** + * Publish the presentation stream. `isStale` is polled once the call reaches + * the front of the publish queue: return true to abandon a publication whose + * capture session has already been replaced or torn down. + */ + publishStream: (stream: MediaStream, isStale?: () => boolean) => Promise; unpublishStream: () => Promise; grantControl: (viewerId: string) => void; revokeControl: (viewerId: string) => void; @@ -86,6 +104,8 @@ export function useWebRTCHostSFU({ const [hasMic, setHasMic] = useState(false); const roomRef = useRef(null); + // Serialises every mutation of the screen-share publication — see publishStream. + const publishQueueRef = useRef>(Promise.resolve()); // Current playback gain, so a viewer who joins later starts at the level the // host already chose rather than snapping back to the default. const speakerGainRef = useRef(DEFAULT_REMOTE_AUDIO_GAIN); @@ -339,12 +359,14 @@ export function useWebRTCHostSFU({ const stopHosting = useCallback(() => { const room = roomRef.current; if (room) { - // Unpublish all tracks but keep room alive for viewers - room.localParticipant.trackPublications.forEach((pub: LocalTrackPublication) => { + // Unpublish all tracks but keep room alive for viewers. Snapshot first: + // unpublishTrack deletes from trackPublications, and walking a Map while + // deleting from it skips entries. + for (const pub of Array.from(room.localParticipant.trackPublications.values())) { if (pub.track) { void room.localParticipant.unpublishTrack(pub.track); } - }); + } void room.disconnect(); roomRef.current = null; } @@ -363,47 +385,128 @@ export function useWebRTCHostSFU({ setHasMic(false); }, []); - // Publish a screen share stream to the LiveKit room - const publishStream = useCallback(async (stream: MediaStream) => { - const room = roomRef.current; - if (room?.state !== LKConnectionState.Connected) { - console.warn('[WebRTCHostSFU] Cannot publish stream: room not connected'); - return; - } - - for (const track of stream.getTracks()) { - if (track.kind === 'video') { - track.contentHint = 'detail'; - await room.localParticipant.publishTrack(track, { - source: Track.Source.ScreenShare, - simulcast: false, - videoEncoding: { - maxBitrate: 8_000_000, - maxFramerate: 60, - }, - }); - } else if (track.kind === 'audio') { - await room.localParticipant.publishTrack(track, { - source: Track.Source.ScreenShareAudio, - }); - } - } + /** + * Run `op` with exclusive access to the screen-share publication. + * + * See the desktop host (useWebRTCHostSFUAPI) for the full story: deciding + * replace-vs-publish is a read followed by an await, so two overlapping + * callers both decide "publish" and livekit rejects the loser with + * `TrackInvalidError: a track with the same ID has already been published`. + * The queue must never settle rejected or every later op would be dropped, + * so the stored tail always swallows; the caller still sees its own error. + */ + const enqueuePublishOp = useCallback((op: () => Promise): Promise => { + const run = publishQueueRef.current.then(op, op); + publishQueueRef.current = run.then( + () => undefined, + () => undefined + ); + return run; }, []); - // Unpublish screen share tracks (room stays connected, mic stays enabled) - const unpublishStream = useCallback(async () => { - const room = roomRef.current; - if (!room) return; + /** + * Publish a screen share stream to the LiveKit room. + * + * Replaces the track inside an existing publication rather than adding a + * second one: viewers fold every subscribed video track into one MediaStream + * and a