fix(llc): fixes for input/output device setting on web #1086
Conversation
|
Warning Rate limit exceeded@Brazol has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds lobby device-selection UI and state, threads initial camera device into StreamLobbyVideo, adds Call APIs to check/set audio output with platform guards, adjusts web media constraint shapes to exact deviceId matches, updates rtc_audio implementations and device enumeration (including iOS earpiece fallback), and minor fixes/changelog updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LobbyScreen
participant DevicePicker
participant Call
participant RtcManager
participant RtcAudioAPI
participant Browser
User->>LobbyScreen: open device picker
LobbyScreen->>DevicePicker: show device list
DevicePicker-->>LobbyScreen: selected device
LobbyScreen->>LobbyScreen: update selectedAudio/Video
User->>LobbyScreen: Join call (with selected devices)
LobbyScreen->>Call: join(..., audioInputDevice, videoInputDevice)
Note over LobbyScreen,Call: During call — change audio output
LobbyScreen->>Call: setAudioOutputDevice(device)
Call->>Call: checkIfAudioOutputChangeSupported()
alt platform == web
Call->>RtcAudioAPI: checkIfAudioOutputChangeSupported()
RtcAudioAPI->>Browser: try audioElement.setSinkId(deviceId)
Browser-->>RtcAudioAPI: success / throws
RtcAudioAPI-->>Call: true / false
else
RtcAudioAPI-->>Call: true
end
alt supported
Call->>RtcManager: apply sinkId to remote audio tracks
RtcManager-->>Call: Result success
else not supported
Call-->>LobbyScreen: Result error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Points needing extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1086 +/- ##
========================================
- Coverage 4.84% 4.83% -0.01%
========================================
Files 591 591
Lines 40500 40508 +8
========================================
- Hits 1961 1960 -1
- Misses 38539 38548 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dogfooding/lib/screens/lobby_screen.dart(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: analyze_legacy_version
🔇 Additional comments (11)
dogfooding/lib/screens/lobby_screen.dart (11)
2-2: LGTM - Required import for device subscription handling.The
dart:asyncimport is necessary forStreamSubscriptionandunawaitedused in the device management implementation.
31-38: LGTM - Device state management setup.The state variables are properly typed and initialized for tracking device selection and managing device change subscriptions.
66-72: LGTM - Device options correctly applied to call connection.The selected devices are properly passed to
CallConnectOptionswhen joining the call.
84-84: LGTM - Proper subscription cleanup.The device change subscription is correctly canceled on dispose.
88-102: LGTM - Robust device change handler.The implementation correctly filters devices by kind and includes a mounted check before updating state. Using
growable: falseis a good optimization.
104-125: LGTM - Audio input picker correctly implemented.The modal flow properly checks for mounted state after the async operation and updates state correctly.
204-206: Verify ValueKey usage doesn't cause unintended side effects.Using
ValueKey(_cameraTrack)will cause the entireStreamLobbyVideowidget to be recreated whenever the camera track changes. While this might be intentional to reset the video view state, it could also cause:
- Loss of internal widget state
- Brief visual flickers during device switching
- Unnecessary rebuilds
Please verify that recreating the entire widget is the desired behavior when switching video input devices, rather than just updating the existing track.
220-246: LGTM - Improved UX with tooltip for blur control.The blur control is now wrapped in a Tooltip with appropriate messages, improving discoverability while preserving the existing functionality.
250-314: LGTM - Well-implemented device selection UI.The device selection controls are properly implemented with:
- Appropriate tooltips for accessibility
- Correct enablement logic based on device availability
- Proper text overflow handling
- Clear visual feedback of selected devices
366-430: LGTM - Robust device picker implementation.The device picker sheet is well-implemented with:
- "System default" option for resetting to default device
- Proper fallback to device ID when label is empty (good defensive coding)
- Appropriate size constraints for the list
432-467: LGTM - Clean device list tile implementation.The device list tile provides clear visual feedback for selection state using modern Flutter color APIs and appropriate styling.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dogfooding/lib/screens/home_screen.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: analyze_legacy_version
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
dogfooding/lib/screens/lobby_screen.dart (2)
54-54: Error handling for device enumeration is still missing.This issue was previously flagged. The
unawaitedcall will silently ignore enumeration errors, leaving device lists empty without user feedback.
157-166: Mounted check and error handling still needed after async camera operations.This issue was previously flagged. After the async camera track operations (lines 158-164), there's no mounted check before calling
setStateat line 166. Additionally, the camera operations lack error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dogfooding/lib/screens/lobby_screen.dart(7 hunks)
🔇 Additional comments (2)
dogfooding/lib/screens/lobby_screen.dart (2)
32-39: Clean device management implementation.The device state management, change handling, and audio input picker are well-structured with appropriate mounted checks and clean separation of concerns.
Also applies to: 44-45, 98-112, 114-135
382-483: Nice device picker UI components.The
_DevicePickerSheetand_DeviceListTilewidgets provide a clean, accessible device selection experience with proper visual feedback for the selected state and a "System default" option.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart (1)
109-116: Remove duplicate emission to the device stream.With the addition of
_devicesController.add(mediaDevices)at line 149, this method now emits the device list twice:
- Once inside
enumerateDevices()at line 149- Again at line 115
This causes duplicate emissions to subscribers of
onDeviceChangestream, potentially triggering unnecessary UI updates.Apply this diff to remove the duplicate emission:
Future<void> _onDeviceChange(_) async { final devicesResult = await enumerateDevices(); final devices = devicesResult.getDataOrNull(); - if (devices == null) return; - - _devicesController.add(devices); + // Devices are now emitted within enumerateDevices() at line 149 }Alternatively, if you want to keep emission only in
_onDeviceChange, remove line 149 instead and ensure all callers go through the stream subscription rather than directenumerateDevices()calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/stream_video/CHANGELOG.md(1 hunks)packages/stream_video/lib/src/call/call.dart(3 hunks)packages/stream_video/lib/src/webrtc/rtc_manager.dart(2 hunks)packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart(2 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/stream_video_flutter/CHANGELOG.md
- packages/stream_video/lib/src/call/call.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: analyze
- GitHub Check: build
🔇 Additional comments (4)
packages/stream_video/lib/src/webrtc/rtc_manager.dart (2)
35-36: LGTM! Clean import with appropriate scoping.The use of the
showclause to import onlycheckIfAudioOutputChangeSupportedis good practice, keeping the namespace clean and making dependencies explicit.
1114-1122: Good defensive check for browser capability.The capability check correctly prevents attempting to set audio output on browsers that don't support
setSinkId. The early return with a descriptive error message ensures graceful handling of unsupported platforms.packages/stream_video/CHANGELOG.md (1)
1-11: LGTM! Changelog properly documents the changes.The changelog updates are well-formatted and clearly document:
- The web platform fixes for input/output device handling
- The new capability check API
The heading level change to
##is consistent with the rest of the changelog structure.packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart (1)
134-147: LGTM! iOS earpiece fallback is correctly implemented.The earpiece enum value is used consistently throughout the codebase and properly parsed from the API. The iOS fallback logic correctly injects the device using the canonical
AudioSettingsRequestDefaultDeviceEnum.earpiecevalue, matching existing patterns across the SDK and test suite.
resolves FLU-306
This PR introduces several fixes and improvements related to audio and video device management on the web platform. It addresses issues encountered during initial device setup and when switching between input/output devices during the call.
It also adds a sample implementation of device setup on the Dogfooding's lobby screen.
Summary by CodeRabbit
New Features
Bug Fixes
UI
Behavior