Skip to content

Commit 1dee1ba

Browse files
authored
Fix media switching during legacy calls (#5069)
* Specify exact for deviceId * Update mediaHandler.spec.ts * fallback to ideal if exact fails * Reduce cognitive complexity for sonar * Add tests
1 parent b274c74 commit 1dee1ba

File tree

2 files changed

+120
-29
lines changed

2 files changed

+120
-29
lines changed

spec/unit/webrtc/mediaHandler.spec.ts

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ describe("Media Handler", function () {
6161
expect(mockMediaDevices.getUserMedia).toHaveBeenCalledWith(
6262
expect.objectContaining({
6363
audio: expect.objectContaining({
64-
deviceId: { ideal: FAKE_AUDIO_INPUT_ID },
64+
deviceId: { exact: FAKE_AUDIO_INPUT_ID },
6565
}),
6666
video: expect.objectContaining({
67-
deviceId: { ideal: FAKE_VIDEO_INPUT_ID },
67+
deviceId: { exact: FAKE_VIDEO_INPUT_ID },
6868
}),
6969
}),
7070
);
@@ -77,7 +77,7 @@ describe("Media Handler", function () {
7777
expect(mockMediaDevices.getUserMedia).toHaveBeenCalledWith(
7878
expect.objectContaining({
7979
audio: expect.objectContaining({
80-
deviceId: { ideal: FAKE_AUDIO_INPUT_ID },
80+
deviceId: { exact: FAKE_AUDIO_INPUT_ID },
8181
}),
8282
}),
8383
);
@@ -109,7 +109,7 @@ describe("Media Handler", function () {
109109
expect(mockMediaDevices.getUserMedia).toHaveBeenCalledWith(
110110
expect.objectContaining({
111111
video: expect.objectContaining({
112-
deviceId: { ideal: FAKE_VIDEO_INPUT_ID },
112+
deviceId: { exact: FAKE_VIDEO_INPUT_ID },
113113
}),
114114
}),
115115
);
@@ -122,10 +122,10 @@ describe("Media Handler", function () {
122122
expect(mockMediaDevices.getUserMedia).toHaveBeenCalledWith(
123123
expect.objectContaining({
124124
audio: expect.objectContaining({
125-
deviceId: { ideal: FAKE_AUDIO_INPUT_ID },
125+
deviceId: { exact: FAKE_AUDIO_INPUT_ID },
126126
}),
127127
video: expect.objectContaining({
128-
deviceId: { ideal: FAKE_VIDEO_INPUT_ID },
128+
deviceId: { exact: FAKE_VIDEO_INPUT_ID },
129129
}),
130130
}),
131131
);
@@ -331,6 +331,80 @@ describe("Media Handler", function () {
331331

332332
expect(stream.getVideoTracks().length).toEqual(0);
333333
});
334+
335+
it("falls back to ideal deviceId when exact deviceId fails", async () => {
336+
// First call with exact should fail
337+
mockMediaDevices.getUserMedia
338+
.mockRejectedValueOnce(new Error("OverconstrainedError"))
339+
.mockImplementation((constraints: MediaStreamConstraints) => {
340+
const stream = new MockMediaStream("local_stream");
341+
if (constraints.audio) {
342+
const track = new MockMediaStreamTrack("audio_track", "audio");
343+
track.settings = { deviceId: FAKE_AUDIO_INPUT_ID };
344+
stream.addTrack(track);
345+
}
346+
return Promise.resolve(stream.typed());
347+
});
348+
349+
const stream = await mediaHandler.getUserMediaStream(true, false);
350+
351+
// Should have been called twice: once with exact, once with ideal
352+
expect(mockMediaDevices.getUserMedia).toHaveBeenCalledTimes(2);
353+
expect(mockMediaDevices.getUserMedia).toHaveBeenNthCalledWith(
354+
1,
355+
expect.objectContaining({
356+
audio: expect.objectContaining({
357+
deviceId: { exact: FAKE_AUDIO_INPUT_ID },
358+
}),
359+
}),
360+
);
361+
expect(mockMediaDevices.getUserMedia).toHaveBeenNthCalledWith(
362+
2,
363+
expect.objectContaining({
364+
audio: expect.objectContaining({
365+
deviceId: { ideal: FAKE_AUDIO_INPUT_ID },
366+
}),
367+
}),
368+
);
369+
expect(stream).toBeTruthy();
370+
});
371+
372+
it("falls back to ideal deviceId for video when exact fails", async () => {
373+
// First call with exact should fail
374+
mockMediaDevices.getUserMedia
375+
.mockRejectedValueOnce(new Error("OverconstrainedError"))
376+
.mockImplementation((constraints: MediaStreamConstraints) => {
377+
const stream = new MockMediaStream("local_stream");
378+
if (constraints.video) {
379+
const track = new MockMediaStreamTrack("video_track", "video");
380+
track.settings = { deviceId: FAKE_VIDEO_INPUT_ID };
381+
stream.addTrack(track);
382+
}
383+
return Promise.resolve(stream.typed());
384+
});
385+
386+
const stream = await mediaHandler.getUserMediaStream(false, true);
387+
388+
// Should have been called twice: once with exact, once with ideal
389+
expect(mockMediaDevices.getUserMedia).toHaveBeenCalledTimes(2);
390+
expect(mockMediaDevices.getUserMedia).toHaveBeenNthCalledWith(
391+
1,
392+
expect.objectContaining({
393+
video: expect.objectContaining({
394+
deviceId: { exact: FAKE_VIDEO_INPUT_ID },
395+
}),
396+
}),
397+
);
398+
expect(mockMediaDevices.getUserMedia).toHaveBeenNthCalledWith(
399+
2,
400+
expect.objectContaining({
401+
video: expect.objectContaining({
402+
deviceId: { ideal: FAKE_VIDEO_INPUT_ID },
403+
}),
404+
}),
405+
);
406+
expect(stream).toBeTruthy();
407+
});
334408
});
335409

336410
describe("getScreensharingStream", () => {

src/webrtc/mediaHandler.ts

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,19 @@ export class MediaHandler extends TypedEventEmitter<
264264
}
265265

266266
if (!canReuseStream) {
267-
const constraints = this.getUserMediaContraints(shouldRequestAudio, shouldRequestVideo);
268-
stream = await navigator.mediaDevices.getUserMedia(constraints);
267+
let constraints: MediaStreamConstraints;
268+
try {
269+
// Not specifying exact for deviceId means switching devices does not always work,
270+
// try with exact and fallback to ideal if it fails
271+
constraints = this.getUserMediaContraints(shouldRequestAudio, shouldRequestVideo, true);
272+
stream = await navigator.mediaDevices.getUserMedia(constraints);
273+
} catch (e) {
274+
logger.warn(
275+
`MediaHandler getUserMediaStreamInternal() error (e=${e}), retrying without exact deviceId`,
276+
);
277+
constraints = this.getUserMediaContraints(shouldRequestAudio, shouldRequestVideo, false);
278+
stream = await navigator.mediaDevices.getUserMedia(constraints);
279+
}
269280
logger.log(
270281
`MediaHandler getUserMediaStreamInternal() calling getUserMediaStream (streamId=${
271282
stream.id
@@ -435,30 +446,36 @@ export class MediaHandler extends TypedEventEmitter<
435446
this.emit(MediaHandlerEvent.LocalStreamsChanged);
436447
}
437448

438-
private getUserMediaContraints(audio: boolean, video: boolean): MediaStreamConstraints {
449+
private getUserMediaContraints(audio: boolean, video: boolean, exactDeviceId?: boolean): MediaStreamConstraints {
439450
const isWebkit = !!navigator.webkitGetUserMedia;
451+
const deviceIdKey = exactDeviceId ? "exact" : "ideal";
452+
453+
const audioConstraints: MediaTrackConstraints = {};
454+
if (this.audioInput) {
455+
audioConstraints.deviceId = { [deviceIdKey]: this.audioInput };
456+
}
457+
if (this.audioSettings) {
458+
audioConstraints.autoGainControl = { ideal: this.audioSettings.autoGainControl };
459+
audioConstraints.echoCancellation = { ideal: this.audioSettings.echoCancellation };
460+
audioConstraints.noiseSuppression = { ideal: this.audioSettings.noiseSuppression };
461+
}
462+
463+
const videoConstraints: MediaTrackConstraints = {
464+
/* We want 640x360. Chrome will give it only if we ask exactly,
465+
FF refuses entirely if we ask exactly, so have to ask for ideal
466+
instead
467+
XXX: Is this still true?
468+
*/
469+
width: isWebkit ? { exact: 640 } : { ideal: 640 },
470+
height: isWebkit ? { exact: 360 } : { ideal: 360 },
471+
};
472+
if (this.videoInput) {
473+
videoConstraints.deviceId = { [deviceIdKey]: this.videoInput };
474+
}
440475

441476
return {
442-
audio: audio
443-
? {
444-
deviceId: this.audioInput ? { ideal: this.audioInput } : undefined,
445-
autoGainControl: this.audioSettings ? { ideal: this.audioSettings.autoGainControl } : undefined,
446-
echoCancellation: this.audioSettings ? { ideal: this.audioSettings.echoCancellation } : undefined,
447-
noiseSuppression: this.audioSettings ? { ideal: this.audioSettings.noiseSuppression } : undefined,
448-
}
449-
: false,
450-
video: video
451-
? {
452-
deviceId: this.videoInput ? { ideal: this.videoInput } : undefined,
453-
/* We want 640x360. Chrome will give it only if we ask exactly,
454-
FF refuses entirely if we ask exactly, so have to ask for ideal
455-
instead
456-
XXX: Is this still true?
457-
*/
458-
width: isWebkit ? { exact: 640 } : { ideal: 640 },
459-
height: isWebkit ? { exact: 360 } : { ideal: 360 },
460-
}
461-
: false,
477+
audio: audio ? audioConstraints : false,
478+
video: video ? videoConstraints : false,
462479
};
463480
}
464481

0 commit comments

Comments
 (0)