diff --git a/spec/unit/webrtc/call.spec.ts b/spec/unit/webrtc/call.spec.ts index 8fb2f3be4..965f41eae 100644 --- a/spec/unit/webrtc/call.spec.ts +++ b/spec/unit/webrtc/call.spec.ts @@ -25,6 +25,7 @@ import { CallType, CallState, CallParty, + CallDirection, } from "../../../src/webrtc/call"; import { MCallAnswer, @@ -1712,4 +1713,110 @@ describe("Call", function () { expect(onReplace).toHaveBeenCalled(); }); }); + describe("should handle glare in negotiation process", () => { + beforeEach(async () => { + // cut methods not want to test + call.hangup = () => null; + call.isLocalOnHold = () => true; + // @ts-ignore + call.updateRemoteSDPStreamMetadata = jest.fn(); + // @ts-ignore + call.getRidOfRTXCodecs = jest.fn(); + // @ts-ignore + call.createAnswer = jest.fn().mockResolvedValue({}); + // @ts-ignore + call.sendVoipEvent = jest.fn(); + }); + + it("and reject remote offer if not polite and have pending local offer", async () => { + // not polite user == CallDirection.Outbound + call.direction = CallDirection.Outbound; + // have already a local offer + // @ts-ignore + call.makingOffer = true; + const offerEvent = makeMockEvent("@test:foo", { + description: { + type: "offer", + sdp: DUMMY_SDP, + }, + }); + // @ts-ignore + call.peerConn = { + signalingState: "have-local-offer", + setRemoteDescription: jest.fn(), + }; + await call.onNegotiateReceived(offerEvent); + expect(call.peerConn?.setRemoteDescription).not.toHaveBeenCalled(); + }); + + it("and not reject remote offer if not polite and do have pending answer", async () => { + // not polite user == CallDirection.Outbound + call.direction = CallDirection.Outbound; + // have not a local offer + // @ts-ignore + call.makingOffer = false; + + // If we have a setRemoteDescription() answer operation pending, then + // we will be "stable" by the time the next setRemoteDescription() is + // executed, so we count this being readyForOffer when deciding whether to + // ignore the offer. + // @ts-ignore + call.isSettingRemoteAnswerPending = true; + const offerEvent = makeMockEvent("@test:foo", { + description: { + type: "offer", + sdp: DUMMY_SDP, + }, + }); + // @ts-ignore + call.peerConn = { + signalingState: "have-local-offer", + setRemoteDescription: jest.fn(), + }; + await call.onNegotiateReceived(offerEvent); + expect(call.peerConn?.setRemoteDescription).toHaveBeenCalled(); + }); + + it("and not reject remote offer if not polite and do not have pending local offer", async () => { + // not polite user == CallDirection.Outbound + call.direction = CallDirection.Outbound; + // have no local offer + // @ts-ignore + call.makingOffer = false; + const offerEvent = makeMockEvent("@test:foo", { + description: { + type: "offer", + sdp: DUMMY_SDP, + }, + }); + // @ts-ignore + call.peerConn = { + signalingState: "stable", + setRemoteDescription: jest.fn(), + }; + await call.onNegotiateReceived(offerEvent); + expect(call.peerConn?.setRemoteDescription).toHaveBeenCalled(); + }); + + it("and if polite do rollback pending local offer", async () => { + // polite user == CallDirection.Inbound + call.direction = CallDirection.Inbound; + // have already a local offer + // @ts-ignore + call.makingOffer = true; + const offerEvent = makeMockEvent("@test:foo", { + description: { + type: "offer", + sdp: DUMMY_SDP, + }, + }); + // @ts-ignore + call.peerConn = { + signalingState: "have-local-offer", + setRemoteDescription: jest.fn(), + }; + await call.onNegotiateReceived(offerEvent); + expect(call.peerConn?.setRemoteDescription).toHaveBeenCalled(); + }); + }); }); diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 4d839827b..d20232367 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -517,8 +517,7 @@ describe("Group Call", function () { await groupCall.setMicrophoneMuted(false); expect(groupCall.isMicrophoneMuted()).toEqual(false); - jest.advanceTimersByTime(groupCall.pttMaxTransmitTime + 100); - + await jest.advanceTimersByTimeAsync(groupCall.pttMaxTransmitTime + 100); expect(groupCall.isMicrophoneMuted()).toEqual(true); }); @@ -585,7 +584,15 @@ describe("Group Call", function () { }); mockCall.sendMetadataUpdate = jest.fn().mockReturnValue(metadataUpdatePromise); + const getUserMediaStreamFlush = Promise.resolve("stream"); + // @ts-ignore + mockCall.cleint = { + getMediaHandler: { + getUserMediaStream: jest.fn().mockReturnValue(getUserMediaStreamFlush), + }, + }; const mutePromise = groupCall.setMicrophoneMuted(true); + await getUserMediaStreamFlush; // we should be muted at this point, before the metadata update has been sent expect(groupCall.isMicrophoneMuted()).toEqual(true); expect(mockCall.localUsermediaFeed.setAudioVideoMuted).toHaveBeenCalled(); @@ -892,14 +899,34 @@ describe("Group Call", function () { expect(await groupCall.setMicrophoneMuted(false)).toBe(false); }); - it("returns false when no permission for audio stream", async () => { + it("returns false when no permission for audio stream and localCallFeed do not have an audio track", async () => { const groupCall = await createAndEnterGroupCall(mockClient, room); + // @ts-ignore + jest.spyOn(groupCall.localCallFeed, "hasAudioTrack", "get").mockReturnValue(false); jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValueOnce( new Error("No Permission"), ); expect(await groupCall.setMicrophoneMuted(false)).toBe(false); }); + it("returns false when user media stream null", async () => { + const groupCall = await createAndEnterGroupCall(mockClient, room); + // @ts-ignore + jest.spyOn(groupCall.localCallFeed, "hasAudioTrack", "get").mockReturnValue(false); + // @ts-ignore + jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockResolvedValue({} as MediaStream); + expect(await groupCall.setMicrophoneMuted(false)).toBe(false); + }); + + it("returns true when no permission for audio stream but localCallFeed has a audio track already", async () => { + const groupCall = await createAndEnterGroupCall(mockClient, room); + // @ts-ignore + jest.spyOn(groupCall.localCallFeed, "hasAudioTrack", "get").mockReturnValue(true); + jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream"); + expect(mockClient.getMediaHandler().getUserMediaStream).not.toHaveBeenCalled(); + expect(await groupCall.setMicrophoneMuted(false)).toBe(true); + }); + it("returns false when unmuting video with no video device", async () => { const groupCall = await createAndEnterGroupCall(mockClient, room); jest.spyOn(mockClient.getMediaHandler(), "hasVideoDevice").mockResolvedValue(false); diff --git a/src/webrtc/callFeed.ts b/src/webrtc/callFeed.ts index 07c906e67..a9cf7a7a9 100644 --- a/src/webrtc/callFeed.ts +++ b/src/webrtc/callFeed.ts @@ -128,7 +128,7 @@ export class CallFeed extends TypedEventEmitter this.emit(CallFeedEvent.ConnectedChanged, this.connected); } - private get hasAudioTrack(): boolean { + public get hasAudioTrack(): boolean { return this.stream.getAudioTracks().length > 0; } diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index a6a629576..6d98586c9 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -655,27 +655,9 @@ export class GroupCall extends TypedEventEmitter< `GroupCall ${this.groupCallId} setMicrophoneMuted() (streamId=${this.localCallFeed.stream.id}, muted=${muted})`, ); - // We needed this here to avoid an error in case user join a call without a device. - // I can not use .then .catch functions because linter :-( - try { - if (!muted) { - const stream = await this.client - .getMediaHandler() - .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()); - if (stream === null) { - // if case permission denied to get a stream stop this here - /* istanbul ignore next */ - logger.log( - `GroupCall ${this.groupCallId} setMicrophoneMuted() no device to receive local stream, muted=${muted}`, - ); - return false; - } - } - } catch (e) { - /* istanbul ignore next */ - logger.log( - `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`, - ); + const hasPermission = await this.checkAudioPermissionIfNecessary(muted); + + if (!hasPermission) { return false; } @@ -700,6 +682,42 @@ export class GroupCall extends TypedEventEmitter< return true; } + /** + * If we allow entering a call without a camera and without video, it can happen that the access rights to the + * devices have not yet been queried. If a stream does not yet have an audio track, we assume that the rights have + * not yet been checked. + * + * `this.client.getMediaHandler().getUserMediaStream` clones the current stream, so it only wanted to be called when + * not Audio Track exists. + * As such, this is a compromise, because, the access rights should always be queried before the call. + */ + private async checkAudioPermissionIfNecessary(muted: boolean): Promise { + // We needed this here to avoid an error in case user join a call without a device. + try { + if (!muted && this.localCallFeed && !this.localCallFeed.hasAudioTrack) { + const stream = await this.client + .getMediaHandler() + .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()); + if (stream?.getTracks().length === 0) { + // if case permission denied to get a stream stop this here + /* istanbul ignore next */ + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no device to receive local stream, muted=${muted}`, + ); + return false; + } + } + } catch (e) { + /* istanbul ignore next */ + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`, + ); + return false; + } + + return true; + } + /** * Sets the mute state of the local participants's video. * @param muted - Whether to mute the video