Skip to content

Commit

Permalink
VBLOCKS-1269 | Don't dispatch deviceinfochange indefinitely (twilio#155)
Browse files Browse the repository at this point in the history
* Don't unnecessary emit deviceinfochange

* Exporting mediadevices properly

* Only init nativeMediaDevices before instantiation

* Lint

* Properly initing

* Tests

* Changelog

* Adding todo note

* Review comments
  • Loading branch information
charliesantos authored Apr 5, 2023
1 parent e9d5dd8 commit 364a4b2
Show file tree
Hide file tree
Showing 5 changed files with 315 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Bug Fixes

- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/14) where `device.audio.disconnect`, `device.audio.incoming` and `device.audio.outgoing` do not have the correct type definitions.

- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/126) where the internal `deviceinfochange` event is being emitted indefinitely, causing high cpu usage.

2.3.2 (February 27, 2023)
===================

Expand Down
4 changes: 2 additions & 2 deletions lib/twilio/audiohelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Device from './device';
import { InvalidArgumentError, NotSupportedError } from './errors';
import Log from './log';
import OutputDeviceCollection from './outputdevicecollection';
import * as defaultMediaDevices from './shims/mediadevices';
import * as getMediaDevicesInstance from './shims/mediadevices';
import { average, difference, isFirefox } from './util';

const MediaDeviceInfoShim = require('./shims/mediadeviceinfo');
Expand Down Expand Up @@ -172,7 +172,7 @@ class AudioHelper extends EventEmitter {
}, options);

this._getUserMedia = getUserMedia;
this._mediaDevices = options.mediaDevices || defaultMediaDevices;
this._mediaDevices = options.mediaDevices || getMediaDevicesInstance();
this._onActiveInputChanged = onActiveInputChanged;

const isAudioContextSupported: boolean = !!(options.AudioContext || options.audioContext);
Expand Down
42 changes: 30 additions & 12 deletions lib/twilio/shims/mediadevices.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const EventTarget = require('./eventtarget');

const POLL_INTERVAL_MS = 500;

const nativeMediaDevices = typeof navigator !== 'undefined' && navigator.mediaDevices;
let nativeMediaDevices = null;

