-
Notifications
You must be signed in to change notification settings - Fork 46
feat(llc, ui): added method with callbacks to handling audio interruptions (iOS, Android) #984
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
WalkthroughThe changes introduce system audio interruption handling for mobile calls by adding new permission declarations in the AndroidManifest, implementing microphone state management during interruptions in the app, and exposing new interruption event classes and callback registration methods in the WebRTC notifier. Android-specific phone permission requests are also added to the home screen initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppContent
participant RtcMediaDeviceNotifier
participant StreamVideo
AppContent->>RtcMediaDeviceNotifier: handleCallInterruptionCallbacks(onBegin, onEnd)
RtcMediaDeviceNotifier->>RtcMediaDeviceNotifier: Register interruption callbacks
Note over AppContent,RtcMediaDeviceNotifier: During a mobile call...
RtcMediaDeviceNotifier-->>AppContent: InterruptionBeginEvent
AppContent->>StreamVideo: setMicrophoneEnabled(false)
RtcMediaDeviceNotifier-->>AppContent: InterruptionEndEvent
AppContent->>StreamVideo: setMicrophoneEnabled(true)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (8)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
dogfooding/lib/screens/home_screen.dart (1)
52-54: LGTM! Proper platform-specific permission handling.The conditional phone permission request for Android aligns correctly with the manifest permissions (
READ_PHONE_STATEandREAD_CALL_STATE) added indogfooding/android/app/src/main/AndroidManifest.xml. The platform check ensures this only runs on Android where these permissions are relevant.Consider handling permission denial gracefully to inform users if phone interruption features may not work:
if (CurrentPlatform.isAndroid) { - Permission.phone.request(); + final phonePermission = await Permission.phone.request(); + if (phonePermission.isDenied) { + debugPrint('Phone permission denied - call interruption detection may not work'); + } }packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart (2)
12-16: LGTM! Clean event class hierarchy.The abstract
InterruptionEventbase class with concreteInterruptionBeginEventandInterruptionEndEventimplementations provides a solid foundation for interruption event handling.Consider if these event classes will be exposed in the public API later, as they're currently defined but not used in the method signature. If they're for future use, consider adding documentation comments.
31-66: Excellent documentation and implementation!The method provides comprehensive documentation covering platform-specific interruption sources and includes practical usage guidance. The delegation to the underlying RTC implementation is appropriate.
Consider mentioning that the READ_PHONE_STATE permission is required in the manifest for Android telephony interruptions:
/// - With telephony: - /// - Phone calls (requires READ_PHONE_STATE permission) + /// - Phone calls (requires READ_PHONE_STATE and READ_CALL_STATE permissions in AndroidManifest.xml)Also consider documenting error handling expectations:
/// This method allows you to pause the call, mute the audio, or perform any other /// necessary actions when interruptions occur. /// + /// Note: Ensure proper error handling in your callback implementations as they + /// will be called from the native platform layer. + /// /// For more details on handling call interruptions, refer to the
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dogfooding/android/app/src/main/AndroidManifest.xml(1 hunks)dogfooding/lib/app/app_content.dart(3 hunks)dogfooding/lib/screens/home_screen.dart(1 hunks)packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video
- GitHub Check: analyze_legacy_version
- GitHub Check: build
- GitHub Check: analyze
🔇 Additional comments (3)
dogfooding/android/app/src/main/AndroidManifest.xml (1)
14-15: LGTM! Permissions are appropriate for audio interruption handling.The
READ_PHONE_STATEandREAD_CALL_STATEpermissions are correctly added to support Android phone call interruption detection. These permissions align with the runtime permission request added indogfooding/lib/screens/home_screen.dartat lines 52-54.dogfooding/lib/app/app_content.dart (2)
16-16: LGTM! Appropriate import for WebRTC functionality.The import with alias
rtcis correctly used for accessing the underlying WebRTC interruption handling capabilities.
109-109: LGTM! Good placement for initialization.Calling
_handleMobileAudioInterruptions()ininitStateensures the audio interruption handling is set up early in the app lifecycle.
packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart
Show resolved
Hide resolved
Co-authored-by: Rene Floor <[email protected]>
…am/stream-video-flutter into feat/audio-interruption-handling
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #984 +/- ##
========================================
- Coverage 3.01% 3.01% -0.01%
========================================
Files 571 571
Lines 38384 38386 +2
========================================
Hits 1156 1156
- Misses 37228 37230 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Rene Floor <[email protected]>
…am/stream-video-flutter into feat/audio-interruption-handling
part of FLU-164
🎯 Goal
Audio interruptions occur when the system or other applications take control of the audio session, temporarily pausing or stopping your app's audio. By implementing this added method, we enable users to respond to those interruptions, such as muting the microphone during an ongoing call.
🛠 Implementation details
Added
handleCallInterruptionCallbackstoRtcMediaDeviceNotifierAdded sample handling (muting the microphone) in the Dogfooding app
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Chores
stream_webrtc_flutterdependency to version 1.0.7 across multiple packages for improved compatibility.