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

Use 'ideal' rather than 'exact' for deviceid #852

Merged
merged 2 commits into from
Mar 5, 2019

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Mar 5, 2019

We were using 'exact' which means we fail outright if the device
we wanted isn't available. This means if a user selects a specific
device then later unplugs it, we fail to open a capture device
the next time they make a call even if there's one available.
Using 'ideal' uses the chosen device in preference, but something
else if it isn't available.

Also log the name of the exception when we fail to open a capture
device to give us more of an idea of what's gone wrong.

Should help fix element-hq/element-web#8993

dbkr added 2 commits March 5, 2019 12:59
We were using 'exact' which means we fail outright if the device
we wanted isn't available. This means if a user selects a specific
device then later unplugs it, we fail to open a capture device
the next time they make a call even if there's one available.
Using 'ideal' uses the chosen device in preference, but something
else if it isn't available.

Also log the name of the exception when we fail to open a capture
device to give us more of an idea of what's gone wrong.

Should help fix element-hq/element-web#8993
@dbkr dbkr requested a review from a team March 5, 2019 13:04
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

This seems like a good improvement, but what will it mean for the capture device settings tab?

It seems like we may also need to add a way to query the JS SDK for "what device did you actually choose" since it's now a non-exact match. This way, the Settings tab has a mechanism to show what device has actually been chosen, instead of just displaying whatever device value was last saved in the settings store (which not match what's actually going to be used).

@dbkr
Copy link
Member Author

dbkr commented Mar 5, 2019

Yeah, the device selection is still a little bit odd. It gets the current preferred device ID from the js-sdk (if any) and sets that as the 'value' of the select, then populates the select with the devices it got from the browser. If the preferred device isn't there any more, you'll therefore have a select with a value that isn't one of the options in the select, in which case browsers will just show the first as selected (ie. the default device). So that means you open settings and it says you have the default device selected when in fact you do have a preferred device.

So another thing we could do would be to add the current preferred device to the select options, although we don't currently save the name (just the ID) so we'd have to display it as 'Unknown Device' or something.

@jryans
Copy link
Collaborator

jryans commented Mar 5, 2019

It gets the current preferred device ID from the js-sdk (if any) and sets that as the 'value' of the select, then populates the select with the devices it got from the browser.

It seems like the sequence is:

  1. VoiceUserSettingsTab calls CallMediaHandler.getAudioOutput() etc. on will mount
  2. CallMediaHandler.getAudioOutput in React SDK reads SettingsStore.getValueAt(SettingLevel.DEVICE, "webrtc_audiooutput") (which would only be storing the preferred device after this change)

So another thing we could do would be to add the current preferred device to the select options, although we don't currently save the name (just the ID) so we'd have to display it as 'Unknown Device' or something.

That sounds like a good improvement to file, if we don't want to do it right now. It sounds like that could good be added later if we want, so I'll mark this as approved and leave the next step up to you.

@dbkr
Copy link
Member Author

dbkr commented Mar 5, 2019

Filed as element-hq/element-web#9045

@dbkr dbkr merged commit 5308595 into develop Mar 5, 2019
@t3chguy t3chguy deleted the dbkr/getusermedia_deviceid_ideal branch May 10, 2022 14:21
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.

People are losing camera/microphone permissions in the desktop app
2 participants