/**
* Make a custom MediaDevices object, and proxy through existing functionality. If
Expand Down Expand Up @@ -40,7 +40,7 @@ class MediaDevicesShim extends EventTarget {

if (typeof nativeMediaDevices.enumerateDevices === 'function') {
nativeMediaDevices.enumerateDevices().then(devices => {
devices.sort(sortDevicesById).forEach([].push, knownDevices);
devices.sort(sortDevicesById).forEach(d => knownDevices.push(d));
});
}

Expand All @@ -49,6 +49,7 @@ class MediaDevicesShim extends EventTarget {
return;
}

// TODO: Remove polling in the next major release.
this._pollInterval = this._pollInterval
|| setInterval(sampleDevices.bind(null, this), POLL_INTERVAL_MS);
}.bind(this));
Expand All @@ -62,22 +63,38 @@ class MediaDevicesShim extends EventTarget {
}
}

if (nativeMediaDevices && typeof nativeMediaDevices.enumerateDevices === 'function') {
MediaDevicesShim.prototype.enumerateDevices = function enumerateDevices() {
MediaDevicesShim.prototype.enumerateDevices = function enumerateDevices() {
if (nativeMediaDevices && typeof nativeMediaDevices.enumerateDevices === 'function') {
return nativeMediaDevices.enumerateDevices(...arguments);
};
}
}
return null;
};

MediaDevicesShim.prototype.getUserMedia = function getUserMedia() {
return nativeMediaDevices.getUserMedia(...arguments);
};

function deviceInfosHaveChanged(newDevices, oldDevices) {
const oldLabels = oldDevices.reduce((map, device) => map.set(device.deviceId, device.label || null), new Map());
newDevices = newDevices.filter(d => d.kind === 'audioinput' || d.kind === 'audiooutput');
oldDevices = oldDevices.filter(d => d.kind === 'audioinput' || d.kind === 'audiooutput');

// On certain browsers, we cannot use deviceId as a key for comparison.
// It's missing along with the device label if the customer has not granted permission.
// The following checks whether some old devices have empty labels and if they are now available.
// This means, the user has granted permissions and the device info have changed.
if (oldDevices.some(d => !d.deviceId) &&
newDevices.some(d => !!d.deviceId)) {
return true;
}

// Use both deviceId and "kind" to create a unique key
// since deviceId is not unique across different kinds of devices.
const oldLabels = oldDevices.reduce((map, device) =>
map.set(`${device.deviceId}-${device.kind}`, device.label), new Map());

return newDevices.some(newDevice => {
const oldLabel = oldLabels.get(newDevice.deviceId);
return typeof oldLabel !== 'undefined' && oldLabel !== newDevice.label;
return newDevices.some(device => {
const oldLabel = oldLabels.get(`${device.deviceId}-${device.kind}`);
return typeof oldLabel !== 'undefined' && oldLabel !== device.label;
});
}

Expand Down Expand Up @@ -164,6 +181,7 @@ function sortDevicesById(a, b) {
return a.deviceId < b.deviceId;
}

module.exports = (function shimMediaDevices() {
module.exports = () => {
nativeMediaDevices = typeof navigator !== 'undefined' ? navigator.mediaDevices : null;
return nativeMediaDevices ? new MediaDevicesShim() : null;
})();
};
2 changes: 2 additions & 0 deletions tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,5 @@ require('./unit/error');
require('./unit/log');
require('./unit/regions');
require('./unit/uuid');

require('./shims/mediadevices');
279 changes: 279 additions & 0 deletions tests/shims/mediadevices.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,279 @@
const assert = require('assert');
const sinon = require('sinon');
const getMediaDevicesInstance = require('../../lib/twilio/shims/mediadevices');

describe('MediaDevicesShim', () => {
const userMediaStream = 'USER-STREAM';

let clock;
let globalMediaDevices;
let getDevices;
let mediaDevices;
let mediaDeviceList;
let nativeMediaDevices;

const sampleDevices = async () => {
await clock.tickAsync(500);
};

beforeEach(() => {
clock = sinon.useFakeTimers(Date.now());

mediaDeviceList = [
{ deviceId: 'id1', kind: 'audioinput', label: 'label1' },
{ deviceId: 'id2', kind: 'audiooutput', label: 'label2' },
{ deviceId: 'id3', kind: 'videoinput', label: 'label3' },
{ deviceId: 'id4', kind: 'videooutput', label: 'label4' },
];

// Always return a deep copy
getDevices = () => new Promise(res => res(mediaDeviceList.map(d => ({ ...d }))));

nativeMediaDevices = {
enumerateDevices: sinon.stub().callsFake(getDevices),
getUserMedia: sinon.stub().returns(Promise.resolve(userMediaStream)),
};

globalMediaDevices = global.navigator.mediaDevices;
global.navigator.mediaDevices = nativeMediaDevices;

mediaDevices = getMediaDevicesInstance();
});

afterEach(() => {
global.navigator.mediaDevices = globalMediaDevices;
clock.restore();
});

describe('.enumerateDevices()', () => {
it('should call native enumerateDevices properly', async () => {
sinon.assert.calledOnce(nativeMediaDevices.enumerateDevices);
const devices = await mediaDevices.enumerateDevices();
sinon.assert.calledTwice(nativeMediaDevices.enumerateDevices);
assert.deepStrictEqual(devices, mediaDeviceList);
});

it('should return null if the browser does not support enumerateDevices', async () => {
nativeMediaDevices.enumerateDevices = null;
const devices = await mediaDevices.enumerateDevices();
assert.deepStrictEqual(devices, null);
});
});

describe('.getUserMedia()', () => {
it('should call native getUserMedia properly', async () => {
const stream = await mediaDevices.getUserMedia({ foo: 'foo' });
sinon.assert.calledOnce(nativeMediaDevices.getUserMedia);
sinon.assert.calledWithExactly(nativeMediaDevices.getUserMedia, { foo: 'foo' })
assert.strictEqual(stream, userMediaStream);
});
});

describe('#devicechange', () => {
let callback;

beforeEach(async () => {
callback = sinon.stub();
mediaDevices.addEventListener('devicechange', callback);
await sampleDevices();
});

it('should stop polling after removing listeners', async () => {
const existingCallCount = nativeMediaDevices.enumerateDevices.callCount;
mediaDevices.removeEventListener('devicechange', callback);
sinon.assert.callCount(nativeMediaDevices.enumerateDevices, existingCallCount);
await sampleDevices();
sinon.assert.callCount(nativeMediaDevices.enumerateDevices, existingCallCount);
});

it('should not emit the first time', async () => {
sinon.assert.notCalled(callback);
});

it('should emit once if a new device is added', async () => {
mediaDeviceList.push({ deviceId: 'id5', kind: 'audioinput', label: 'label5' });
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

it('should emit once if a device is removed', async () => {
mediaDeviceList.pop();
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

it('should emit once if a device is removed and a new device is added', async () => {
mediaDeviceList.pop();
mediaDeviceList.push({ deviceId: 'id5', kind: 'audioinput', label: 'label5' });
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

describe('when native event is supported', () => {
const setup = async () => {
nativeMediaDevices.ondevicechange = null;
mediaDevices = getMediaDevicesInstance();
callback = sinon.stub();
mediaDevices.addEventListener('devicechange', callback);
await sampleDevices();
};

beforeEach(async () => await setup());

it('should not emit manually', async () => {
mediaDeviceList.pop();
await sampleDevices();
sinon.assert.notCalled(callback);
});

it('should reemit if addEventListener is not supported', async () => {
await sampleDevices();
nativeMediaDevices.ondevicechange({ type: 'devicechange' });
sinon.assert.calledOnce(callback);
});

it('should reemit if addEventListener is supported', async () => {
nativeMediaDevices.addEventListener = (eventName, dispatchEvent) => {
nativeMediaDevices[`_emit${eventName}`] = () => dispatchEvent({ type: eventName });
};
await setup();
await sampleDevices();
nativeMediaDevices._emitdevicechange();
sinon.assert.calledOnce(callback);
});
});
});

describe('#deviceinfochange', () => {
let callback;

const setup = async () => {
callback = sinon.stub();
mediaDevices = getMediaDevicesInstance();
mediaDevices.addEventListener('deviceinfochange', callback);
await sampleDevices();
};

beforeEach(async () => {
mediaDeviceList.forEach(d => d.label = '');
await setup();
});

it('should stop polling after removing listeners', async () => {
const existingCallCount = nativeMediaDevices.enumerateDevices.callCount;
mediaDevices.removeEventListener('deviceinfochange', callback);
sinon.assert.callCount(nativeMediaDevices.enumerateDevices, existingCallCount);
await sampleDevices();
sinon.assert.callCount(nativeMediaDevices.enumerateDevices, existingCallCount);
});

it('should not emit the first time', async () => {
sinon.assert.notCalled(callback);
});

it('should emit once when device labels become available', async () => {
mediaDeviceList.forEach((d, i) => d.label = `label${i}`);
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

it('should emit once when only audioinput or audiooutput device labels become available', async () => {
mediaDeviceList.forEach((d, i) => {
if (d.kind === 'audioinput' || d.kind === 'audiooutput') {
d.label = `label${i}`;
}
});
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

it('should emit once when there are duplicate ids across different types of devices', async () => {
mediaDeviceList.forEach((d, i) => {
d.deviceId = 'foo';
d.label = '';
});
await setup();
mediaDeviceList.forEach((d, i) => {
d.deviceId = 'foo';
d.label = `label${i}`;
});
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

it('should not emit when device labels become available for a videoinput or videooutput device', async () => {
mediaDeviceList.forEach((d, i) => {
if (d.kind === 'videoinput' || d.kind === 'videooutput') {
d.label = `label${i}`;
}
});
await sampleDevices();
sinon.assert.notCalled(callback);
await sampleDevices();
sinon.assert.notCalled(callback);
});

it('should emit once when ids are all empty initially', async () => {
mediaDeviceList.forEach((d, i) => {
d.deviceId = '';
d.label = '';
});
await setup();
mediaDeviceList.forEach((d, i) => {
d.deviceId = `id${i}`;
d.label = `label${i}`;
});
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
await sampleDevices();
sinon.assert.calledOnce(callback);
});

describe('when native event is supported', () => {
const setupForNative = async () => {
nativeMediaDevices.ondeviceinfochange = null;
await setup();
};

beforeEach(async () => await setupForNative());

it('should not emit manually', async () => {
mediaDeviceList.forEach((d, i) => d.label = `label${i}`);
await sampleDevices();
sinon.assert.notCalled(callback);
});

it('should reemit if addEventListener is not supported', async () => {
await sampleDevices();
nativeMediaDevices.ondeviceinfochange({ type: 'deviceinfochange' });
sinon.assert.calledOnce(callback);
});

it('should reemit if addEventListener is supported', async () => {
nativeMediaDevices.addEventListener = (eventName, dispatchEvent) => {
nativeMediaDevices[`_emit${eventName}`] = () => dispatchEvent({ type: eventName });
};
await setupForNative();
await sampleDevices();
nativeMediaDevices._emitdeviceinfochange();
sinon.assert.calledOnce(callback);
});
});
});
});

0 comments on commit 364a4b2

Please sign in to comment.