Skip to content

Commit

Permalink
Fix screenshare failing after several attempts
Browse files Browse the repository at this point in the history 
Re-use any existing transceivers when screen sharing. This prevents
transceivers accumulating and making the SDP too big: see linked bug.

This also switches from `addTrack()` to `addTransceiver ()` which is
not that large of a change, other than having to explicitly find the
transceivers after an offer has arrived rather than just adding tracks
and letting WebRTC take care of it.

Fixes element-hq/element-call#625
  • Loading branch information
@dbkr
dbkr committed Oct 18, 2022
1 parent dfe535b commit 92efd30
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 47 deletions.
151 changes: 105 additions & 46 deletions src/webrtc/call.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,31 @@ export type CallEventHandlerMap = {
[CallEvent.SendVoipEvent]: (event: Record<string, any>) => void;
};

enum TransceiverIndex {
UserMediaAudio = 0,
UserMediaVideo = 1,
ScreenshareAudio = 2,
ScreenshareVideo = 3,
}

function getTransceiverIndex(purpose: SDPStreamMetadataPurpose, kind: string): TransceiverIndex {
if (purpose == SDPStreamMetadataPurpose.Usermedia) {
if (kind == MediaType.AUDIO) {
return TransceiverIndex.UserMediaAudio;
} else if (kind == MediaType.VIDEO) {
return TransceiverIndex.UserMediaVideo;
}
} else {
if (kind == MediaType.AUDIO) {
return TransceiverIndex.ScreenshareAudio;
} else if (kind == MediaType.VIDEO) {
return TransceiverIndex.ScreenshareVideo;
}
}

throw new Error(`Unknown media purpose / kind: ${purpose} / ${kind}`);
}

/**
* Construct a new Matrix Call.
* @constructor
Expand Down Expand Up @@ -345,8 +370,10 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
private candidateSendTries = 0;
private candidatesEnded = false;
private feeds: Array<CallFeed> = [];
private usermediaSenders: Array<RTCRtpSender> = [];
private screensharingSenders: Array<RTCRtpSender> = [];

// our transceivers for each purpose and type of media, according to SenderIndex
private transceivers: RTCRtpTransceiver[] = [null, null, null, null];

private inviteOrAnswerSent = false;
private waitForLocalAVStream: boolean;
private successor: MatrixCall;
Expand Down Expand Up @@ -634,6 +661,18 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
audioMuted,
videoMuted,
}));

// gather transceivers from the new tracks so that we can use the same ones for tracks that
// we add later. We only do this for user media streams though: screenshare streams just always
// get their own unidirectional transceiver since a bidirectional screen share is pretty rare
// (we *could* re-use an existing recvonly transceiver for this, but it's simpler to just not).
if (purpose == SDPStreamMetadataPurpose.Usermedia) {
for (const track of stream.getTracks()) {
const transceiver = this.peerConn.getTransceivers().find(t => t.receiver.track == track);
this.transceivers[getTransceiverIndex(purpose, track.kind)] = transceiver;
}
}

this.emit(CallEvent.FeedsChanged, this.feeds);

logger.info(
Expand Down Expand Up @@ -675,6 +714,12 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
stream,
purpose,
}));

for (const track of stream.getTracks()) {
const transceiver = this.peerConn.getTransceivers().find(t => t.receiver.track == track);
this.transceivers[getTransceiverIndex(purpose, track.kind)] = transceiver;
}

this.emit(CallEvent.FeedsChanged, this.feeds);

logger.info(`Call ${this.callId} pushed remote stream (id="${stream.id}", active="${stream.active}")`);
Expand Down Expand Up @@ -722,11 +767,6 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
this.feeds.push(callFeed);

if (addToPeerConnection) {
const senderArray = callFeed.purpose === SDPStreamMetadataPurpose.Usermedia ?
this.usermediaSenders : this.screensharingSenders;
// Empty the array
senderArray.splice(0, senderArray.length);

for (const track of callFeed.stream.getTracks()) {
logger.info(
`Call ${this.callId} ` +
Expand All @@ -738,7 +778,24 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
`enabled=${track.enabled}` +
`) to peer connection`,
);
senderArray.push(this.peerConn.addTrack(track, callFeed.stream));

const tIdx = getTransceiverIndex(callFeed.purpose, track.kind);
if (this.transceivers[tIdx]) {
// we already have a sender, so we re-use it. We try to re-use transceivers as much
// as possible because they can't be removed once added, so otherwise they just
// accumulate which makes the SDP very large very quickly: in fact it only takes
// about 6 video tracks to exceed the maximum size of an Olm-encrypted
// Matrix event.
this.transceivers[tIdx].sender.replaceTrack(track);
this.transceivers[tIdx].direction =
this.transceivers[tIdx].direction === "inactive" ? "sendonly" : "sendrecv";
} else {
// create a new one: pass the track in and everything happens automatically
this.transceivers[tIdx] = this.peerConn.addTransceiver(track, {
streams: [callFeed.stream],
direction: callFeed.purpose === SDPStreamMetadataPurpose.Usermedia ? "sendrecv" : "sendonly",
});
}
}
}

Expand All @@ -759,20 +816,20 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
* @param callFeed to remove
*/
public removeLocalFeed(callFeed: CallFeed): void {
const senderArray = callFeed.purpose === SDPStreamMetadataPurpose.Usermedia
? this.usermediaSenders
: this.screensharingSenders;
const audioTransceiver = this.transceivers[getTransceiverIndex(callFeed.purpose, "audio")];
const videoTransceiver = this.transceivers[getTransceiverIndex(callFeed.purpose, "video")];

for (const sender of senderArray) {
this.peerConn.removeTrack(sender);
for (const transceiver of [audioTransceiver, videoTransceiver]) {
// this is slightly mixing the track and transceiver API but is basically just shorthand.
// There is no way to actually remove a transceiver, so this just sets it to inactive
// (or recvonly) and replaces the source with nothing.
if (transceiver && transceiver.sender) this.peerConn.removeTrack(transceiver.sender);
}

if (callFeed.purpose === SDPStreamMetadataPurpose.Screenshare) {
this.client.getMediaHandler().stopScreensharingStream(callFeed.stream);
}

// Empty the array
senderArray.splice(0, senderArray.length);
this.deleteFeed(callFeed);
}

Expand Down Expand Up @@ -1139,9 +1196,19 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
return false;
}
} else {
for (const sender of this.screensharingSenders) {
this.peerConn.removeTrack(sender);
const audioTransceiver = this.transceivers[getTransceiverIndex(
SDPStreamMetadataPurpose.Screenshare, "audio",
)];
const videoTransceiver = this.transceivers[getTransceiverIndex(
SDPStreamMetadataPurpose.Screenshare, "video",
)];

for (const transceiver of [audioTransceiver, videoTransceiver]) {
// this is slightly mixing the track and transceiver API but is basically just shorthand
// for removing the sender.
if (transceiver && transceiver.sender) this.peerConn.removeTrack(transceiver.sender);
}

this.client.getMediaHandler().stopScreensharingStream(this.localScreensharingStream);
this.deleteFeedByStream(this.localScreensharingStream);
return false;
Expand All @@ -1167,9 +1234,11 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
const track = stream.getTracks().find((track) => {
return track.kind === "video";
});
const sender = this.usermediaSenders.find((sender) => {
return sender.track?.kind === "video";
});

const sender = this.transceivers[getTransceiverIndex(
SDPStreamMetadataPurpose.Usermedia, "video",
)].sender;

sender.replaceTrack(track);

this.pushNewLocalFeed(stream, SDPStreamMetadataPurpose.Screenshare, false);
Expand All @@ -1183,9 +1252,9 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
const track = this.localUsermediaStream.getTracks().find((track) => {
return track.kind === "video";
});
const sender = this.usermediaSenders.find((sender) => {
return sender.track?.kind === "video";
});
const sender = this.transceivers[getTransceiverIndex(
SDPStreamMetadataPurpose.Usermedia, "video",
)].sender;
sender.replaceTrack(track);

this.client.getMediaHandler().stopScreensharingStream(this.localScreensharingStream);
Expand Down Expand Up @@ -1219,14 +1288,10 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
this.localUsermediaStream.addTrack(track);
}

const newSenders = [];

for (const track of stream.getTracks()) {
const oldSender = this.usermediaSenders.find((sender) => {
return sender.track?.kind === track.kind;
});
const tIdx = getTransceiverIndex(SDPStreamMetadataPurpose.Usermedia, track.kind);

let newSender: RTCRtpSender;
const oldSender = this.transceivers[tIdx].sender;

try {
logger.info(
Expand All @@ -1239,7 +1304,6 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
`) to peer connection`,
);
await oldSender.replaceTrack(track);
newSender = oldSender;
} catch (error) {
logger.info(
`Call ${this.callId} `+
Expand All @@ -1250,13 +1314,13 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
`streamPurpose="${callFeed.purpose}"` +
`) to peer connection`,
);
newSender = this.peerConn.addTrack(track, this.localUsermediaStream);
}

newSenders.push(newSender);
this.transceivers[tIdx] = this.peerConn.addTransceiver(track, {
streams: [this.localUsermediaStream],
direction: "sendrecv",
});
}
}

