Remove stream-replacement (#2551)

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();
+        });
+    });
 });

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,
+            });
         });
-        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 by default", () => {
+            it("should mute audio by default", () => {
+                expect(feed.isAudioMuted()).toBeTruthy();
+            });
 
-        feedNewStreamCallback.mockReset();
+            it("should mute video by default", () => {
+                expect(feed.isVideoMuted()).toBeTruthy();
+            });
+        });
 
-        replacementStream.addTrack(new MockMediaStreamTrack("track_id", "audio"));
-        expect(feedNewStreamCallback).toHaveBeenCalledWith(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();
+            });
+
+            it("should un-mute video", () => {
+                // @ts-ignore Mock
+                feed.stream.addTrack(new MockMediaStreamTrack("track", "video", true));
+                expect(feed.isVideoMuted()).toBeFalsy();
+            });
+        });
+
+        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();
+            });
+        });
     });
 });

src/webrtc/call.ts

@@ -523,24 +523,22 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             return;
         }
 
-        // Try to find a feed with the same purpose as the new stream,
-        // if we find it replace the old stream with the new one
-        const existingFeed = this.getRemoteFeeds().find((feed) => 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<CallEvent, CallEventHandlerMap
             return;
         }
 
-        // Try to find a feed with the same stream id as the new stream,
-        // if we find it replace the old stream with the new one
-        const feed = this.getFeedByStreamId(stream.id);
-        if (feed) {
-            feed.setNewStream(stream);
-        } else {
-            this.feeds.push(new CallFeed({
-                client: this.client,
-                roomId: this.roomId,
-                audioMuted: false,
-                videoMuted: false,
-                userId,
-                stream,
-                purpose,
-            }));
-            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,
+            audioMuted: false,
+            videoMuted: false,
+            userId,
+            stream,
+            purpose,
+        }));
+        this.emit(CallEvent.FeedsChanged, this.feeds);
+
         logger.info(`Pushed remote stream (id="${stream.id}", active="${stream.active}")`);
     }
 
@@ -592,25 +588,23 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
         setTracksEnabled(stream.getAudioTracks(), true);
         setTracksEnabled(stream.getVideoTracks(), true);
 
-        // We try to replace an existing feed if there already is one with the same purpose
-        const existingFeed = this.getLocalFeeds().find((feed) => 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,
+        );
     }
 
     /**

src/webrtc/callFeed.ts

@@ -174,15 +174,6 @@ export class CallFeed extends TypedEventEmitter<CallFeedEvent, EventHandlerMap>
         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