Skip to content
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

Improve fullscreen, orientation and PiP handling #286

Merged
merged 2 commits into from
Jan 19, 2021
Merged

Improve fullscreen, orientation and PiP handling #286

merged 2 commits into from
Jan 19, 2021

Conversation

ThreeFive-O
Copy link
Contributor

@ThreeFive-O ThreeFive-O commented Jan 17, 2021

Fixes #174.

See: https://developer.android.com/training/system-ui/navigation

When the user reopens the activity, onCreate() won't get called, so the system bars will remain visible. If you want system UI changes to persist as the user navigates in and out of your activity, set UI flags in onResume() or onWindowFocusChanged().

Copy link
Member

@Maxr1998 Maxr1998 left a comment

Choose a reason for hiding this comment

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

Good catch, thanks for the fix. Could you move the onResume function below onStart please? That order makes more sense imo considering the activity lifecycle.

@ThreeFive-O
Copy link
Contributor Author

@Maxr1998 Thank you for the feedback. Fixed the feedback and the code comment which was outdated.

One thing I noticed is that onResume doesn't fully fix the case when returning (maximizing) from Picture in Picture (PiP) mode. In most cases the navigation bar is still visible, but can be hidden by tapping the player window. In rare cases the navigation bar hides when returning from PiP.
Returning from PiP calls onResume, but there must be some other problem involved.
I'm on Android Pie on a Samsung phone.

Any idea?

@Maxr1998
Copy link
Member

I've observed that as well when first implementing PiP, but never found a solution/fix for the issue. I'm thinking it's a bug in the Android framework or ExoPlayer library, but I can't really tell.

YouTube opens the video in portrait mode when returning from PiP, so the bug doesn't apply there. Maybe we should do the same to cover it, it also makes sense since you mostly watch through PiP while in portrait.

@ThreeFive-O
Copy link
Contributor Author

ThreeFive-O commented Jan 18, 2021

@Maxr1998 It took me a while to figure out the fullscreen logic correctly, but I guess with the latest commit I addressed all issues.

The behavior of the latest commit is as follows:

  • When starting the video playback we request landscape orientation. The configuration change handler will take care about fullscreen mode.
  • When playback is stopped and app is paused (because of an app switch) the onResume handler will restore the last viewing mode (fullscreen when it was fullscreen; normal mode when it was normal mode)
  • When switching to PiP mode the configuration changes for enabling/disabling fullscreen mode are not handled. This fixes the rogue orientation changes in the configuration change handle. (Wild guess: Looks like delayed configuration change events handled after leaving PiP mode which arrive in unsorted manner and therefore can cause false reporting of orientation changes)

Please give it a try.

Copy link
Member

@Maxr1998 Maxr1998 left a comment

Choose a reason for hiding this comment

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

Thanks for the other set of changes, looks good to me! Just some small formatting nitpicks to fix, then it's ready to merge!

@ThreeFive-O ThreeFive-O requested a review from Maxr1998 January 19, 2021 08:11
@ThreeFive-O
Copy link
Contributor Author

Thank you @Maxr1998 and @nielsvanvelzen for your code reviews and improvements.

- Enter fullscreen in onResume if applicable
- Allow PiP in N+
@Maxr1998 Maxr1998 changed the title Fix #174: hide navigation bar on resume Improve fullscreen, orientation and PiP handling Jan 19, 2021
@Maxr1998
Copy link
Member

Maxr1998 commented Jan 19, 2021

Added another small fix (the isPictureInPictureMode call would have crashed below N) and rebased/squashen all commits onto the lastest master.

@Maxr1998 Maxr1998 added bug Something isn't working exoplayer Related to the ExoPlayer integration labels Jan 19, 2021
Fixed by delaying the enableFullscreen call in onConfigurationChanged
@nielsvanvelzen nielsvanvelzen merged commit 68ece25 into jellyfin:master Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exoplayer Related to the ExoPlayer integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation bar does not hide itself after initial playback
3 participants