this.usermediaSenders = newSenders;
}

/**
Expand Down Expand Up @@ -1916,8 +1980,10 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
// clunky because TypeScript can't follow the types through if we use an expression as the key
if (this.state === CallState.CreateOffer) {
content.offer = this.peerConn.localDescription?.toJSON();
logger.debug(`offer is`, content.offer);
} else {
content.description = this.peerConn.localDescription?.toJSON();
logger.debug(`description is`, content.description);
}

content.capabilities = {
Expand Down Expand Up @@ -2109,17 +2175,10 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
}
}

for (const trans of this.peerConn.getTransceivers()) {
if (
this.screensharingSenders.includes(trans.sender) &&
(
trans.sender.track?.kind === "video" ||
trans.receiver.track?.kind === "video"
)
) {
trans.setCodecPreferences(codecs);
}
}
const screenshareVideoTransceiver = this.transceivers[getTransceiverIndex(
SDPStreamMetadataPurpose.Screenshare, "video",
)];
if (screenshareVideoTransceiver) screenshareVideoTransceiver.setCodecPreferences(codecs);
}

private onNegotiationNeeded = async (): Promise<void> => {
Expand Down
4 changes: 3 additions & 1 deletion src/webrtc/groupCall.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,9 @@ export class GroupCall extends TypedEventEmitter<
return false;
}
} else {
await Promise.all(this.calls.map(call => call.removeLocalFeed(call.localScreensharingFeed)));
await Promise.all(this.calls.map(call => {
if (call.localScreensharingFeed) call.removeLocalFeed(call.localScreensharingFeed);
}));
this.client.getMediaHandler().stopScreensharingStream(this.localScreenshareFeed.stream);
this.removeScreenshareFeed(this.localScreenshareFeed);
this.localScreenshareFeed = undefined;
Expand Down

0 comments on commit 92efd30

Please sign in to comment.