diff --git a/spec/unit/webrtc/call.spec.ts b/spec/unit/webrtc/call.spec.ts index df731d66960..9828dfaa372 100644 --- a/spec/unit/webrtc/call.spec.ts +++ b/spec/unit/webrtc/call.spec.ts @@ -391,71 +391,6 @@ describe('Call', function() { }).track.id).toBe("video_track"); }); - describe("should handle stream replacement", () => { - it("with both purpose and id", async () => { - await startVoiceCall(client, call); - - call.updateRemoteSDPStreamMetadata({ - "remote_stream1": { - purpose: SDPStreamMetadataPurpose.Usermedia, - }, - }); - call.pushRemoteFeed(new MockMediaStream("remote_stream1", [])); - const feed = call.getFeeds().find((feed) => feed.stream.id === "remote_stream1"); - - call.updateRemoteSDPStreamMetadata({ - "remote_stream2": { - purpose: SDPStreamMetadataPurpose.Usermedia, - }, - }); - call.pushRemoteFeed(new MockMediaStream("remote_stream2", [])); - - expect(feed?.stream?.id).toBe("remote_stream2"); - }); - - it("with just purpose", async () => { - await startVoiceCall(client, call); - - call.updateRemoteSDPStreamMetadata({ - "remote_stream1": { - purpose: SDPStreamMetadataPurpose.Usermedia, - }, - }); - call.pushRemoteFeed(new MockMediaStream("remote_stream1", [])); - const feed = call.getFeeds().find((feed) => feed.stream.id === "remote_stream1"); - - call.updateRemoteSDPStreamMetadata({ - "remote_stream2": { - purpose: SDPStreamMetadataPurpose.Usermedia, - }, - }); - call.pushRemoteFeed(new MockMediaStream("remote_stream2", [])); - - expect(feed?.stream?.id).toBe("remote_stream2"); - }); - - it("should not replace purpose is different", async () => { - await startVoiceCall(client, call); - - call.updateRemoteSDPStreamMetadata({ - "remote_stream1": { - purpose: SDPStreamMetadataPurpose.Usermedia, - }, - }); - call.pushRemoteFeed(new MockMediaStream("remote_stream1", [])); - const feed = call.getFeeds().find((feed) => feed.stream.id === "remote_stream1"); - - call.updateRemoteSDPStreamMetadata({ - "remote_stream2": { - purpose: SDPStreamMetadataPurpose.Screenshare, - }, - }); - call.pushRemoteFeed(new MockMediaStream("remote_stream2", [])); - - expect(feed?.stream?.id).toBe("remote_stream1"); - }); - }); - it("should handle SDPStreamMetadata changes", async () => { await startVoiceCall(client, call); @@ -758,4 +693,63 @@ describe('Call', function() { expect(supportsMatrixCall()).toBe(false); }); }); + + describe("ignoring streams with ids for which we already have a feed", () => { + const STREAM_ID = "stream_id"; + const FEEDS_CHANGED_CALLBACK = jest.fn(); + + beforeEach(async () => { + await startVoiceCall(client, call); + call.on(CallEvent.FeedsChanged, FEEDS_CHANGED_CALLBACK); + jest.spyOn(call, "pushLocalFeed"); + }); + + afterEach(() => { + FEEDS_CHANGED_CALLBACK.mockReset(); + }); + + it("should ignore stream passed to pushRemoteFeed()", async () => { + await call.onAnswerReceived({ + getContent: () => { + return { + version: 1, + call_id: call.callId, + party_id: 'party_id', + answer: { + sdp: DUMMY_SDP, + }, + [SDPStreamMetadataKey]: { + [STREAM_ID]: { + purpose: SDPStreamMetadataPurpose.Usermedia, + }, + }, + }; + }, + }); + + call.pushRemoteFeed(new MockMediaStream(STREAM_ID)); + call.pushRemoteFeed(new MockMediaStream(STREAM_ID)); + + expect(call.getRemoteFeeds().length).toBe(1); + expect(FEEDS_CHANGED_CALLBACK).toHaveBeenCalledTimes(1); + }); + + it("should ignore stream passed to pushRemoteFeedWithoutMetadata()", async () => { + call.pushRemoteFeedWithoutMetadata(new MockMediaStream(STREAM_ID)); + call.pushRemoteFeedWithoutMetadata(new MockMediaStream(STREAM_ID)); + + expect(call.getRemoteFeeds().length).toBe(1); + expect(FEEDS_CHANGED_CALLBACK).toHaveBeenCalledTimes(1); + }); + + it("should ignore stream passed to pushNewLocalFeed()", async () => { + call.pushNewLocalFeed(new MockMediaStream(STREAM_ID), SDPStreamMetadataPurpose.Screenshare); + call.pushNewLocalFeed(new MockMediaStream(STREAM_ID), SDPStreamMetadataPurpose.Screenshare); + + // We already have one local feed from placeVoiceCall() + expect(call.getLocalFeeds().length).toBe(2); + expect(FEEDS_CHANGED_CALLBACK).toHaveBeenCalledTimes(1); + expect(call.pushLocalFeed).toHaveBeenCalled(); + }); + }); }); diff --git a/spec/unit/webrtc/callFeed.spec.ts b/spec/unit/webrtc/callFeed.spec.ts index e8881781dd9..635fa14fd8f 100644 --- a/spec/unit/webrtc/callFeed.spec.ts +++ b/spec/unit/webrtc/callFeed.spec.ts @@ -15,13 +15,11 @@ limitations under the License. */ import { SDPStreamMetadataPurpose } from "../../../src/webrtc/callEventTypes"; -import { CallFeed, CallFeedEvent } from "../../../src/webrtc/callFeed"; -import { MockMediaStream, MockMediaStreamTrack } from "../../test-utils/webrtc"; +import { CallFeed } from "../../../src/webrtc/callFeed"; import { TestClient } from "../../TestClient"; +import { MockMediaStream, MockMediaStreamTrack } from "../../test-utils/webrtc"; describe("CallFeed", () => { - const roomId = "room_id"; - let client; beforeEach(() => { @@ -32,30 +30,60 @@ describe("CallFeed", () => { client.stop(); }); - it("should handle stream replacement", () => { - const feedNewStreamCallback = jest.fn(); - const feed = new CallFeed({ - client, - roomId, - userId: "user1", - // @ts-ignore Mock - stream: new MockMediaStream("stream1"), - id: "id", - purpose: SDPStreamMetadataPurpose.Usermedia, - audioMuted: false, - videoMuted: false, + describe("muting", () => { + let feed: CallFeed; + + beforeEach(() => { + feed = new CallFeed({ + client, + roomId: "room1", + userId: "user1", + // @ts-ignore Mock + stream: new MockMediaStream("stream1"), + purpose: SDPStreamMetadataPurpose.Usermedia, + audioMuted: false, + videoMuted: false, + }); + }); + + describe("muting by default", () => { + it("should mute audio by default", () => { + expect(feed.isAudioMuted()).toBeTruthy(); + }); + + it("should mute video by default", () => { + expect(feed.isVideoMuted()).toBeTruthy(); + }); }); - feed.on(CallFeedEvent.NewStream, feedNewStreamCallback); - const replacementStream = new MockMediaStream("stream2"); - // @ts-ignore Mock - feed.setNewStream(replacementStream); - expect(feedNewStreamCallback).toHaveBeenCalledWith(replacementStream); - expect(feed.stream).toBe(replacementStream); + describe("muting after adding a track", () => { + it("should un-mute audio", () => { + // @ts-ignore Mock + feed.stream.addTrack(new MockMediaStreamTrack("track", "audio", true)); + expect(feed.isAudioMuted()).toBeFalsy(); + }); - feedNewStreamCallback.mockReset(); + it("should un-mute video", () => { + // @ts-ignore Mock + feed.stream.addTrack(new MockMediaStreamTrack("track", "video", true)); + expect(feed.isVideoMuted()).toBeFalsy(); + }); + }); - replacementStream.addTrack(new MockMediaStreamTrack("track_id", "audio")); - expect(feedNewStreamCallback).toHaveBeenCalledWith(replacementStream); + describe("muting after calling setAudioVideoMuted()", () => { + it("should mute audio by default ", () => { + // @ts-ignore Mock + feed.stream.addTrack(new MockMediaStreamTrack("track", "audio", true)); + feed.setAudioVideoMuted(true, false); + expect(feed.isAudioMuted()).toBeTruthy(); + }); + + it("should mute video by default", () => { + // @ts-ignore Mock + feed.stream.addTrack(new MockMediaStreamTrack("track", "video", true)); + feed.setAudioVideoMuted(false, true); + expect(feed.isVideoMuted()).toBeTruthy(); + }); + }); }); }); diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts index e0f9ae2ff7b..ebc7464f06d 100644 --- a/src/webrtc/call.ts +++ b/src/webrtc/call.ts @@ -523,24 +523,22 @@ export class MatrixCall extends TypedEventEmitter feed.purpose === purpose); - if (existingFeed) { - existingFeed.setNewStream(stream); - } else { - this.feeds.push(new CallFeed({ - client: this.client, - roomId: this.roomId, - userId, - stream, - purpose, - audioMuted, - videoMuted, - })); - this.emit(CallEvent.FeedsChanged, this.feeds); + if (this.getFeedByStreamId(stream.id)) { + logger.warn(`Ignoring stream with id ${stream.id} because we already have a feed for it`); + return; } + this.feeds.push(new CallFeed({ + client: this.client, + roomId: this.roomId, + userId, + stream, + purpose, + audioMuted, + videoMuted, + })); + this.emit(CallEvent.FeedsChanged, this.feeds); + logger.info(`Pushed remote stream (id="${stream.id}", active="${stream.active}", purpose=${purpose})`); } @@ -562,24 +560,22 @@ export class MatrixCall extends TypedEventEmitter feed.purpose === purpose); - if (existingFeed) { - existingFeed.setNewStream(stream); - } else { - this.pushLocalFeed( - new CallFeed({ - client: this.client, - roomId: this.roomId, - audioMuted: false, - videoMuted: false, - userId, - stream, - purpose, - }), - addToPeerConnection, - ); - this.emit(CallEvent.FeedsChanged, this.feeds); + if (this.getFeedByStreamId(stream.id)) { + logger.warn(`Ignoring stream with id ${stream.id} because we already have a feed for it`); + return; } + + this.pushLocalFeed( + new CallFeed({ + client: this.client, + roomId: this.roomId, + audioMuted: false, + videoMuted: false, + userId, + stream, + purpose, + }), + addToPeerConnection, + ); } /** diff --git a/src/webrtc/callFeed.ts b/src/webrtc/callFeed.ts index f1fc6540af5..a8a20205491 100644 --- a/src/webrtc/callFeed.ts +++ b/src/webrtc/callFeed.ts @@ -174,15 +174,6 @@ export class CallFeed extends TypedEventEmitter return this.speaking; } - /** - * Replaces the current MediaStream with a new one. - * This method should be only used by MatrixCall. - * @param newStream new stream with which to replace the current one - */ - public setNewStream(newStream: MediaStream): void { - this.updateStream(this.stream, newStream); - } - /** * Set one or both of feed's internal audio and video video mute state * Either value may be null to leave it as-is