diff --git a/packages/react-client/src/FishjamProvider.tsx b/packages/react-client/src/FishjamProvider.tsx index d6ad0b8f..69537ad3 100644 --- a/packages/react-client/src/FishjamProvider.tsx +++ b/packages/react-client/src/FishjamProvider.tsx @@ -1,4 +1,4 @@ -import { type FishjamClient, getLogger, type ReconnectConfig } from "@fishjam-cloud/ts-client"; +import type { FishjamClient, ReconnectConfig } from "@fishjam-cloud/ts-client"; import { type DeviceError as CoreDeviceError, type DeviceItem, @@ -6,12 +6,14 @@ import { type IDevicePersistence, type InitializeDevicesResult as CoreInitializeDevicesResult, type LocalDeviceState, + type PlatformMediaStream, type PlatformMediaStreamTrack, type TrackDeviceController, type TrackMiddleware as CoreTrackMiddleware, + type TracksMiddleware as CoreTracksMiddleware, WebDeviceManager, } from "@fishjam-cloud/tsunami"; -import { type PropsWithChildren, type RefObject, useCallback, useMemo, useRef, useSyncExternalStore } from "react"; +import { type PropsWithChildren, useCallback, useMemo, useRef, useSyncExternalStore } from "react"; import { CameraContext } from "./contexts/camera"; import { CustomSourceContext } from "./contexts/customSource"; @@ -22,11 +24,15 @@ import { InitDevicesContext } from "./contexts/initDevices"; import { MicrophoneContext } from "./contexts/microphone"; import { PeerStatusContext } from "./contexts/peerStatus"; import { ScreenshareContext } from "./contexts/screenshare"; -import { useCustomSourceManager } from "./hooks/internal/useCustomSourceManager"; import { useFishjamClientState } from "./hooks/internal/useFishjamClientState"; import { usePeerStatus } from "./hooks/internal/usePeerStatus"; -import { useScreenShareManager } from "./hooks/internal/useScreenshareManager"; -import type { DeviceManager, TrackManager } from "./types/internal"; +import type { + CustomSourceManager, + CustomSourceState, + DeviceManager, + TrackManager, + UseScreenshareResult, +} from "./types/internal"; import type { BandwidthLimits, DeviceError, @@ -34,6 +40,7 @@ import type { PersistLastDeviceHandlers, StreamConfig, TrackMiddleware, + TracksMiddleware, } from "./types/public"; import { getLastDevice, saveLastDevice } from "./utils/localStorage"; @@ -89,6 +96,8 @@ const asLegacyDeviceError = (error: CoreDeviceError | null): DeviceError | null const asDomTrack = (track: PlatformMediaStreamTrack | null): MediaStreamTrack | null => track as MediaStreamTrack | null; +const asDomStream = (stream: PlatformMediaStream | null): MediaStream | null => stream as MediaStream | null; + const asStartDeviceResult = async ( result: Promise<[PlatformMediaStreamTrack, null] | [null, CoreDeviceError]>, ): Promise<[MediaStreamTrack, null] | [null, DeviceError]> => { @@ -244,24 +253,32 @@ export function FishjamProvider(props: FishjamProviderProps) { [client], ); - const logger = useMemo(() => getLogger(props.debug ?? false), [props.debug]); - - const screenShareManager = useScreenShareManager({ - fishjamClient: client, - peerStatus, - logger, - }); + const screenShareManager: UseScreenshareResult = useMemo( + () => ({ + startStreaming: (constraints) => client.startScreenShare(constraints), + stopStreaming: () => client.stopScreenShare(), + stream: asDomStream(clientState.screenShare.stream), + videoTrack: asDomTrack(clientState.screenShare.videoTrack), + audioTrack: asDomTrack(clientState.screenShare.audioTrack), + currentTracksMiddleware: clientState.screenShare.middleware as TracksMiddleware | null, + setTracksMiddleware: (middleware) => + client.setScreenShareTracksMiddleware(middleware as CoreTracksMiddleware | null), + }), + [client, clientState.screenShare], + ); - const customSourceManager = useCustomSourceManager({ - fishjamClient: client, - peerStatus, - logger, - }); + const customSourceManager: CustomSourceManager = useMemo( + () => ({ + setStream: (sourceId, stream) => client.setCustomSource(sourceId, stream), + getSource: (sourceId) => clientState.customSources[sourceId] as CustomSourceState | undefined, + }), + [client, clientState.customSources], + ); const fishjamClientState = useFishjamClientState(client); return ( - }> + }> diff --git a/packages/react-client/src/contexts/customSource.ts b/packages/react-client/src/contexts/customSource.ts index 120f7f68..11e5e324 100644 --- a/packages/react-client/src/contexts/customSource.ts +++ b/packages/react-client/src/contexts/customSource.ts @@ -1,5 +1,5 @@ import { createContext } from "react"; -import type { CustomSourceManager } from "../hooks/internal/useCustomSourceManager"; +import type { CustomSourceManager } from "../types/internal"; export const CustomSourceContext = createContext(null); diff --git a/packages/react-client/src/contexts/screenshare.ts b/packages/react-client/src/contexts/screenshare.ts index 1016800c..8817d1b6 100644 --- a/packages/react-client/src/contexts/screenshare.ts +++ b/packages/react-client/src/contexts/screenshare.ts @@ -1,5 +1,5 @@ import { createContext } from "react"; -import type { UseScreenshareResult } from "../hooks/internal/useScreenshareManager"; +import type { UseScreenshareResult } from "../types/internal"; export const ScreenshareContext = createContext(null); diff --git a/packages/react-client/src/hooks/internal/useCustomSourceManager.ts b/packages/react-client/src/hooks/internal/useCustomSourceManager.ts deleted file mode 100644 index b4974966..00000000 --- a/packages/react-client/src/hooks/internal/useCustomSourceManager.ts +++ /dev/null @@ -1,185 +0,0 @@ -import { type Logger, type TrackMetadata, TrackTypeError } from "@fishjam-cloud/ts-client"; -import type { FishjamClient } from "@fishjam-cloud/tsunami"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; - -import type { CustomSourceState, CustomSourceTracks } from "../../types/internal"; -import type { PeerStatus } from "../../types/public"; - -type CustomSourceManagerProps = { - fishjamClient: FishjamClient; - peerStatus: PeerStatus; - logger: Logger; -}; - -export type CustomSourceManager = { - setStream: (sourceId: string, stream: MediaStream | null) => Promise; - getSource: (sourceId: string) => CustomSourceState | undefined; -}; - -export function useCustomSourceManager({ - fishjamClient, - peerStatus, - logger, -}: CustomSourceManagerProps): CustomSourceManager { - const [sources, setSources] = useState>({}); - // setStream reads the current sources synchronously (to diff against the old stream and to - // remove stale tracks), but must stay referentially stable so consumers can safely depend on - // it in effects. Reading through a ref keeps it out of the useCallback deps. - const sourcesRef = useRef(sources); - sourcesRef.current = sources; - - // Replacing a source's stream issues two setStream calls in the same tick (unpublish the old, - // publish the new). Run concurrently they would read the same snapshot and race to remove the - // same track IDs — and when the publish call loses that race, the new stream is never - // published. This chain serializes the calls instead. - const setStreamQueue = useRef(Promise.resolve()); - - const pendingSources = useMemo( - () => Object.entries(sources).filter(([_, source]) => source.trackIds === undefined), - [sources], - ); - - const getDisplayName = useCallback(() => { - const name = fishjamClient.getLocalPeer()?.metadata?.peer?.displayName; - if (typeof name == "string") return name; - }, [fishjamClient]); - - const addTrackToFishjamClient = useCallback( - async (track: MediaStreamTrack, trackMetadata: TrackMetadata) => { - try { - return fishjamClient.addTrack(track, trackMetadata); - } catch (err) { - if (err instanceof TrackTypeError) { - logger.warn(err.message); - return undefined; - } - throw err; - } - }, - [fishjamClient, logger], - ); - - const startStreaming = useCallback( - async (source: CustomSourceState): Promise => { - const stream = source?.stream; - - const video = stream.getVideoTracks().at(0); - const audio = stream.getAudioTracks().at(0); - - const promises = []; - const displayName = getDisplayName(); - if (video) { - const videoMetadata = { type: "customVideo", displayName, paused: false } as const; - promises.push(addTrackToFishjamClient(video, videoMetadata)); - } - if (audio) { - const audioMetadata = { type: "customAudio", displayName, paused: false } as const; - promises.push(addTrackToFishjamClient(audio, audioMetadata)); - } - - if (promises.length === 0) { - logger.warn("Attempted to add empty MediaStream as custom source."); - return source; - } - const [videoId, audioId] = await Promise.all(promises); - return { ...source, trackIds: { videoId, audioId } }; - }, - [addTrackToFishjamClient, getDisplayName, logger], - ); - - const removeTracks = useCallback( - async ({ videoId, audioId }: CustomSourceTracks) => { - const promises = []; - if (videoId) promises.push(fishjamClient.removeTrack(videoId)); - if (audioId) promises.push(fishjamClient.removeTrack(audioId)); - await Promise.all(promises); - }, - [fishjamClient], - ); - - const getSource = useCallback((sourceId: string) => sources[sourceId], [sources]); - - // Updates both the state (what consumers render) and the ref (what queued setStream calls - // read synchronously, before React re-renders). - const updateSources = useCallback( - (update: (old: Record) => Record) => { - sourcesRef.current = update(sourcesRef.current); - setSources(update); - }, - [], - ); - - const applySetStream = useCallback( - async (sourceId: string, stream: MediaStream | null) => { - const oldSource = sourcesRef.current[sourceId]; - if (stream === oldSource?.stream) return; - - if (oldSource?.trackIds) await removeTracks(oldSource.trackIds); - - if (stream !== null) { - updateSources((old) => ({ ...old, [sourceId]: { stream } })); - } else if (oldSource) { - updateSources((old) => Object.fromEntries(Object.entries(old).filter(([id]) => id !== sourceId))); - } - }, - [removeTracks, updateSources], - ); - - const setStream = useCallback( - (sourceId: string, stream: MediaStream | null) => { - const run = setStreamQueue.current.then(() => applySetStream(sourceId, stream)); - // Chain a never-rejecting link so one failed call cannot poison the queue; the caller - // still observes failures through the returned promise. - setStreamQueue.current = run.catch(() => undefined); - return run; - }, - [applySetStream], - ); - - useEffect(() => { - const onConnected = async () => { - if (pendingSources.length === 0) return; - - const results = await Promise.all( - pendingSources.map(async ([id, source]) => [id, await startStreaming(source)] as const), - ); - - // While the tracks were being added, setStream may have unpublished the source or replaced - // its stream. Record track IDs only where the entry is still the one we started streaming - // (same stream, still no track IDs) — anything else must not be patched (it would resurrect - // an unpublished source or attach the IDs to a different stream), and the tracks we just - // added for it are orphans to unpublish again. - const isStillCurrent = ([id, started]: (typeof results)[number]) => { - const current = sourcesRef.current[id]; - return current !== undefined && current.stream === started.stream && current.trackIds === undefined; - }; - const patch = results.filter(isStillCurrent); - const orphans = results.filter((result) => !isStillCurrent(result)); - - if (patch.length > 0) { - updateSources((old) => ({ ...old, ...Object.fromEntries(patch) })); - } - for (const [, started] of orphans) { - if (started.trackIds) await removeTracks(started.trackIds); - } - }; - - const onDisconnected = () => { - updateSources((old) => - Object.fromEntries(Object.entries(old).map(([id, source]) => [id, { ...source, trackIds: undefined }])), - ); - }; - - // addTrack can reject for reasons other than TrackTypeError (e.g. a disconnect mid-add); - // surface it instead of leaving an unhandled rejection. - if (peerStatus === "connected") - onConnected().catch((error) => logger.error("Failed to publish custom sources", error)); - - fishjamClient.on("disconnected", onDisconnected); - return () => { - fishjamClient.off("disconnected", onDisconnected); - }; - }, [pendingSources, fishjamClient, peerStatus, startStreaming, removeTracks, updateSources, logger]); - - return { setStream, getSource }; -} diff --git a/packages/react-client/src/hooks/internal/useScreenshareManager.ts b/packages/react-client/src/hooks/internal/useScreenshareManager.ts deleted file mode 100644 index 023a6cd4..00000000 --- a/packages/react-client/src/hooks/internal/useScreenshareManager.ts +++ /dev/null @@ -1,240 +0,0 @@ -import { type Logger, type TrackMetadata, TrackTypeError } from "@fishjam-cloud/ts-client"; -import type { FishjamClient } from "@fishjam-cloud/tsunami"; -import { useCallback, useEffect, useRef, useState } from "react"; - -import type { ScreenShareState } from "../../types/internal"; -import type { PeerStatus, TracksMiddleware } from "../../types/public"; -import { useCurrentCallback } from "./useCurrentCallback"; - -export type UseScreenshareResult = { - /** - * Invokes the screen sharing prompt in the user's browser and starts streaming upon approval. - */ - startStreaming: (props?: { - audioConstraints?: boolean | MediaTrackConstraints; - videoConstraints?: boolean | MediaTrackConstraints; - }) => Promise; - /** - * Stops the stream and cancels browser screen sharing. - */ - stopStreaming: () => Promise; - /** - * The MediaStream object containing both tracks. - */ - stream: MediaStream | null; - /** - * The separate video MediaStreamTrack. - */ - videoTrack: MediaStreamTrack | null; - /** - * The separate audio MediaStreamTrack. - */ - audioTrack: MediaStreamTrack | null; - /** - * The middleware currently assigned to process the tracks. - * A screenshare may include both audio and video tracks, and this middleware is capable of processing - * each track type. - */ - currentTracksMiddleware: TracksMiddleware | null; - /** - * Sets the middleware responsible for processing the tracks. - * @param middleware The middleware to set, which can be a TracksMiddleware instance or null to remove the middleware. - * @returns A Promise that resolves once the middleware is successfully set. - */ - setTracksMiddleware: (middleware: TracksMiddleware | null) => Promise; -}; - -interface ScreenShareManagerProps { - fishjamClient: FishjamClient; - peerStatus: PeerStatus; - logger: Logger; -} - -export const useScreenShareManager = ({ - fishjamClient, - peerStatus, - logger, -}: ScreenShareManagerProps): UseScreenshareResult => { - const [state, setState] = useState({ stream: null, trackIds: null }); - - const cleanMiddlewareFnRef = useRef<(() => void) | null>(null); - - const stream = state.stream ?? null; - const tracksMiddleware = state.tracksMiddleware ?? null; - const [mediaVideoTrack, mediaAudioTrack] = stream ? getTracksFromStream(stream) : [null, null]; - - const getDisplayName = useCallback(() => { - const name = fishjamClient.getLocalPeer()?.metadata?.peer?.displayName; - if (typeof name === "string") return name; - }, [fishjamClient]); - - const addTrackToFishjamClient = useCallback( - async (track: MediaStreamTrack, trackMetadata: TrackMetadata) => { - try { - // Awaited so a rejected addTrack (the core surfaces publish failures as - // rejections) lands in this catch and keeps the local track alive. - return await fishjamClient.addTrack(track, trackMetadata); - } catch (err) { - if (err instanceof TrackTypeError) { - logger.warn(err.message); - return undefined; - } - throw err; - } - }, - [fishjamClient, logger], - ); - - const startStreaming: UseScreenshareResult["startStreaming"] = useCallback( - async (props) => { - const displayStream = await navigator.mediaDevices.getDisplayMedia({ - video: props?.videoConstraints ?? true, - audio: props?.audioConstraints ?? true, - }); - - const displayName = getDisplayName(); - - let [video, audio] = getTracksFromStream(displayStream); - - if (tracksMiddleware) { - const { videoTrack, audioTrack, onClear } = await tracksMiddleware(video, audio); - video = videoTrack; - audio = audioTrack; - cleanMiddlewareFnRef.current = onClear; - } - - // TODO: FCE-2659 Refactor this hook so this check is not required. - // This check is needed to support screensharing in livestreams which don't use the FishjamClient. - // trackIds are simply ignored because they are not used in this use case. - if (fishjamClient.status === "initialized") { - const addTrackPromises = [ - addTrackToFishjamClient(video, { displayName, type: "screenShareVideo", paused: false }), - ]; - if (audio) - addTrackPromises.push( - addTrackToFishjamClient(audio, { displayName, type: "screenShareAudio", paused: false }), - ); - - const [videoId, audioId] = await Promise.all(addTrackPromises); - setState({ stream: displayStream, trackIds: { videoId, audioId } }); - } else { - setState({ stream: displayStream, trackIds: {} }); - } - }, - [tracksMiddleware, getDisplayName, addTrackToFishjamClient, fishjamClient], - ); - - const replaceTracks = useCallback( - async (newVideoTrack: MediaStreamTrack, newAudioTrack: MediaStreamTrack | null) => { - if (!state?.stream) return; - - const addTrackPromises: Promise[] = []; - - if (newVideoTrack && state.trackIds.videoId) - addTrackPromises.push(fishjamClient.replaceTrack(state.trackIds.videoId, newVideoTrack)); - if (newAudioTrack && state.trackIds.audioId) - addTrackPromises.push(fishjamClient.replaceTrack(state.trackIds.audioId, newAudioTrack)); - - await Promise.all(addTrackPromises); - }, - [state.stream, state.trackIds?.videoId, state.trackIds?.audioId, fishjamClient], - ); - - const cleanMiddleware = useCallback(() => { - cleanMiddlewareFnRef.current?.(); - cleanMiddlewareFnRef.current = null; - }, []); - - const setTracksMiddleware = useCallback( - async (middleware: TracksMiddleware | null): Promise => { - if (!state?.stream) return; - - const [video, audio] = getTracksFromStream(state.stream); - - cleanMiddleware(); - - const { videoTrack, audioTrack, onClear } = (await middleware?.(video, audio)) ?? { - videoTrack: video, - audioTrack: audio, - onClear: null, - }; - cleanMiddlewareFnRef.current = onClear; - await replaceTracks(videoTrack, audioTrack); - }, - [state.stream, cleanMiddleware, replaceTracks], - ); - - // Stable identity with a live closure: peerStatus must be observed at call - // time so a captured reference doesn't skip the SFU removeTrack calls. - const stopStreaming: UseScreenshareResult["stopStreaming"] = useCurrentCallback(async () => { - if (!state.stream) { - logger.warn("No stream to stop"); - return; - } - const [video, audio] = getTracksFromStream(state.stream); - - video.stop(); - if (audio) audio.stop(); - - if (peerStatus === "connected") { - const removeTrackPromises: Promise[] = []; - if (state.trackIds.videoId) removeTrackPromises.push(fishjamClient.removeTrack(state.trackIds.videoId)); - if (state.trackIds.audioId) removeTrackPromises.push(fishjamClient.removeTrack(state.trackIds.audioId)); - - await Promise.all(removeTrackPromises); - } - - cleanMiddleware(); - setState((prev) => ({ stream: null, trackIds: null, tracksMiddleware: prev.tracksMiddleware })); - }); - - useEffect(() => { - if (!state.stream) return; - const [video, audio] = getTracksFromStream(state.stream); - - const trackEndedHandler = () => { - void stopStreaming().catch((err) => { - logger.error(err); - }); - }; - - video.addEventListener("ended", trackEndedHandler); - audio?.addEventListener("ended", trackEndedHandler); - - return () => { - video.removeEventListener("ended", trackEndedHandler); - audio?.removeEventListener("ended", trackEndedHandler); - }; - }, [state, stopStreaming, logger]); - - useEffect(() => { - const onDisconnected = () => { - if (!stream) return; - void stopStreaming().catch((err) => { - logger.error(err); - }); - }; - fishjamClient.on("disconnected", onDisconnected); - - return () => { - fishjamClient.removeListener("disconnected", onDisconnected); - }; - }, [stopStreaming, fishjamClient, stream, logger]); - - return { - startStreaming, - stopStreaming, - stream, - videoTrack: mediaVideoTrack, - audioTrack: mediaAudioTrack, - setTracksMiddleware, - currentTracksMiddleware: tracksMiddleware, - }; -}; - -const getTracksFromStream = (stream: MediaStream): [MediaStreamTrack, MediaStreamTrack | null] => { - const video = stream.getVideoTracks()[0]; - const audio = stream.getAudioTracks()[0] ?? null; - - return [video, audio]; -}; diff --git a/packages/react-client/src/tests/screenShare.spec.ts b/packages/react-client/src/tests/screenShare.spec.ts index 261e5960..26cf90a2 100644 --- a/packages/react-client/src/tests/screenShare.spec.ts +++ b/packages/react-client/src/tests/screenShare.spec.ts @@ -111,11 +111,9 @@ describe("useScreenShare", () => { // The middleware output is pushed to the SFU via replaceTrack. expect(client.replaceTrack).toHaveBeenCalled(); - // KNOWN QUIRK (FCE-3574): setTracksMiddleware never writes the middleware - // back into state, so `currentTracksMiddleware` stays null and the middleware - // is NOT re-applied on a subsequent startStreaming. Captured here so the - // rewrite has to make a deliberate decision to fix it (the assertion will - // flip when it does). - expect(result.current.currentTracksMiddleware).toBeNull(); + // FCE-3574 (fixed by the tsunami rewrite): the middleware is persisted, so + // `currentTracksMiddleware` reflects it and it is re-applied on a + // subsequent startStreaming. + expect(result.current.currentTracksMiddleware).not.toBeNull(); }); }); diff --git a/packages/react-client/src/types/internal.ts b/packages/react-client/src/types/internal.ts index b39c54ec..01788be9 100644 --- a/packages/react-client/src/types/internal.ts +++ b/packages/react-client/src/types/internal.ts @@ -67,3 +67,44 @@ export type DeviceManager = { deviceError: DeviceError | null; selectedDevice: MediaDeviceInfo | null; }; + +export type UseScreenshareResult = { + /** + * Invokes the screen sharing prompt in the user's browser and starts streaming upon approval. + */ + startStreaming: (props?: { + audioConstraints?: boolean | MediaTrackConstraints; + videoConstraints?: boolean | MediaTrackConstraints; + }) => Promise; + /** + * Stops the stream and cancels browser screen sharing. + */ + stopStreaming: () => Promise; + /** + * The MediaStream object containing both tracks. + */ + stream: MediaStream | null; + /** + * The separate video MediaStreamTrack. + */ + videoTrack: MediaStreamTrack | null; + /** + * The separate audio MediaStreamTrack. + */ + audioTrack: MediaStreamTrack | null; + /** + * The middleware currently assigned to process the tracks. + * By default, the middleware function returns the original track. + */ + currentTracksMiddleware: TracksMiddleware | null; + /** + * Sets a new middleware function to process the tracks. + * @param middleware The middleware function to set, which can be a TracksMiddleware function or null to remove the middleware. + */ + setTracksMiddleware: (middleware: TracksMiddleware | null) => Promise; +}; + +export type CustomSourceManager = { + setStream: (sourceId: string, stream: MediaStream | null) => Promise; + getSource: (sourceId: string) => CustomSourceState | undefined; +};