-
-
Notifications
You must be signed in to change notification settings - Fork 650
Fix media switching during legacy calls #5069
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
Conversation
BillCarsonFr
left a comment
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.
See my comment
| try { | ||
| // Not specifying exact for deviceId means switching devices does not always work, | ||
| // try with exact and fallback to ideal if it fails | ||
| constraints = this.getUserMediaContraints(shouldRequestAudio, shouldRequestVideo, true); |
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.
I don't understand, as per doc
ideal: A string or an array of strings, specifying ideal values for the property. If possible, one of the listed values will be used, but if it's not possible, the user agent will use the closest possible match.
Why doing exact then ideal would work differently than just ideal? what am I missing?
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.
on macOS, Using "ideal" at the moment is not working when switching camera and providing a specific deviceId. I don't know why maybe a system bug.
By the sounds of the comments further down in the code, this api is not very consistent cross browser either.
toger5
left a comment
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.
What is needed to resolve this question:
XXX: Is this still true?
toger5
left a comment
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.
Otherwise this looks good.
Approving because for me the comment on XXX: Is this still true? is not blocking.
|
@toger5 That is old code, I just changed the formatting of it to pass the sonar cloud checks. By the sounds of the comment, the author was not sure if it was still an issue. We'd probably need to go back and have a look at what the original issue was for that. I don't think it would affect the solution for this problem, but I can check after the fact. |
Thanks for this info. I missed that it is old code. |
Fixes element-hq/element-web#31123
element web PR is here for testing the netlify
A couple of cases tested manually. The main case (case 1) is easily reproducible on macOS(all browsers I believe) and was reported for Windows. There was one report it couldn't be reproduced on Linux.
Case 1: Switing camera broken(With no fix applied this is easily reproducible)
Observed: Local feed stays on camera 1 and remote feed from camera 1 received by Alice
Expected: Local feed changes successfully to camera and remote feed from camera 2 received by Alice
Case 2: (When only using "exact" this is reproducible, hence the need for the try/catch)
Observed: An error occurs that media cannot be connected (as we have selected "exact" and the last device saved in settings was the disconnected camera)
Expected: Camera 2 falls back to Camera 1 and this is used for local/remote feeds.