Fix PiP inconsistencies on Android #424
Conversation
zigavehovec
requested review from
strangesource and
rolandkakonyi
and removed request for
a team
CHANGELOG.md
Outdated
|
|
||
| ### Fixed | ||
|
|
||
| - Android: Entering Picture-in-Picture automatically from all screens when navigating the app to the background (after navigating to Basic Picture-in-Picture screen once) |
There was a problem hiding this comment.
This entry is very specific for our samples, is this something that only happens in our samples? If so we should make this clear. Otherwise, can we make it a bit more generic?
There was a problem hiding this comment.
It's kind of both, I would suppose that it should happen to other single activity apps. I'll write it in a more generic way.
| // Explicitly call `exitPictureInPicture()` on PlayerView when exiting PiP state, otherwise | ||
| // the `PictureInPictureExit` event won't get dispatched. | ||
| playerView?.exitPictureInPicture() | ||
| private fun isInPictureInPictureMode(): Boolean { |
There was a problem hiding this comment.
This function has a very similar name to the property isPictureInPictureMode which makes it quite confusing. Should we rename to e.g. isCurrentActivityInPictureInPictureMode or convert it to an extension function on the activity?
Co-authored-by: Lukas Knoch-Girstmair <strangesource@users.noreply.github.com>
There was a problem hiding this comment.
iOS still works :)
I can't judge the Android part.
Could you also update the guide here?
https://developer.bitmovin.com/playback/docs/enabling-picture-in-picture-mode
For the record: I also tested Android and it works!
Added a section in the guide here: https://developer.bitmovin.com/playback/docs/enabling-picture-in-picture-mode#adjust-the-ui-for-picture-in-picture-mode-on-android |
Description
The PiP implementation on Android was very complex and made some wrong assumptions, which caused a few bugs. Most notably:
Note that we do have
PictureInPictureConfig.shouldEnterOnBackground, but it works on iOS only and implementing it carries some lifecycle handling complexity (this bug), so implementing this should be considered as a feature request.Reference: https://github.com/bitmovin/bitmovin-player-react-native/pull/424/files#diff-884e35b040de326e767bfbc98ce73c052358fe2319d57d4716e044dbe68b6aa1L176
RNPictureInPictureHandlerdid toolbar hiding from the native side which is a wrong layer to do that. This caused the app to be without a toolbar after: Going into PiP mode -> Dismissing PiP window (stopping the app) -> Opening the app again.Reference: https://github.com/bitmovin/bitmovin-player-react-native/pull/424/files#diff-884e35b040de326e767bfbc98ce73c052358fe2319d57d4716e044dbe68b6aa1L115
Changes
RNPictureInPictureHandler, aligning it with the Flutter implementation: Introduce Picture in Picture bitmovin-player-flutter#87Checklist
CHANGELOGentry