Remember selected input devices #4218
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danxuliu
added
3. to review
bug
feature: frontend 🖌️
Member
Author
|
/backport to stable20 |
nickvergessen
requested changes
Sep 24, 2020
Member
nickvergessen
left a comment
nickvergessen
left a comment
There was a problem hiding this comment.
Works good in general, but I think the null device needs a bit more handling in this case. When you ever selected "No camera", you leave the call and join again, you will not be prompted for video permissions and your button will always say "No camera". Maybe clicking that button could instead bring you to the settings so you can select one?
Before the selected input devices were used as long as the device was still available and Talk was still open. However, if the device was disconnected and connected it was not automatically selected again, even if Talk was still open. Now the last selected device of each kind is remembered, so it will be automatically used if available when joining a call, even if it was disconnected and connected or if Talk was closed and opened. Additionally, if audio or video devices were explicitly disabled this will also be remembered and respected when Talk is open again. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When all the tracks from a stream were ended the stream was immediately removed. However, if there were pending audio or video changes the stream could be reused to add new tracks once "getUserMedia" finishes without having to create a new one. Moreover, this also fixes an issue when changing audio and video devices at the "same" time. When a device is changed first the tracks for the previous device are stopped. If both the audio and video devices were changed then all the tracks of the stream were ended, and thus the stream was removed. However, the stream was still referenced in the "localStreamsChanged" arrays of the handlers, so when "getUserMedia" returned the new track the old stream was not found, a new one was created and the track was added to it. As this happened for both audio and video two different streams ended being created, each one only with audio tracks or video tracks, and as both streams were passed to "localStreamChanged" either the audio or the video, depending on the last stream passed, were disabled due to the stream not having tracks of that type. Something similar happened too when "getUserMedia" failed, although in that case the old stream was the one passed to "localStreamChanged". Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Before the audio and video input id changes were marked as pending only while waiting for "getUserMedia" to finish. However, in the initial phase of handling input id changes the tracks for the previous device are stopped, which could cause the stream to be removed (for example, if there was just a single active device) even if it is still referenced in the "localStreamsChanged" array. Now handling input id changes are marked as pending since the beginning, which prevents ended streams to be removed while they may still be needed later. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When the devices are enumerated the selected audio and video input id may change depending on which devices are available. Therefore, when "getUserMedia" is called if an enumeration has been triggered but it has not finished yet now it will be waited for the enumeration to finish before getting the user media. Enumerating the devices is a fast operation, so this should not cause any noticeable delay when getting the user media. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When starting the local media for the first time the media devices could have not been enumerated yet, so even if the remembered devices are available they were not used in the first "getUserMedia" request. However, as the devices are enumerated after getting the user media (as that is the only time when it is guaranteed that the full device information can be got) this caused the remembered devices to be then found and further "getUserMedia" requests to be made for them. To avoid an initial "getUserMedia" request with default ids followed by other requests with the remembered ones now the media devices are tried to be enumerated before starting a call. This does not guarantee that the extra "getUserMedia" requests will be avoided, as if persistent media permissions are not granted the browser may not provide the full device IDs, but at least avoids them in some cases. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
force-pushed
the
remember-selected-input-devices
branch
from
f3ad9de to
9a902a7
Compare
Member
Author
Yes, sounds good. I would do it in a follow up pull request, though, as there are currently some issues when getting the media fails that should probably be fixed first (as that may affect the code to differentiate between media explicitly disabled and media failed). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #3913
Before the selected input devices were used as long as the device was still available and Talk was still open. However, if the device was disconnected and connected it was not automatically selected again, even if Talk was still open. Now the last selected device of each kind is remembered, so it will be automatically used if available when joining a call, even if it was disconnected and connected or if Talk was closed and opened.
Additionally, if audio or video devices were explicitly disabled this will also be remembered and respected when Talk is open again.
Note that the last selected device is automatically used if available when joining a call, but it will not be automatically used if it becomes available during a call. While it would be possible to do that at least for me it felt strange and unexpected (but I would have no problem enabling that if desired).
Besides that this pull request also fixes some related issues that have given me a big headache :-P