chore(ui): improved Android PiP implementation #1003
Conversation
|
""" WalkthroughThis update refactors Android Picture-in-Picture (PiP) support in the video call Flutter package. It introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant FlutterApp
participant StreamFlutterActivity
participant PictureInPictureHelper
participant AndroidSystem
FlutterApp->>StreamFlutterActivity: App starts (extends StreamFlutterActivity)
StreamFlutterActivity->>PictureInPictureHelper: configureFlutterEngine()
PictureInPictureHelper->>FlutterApp: Setup MethodChannel for PiP control
FlutterApp->>PictureInPictureHelper: setPictureInPictureAllowed (via channel)
PictureInPictureHelper-->>FlutterApp: Updates PiP allowed state
StreamFlutterActivity->>PictureInPictureHelper: onUserLeaveHint() or onPause()
PictureInPictureHelper->>AndroidSystem: Request enterPictureInPictureMode if allowed
AndroidSystem-->>StreamFlutterActivity: Notify onPictureInPictureModeChanged
StreamFlutterActivity->>PictureInPictureHelper: notifyPictureInPictureModeChanged()
PictureInPictureHelper->>FlutterApp: Notify PiP mode state via channel
FlutterApp->>AndroidPipOverlay: Show/hide overlay based on PiP mode
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected related to the linked issues. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/android_pip_overlay.dart (1)
23-40: Consider simplifying widget hierarchy and state management.The implementation is solid, but there are two minor optimization opportunities:
Redundant layout widgets:
SizedBox.expandandPositioned.fillboth expand to fill available space. Consider using justSizedBox.expandor removing thePositioned.fillwrapper.State management: The widget extends
StatefulWidgetbut doesn't appear to have any mutable state. Consider usingStatelessWidgetunless state management is planned for future enhancements.-class AndroidPipOverlay extends StatefulWidget { +class AndroidPipOverlay extends StatelessWidget { const AndroidPipOverlay({ super.key, required this.call, this.sort, this.customBuilder, }); final Call call; final Comparator<CallParticipantState>? sort; final CallWidgetBuilder? customBuilder; @override - State<AndroidPipOverlay> createState() => _AndroidPipOverlayState(); -} - -class _AndroidPipOverlayState extends State<AndroidPipOverlay> { - @override Widget build(BuildContext context) { return Material( color: Colors.black, child: SizedBox.expand( - child: Positioned.fill( - child: widget.customBuilder?.call(context, widget.call) ?? + child: customBuilder?.call(context, call) ?? StreamCallParticipants( - call: widget.call, + call: call, layoutMode: ParticipantLayoutMode.pictureInPicture, - sort: widget.sort, + sort: sort, ), - ), ), ); } }dogfooding/android/app/src/main/kotlin/io/getstream/video/flutter/dogfooding/MainActivity.kt (2)
24-24: Remove unnecessary empty line.override fun configureFlutterEngine(flutterEngine: FlutterEngine) { super.configureFlutterEngine(flutterEngine) - MethodChannel(flutterEngine.dartExecutor.binaryMessenger, CHANNEL).setMethodCallHandler { call, result ->
24-24: Remove trailing whitespace.- super.configureFlutterEngine(flutterEngine) - + super.configureFlutterEngine(flutterEngine) +packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterActivity.kt (2)
50-50: Remove trailing whitespace.-} +}
24-26: Consider improving lambda clarity.The lambda
{ this }could be more explicit for better readability.- PictureInPictureHelper.initializeWithFlutterEngine(flutterEngine) { this } + PictureInPictureHelper.initializeWithFlutterEngine(flutterEngine) { + this@StreamFlutterActivity + }packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/stream_picture_in_picture_android_view.dart (1)
140-166: Consider using null-safe operators for better defensive programming.While the current guards ensure
_overlayEntrywon't be null, using null-safe operators would make the code more robust against future changes.- overlay.insert(_overlayEntry!); + _overlayEntry?.let { overlay.insert(it) }; _isOverlayVisible = true;- _overlayEntry!.remove(); + _overlayEntry?.remove(); _overlayEntry = null; _isOverlayVisible = false;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
dogfooding/android/app/src/main/kotlin/io/getstream/video/flutter/dogfooding/MainActivity.kt(1 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/MethodCallHandlerImpl.kt(1 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterActivity.kt(1 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt(2 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart(2 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/android_pip_overlay.dart(1 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/stream_picture_in_picture_android_view.dart(1 hunks)packages/stream_video_flutter/lib/stream_video_flutter.dart(1 hunks)packages/stream_video_flutter/lib/stream_video_flutter_background.dart(1 hunks)packages/stream_video_flutter/lib/stream_video_flutter_method_channel.dart(1 hunks)packages/stream_video_flutter/lib/stream_video_flutter_platform_interface.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build
- GitHub Check: analyze
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
🔇 Additional comments (27)
packages/stream_video_flutter/lib/stream_video_flutter.dart (1)
29-31: LGTM: Well-organized exports for new PiP components.The new exports are properly placed among other Picture-in-Picture related components and follow the consistent export format used throughout the file.
packages/stream_video_flutter/lib/stream_video_flutter_platform_interface.dart (1)
64-70: LGTM: Proper deprecation with clear migration guidance.The deprecation annotation provides clear guidance about the replacement (
StreamPictureInPictureAndroidView) and the error message correction fromshowPictureInPicturetosetPictureInPictureEnabledimproves accuracy.packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/MethodCallHandlerImpl.kt (1)
107-107: LGTM: Removal aligns with automatic PiP handling architecture.The removal of manual PiP enable/disable method call handling is consistent with the refactoring to automatic PiP management via
StreamFlutterActivity, simplifying the method channel implementation.packages/stream_video_flutter/lib/stream_video_flutter_background.dart (1)
66-70: LGTM: Clean deprecation with backward compatibility.The no-op implementation with detailed deprecation message provides a smooth migration path while maintaining backward compatibility. The approach is consistent with the platform interface deprecation.
packages/stream_video_flutter/lib/stream_video_flutter_method_channel.dart (1)
136-142: LGTM! Proper deprecation implementation.The method is correctly deprecated with a clear message explaining the new approach. The no-op implementation maintains backward compatibility.
dogfooding/android/app/src/main/kotlin/io/getstream/video/flutter/dogfooding/MainActivity.kt (3)
12-19: Correct migration to StreamFlutterActivity.The activity now properly extends
StreamFlutterActivitywhich provides automatic PiP support, removing the need for manual PiP handling.
12-12: LGTM! Correct import for the new PiP implementation.The import correctly references the new
StreamFlutterActivityclass from the Stream Video Flutter plugin.
19-19: Correct migration to StreamFlutterActivity for automatic PiP support.This change properly implements the new PiP architecture by extending
StreamFlutterActivity, which handles PiP lifecycle events automatically.packages/stream_video_flutter/CHANGELOG.md (2)
3-9: Well-documented PiP improvements.The changelog entry clearly describes the improvements, provides migration instructions, and notes backward compatibility.
3-9: LGTM! Comprehensive PiP improvement documentation.The changelog entry clearly documents:
- The new
StreamFlutterActivityfor automatic PiP support- Key improvements and fixes
- Backwards compatibility information
- Clear migration instructions
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterActivity.kt (5)
8-19: Well-designed automatic PiP support.The abstract class properly encapsulates PiP lifecycle management, making it easy for developers to adopt by simply extending this class.
21-26: LGTM! Proper PiP helper initialization.The method correctly initializes the
PictureInPictureHelperwith the Flutter engine and provides the activity context via a lambda.
28-32: LGTM! Standard PiP trigger on user leave hint.Correctly implements the Android pattern for entering PiP mode when the user presses the home button or switches apps.
42-49: LGTM! Proper PiP mode change notification.Correctly notifies Flutter when the PiP mode state changes, allowing the Flutter side to update its UI accordingly.
34-40: Duplicate PiP triggers are no-opsThe
handlePipTriggerimplementation in
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt
checksactivity.isInPictureInPictureModeand returns immediately, preventing any redundant calls from bothonUserLeaveHint()andonPause(). No changes required.packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart (2)
224-228: Correct integration of Android PiP widget.The
StreamPictureInPictureAndroidViewis properly integrated into the widget tree, following the same pattern as the iOS implementation and respecting the PiP configuration settings.
224-228: LGTM! Consistent PiP widget integration for Android.The Android PiP implementation now follows the same pattern as iOS, with a dedicated
StreamPictureInPictureAndroidViewwidget integrated directly into the widget tree. This is cleaner than the previous lifecycle-based approach.packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/stream_picture_in_picture_android_view.dart (5)
38-47: Well-structured initialization logic.The initialization properly checks the configuration before setting up listeners and correctly initializes all necessary components for PiP functionality.
49-72: Excellent widget update handling.The method properly handles configuration changes and call updates with appropriate cleanup and re-initialization. This ensures the PiP state remains consistent with the widget configuration.
74-84: Proper resource cleanup in dispose.The dispose method correctly cleans up all resources including listeners, overlays, and subscriptions, preventing memory leaks.
168-186: Comprehensive PiP eligibility checks.The method properly validates all conditions including configuration settings, call status, and screen sharing state to determine if PiP should be allowed.
188-192: Appropriate build implementation.Returning
SizedBox.shrink()is correct since this widget manages PiP state without rendering visible content in the widget tree.packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt (5)
16-42: Well-structured Flutter integration setup.The initialization properly sets up the method channel communication with Flutter and handles the
setPictureInPictureAllowedmethod correctly.
44-54: Proper PiP state management.The method correctly updates the PiP allowed flag and handles disabling PiP mode when necessary with appropriate null safety checks.
68-81: Excellent PiP trigger handling with proper guards.The method correctly avoids redundant PiP entries and properly notifies Flutter of mode changes. Good defensive programming by checking the current state first.
83-106: Comprehensive PiP mode entry with proper version handling.The implementation correctly handles different Android versions, sets appropriate aspect ratios based on orientation, and utilizes Android 13+ seamless resize feature when available.
112-121: Excellent backward compatibility handling.The deprecated method properly maintains backward compatibility while guiding users to the new API with clear deprecation messages.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1003 +/- ##
========================================
- Coverage 4.17% 4.16% -0.01%
========================================
Files 571 573 +2
Lines 38459 38508 +49
========================================
- Hits 1605 1604 -1
- Misses 36854 36904 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt (1)
7-7: Remove unused import.The
android.graphics.Rectimport is not used anywhere in the code.-import android.graphics.Rect
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: stream_video
- GitHub Check: stream_video_push_notification
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_screen_sharing
- GitHub Check: build
- GitHub Check: analyze
🔇 Additional comments (3)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt (3)
69-78: Excellent PiP trigger handling with redundancy check.Good implementation that prevents redundant PiP entries and uses the new permission-based approach effectively.
84-107: Well-implemented PiP mode entry with proper version handling.Excellent implementation that:
- Includes proper Android version checks
- Handles aspect ratio based on orientation
- Uses modern PiP features like seamless resize when available
- Provides fallback for older Android versions
The orientation-based aspect ratio calculation is particularly smart.
113-125: Exemplary deprecation handling with clear migration guidance.This is an excellent example of how to deprecate APIs properly:
- Clear deprecation annotation with specific alternative
- Detailed error message with step-by-step migration instructions
- Maintains backward compatibility while guiding developers to the new approach
This aligns perfectly with the PR objective of maintaining backward compatibility while encouraging migration.
...ain/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt
Outdated
Show resolved
Hide resolved
...ain/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt
Show resolved
Hide resolved
| private fun handleMethodCall(call: MethodCall, result: MethodChannel.Result) { | ||
| when (call.method) { | ||
| "setPictureInPictureAllowed" -> { | ||
| val isAllowed = call.arguments as Boolean | ||
| setPictureInPictureAllowed(isAllowed) | ||
| result.success(null) | ||
| } | ||
| else -> { | ||
| result.notImplemented() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety for method call arguments.
The type casting on line 35 could throw a ClassCastException if the Flutter side passes incorrect argument types.
private fun handleMethodCall(call: MethodCall, result: MethodChannel.Result) {
when (call.method) {
"setPictureInPictureAllowed" -> {
- val isAllowed = call.arguments as Boolean
- setPictureInPictureAllowed(isAllowed)
- result.success(null)
+ try {
+ val isAllowed = call.arguments as? Boolean
+ if (isAllowed != null) {
+ setPictureInPictureAllowed(isAllowed)
+ result.success(null)
+ } else {
+ result.error("INVALID_ARGUMENT", "Expected Boolean argument", null)
+ }
+ } catch (e: Exception) {
+ result.error("METHOD_CALL_ERROR", e.message, null)
+ }
}
else -> {
result.notImplemented()
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun handleMethodCall(call: MethodCall, result: MethodChannel.Result) { | |
| when (call.method) { | |
| "setPictureInPictureAllowed" -> { | |
| val isAllowed = call.arguments as Boolean | |
| setPictureInPictureAllowed(isAllowed) | |
| result.success(null) | |
| } | |
| else -> { | |
| result.notImplemented() | |
| } | |
| } | |
| } | |
| private fun handleMethodCall(call: MethodCall, result: MethodChannel.Result) { | |
| when (call.method) { | |
| "setPictureInPictureAllowed" -> { | |
| try { | |
| val isAllowed = call.arguments as? Boolean | |
| if (isAllowed != null) { | |
| setPictureInPictureAllowed(isAllowed) | |
| result.success(null) | |
| } else { | |
| result.error("INVALID_ARGUMENT", "Expected Boolean argument", null) | |
| } | |
| } catch (e: Exception) { | |
| result.error("METHOD_CALL_ERROR", e.message, null) | |
| } | |
| } | |
| else -> { | |
| result.notImplemented() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt
between lines 32 and 43, the direct cast of call.arguments to Boolean can cause
a ClassCastException if the argument type is incorrect. To fix this, safely
check the type of call.arguments before casting, for example by using a
conditional cast or verifying the argument type, and handle the case where the
argument is not a Boolean by returning an error or defaulting safely.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/stream_video_flutter/CHANGELOG.md (2)
5-5: Add the missing article in the “Automatic Activation” bullet
Minor grammar tweak for smoother reading.-* **Automatic Activation:** PiP now triggers automatically when users press home button or background the app during calls. +* **Automatic Activation:** PiP now triggers automatically when users press the Home button or background the app during calls.
7-7: Clarify phrasing in the “Migration Required” bullet
Makes the instruction more explicit and idiomatic.-* **Migration Required:** In your `MainActivity`, remove the manual `onUserLeaveHint()` implementation and extend the MainActivity with `StreamFlutterActivity`. Previously required manually calling `PictureInPictureHelper.enterPictureInPictureIfInCall(this)` - now handled automatically. +* **Migration Required:** In your `MainActivity`, remove the manual `onUserLeaveHint()` implementation and have your `MainActivity` extend `StreamFlutterActivity`. Previously, you had to call `PictureInPictureHelper.enterPictureInPictureIfInCall(this)` manually—this is now handled automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stream_video_flutter/CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/stream_video_flutter/CHANGELOG.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...triggers automatically when users press home button or background the app during cal...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: build
- GitHub Check: stream_video_push_notification
- GitHub Check: analyze
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
...oid/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterActivity.kt
Show resolved
Hide resolved
.../src/call_screen/call_content/picture_in_picture/stream_picture_in_picture_android_view.dart
Show resolved
Hide resolved
...ain/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt (2)
18-22: Consider adding cleanup method for memory leak prevention.The static references to
methodChannelandgetActivitycould potentially cause memory leaks if not properly cleaned up when the Flutter engine is destroyed.Consider adding a cleanup method:
+ fun cleanup() { + methodChannel?.setMethodCallHandler(null) + methodChannel = null + getActivity = null + }
32-43: Add type safety for method call arguments.The type casting on line 35 could throw a
ClassCastExceptionif the Flutter side passes incorrect argument types.private fun handleMethodCall(call: MethodCall, result: MethodChannel.Result) { when (call.method) { "setPictureInPictureAllowed" -> { - val isAllowed = call.arguments as Boolean - setPictureInPictureAllowed(isAllowed) - result.success(null) + try { + val isAllowed = call.arguments as? Boolean + if (isAllowed != null) { + setPictureInPictureAllowed(isAllowed) + result.success(null) + } else { + result.error("INVALID_ARGUMENT", "Expected Boolean argument", null) + } + } catch (e: Exception) { + result.error("METHOD_CALL_ERROR", e.message, null) + } } else -> { result.notImplemented() } } }
🧹 Nitpick comments (1)
packages/stream_video_flutter/CHANGELOG.md (1)
5-5: Fix minor grammatical issue.Consider adding the missing article "the" for better readability.
-* **Automatic Activation:** PiP now triggers automatically when users press home button or background the app during calls. +* **Automatic Activation:** PiP now triggers automatically when users press the home button or background the app during calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterFragmentActivity.kt(1 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt(2 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/stream_picture_in_picture_android_view.dart(1 hunks)packages/stream_video_flutter/lib/stream_video_flutter_background.dart(0 hunks)packages/stream_video_flutter/lib/stream_video_flutter_method_channel.dart(0 hunks)packages/stream_video_flutter/lib/stream_video_flutter_platform_interface.dart(0 hunks)
💤 Files with no reviewable changes (3)
- packages/stream_video_flutter/lib/stream_video_flutter_platform_interface.dart
- packages/stream_video_flutter/lib/stream_video_flutter_background.dart
- packages/stream_video_flutter/lib/stream_video_flutter_method_channel.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video_flutter/lib/src/call_screen/call_content/picture_in_picture/stream_picture_in_picture_android_view.dart
🧰 Additional context used
🪛 LanguageTool
packages/stream_video_flutter/CHANGELOG.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...triggers automatically when users press home button or background the app during cal...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: analyze
- GitHub Check: analyze_legacy_version
🔇 Additional comments (11)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterFragmentActivity.kt (4)
8-18: LGTM! Well-documented base class design.The class documentation clearly explains the purpose and provides a practical usage example. The abstract class design appropriately allows for extension while providing automatic PiP functionality.
28-32: LGTM! Proper PiP trigger on user leave.The implementation correctly calls the parent method and triggers PiP when the user leaves the app (e.g., home button press).
34-40: Good defensive programming with isFinishing check.The condition
!isFinishingprevents PiP activation when the activity is being destroyed, which is the correct behavior.
42-49: LGTM! Proper PiP mode change notification.The implementation correctly notifies Flutter about PiP mode state changes, enabling proper UI updates on the Flutter side.
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/PictureInPictureHelper.kt (6)
24-30: LGTM! Clean method channel initialization.The method channel setup is well-structured and correctly initializes the communication bridge between Flutter and native Android.
45-55: LGTM! Proper PiP state management with automatic exit.The implementation correctly manages the PiP allowed state and automatically exits PiP mode when disabled, ensuring consistent behavior.
69-78: Excellent defensive programming for PiP triggers.The redundancy check prevents unnecessary PiP calls and the condition gating ensures PiP only activates when appropriate. This is a significant improvement over the previous implementation.
80-84: LGTM! Proper main thread enforcement addressed.The implementation correctly ensures method channel calls are made on the main thread using
runOnUiThread, addressing the previous review concern.
86-109: Well-implemented PiP mode entry with proper aspect ratio handling.The implementation correctly:
- Checks capabilities before attempting PiP entry
- Sets appropriate aspect ratios based on orientation
- Enables seamless resize on supported Android versions
- Provides fallback for older Android versions
111-113: LGTM! Simple and effective capability check.The method correctly verifies device PiP support before attempting to enter PiP mode.
packages/stream_video_flutter/CHANGELOG.md (1)
3-9: Comprehensive documentation of breaking changes.The changelog clearly explains the PiP improvements, migration requirements, and removed deprecated methods. This provides excellent guidance for developers upgrading to the new implementation.
...main/kotlin/io/getstream/video/flutter/stream_video_flutter/StreamFlutterFragmentActivity.kt
Show resolved
Hide resolved
Resolves FLU-178, FLU-174
Part of FLU-173
🎯 Goal
Reworked Android PiP implementation to avoid other widgets overlapping the picture-in-picture video track. PiP mode is now triggered from a native Android side and will show a dedicated overlay on the Flutter side.
Simple migration is required to make PiP work on Android.
screen-20250701-161136.mp4
Summary by CodeRabbit
Summary by CodeRabbit
New Features
AndroidPipOverlayandStreamPictureInPictureAndroidViewfor improved PiP video presentation.StreamFlutterActivityorStreamFlutterFragmentActivityinstead ofFlutterActivityto enable built-in PiP support.Bug Fixes
Deprecations
Documentation