diff --git a/.changeset/sad-mirrors-yawn.md b/.changeset/sad-mirrors-yawn.md new file mode 100644 index 000000000..5119cf825 --- /dev/null +++ b/.changeset/sad-mirrors-yawn.md @@ -0,0 +1,5 @@ +--- +"@whereby.com/media": patch +--- + +Fix stopOrResumeVideo race diff --git a/packages/media/src/webrtc/P2pRtcManager.ts b/packages/media/src/webrtc/P2pRtcManager.ts index 79236b741..df790423a 100644 --- a/packages/media/src/webrtc/P2pRtcManager.ts +++ b/packages/media/src/webrtc/P2pRtcManager.ts @@ -127,6 +127,7 @@ export default class P2pRtcManager implements RtcManager { _closed: boolean; analytics: P2PAnalytics; _rtcStatsDisconnectTimeout?: ReturnType; + _webcamPaused?: boolean; constructor({ selfId, room, emitter, serverSocket, webrtcProvider, features }: RtcManagerOptions) { const { name, session, iceServers, turnServers, mediaserverConfigTtlSeconds } = room; @@ -216,9 +217,12 @@ export default class P2pRtcManager implements RtcManager { addCameraStream( stream: MediaStream, - { beforeEffectTracks = [] }: AddCameraStreamOptions = { beforeEffectTracks: [] }, + { videoPaused, beforeEffectTracks = [] }: AddCameraStreamOptions = { beforeEffectTracks: [] }, ) { logger.info("addCameraStream: [stream.id: %s]", stream.id); + + this._webcamPaused = videoPaused; + if (stream === this._localCameraStream) { // this can happen after reconnect. We do not want to add the stream to the // peerconnection again. @@ -1309,6 +1313,9 @@ export default class P2pRtcManager implements RtcManager { stopOrResumeVideo(localStream: MediaStream, enable: boolean) { logger.info("stopOrResumeVideo() [enable: %s]", enable); + + this._webcamPaused = !enable; + // actually turn off the camera. Chrome-only (Firefox has different plans) if (!["chrome", "safari"].includes(browserName)) { return; @@ -1350,6 +1357,13 @@ export default class P2pRtcManager implements RtcManager { .getUserMedia({ video: constraints }) .then((stream) => { const track = stream.getVideoTracks()[0]; + if (this._webcamPaused) { + // if the user paused video inbetween the gUM call and the result, + // we have to stop the track to avoid leaving the camera light on + // and prevent sending video when we shouldn't be + track.stop(); + return; + } localStream.addTrack(track); this._monitorVideoTrack(track); this._emit(CONNECTION_STATUS.EVENTS.LOCAL_STREAM_TRACK_ADDED as string, { diff --git a/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts b/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts index 5e9271173..fd8da549d 100644 --- a/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts +++ b/packages/media/src/webrtc/VegaRtcManager/__tests__/index.spec.ts @@ -328,16 +328,175 @@ describe("VegaRtcManager", () => { }); }); - it("should sendWebcam(track)", async () => { - const expectedTrack = gumStream.getVideoTracks()[0]; - jest.spyOn(rtcManager, "_sendWebcam"); + describe("when there is a webcam producer", () => { + let stream: MediaStream; + let existingTrack: MediaStreamTrack; + + beforeEach(async () => { + existingTrack = helpers.createMockedMediaStreamTrack({ + id: "id", + kind: "video", + }); + stream = helpers.createMockedMediaStream([existingTrack]); + }); - rtcManager.stopOrResumeVideo(localStream, true); + it("should replace the webcam producer track", async () => { + const mockVideoProducer = new MockProducer({ kind: "video" }); + jest.spyOn(mockVideoProducer, "replaceTrack"); + jest.spyOn(mockSendTransport, "produce").mockImplementation( + ({ track }: { track: MediaStreamTrack }) => { + if (track.kind === "video") return mockVideoProducer; + else return new MockProducer({ kind: "audio" }); + }, + ); - jest.advanceTimersToNextTimerAsync(); - await new Promise(process.nextTick); + rtcManager.setupSocketListeners(); + rtcManager.addCameraStream(stream); + + const expectedTrack = gumStream.getVideoTracks()[0]; + + rtcManager.stopOrResumeVideo(localStream, true); + await jest.runAllTimersAsync(); + + expect(mockVideoProducer.replaceTrack).toHaveBeenCalledWith({ track: expectedTrack }); + sfuWebsocketServer.close(); + }); + }); + + describe("when there is no webcam producer", () => { + let stream: MediaStream; + beforeEach(async () => { + stream = helpers.createMockedMediaStream([]); + }); + + it("should create the webcam producer", async () => { + const mockVideoProducer = new MockProducer({ kind: "video" }); + jest.spyOn(mockVideoProducer, "replaceTrack"); + jest.spyOn(mockSendTransport, "produce").mockImplementation( + ({ track }: { track: MediaStreamTrack }) => { + if (track.kind === "video") return mockVideoProducer; + else return new MockProducer({ kind: "audio" }); + }, + ); + + rtcManager.setupSocketListeners(); + rtcManager.addCameraStream(stream); + + const expectedTrack = gumStream.getVideoTracks()[0]; + + rtcManager.stopOrResumeVideo(localStream, true); + await jest.runAllTimersAsync(); + + expect(mockSendTransport.produce).toHaveBeenCalledTimes(1); + expect(mockSendTransport.produce).toHaveBeenCalledWith( + expect.objectContaining({ track: expectedTrack }), + ); + sfuWebsocketServer.close(); + }); + }); + + describe("when webcam is paused shortly after enabling", () => { + describe("when there is a webcam producer", () => { + let stream: MediaStream; + let existingTrack: MediaStreamTrack; + + beforeEach(async () => { + existingTrack = helpers.createMockedMediaStreamTrack({ + id: "id", + kind: "video", + }); + stream = helpers.createMockedMediaStream([existingTrack]); + }); + + it("should not replace the webcam producer track", async () => { + const mockVideoProducer = new MockProducer({ kind: "video" }); + jest.spyOn(mockVideoProducer, "replaceTrack"); + jest.spyOn(mockSendTransport, "produce").mockImplementation( + ({ track }: { track: MediaStreamTrack }) => { + if (track.kind === "video") return mockVideoProducer; + else return new MockProducer({ kind: "audio" }); + }, + ); + const expectedTrack = gumStream.getVideoTracks()[0]; + jest.spyOn(expectedTrack, "stop"); + + rtcManager.setupSocketListeners(); + rtcManager.addCameraStream(stream); + rtcManager.stopOrResumeVideo(localStream, true); + rtcManager.stopOrResumeVideo(localStream, false); + await Promise.resolve(); + + expect(mockVideoProducer.replaceTrack).not.toHaveBeenCalled(); + sfuWebsocketServer.close(); + }); + }); + + describe("when there is no webcam producer", () => { + let stream: MediaStream; + beforeEach(async () => { + stream = helpers.createMockedMediaStream([]); + }); + + it("should not create the webcam producer", async () => { + const mockVideoProducer = new MockProducer({ kind: "video" }); + jest.spyOn(mockVideoProducer, "replaceTrack"); + jest.spyOn(mockSendTransport, "produce").mockImplementation( + ({ track }: { track: MediaStreamTrack }) => { + if (track.kind === "video") return mockVideoProducer; + else return new MockProducer({ kind: "audio" }); + }, + ); + + const expectedTrack = gumStream.getVideoTracks()[0]; + + rtcManager.setupSocketListeners(); + rtcManager.addCameraStream(stream); + rtcManager.stopOrResumeVideo(localStream, true); + rtcManager.stopOrResumeVideo(localStream, false); + await Promise.resolve(); + + expect(mockSendTransport.produce).not.toHaveBeenCalled(); + sfuWebsocketServer.close(); + }); + }); + + it("doesn't emit any events", async () => { + const expectedTrack = gumStream.getVideoTracks()[0]; + jest.spyOn(rtcManager, "_sendWebcam"); + + rtcManager.stopOrResumeVideo(localStream, true); + rtcManager.stopOrResumeVideo(localStream, false); + + jest.advanceTimersToNextTimerAsync(); + await new Promise(process.nextTick); + + expect(emitter.emit).not.toHaveBeenCalled(); + }); + + it("should not add video track to local stream", async () => { + const expectedTrack = gumStream.getVideoTracks()[0]; + + rtcManager.stopOrResumeVideo(localStream, true); + rtcManager.stopOrResumeVideo(localStream, false); + + jest.advanceTimersToNextTimerAsync(); + await new Promise(process.nextTick); + + expect(localStream.addTrack).not.toHaveBeenCalledWith(); + }); + + it("stops the new video track", async () => { + const expectedTrack = gumStream.getVideoTracks()[0]; + jest.spyOn(expectedTrack, "stop"); + + rtcManager.stopOrResumeVideo(localStream, true); + rtcManager.stopOrResumeVideo(localStream, false); - expect(rtcManager._sendWebcam).toHaveBeenCalledWith(expectedTrack); + jest.advanceTimersToNextTimerAsync(); + await new Promise(process.nextTick); + + expect(expectedTrack.stop).toHaveBeenCalled(); + }); }); }); }); diff --git a/packages/media/src/webrtc/VegaRtcManager/index.ts b/packages/media/src/webrtc/VegaRtcManager/index.ts index 3a8b6f35b..aa9079a40 100644 --- a/packages/media/src/webrtc/VegaRtcManager/index.ts +++ b/packages/media/src/webrtc/VegaRtcManager/index.ts @@ -1647,6 +1647,14 @@ export default class VegaRtcManager implements RtcManager { .getUserMedia({ video: constraints }) .then((stream) => { const track = stream.getVideoTracks()[0]; + if (this._webcamPaused) { + // if the user paused video inbetween the gUM call and the result, + // we have to stop the track to avoid leaving the camera light on + // and prevent sending video when we shouldn't be + track.stop(); + return; + } + localStream.addTrack(track); this._monitorVideoTrack(track); diff --git a/packages/media/tests/webrtc/P2pRtcManager.spec.ts b/packages/media/tests/webrtc/P2pRtcManager.spec.ts index 98996347c..894a1bd85 100644 --- a/packages/media/tests/webrtc/P2pRtcManager.spec.ts +++ b/packages/media/tests/webrtc/P2pRtcManager.spec.ts @@ -1310,6 +1310,52 @@ describe("P2pRtcManager", () => { expect(rtcManager._replaceTrackToPeerConnections).toHaveBeenCalledWith(stoppedTrack, expectedTrack); }); + + describe("when video is disabled shortly after enabling", () => { + it("should not add video track to local stream", async () => { + const expectedTrack = gumStream.getVideoTracks()[0]; + + const enablePromise = rtcManager.stopOrResumeVideo(localStream, true); + const disablePromise = rtcManager.stopOrResumeVideo(localStream, false); + await enablePromise; + + expect(localStream.addTrack).not.toHaveBeenCalledWith(expectedTrack); + await disablePromise; + }); + + it("should not emit event", async () => { + const enablePromise = rtcManager.stopOrResumeVideo(localStream, true); + const disablePromise = rtcManager.stopOrResumeVideo(localStream, false); + await enablePromise; + + expect(emitterStub.emit).not.toHaveBeenCalled(); + await disablePromise; + }); + + it("should not add track to peer connection(s)", async () => { + jest.spyOn(rtcManager, "_addTrackToPeerConnections"); + + const enablePromise = rtcManager.stopOrResumeVideo(localStream, true); + const disablePromise = rtcManager.stopOrResumeVideo(localStream, false); + await enablePromise; + + expect(rtcManager._addTrackToPeerConnections).not.toHaveBeenCalled(); + await disablePromise; + }); + + it("should not replace track in peer connection(s) when stopped track exists", async () => { + jest.spyOn(rtcManager, "_replaceTrackToPeerConnections"); + const stoppedTrack = helpers.createMockedMediaStreamTrack({ kind: "video" }); + rtcManager._stoppedVideoTrack = stoppedTrack; + + const enablePromise = rtcManager.stopOrResumeVideo(localStream, true); + const disablePromise = rtcManager.stopOrResumeVideo(localStream, false); + await enablePromise; + + expect(rtcManager._replaceTrackToPeerConnections).not.toHaveBeenCalled(); + await disablePromise; + }); + }); }); describe("handling localStream `stopresumevideo` event", () => {