From 1e953209a039e0c6f90751ad2de7a98d86966f4a Mon Sep 17 00:00:00 2001 From: Kevin Hanna Date: Tue, 10 Dec 2024 12:54:39 +0000 Subject: [PATCH] media: removeDeviceHandlerFactory VegaRtcManager init option This was added to support using the sdk in a node client, but now we have two custom factories it makes sense to use them in the same way --- .../src/redux/slices/rtcConnection/index.ts | 3 +- packages/media/src/utils/getHandler.ts | 10 +++- .../media/src/webrtc/RtcManagerDispatcher.ts | 1 - .../VegaRtcManager/__tests__/index.spec.ts | 54 ++++++++++--------- .../media/src/webrtc/VegaRtcManager/index.ts | 19 ++++--- .../tests/webrtc/RtcManagerDispatcher.spec.ts | 13 ----- 6 files changed, 47 insertions(+), 53 deletions(-) diff --git a/packages/core/src/redux/slices/rtcConnection/index.ts b/packages/core/src/redux/slices/rtcConnection/index.ts index 4abbfe4ef..dfc7f0f6e 100644 --- a/packages/core/src/redux/slices/rtcConnection/index.ts +++ b/packages/core/src/redux/slices/rtcConnection/index.ts @@ -13,7 +13,7 @@ import { createReactor, startAppListening } from "../../listenerMiddleware"; import { selectRemoteClients, streamStatusUpdated } from "../remoteParticipants"; import { StreamState } from "../../../RoomParticipant"; import { selectAppIsNodeSdk, selectAppIsActive, doAppStop } from "../app"; -import { Safari12 as MediasoupDeviceHandler } from "mediasoup-client/lib/handlers/Safari12.js"; + import { selectIsCameraEnabled, selectIsMicrophoneEnabled, @@ -197,7 +197,6 @@ export const doConnectRtc = createAppThunk(() => (dispatch, getState) => { vp9On: false, h264On: false, simulcastScreenshareOn: false, - deviceHandlerFactory: isNodeSdk ? MediasoupDeviceHandler.createFactory() : undefined, }, }); diff --git a/packages/media/src/utils/getHandler.ts b/packages/media/src/utils/getHandler.ts index 97d90d44b..32082121e 100644 --- a/packages/media/src/utils/getHandler.ts +++ b/packages/media/src/utils/getHandler.ts @@ -1,8 +1,14 @@ import { detectDevice } from "mediasoup-client"; +import { BuiltinHandlerName } from "mediasoup-client/lib/types"; import { UAParser } from "ua-parser-js"; -export const getHandler = (features: Record) => { - let handlerName = +type SupportedDevice = BuiltinHandlerName | "NodeJS" | "Safari17" | undefined; +export const getHandler = (features: Record): SupportedDevice => { + if (features.isNodeSdk) { + return "NodeJS"; + } + + let handlerName: SupportedDevice = detectDevice() || (/applecoremedia|applewebkit|safari/i.test(navigator.userAgent) ? "Safari17" : undefined); if (handlerName === "Safari12" && typeof navigator === "object" && typeof navigator.userAgent === "string") { diff --git a/packages/media/src/webrtc/RtcManagerDispatcher.ts b/packages/media/src/webrtc/RtcManagerDispatcher.ts index f2ea030c4..b7feab146 100644 --- a/packages/media/src/webrtc/RtcManagerDispatcher.ts +++ b/packages/media/src/webrtc/RtcManagerDispatcher.ts @@ -34,7 +34,6 @@ export default class RtcManagerDispatcher { webrtcProvider, features, eventClaim, - deviceHandlerFactory: features?.deviceHandlerFactory, }; const isSfu = !!room.sfuServer; if (this.currentManager) { diff --git a/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts b/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts index 0a9d7eef3..360934eb2 100644 --- a/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts +++ b/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts @@ -1,4 +1,4 @@ -import * as mediasoupClient from "mediasoup-client"; +const mediasoupClient = jest.requireActual("mediasoup-client"); import VegaRtcManager from "../"; @@ -10,10 +10,18 @@ import WS from "jest-websocket-mock"; import Logger from "../../../utils/Logger"; import { setTimeout } from "timers/promises"; -jest.mock("../../../utils/getHandler"); jest.mock("../../../utils/Safari17Handler"); -const { getHandler } = jest.requireMock("../../../utils/getHandler"); const { Safari17 } = jest.requireMock("../../../utils/Safari17Handler"); +jest.mock("mediasoup-client/lib/handlers/Safari12"); +const { Safari12 } = jest.requireMock("mediasoup-client/lib/handlers/Safari12"); +jest.mock("../../../utils/getHandler"); +const { getHandler } = jest.requireMock("../../../utils/getHandler"); + +jest.mock("mediasoup-client", () => ({ + ...mediasoupClient, + Device: jest.fn(), +})); +const mediasoupClientMock = jest.requireMock("mediasoup-client"); const logger = new Logger(); @@ -24,7 +32,6 @@ jest.mock("webrtc-adapter", () => { }); const originalNavigator = global.navigator; -const originalMediasoupDevice = mediasoupClient.Device; describe("VegaRtcManager", () => { let navigator: any; @@ -67,16 +74,12 @@ describe("VegaRtcManager", () => { value: navigator, }); - Object.defineProperty(mediasoupClient, "Device", { - value: jest.fn().mockImplementation(() => { - return { - load: jest.fn(), - rtpCapabilities: {}, - createSendTransport: () => mockSendTransport, - createRecvTransport: () => new MockTransport(), - }; - }), - }); + mediasoupClientMock.Device.mockImplementation(() => ({ + load: jest.fn(), + rtpCapabilities: {}, + createSendTransport: () => mockSendTransport, + createRecvTransport: () => new MockTransport(), + })); rtcManager = new VegaRtcManager({ selfId: helpers.randomString("client-"), @@ -99,17 +102,17 @@ describe("VegaRtcManager", () => { Object.defineProperty(global, "navigator", { value: originalNavigator, }); - Object.defineProperty(mediasoupClient, "Device", { - value: originalMediasoupDevice, - }); }); describe("constructor", () => { const selfId = helpers.randomString("client-"); const room = { name: helpers.randomString("/room-"), iceServers: {} }; - it("handles custom device handler factories", () => { - const deviceHandlerFactory = function () {}; + it("uses the custom Safari17 handler", () => { + const factory = jest.fn(); + getHandler.mockImplementation(() => "Safari17"); + Safari17.createFactory.mockImplementation(() => factory); + //eslint-disable-next-line no-new new VegaRtcManager({ selfId, @@ -117,15 +120,15 @@ describe("VegaRtcManager", () => { emitter, serverSocket, webrtcProvider, - deviceHandlerFactory, }); - expect(mediasoupClient.Device).toHaveBeenCalledWith({ handlerFactory: deviceHandlerFactory }); + + expect(mediasoupClientMock.Device).toHaveBeenCalledWith({ handlerFactory: factory }); }); - it("uses the custom Safari17 handler", () => { - getHandler.mockImplementation(() => "Safari17"); + it("uses the nodejs handler when isNodeJs", () => { + getHandler.mockImplementation(() => "NodeJS"); const factory = jest.fn(); - Safari17.createFactory.mockImplementation(() => factory); + Safari12.createFactory.mockImplementation(() => factory); //eslint-disable-next-line no-new new VegaRtcManager({ @@ -134,9 +137,10 @@ describe("VegaRtcManager", () => { emitter, serverSocket, webrtcProvider, + features: { isNodeSdk: true }, }); - expect(mediasoupClient.Device).toHaveBeenCalledWith({ handlerFactory: factory }); + expect(mediasoupClientMock.Device).toHaveBeenCalledWith({ handlerFactory: factory }); }); }); diff --git a/packages/media/src/webrtc/VegaRtcManager/index.ts b/packages/media/src/webrtc/VegaRtcManager/index.ts index 67d52228e..6c47b5197 100644 --- a/packages/media/src/webrtc/VegaRtcManager/index.ts +++ b/packages/media/src/webrtc/VegaRtcManager/index.ts @@ -1,6 +1,8 @@ import { Device } from "mediasoup-client"; import adapterRaw from "webrtc-adapter"; import { v4 as uuidv4 } from "uuid"; +// of the provided ones, this seems to work best in NodeJS +import { Safari12 as NodeDeviceHandler } from "mediasoup-client/lib/handlers/Safari12.js"; import rtcManagerEvents from "../rtcManagerEvents"; import rtcStats from "../rtcStatsService"; @@ -99,7 +101,6 @@ export default class VegaRtcManager implements RtcManager { webrtcProvider, features, eventClaim, - deviceHandlerFactory, }: { selfId: any; room: any; @@ -108,7 +109,6 @@ export default class VegaRtcManager implements RtcManager { webrtcProvider: any; features?: any; eventClaim?: string; - deviceHandlerFactory?: any; }) { const { session, iceServers, sfuServer, mediaserverConfigTtlSeconds } = room; @@ -128,15 +128,14 @@ export default class VegaRtcManager implements RtcManager { const handlerName = getHandler(this._features); - if (handlerName === "Safari17" && !deviceHandlerFactory) { - // Patched Safari12 handler to fix simulcast bandwith limitsp - deviceHandlerFactory = Safari17.createFactory(); - } - - if (deviceHandlerFactory) { - this._mediasoupDevice = new Device({ handlerFactory: deviceHandlerFactory }); + if (handlerName === "Safari17") { + // Patched Safari12 handler to fix simulcast bandwith limits + this._mediasoupDevice = new Device({ handlerFactory: Safari17.createFactory() }); + } else if (handlerName === "NodeJS") { + this._mediasoupDevice = new Device({ handlerFactory: NodeDeviceHandler.createFactory() }); } else { - this._mediasoupDevice = new Device({ handlerName: handlerName as BuiltinHandlerName }); + console.warn("handlerName", handlerName, features); + this._mediasoupDevice = new Device({ handlerName }); } this._routerRtpCapabilities = null; diff --git a/packages/media/tests/webrtc/RtcManagerDispatcher.spec.ts b/packages/media/tests/webrtc/RtcManagerDispatcher.spec.ts index 6fb4ec801..f67d9398c 100644 --- a/packages/media/tests/webrtc/RtcManagerDispatcher.spec.ts +++ b/packages/media/tests/webrtc/RtcManagerDispatcher.spec.ts @@ -68,19 +68,6 @@ describe("RtcManagerDispatcher", () => { const rtcManager = mockEmitRoomJoined({ sfuServer: { url: helpers.randomString("sfu-") + ":443" } }); expect(rtcManager).toBeInstanceOf(VegaRtcManager); }); - it("allows custom device handler factories when sfuServer", () => { - features.deviceHandlerFactory = function () {}; - jest.mock("../../src/webrtc/VegaRtcManager", () => { - return { - default: jest.fn(), - }; - }); - const rtcManager = mockEmitRoomJoined({ sfuServer: { url: helpers.randomString("sfu-") + ":443" } }); - expect(rtcManager).toBeInstanceOf(VegaRtcManager); - expect(mediasoupClient.Device).toHaveBeenCalledWith({ - handlerFactory: features.deviceHandlerFactory, - }); - }); it("emits nothing when error is set", () => { const rtcManager = mockEmitRoomJoined({ error: "yo" });