Skip to content

Break loop in acquiring media on Safari #3311

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

Merged
merged 2 commits into from
Jun 4, 2025
Merged

Conversation

robintown
Copy link
Member

@robintown robintown commented Jun 3, 2025

As of this week we've started seeing an issue in mobile Safari where Element Call will fail to acquire media, calling getUserMedia over and over in a loop. How this happened:

  • It seems that, for whatever reason (a recent Safari update?) the browser will emit a MediaDevices 'devicechange' event whenever you call getUserMedia, even though there is no change in the devices returned by enumerateDevices.
  • Then, because Element Call's reactivity happened to be a little too coarse-grained in a couple of spots (specifically, the LobbyView and MuteStates files), every 'devicechange' event would have the unintentional side effect of triggering a new call to getUserMedia. And so, the loop begins.

While it is pretty annoying that Safari has started emitting spurious 'devicechange' events, we can easily work around this situation by making our reactivity more fine-grained, breaking the loop. And this may result in performance improvements for all browsers, so, why not.

Here's the specifics of the loop:

  • LobbyView calls usePreviewTracks
  • usePreviewTracks has an effect which calls getUserMedia
  • The browser emits a MediaDevices 'devicechange' event
  • Media devices observer emits a new devices map
  • MediaDevicesContext invalidates its available devices memo and returns a new devices map
  • Devices map is passed to MuteStates
  • MuteStates (unnecessarily) invalidates its mute states memos and returns new ones
  • Mute states are passed to the LobbyView
  • LobbyView (unnecessarily) invalidates its onError callback and passes a new one to usePreviewTracks
  • usePreviewTracks uses the onError callback as one of the effect dependencies, so the effect runs again, and we call getUserMedia again

@robintown robintown added the PR-Bug-Fix Release note category. A PR that fixes a bug. label Jun 3, 2025
@robintown robintown marked this pull request as ready for review June 3, 2025 22:06
@robintown robintown requested a review from a team as a code owner June 3, 2025 22:06
@robintown robintown requested a review from BillCarsonFr June 3, 2025 22:06
@robintown
Copy link
Member Author

Up to the reviewer whether this needs a test, but… the failure mode is pretty idiosyncratic, and testing the lobby component is going to be painful, so I'd prefer to not!

Copy link
Member

@BillCarsonFr BillCarsonFr left a comment

Choose a reason for hiding this comment

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

I approve but I am a bit confused by the change, and I wonder if it is not working around the real problem?

This gives us the additional insurance of breaking the Safari media acquisition loop at the source by admitting that they can be spurious in practice. Safari, why!?
@robintown robintown merged commit ef41ef8 into livekit Jun 4, 2025
19 checks passed
Comment on lines +147 to +150
// time of writing, we are seeing mobile Safari firing spurious
// 'devicechange' events (where no change has actually occurred) when
// we call MediaDevices.getUserMedia. So, filter by deep equality.
).pipe(startWith([]), distinctUntilChanged<MediaDeviceInfo[]>(isEqual)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice that distinctUntilChanged can get a comparator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Bug-Fix Release note category. A PR that fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants