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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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:
hubs/src/utils/media-devices-manager.js
Line 182 in c03f521
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
.
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. |
Prepend a
System Default
option for eachMediaDeviceKind
(audioinput
,audiooutput
andvideoinput
).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: