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 video track when muting video #3028

Merged
merged 3 commits into from
Jan 6, 2023
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
8 changes: 6 additions & 2 deletions spec/unit/webrtc/groupCall.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,10 @@ describe("Group Call", function () {
it("should mute local video when calling setLocalVideoMuted()", async () => {
const groupCall = await createAndEnterGroupCall(mockClient, room);

groupCall.localCallFeed!.setAudioVideoMuted = jest.fn();
jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream");
jest.spyOn(groupCall, "updateLocalUsermediaStream");
jest.spyOn(groupCall.localCallFeed!, "setAudioVideoMuted");

const setAVMutedArray: ((audioMuted: boolean | null, videoMuted: boolean | null) => void)[] = [];
const tracksArray: MediaStreamTrack[] = [];
const sendMetadataUpdateArray: (() => Promise<void>)[] = [];
Expand All @@ -824,7 +827,8 @@ describe("Group Call", function () {
await groupCall.setLocalVideoMuted(true);

groupCall.localCallFeed!.stream.getVideoTracks().forEach((track) => expect(track.enabled).toBe(false));
expect(groupCall.localCallFeed!.setAudioVideoMuted).toHaveBeenCalledWith(null, true);
expect(mockClient.getMediaHandler().getUserMediaStream).toHaveBeenCalledWith(true, false);
expect(groupCall.updateLocalUsermediaStream).toHaveBeenCalled();
setAVMutedArray.forEach((f) => expect(f).toHaveBeenCalledWith(null, true));
tracksArray.forEach((track) => expect(track.enabled).toBe(false));
sendMetadataUpdateArray.forEach((f) => expect(f).toHaveBeenCalled());
Expand Down
18 changes: 8 additions & 10 deletions spec/unit/webrtc/mediaHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,20 +308,18 @@ describe("Media Handler", function () {
expect(stream2.isCloneOf(stream1)).toEqual(false);
});

it("strips unwanted audio tracks from re-used stream", async () => {
const stream1 = await mediaHandler.getUserMediaStream(true, true);
const stream2 = (await mediaHandler.getUserMediaStream(false, true)) as unknown as MockMediaStream;
it("creates new stream when we no longer want audio", async () => {
await mediaHandler.getUserMediaStream(true, true);
const stream = await mediaHandler.getUserMediaStream(false, true);

expect(stream2.isCloneOf(stream1)).toEqual(true);
expect(stream2.getAudioTracks().length).toEqual(0);
expect(stream.getAudioTracks().length).toEqual(0);
});

it("strips unwanted video tracks from re-used stream", async () => {
const stream1 = await mediaHandler.getUserMediaStream(true, true);
const stream2 = (await mediaHandler.getUserMediaStream(true, false)) as unknown as MockMediaStream;
it("creates new stream when we no longer want video", async () => {
await mediaHandler.getUserMediaStream(true, true);
const stream = await mediaHandler.getUserMediaStream(true, false);

expect(stream2.isCloneOf(stream1)).toEqual(true);
expect(stream2.getVideoTracks().length).toEqual(0);
expect(stream.getVideoTracks().length).toEqual(0);
});
});

Expand Down
31 changes: 28 additions & 3 deletions src/webrtc/call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,10 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
setTracksEnabled(stream.getAudioTracks(), audioEnabled);
setTracksEnabled(stream.getVideoTracks(), videoEnabled);

// We want to keep the same stream id, so we replace the tracks rather than the whole stream
// We want to keep the same stream id, so we replace the tracks rather
// than the whole stream.

// Firstly, we replace the tracks in our localUsermediaStream.
for (const track of this.localUsermediaStream!.getTracks()) {
this.localUsermediaStream!.removeTrack(track);
track.stop();
Expand All @@ -1289,10 +1292,23 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
this.localUsermediaStream!.addTrack(track);
}

// Secondly, we remove tracks that we no longer need from the peer
// connection, if any. This only happens when we mute the video atm.
// This will change the transceiver direction to "inactive" and
// therefore cause re-negotiation.
for (const kind of ["audio", "video"]) {
const sender = this.transceivers.get(getTransceiverKey(SDPStreamMetadataPurpose.Usermedia, kind))?.sender;
// Only remove the track if we aren't going to immediately replace it
if (sender && !stream.getTracks().find((t) => t.kind === kind)) {
this.peerConn?.removeTrack(sender);
}
}
// Thirdly, we replace the old tracks, if possible.
for (const track of stream.getTracks()) {
const tKey = getTransceiverKey(SDPStreamMetadataPurpose.Usermedia, track.kind);

const oldSender = this.transceivers.get(tKey)?.sender;
const transceiver = this.transceivers.get(tKey);
const oldSender = transceiver?.sender;
let added = false;
if (oldSender) {
try {
Expand All @@ -1306,6 +1322,10 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
`) to peer connection`,
);
await oldSender.replaceTrack(track);
// Set the direction to indicate we're going to be sending.
// This is only necessary in the cases where we're upgrading
// the call to video after downgrading it.
transceiver.direction = transceiver.direction === "inactive" ? "sendonly" : "sendrecv";
added = true;
} catch (error) {
logger.warn(`replaceTrack failed: adding new transceiver instead`, error);
Expand Down Expand Up @@ -1349,7 +1369,12 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
await this.upgradeCall(false, true);
return this.isLocalVideoMuted();
}
this.localUsermediaFeed?.setAudioVideoMuted(null, muted);
if (this.opponentSupportsSDPStreamMetadata()) {
const stream = await this.client.getMediaHandler().getUserMediaStream(true, !muted);
await this.updateLocalUsermediaStream(stream);
} else {
this.localUsermediaFeed?.setAudioVideoMuted(null, muted);
}
this.updateMuteStatus();
await this.sendMetadataUpdate();
return this.isLocalVideoMuted();
Expand Down
3 changes: 3 additions & 0 deletions src/webrtc/groupCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ export class GroupCall extends TypedEventEmitter<
logger.log(
`groupCall ${this.groupCallId} setLocalVideoMuted stream ${this.localCallFeed.stream.id} muted ${muted}`,
);

const stream = await this.client.getMediaHandler().getUserMediaStream(true, !muted);
robintown marked this conversation as resolved.
Show resolved Hide resolved
await this.updateLocalUsermediaStream(stream);
this.localCallFeed.setAudioVideoMuted(null, muted);
setTracksEnabled(this.localCallFeed.stream.getVideoTracks(), !muted);
} else {
Expand Down
34 changes: 20 additions & 14 deletions src/webrtc/mediaHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,24 +203,30 @@ export class MediaHandler extends TypedEventEmitter<

let canReuseStream = true;
if (this.localUserMediaStream) {
// This figures out if we can reuse the current localUsermediaStream
// based on whether or not the "mute state" (presence of tracks of a
// given kind) matches what is being requested
if (shouldRequestAudio !== this.localUserMediaStream.getAudioTracks().length > 0) {
canReuseStream = false;
}
if (shouldRequestVideo !== this.localUserMediaStream.getVideoTracks().length > 0) {
canReuseStream = false;
}

// This code checks that the device ID is the same as the localUserMediaStream stream, but we update
// the localUserMediaStream whenever the device ID changes (apart from when restoring) so it's not
// clear why this would ever be different, unless there's a race.
if (shouldRequestAudio) {
if (
this.localUserMediaStream.getAudioTracks().length === 0 ||
this.localUserMediaStream.getAudioTracks()[0]?.getSettings()?.deviceId !== this.audioInput
) {
canReuseStream = false;
}
if (
shouldRequestAudio &&
this.localUserMediaStream.getAudioTracks()[0]?.getSettings()?.deviceId !== this.audioInput
) {
canReuseStream = false;
}
if (shouldRequestVideo) {
if (
this.localUserMediaStream.getVideoTracks().length === 0 ||
this.localUserMediaStream.getVideoTracks()[0]?.getSettings()?.deviceId !== this.videoInput
) {
canReuseStream = false;
}
if (
shouldRequestVideo &&
this.localUserMediaStream.getVideoTracks()[0]?.getSettings()?.deviceId !== this.videoInput
) {
canReuseStream = false;
}
} else {
canReuseStream = false;
Expand Down