Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove stream-replacement #2551

Merged
merged 5 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 59 additions & 65 deletions spec/unit/webrtc/call.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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();
});
});
});
78 changes: 53 additions & 25 deletions spec/unit/webrtc/callFeed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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();
});
});
});
});
94 changes: 44 additions & 50 deletions src/webrtc/call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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})`);
}

Expand All @@ -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}")`);
}

Expand All @@ -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,
);
}

/**
Expand Down
9 changes: 0 additions & 9 deletions src/webrtc/callFeed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down