[BUG] High CPU usage: "deviceinfochange" event is being dispatched indefinitely

[cmaiquel]

• I have verified that the issue occurs with the latest twilio.js release and is not marked as a known issue in the CHANGELOG.md.
• I reviewed the Common Issues and open GitHub issues and verified that this report represents a potentially new issue.
• I verified that the Quickstart application works in my environment.
• I am not sharing any Personally Identifiable Information (PII)
  or sensitive account information (API keys, credentials, etc.) when reporting this issue.

Code to reproduce the issue:

mediadevices.js
When a media device has an empty label, the method "deviceInfosHaveChanged" always returns true
A macOS "videoinput" device with an empty label, causing this behavior:

    {
        "deviceId": "",
        "kind": "videoinput",
        "label": "",
        "groupId": "zxzczxzccxcxxzczczxczxc"
    }

// Always returns true if there's a media device with an empty label, even when there's no difference between newDevices and oldDevices
function deviceInfosHaveChanged(newDevices, oldDevices) {
    var oldLabels = oldDevices.reduce(function (map, device) { return map.set(device.deviceId, device.label || null); }, new Map());
    return newDevices.some(function (newDevice) {
        var oldLabel = oldLabels.get(newDevice.deviceId);
        return typeof oldLabel !== 'undefined' && oldLabel !== newDevice.label;
    });
}

// As a result 'deviceinfochange' is triggered each time this method is invoked
// and this method is being invoked in a "poll" interval of 500ms
function sampleDevices(mediaDevices) {
    nativeMediaDevices.enumerateDevices().then(function (newDevices) {
        var knownDevices = mediaDevices._knownDevices;
        var 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(known devices, oldDevices)) {
            mediaDevices.dispatchEvent(new Event('deviceinfochange'));
        }
    });
}

Expected behavior:

Do not dispatch "deviceinfochange" in an indefinite loop for empty labeled devices.

Software versions:

• Browser(s): Chrome Version 107.0.5304.110 (Official Build) (x86_64)
• Operating System: macOS 12.6.1
• twilio.js: 2.1.2
• Third-party libraries (e.g., Angular, React, etc.):

[charliesantos]

Hi @cmaiquel , thanks for submitting. Usually devices will show with empty labels if you haven't asked for media permissions, or the user denied media access. We recommend to call getUserMedia and get the necessary permissions before enumerating devices or placing a call.

With that said, are you able to reproduce this issue after getting the necessary media permissions?

[cmaiquel]

@charliesantos I'm using the Voice SDK for voice calls. Ideally, I wouldn't need the video device permissions.

[charliesantos]

I submitted an internal ticket to address this @cmaiquel .

[cmaiquel]

Thanks @charliesantos.
There should also be a better way to detect device changes instead of calling enumerate devices every 500ms.
I think that "devicechange" event from MediaDevices interface is widely supported nowadays. https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/devicechange_event

[charliesantos]

@cmaiquel I agree! This should get addressed in our modernization effort for the SDK.

[kamalbennani]

I kinda agree with @cmaiquel.
We are also facing the same issue kind of, because of this shim device detection, which sometimes causes double-stream audio.

Lately, we moved the device management logic into a separate logic instead of using Twilio internal implementation, and we react to the device change based on the modern browser APIs, but the issue, is when we try to let the Twilio SDK know about a newly plugged device, the SDK will just crash and fail clean-up resources (cleaning up the master audio, etc ...), as the SDK is not yet aware of this new plugged Device because of this 500ms interval.

I think based on your Browser Support List, this most probably needs to be dropped off in favour of the native browser APIs.

[charliesantos]

I completely agree @kamalbennani @cmaiquel !

[charliesantos]

@cmaiquel @kamalbennani we started the work on this issue. Just letting you know that we are probably not going to drop the polling logic since some of our supported browsers still does not support the native API. However, we will figure out how to feature detect this one so we don't have to poll on browsers that do support the native API.

[charliesantos]

Should be fixed in #155
However, we still need to keep the polling part to keep the internal deviceinfochange, since the native devicechange event is not emitted when device labels are updated. We are thinking of creating a parameter to allow disabling of polling to save more CPU, for those that really needs it.

[charliesantos]

Should be fixed in 2.4.0

[cmaiquel]

Thanks! I will try the fix.