Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "system default" option for media devices #5955

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

johnshaughnessy
Copy link
Contributor

@johnshaughnessy johnshaughnessy commented Feb 18, 2023

Prepend a System Default option for each MediaDeviceKind (audioinput, audiooutput and videoinput).

image

Also remove a few unused functions, indirection, and dry up some code where possible.

This fixes a bug where resetting the user preference back to the default in the preferences screen would:

  • Enable whichever device is the system default, but
  • Show whichever device happens to be first in the list.

Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks mostly good. The main thing I see is that we are reverting the System Default setting to the actual default device in some places including the Mic Setup Modal or the Audio Toolbar.

In the preferences screen everything works as expected because we are saving the pref and calling startMicShare with updatePrefs set to false so we don't overwrite the store.

When calling it from other places like useMicrophone, useMicrophoneStatus or ui-root we en up saving the currently used device in the preferences so we are overwriting it:

const micDeviceId = this.deviceIdForMicDeviceLabel(this.micLabelForAudioTrack(this.audioTrack));

We have to save in preferences the currently used deviceId not the one coming from the audio track. Also we probably need to update selectedMicDeviceId to don't return the audio track based label but the currently stored deviceId similarly to how we do it in selectedSpeakersDeviceId.

@keianhzo
Copy link
Contributor

keianhzo commented Mar 1, 2023

Thinking about this again, I think the best approach is not move the mic setting responsibility to the media devices manager and just update the preferences from anywhere else. I've made a proposal in this fork: eec7740 Feel free to merge that commit or disregard if you don't feel it's the right thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants