From 096f7ad1a548a1d5141c746fa72e4d4b2e251901 Mon Sep 17 00:00:00 2001 From: charliesantos Date: Fri, 15 Sep 2023 13:27:41 -0700 Subject: [PATCH 1/4] VBLOCKS-2214 | Remove unnecessary enumerateDevices calls --- lib/twilio/audiohelper.ts | 71 ++++---- lib/twilio/device.ts | 10 +- lib/twilio/shims/mediadevices.ts | 190 --------------------- tests/audiohelper.js | 46 +---- tests/index.ts | 2 - tests/shims/mediadevices.js | 279 ------------------------------- tests/unit/device.ts | 13 +- 7 files changed, 60 insertions(+), 551 deletions(-) delete mode 100644 lib/twilio/shims/mediadevices.ts delete mode 100644 tests/shims/mediadevices.js diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 5b37b694..e3011b5b 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -8,7 +8,6 @@ import { InvalidArgumentError, NotSupportedError } from './errors'; import Log from './log'; import OutputDeviceCollection from './outputdevicecollection'; import MediaDeviceInfoShim from './shims/mediadeviceinfo'; -import getMediaDevicesInstance from './shims/mediadevices'; import { average, difference, isFirefox } from './util'; /** @@ -176,11 +175,11 @@ class AudioHelper extends EventEmitter { }, options); this._getUserMedia = getUserMedia; - this._mediaDevices = options.mediaDevices || getMediaDevicesInstance() as AudioHelper.MediaDevicesLike; + this._mediaDevices = options.mediaDevices || navigator.mediaDevices; this._onActiveInputChanged = onActiveInputChanged; this._enumerateDevices = typeof options.enumerateDevices === 'function' ? options.enumerateDevices - : this._mediaDevices && this._mediaDevices.enumerateDevices; + : this._mediaDevices && this._mediaDevices.enumerateDevices.bind(this._mediaDevices); const isAudioContextSupported: boolean = !!(options.AudioContext || options.audioContext); const isEnumerationSupported: boolean = !!this._enumerateDevices; @@ -308,10 +307,41 @@ class AudioHelper extends EventEmitter { if (this._mediaDevices.removeEventListener) { this._mediaDevices.removeEventListener('devicechange', this._updateAvailableDevices); - this._mediaDevices.removeEventListener('deviceinfochange', this._updateAvailableDevices); } } + /** + * Update the available input and output devices + * @private + */ + _updateAvailableDevices = (): Promise => { + if (!this._mediaDevices || !this._enumerateDevices) { + return Promise.reject('Enumeration not supported'); + } + + return this._enumerateDevices().then((devices: MediaDeviceInfo[]) => { + this._updateDevices(devices.filter((d: MediaDeviceInfo) => d.kind === 'audiooutput'), + this.availableOutputDevices, + this._removeLostOutput); + + this._updateDevices(devices.filter((d: MediaDeviceInfo) => d.kind === 'audioinput'), + this.availableInputDevices, + this._removeLostInput); + + const defaultDevice = this.availableOutputDevices.get('default') + || Array.from(this.availableOutputDevices.values())[0]; + + [this.speakerDevices, this.ringtoneDevices].forEach(outputDevices => { + if (!outputDevices.get().size && this.availableOutputDevices.size && this.isOutputSelectionSupported) { + outputDevices.set(defaultDevice.deviceId) + .catch((reason) => { + this._log.warn(`Unable to set audio output devices. ${reason}`); + }); + } + }); + }); + } + /** * Enable or disable the disconnect sound. * @param doEnable Passing `true` will enable the sound and `false` will disable the sound. @@ -426,7 +456,6 @@ class AudioHelper extends EventEmitter { if (this._mediaDevices.addEventListener) { this._mediaDevices.addEventListener('devicechange', this._updateAvailableDevices); - this._mediaDevices.addEventListener('deviceinfochange', this._updateAvailableDevices); } this._updateAvailableDevices().then(() => { @@ -542,37 +571,6 @@ class AudioHelper extends EventEmitter { }); } - /** - * Update the available input and output devices - */ - private _updateAvailableDevices = (): Promise => { - if (!this._mediaDevices || !this._enumerateDevices) { - return Promise.reject('Enumeration not supported'); - } - - return this._enumerateDevices().then((devices: MediaDeviceInfo[]) => { - this._updateDevices(devices.filter((d: MediaDeviceInfo) => d.kind === 'audiooutput'), - this.availableOutputDevices, - this._removeLostOutput); - - this._updateDevices(devices.filter((d: MediaDeviceInfo) => d.kind === 'audioinput'), - this.availableInputDevices, - this._removeLostInput); - - const defaultDevice = this.availableOutputDevices.get('default') - || Array.from(this.availableOutputDevices.values())[0]; - - [this.speakerDevices, this.ringtoneDevices].forEach(outputDevices => { - if (!outputDevices.get().size && this.availableOutputDevices.size && this.isOutputSelectionSupported) { - outputDevices.set(defaultDevice.deviceId) - .catch((reason) => { - this._log.warn(`Unable to set audio output devices. ${reason}`); - }); - } - }); - }); - } - /** * Update a set of devices. * @param updatedDevices - An updated list of available Devices @@ -680,7 +678,6 @@ namespace AudioHelper { * that were lost as a result of this deviceChange event. * @example `device.audio.on('deviceChange', lostActiveDevices => { })` * @event - * @private */ declare function deviceChangeEvent(lostActiveDevices: MediaDeviceInfo[]): void; diff --git a/lib/twilio/device.ts b/lib/twilio/device.ts index c41404a2..1d058636 100644 --- a/lib/twilio/device.ts +++ b/lib/twilio/device.ts @@ -939,7 +939,15 @@ class Device extends EventEmitter { const config: Call.Config = { audioHelper: this._audio, - getUserMedia: this._options.getUserMedia || getUserMedia, + getUserMedia: (...args) => + (this._options.getUserMedia || getUserMedia)(...args) + .then((gumResponse: any) => { + this._audio?._updateAvailableDevices().catch(error => { + // Ignore error, we don't want to break the call flow + this._log.warn('Unable to updateAvailableDevices after gUM call', error); + }); + return Promise.resolve(gumResponse); + }), isUnifiedPlanDefault: Device._isUnifiedPlanDefault, onIgnore: (): void => { this._soundcache.get(Device.SoundName.Incoming).stop(); diff --git a/lib/twilio/shims/mediadevices.ts b/lib/twilio/shims/mediadevices.ts deleted file mode 100644 index 212836ad..00000000 --- a/lib/twilio/shims/mediadevices.ts +++ /dev/null @@ -1,190 +0,0 @@ -/** - * @packageDocumentation - * @module Voice - * @internalapi - */ -// @ts-nocheck - -import EventTarget from './eventtarget'; - -const POLL_INTERVAL_MS = 500; - -let nativeMediaDevices = null; - -/** - * Make a custom MediaDevices object, and proxy through existing functionality. If - * devicechange is present, we simply reemit the event. If not, we will do the - * detection ourselves and fire the event when necessary. The same logic exists - * for deviceinfochange for consistency, however deviceinfochange is our own event - * so it is unlikely that it will ever be native. The w3c spec for devicechange - * is unclear as to whether MediaDeviceInfo changes (such as label) will - * trigger the devicechange event. We have an open question on this here: - * https://bugs.chromium.org/p/chromium/issues/detail?id=585096 - */ -class MediaDevicesShim extends EventTarget { - constructor() { - super(); - - this._defineEventHandler('devicechange'); - this._defineEventHandler('deviceinfochange'); - - const knownDevices = []; - Object.defineProperties(this, { - _deviceChangeIsNative: { value: reemitNativeEvent(this, 'devicechange') }, - _deviceInfoChangeIsNative: { value: reemitNativeEvent(this, 'deviceinfochange') }, - _knownDevices: { value: knownDevices }, - _pollInterval: { - value: null, - writable: true, - }, - }); - - if (typeof nativeMediaDevices.enumerateDevices === 'function') { - nativeMediaDevices.enumerateDevices().then(devices => { - devices.sort(sortDevicesById).forEach(d => knownDevices.push(d)); - }); - } - - this._eventEmitter.on('newListener', function maybeStartPolling(eventName) { - if (eventName !== 'devicechange' && eventName !== 'deviceinfochange') { - return; - } - - // TODO: Remove polling in the next major release. - this._pollInterval = this._pollInterval - || setInterval(sampleDevices.bind(null, this), POLL_INTERVAL_MS); - }.bind(this)); - - this._eventEmitter.on('removeListener', function maybeStopPolling() { - if (this._pollInterval && !hasChangeListeners(this)) { - clearInterval(this._pollInterval); - this._pollInterval = null; - } - }.bind(this)); - } -} - -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) { - 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(device => { - const oldLabel = oldLabels.get(`${device.deviceId}-${device.kind}`); - return typeof oldLabel !== 'undefined' && oldLabel !== device.label; - }); -} - -function devicesHaveChanged(newDevices, oldDevices) { - return newDevices.length !== oldDevices.length - || propertyHasChanged('deviceId', newDevices, oldDevices); -} - -function hasChangeListeners(mediaDevices) { - return ['devicechange', 'deviceinfochange'].reduce((count, event) => count + mediaDevices._eventEmitter.listenerCount(event), 0) > 0; -} - -/** - * Sample the current set of devices and emit devicechange event if a device has been - * added or removed, and deviceinfochange if a device's label has changed. - * @param {MediaDevicesShim} mediaDevices - * @private - */ -function sampleDevices(mediaDevices) { - nativeMediaDevices.enumerateDevices().then(newDevices => { - const knownDevices = mediaDevices._knownDevices; - const oldDevices = knownDevices.slice(); - - // Replace known devices in-place - [].splice.apply(knownDevices, [0, knownDevices.length] - .concat(newDevices.sort(sortDevicesById))); - - if (!mediaDevices._deviceChangeIsNative - && devicesHaveChanged(knownDevices, oldDevices)) { - mediaDevices.dispatchEvent(new Event('devicechange')); - } - - if (!mediaDevices._deviceInfoChangeIsNative - && deviceInfosHaveChanged(knownDevices, oldDevices)) { - mediaDevices.dispatchEvent(new Event('deviceinfochange')); - } - }); -} - -/** - * Accepts two sorted arrays and the name of a property to compare on objects from each. - * Arrays should also be of the same length. - * @param {string} propertyName - Name of the property to compare on each object - * @param {Array} as - The left-side array of objects to compare. - * @param {Array} bs - The right-side array of objects to compare. - * @private - * @returns {boolean} True if the property of any object in array A is different than - * the same property of its corresponding object in array B. - */ -function propertyHasChanged(propertyName, as, bs) { - return as.some((a, i) => a[propertyName] !== bs[i][propertyName]); -} - -/** - * Re-emit the native event, if the native mediaDevices has the corresponding property. - * @param {MediaDevicesShim} mediaDevices - * @param {string} eventName - Name of the event - * @private - * @returns {boolean} Whether the native mediaDevice had the corresponding property - */ -function reemitNativeEvent(mediaDevices, eventName) { - const methodName = `on${eventName}`; - - function dispatchEvent(event) { - mediaDevices.dispatchEvent(event); - } - - if (methodName in nativeMediaDevices) { - // Use addEventListener if it's available so we don't stomp on any other listeners - // for this event. Currently, navigator.mediaDevices.addEventListener does not exist in Safari. - if ('addEventListener' in nativeMediaDevices) { - nativeMediaDevices.addEventListener(eventName, dispatchEvent); - } else { - nativeMediaDevices[methodName] = dispatchEvent; - } - - return true; - } - - return false; -} - -function sortDevicesById(a, b) { - return a.deviceId < b.deviceId; -} - -const getMediaDevicesInstance = () => { - nativeMediaDevices = typeof navigator !== 'undefined' ? navigator.mediaDevices : null; - return nativeMediaDevices ? new MediaDevicesShim() : null; -}; - -export default getMediaDevicesInstance; diff --git a/tests/audiohelper.js b/tests/audiohelper.js index 54699418..050b887b 100644 --- a/tests/audiohelper.js +++ b/tests/audiohelper.js @@ -15,7 +15,11 @@ describe('AudioHelper', () => { let oldNavigator; beforeEach(() => { - audio = new AudioHelper(noop, noop, getUserMedia, { mediaDevices: {} }); + audio = new AudioHelper(noop, noop, getUserMedia, { mediaDevices: { + enumerateDevices: function(){ + return Promise.resolve(); + } + } }); }); before(() => { @@ -505,45 +509,5 @@ describe('AudioHelper', () => { }))); }); }); - - describe('mediaDevices:deviceinfochange', () => { - let audio; - const wait = () => new Promise(r => setTimeout(r, 0)); - const noop = () => {}; - - beforeEach(() => { - audio = new AudioHelper(noop, noop, getUserMedia, { - mediaDevices, - setSinkId: () => {} - }); - audio.speakerDevices = {}; - audio.ringtoneDevices = { - get: () => ({ size: 1 }), - set: sinon.stub(), - }; - audio.speakerDevices = { - get: () => ({ size: 0 }), - set: sinon.stub().rejects('foobar'), - }; - audio._log.warn = sinon.stub(); - audio.isOutputSelectionSupported = true; - }); - - it('should set output devices', () => { - handlers.get('deviceinfochange')(); - wait().then(() => sinon.assert.called(audio.speakerDevices.set)); - }); - - it('should catch error when setting output devices', () => { - handlers.get('deviceinfochange')(); - wait().then(() => sinon.assert.called(audio._log.warn)); - }); - - it('should not set device when isSupported is false', () => { - audio.isOutputSelectionSupported = false; - handlers.get('deviceinfochange')(); - wait().then(() => sinon.assert.notCalled(audio.speakerDevices.set)); - }); - }); }); }); diff --git a/tests/index.ts b/tests/index.ts index d746c741..169c2df9 100644 --- a/tests/index.ts +++ b/tests/index.ts @@ -102,5 +102,3 @@ require('./unit/error'); require('./unit/log'); require('./unit/regions'); require('./unit/uuid'); - -require('./shims/mediadevices'); diff --git a/tests/shims/mediadevices.js b/tests/shims/mediadevices.js deleted file mode 100644 index 4fd7d372..00000000 --- a/tests/shims/mediadevices.js +++ /dev/null @@ -1,279 +0,0 @@ -const assert = require('assert'); -const sinon = require('sinon'); -const getMediaDevicesInstance = require('../../lib/twilio/shims/mediadevices').default; - -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); - }); - }); - }); -}); diff --git a/tests/unit/device.ts b/tests/unit/device.ts index 53379f60..a3f3120a 100644 --- a/tests/unit/device.ts +++ b/tests/unit/device.ts @@ -24,6 +24,7 @@ const ClientCapability = require('twilio').jwt.ClientCapability; describe('Device', function() { let activeCall: any; let audioHelper: any; + let callConfig: any; let clock: SinonFakeTimers; let connectOptions: Record | undefined; let device: Device; @@ -32,6 +33,7 @@ describe('Device', function() { let publisher: any; let stub: SinonStubbedInstance; let token: string; + let updateAvailableDevicesStub: any; let updateInputStream: Function; let updateSinkIds: Function; @@ -50,12 +52,14 @@ describe('Device', function() { const audioHelper = createEmitterStub(require('../../lib/twilio/audiohelper').default); audioHelper._enabledSounds = enabledSounds; audioHelper._getEnabledSounds = () => enabledSounds; + audioHelper._updateAvailableDevices = updateAvailableDevicesStub; audioHelper.disconnect = () => enabledSounds[Device.SoundName.Disconnect]; audioHelper.incoming = () => enabledSounds[Device.SoundName.Incoming]; audioHelper.outgoing = () => enabledSounds[Device.SoundName.Outgoing]; return audioHelper; }; - const Call = (_?: any, _connectOptions?: Record) => { + const Call = (_config?: any, _connectOptions?: Record) => { + callConfig = _config; connectOptions = _connectOptions; return activeCall = createEmitterStub(require('../../lib/twilio/call').default); }; @@ -78,6 +82,7 @@ describe('Device', function() { beforeEach(() => { pstream = null; publisher = null; + updateAvailableDevicesStub = sinon.stub().returns(Promise.reject()); clock = sinon.useFakeTimers(Date.now()); token = createToken('alice'); device = new Device(token, setupOptions); @@ -196,6 +201,12 @@ describe('Device', function() { }); describe('.connect(params?, audioConstraints?, iceServers?)', () => { + it('should update device list after a getUserMediaCall', async () => { + await device.connect(); + await callConfig.getUserMedia(); + sinon.assert.calledOnce(updateAvailableDevicesStub); + }); + it('should reject if there is already an active call', async () => { await device.connect(); await assert.rejects(() => device.connect(), /A Call is already active/); From dfc5c22deae615ea2582c3f3c3eb69b198c73cf8 Mon Sep 17 00:00:00 2001 From: charliesantos Date: Tue, 19 Sep 2023 14:45:56 -0700 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4d92744..39c741fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Changes ------- +- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/197) where audio in Chrome browser is choppy when another application is using the audio devices. - Added missing documentation for the following events: - `call.on('ringing', handler)` - `call.on('warning', handler)` From 2359e9fe30caed47043ecbf54a386bc0c7a11fab Mon Sep 17 00:00:00 2001 From: charliesantos Date: Wed, 20 Sep 2023 09:49:23 -0700 Subject: [PATCH 3/4] Using callback instead --- lib/twilio/call.ts | 9 +++++++++ lib/twilio/device.ts | 23 +++++++++++++---------- tests/unit/call.ts | 13 +++++++++++-- tests/unit/device.ts | 6 ++---- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/lib/twilio/call.ts b/lib/twilio/call.ts index fbcc31bd..f016ddd1 100644 --- a/lib/twilio/call.ts +++ b/lib/twilio/call.ts @@ -669,6 +669,10 @@ class Call extends EventEmitter { data: { audioConstraints }, }, this); + if (this._options.onGetUserMedia) { + this._options.onGetUserMedia(); + } + connect(); }, (error: Record) => { let twilioError; @@ -1914,6 +1918,11 @@ namespace Call { */ offerSdp?: string | null; + /** + * Called after a successful getUserMedia call + */ + onGetUserMedia?: () => void; + /** * Whether this is a preflight call or not */ diff --git a/lib/twilio/device.ts b/lib/twilio/device.ts index 1d058636..f85f95a2 100644 --- a/lib/twilio/device.ts +++ b/lib/twilio/device.ts @@ -939,15 +939,7 @@ class Device extends EventEmitter { const config: Call.Config = { audioHelper: this._audio, - getUserMedia: (...args) => - (this._options.getUserMedia || getUserMedia)(...args) - .then((gumResponse: any) => { - this._audio?._updateAvailableDevices().catch(error => { - // Ignore error, we don't want to break the call flow - this._log.warn('Unable to updateAvailableDevices after gUM call', error); - }); - return Promise.resolve(gumResponse); - }), + getUserMedia: this._options.getUserMedia || getUserMedia, isUnifiedPlanDefault: Device._isUnifiedPlanDefault, onIgnore: (): void => { this._soundcache.get(Device.SoundName.Incoming).stop(); @@ -977,6 +969,7 @@ class Device extends EventEmitter { getInputStream: (): MediaStream | null => this._options.fileInputStream || this._callInputStream, getSinkIds: (): string[] => this._callSinkIds, maxAverageBitrate: this._options.maxAverageBitrate, + onGetUserMedia: () => this._onGetUserMedia(), preflight: this._options.preflight, rtcConstraints: this._options.rtcConstraints, shouldPlayDisconnect: () => this._audio?.disconnect(), @@ -1096,10 +1089,20 @@ class Device extends EventEmitter { } } + /** + * Called after a successful getUserMedia call + */ + private _onGetUserMedia = () => { + this._audio?._updateAvailableDevices().catch(error => { + // Ignore error, we don't want to break the call flow + this._log.warn('Unable to updateAvailableDevices after gUM call', error); + }); + } + /** * Called when a 'close' event is received from the signaling stream. */ - private _onSignalingClose = () => { + private _onSignalingClose = () => { this._stream = null; this._streamConnectedPromise = null; } diff --git a/tests/unit/call.ts b/tests/unit/call.ts index a967eaf1..4b389f01 100644 --- a/tests/unit/call.ts +++ b/tests/unit/call.ts @@ -399,12 +399,14 @@ describe('Call', function() { }); context('when getInputStream is present and succeeds', () => { - let getInputStream; + let getInputStream: any; + let onGetUserMedia: any; let wait: Promise; beforeEach(() => { + onGetUserMedia = sinon.stub(); getInputStream = sinon.spy(() => 'foo'); - Object.assign(options, { getInputStream }); + Object.assign(options, { getInputStream, onGetUserMedia }); conn = new Call(config, options); mediaHandler.setInputTracksFromStream = sinon.spy(() => { @@ -426,6 +428,13 @@ describe('Call', function() { }); }); + it('should call onGetUserMedia callback', () => { + conn.accept(); + return wait.then(() => { + sinon.assert.calledWith(onGetUserMedia); + }); + }); + context('when incoming', () => { beforeEach(() => { getInputStream = sinon.spy(() => 'foo'); diff --git a/tests/unit/device.ts b/tests/unit/device.ts index a3f3120a..704f480d 100644 --- a/tests/unit/device.ts +++ b/tests/unit/device.ts @@ -24,7 +24,6 @@ const ClientCapability = require('twilio').jwt.ClientCapability; describe('Device', function() { let activeCall: any; let audioHelper: any; - let callConfig: any; let clock: SinonFakeTimers; let connectOptions: Record | undefined; let device: Device; @@ -58,8 +57,7 @@ describe('Device', function() { audioHelper.outgoing = () => enabledSounds[Device.SoundName.Outgoing]; return audioHelper; }; - const Call = (_config?: any, _connectOptions?: Record) => { - callConfig = _config; + const Call = (_?: any, _connectOptions?: Record) => { connectOptions = _connectOptions; return activeCall = createEmitterStub(require('../../lib/twilio/call').default); }; @@ -203,7 +201,7 @@ describe('Device', function() { describe('.connect(params?, audioConstraints?, iceServers?)', () => { it('should update device list after a getUserMediaCall', async () => { await device.connect(); - await callConfig.getUserMedia(); + connectOptions!.onGetUserMedia(); sinon.assert.calledOnce(updateAvailableDevicesStub); }); From 3ddb2925e110d4b9028fa3e32052ed8c3d823475 Mon Sep 17 00:00:00 2001 From: charliesantos Date: Wed, 20 Sep 2023 12:10:44 -0700 Subject: [PATCH 4/4] Changelog tweak --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39c741fa..cc69f967 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Changes ------- -- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/197) where audio in Chrome browser is choppy when another application is using the audio devices. +- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/197) where audio in the Chrome browser is choppy when another application is also using the audio devices. - Added missing documentation for the following events: - `call.on('ringing', handler)` - `call.on('warning', handler)`