Fix: Issue where upon video completion in fullscreen landscape, the device does not revert back to portrait mode#13178
Conversation
|
I forgot to remove the Debug part. I'll do that now. |
| if (player.getCurrentState() == STATE_COMPLETED) { | ||
| player.getFragmentListener() | ||
| .ifPresent(PlayerServiceEventListener::onScreenRotationButtonClicked); | ||
| } |
There was a problem hiding this comment.
Well you are not checking if you really need to do the rotation.
It works fine if you had vertical orientation before entering the full-screen so rotation in the end will return you to starting position. But if before video you had horizontal orientation, rotation in the end will rotate screen to vertical position. So it is not solving the bug but transfers it from vertical to horizontal orientation...
I don't know if there is any kind of tracking of previous rotation and not sure how it worked (if it really worked) before 0.28.1 mentioned in the linked issue or not but you need to track it
There was a problem hiding this comment.
I'll look into it. Thanks.
There was a problem hiding this comment.
I think I may have found a fix for that.
So from what I understand, when the screen orientation is locked, the video starts in portrait mode and goes into landscape mode when the fullscreen button is clicked. At the end of the video, it goes back to portrait mode like it was before and exits fullscreen mode.
When the screen orientation is unlocked, if the video ends while in portrait or landscape mode, it should remain in that mode because the phone is still in portrait or landscape.
Now by default, it exits fullscreen mode no matter if it locked or unlocked. I'm not sure if that's a wanted behavior by whoever wrote that but at least the orientation seems to be fixed.
For the fix I checked if the orientation is locked or unlocked before making the rotation.
There was a problem hiding this comment.
I think there are 2 options:
- The one you already implemented, to always rotate back to portrait if the screen orientation is locked
- only rotate back to portrait if the orientation is locked and the option "Start main player in fullscreen" is enabled.
There was a problem hiding this comment.
I think there are 2 options:
* The one you already implemented, to always rotate back to portrait if the screen orientation is locked
It is not "always rotate back to portrair", it is "always rotate regardless original orientation"
* only rotate back to portrait if the orientation is locked and the option "Start main player in fullscreen" is enabled.
Does it have any sense at all?
There was a problem hiding this comment.
@volatilespecial You shouldn't always rotate, you should rotate smartly
For the fix I checked if the orientation is locked or unlocked before making the rotation.
What it changes? Yeah it probably should also be done
For following assume that screen rotation is locked:
With no rotation on fullscreen exit: (current behavior)
Vertical position. Open fullscreen -> horizontal position. Exit fullscreen -> horizontal position (bug)
Horizontal position. Open fullscreen -> horizontal position. Exit fullscreen -> horizontal position
With always rotate on fullscreen exit: (your PR)
Vertical position. Open fullscreen -> horizontal position. Exit fullscreen -> vertical position
Horizontal position. Open fullscreen -> horizontal position. Exit fullscreen -> vertical position (bug)
There was a problem hiding this comment.
You're right. I'll look into it and fix that.
|
Personally, I would recommend adding a configuration option to rotate back to portait on video end. Perhaps only show it in case the option "Start main player in fullscreen" is enabled. |
It is a one more option. We try to constraint amount of options because most of it won't be discovered by users and moreover. Additionally the behavior itself is considered buggy by all people. Especially because it not only turns the screen for newpipe but turns screen for whole android so it means you mess with user expectations that your app doesn't stab you... |
|
I understand your point. Would just be great if advanced users could achieve their desired behaviour without actually forking the repo. Do you have any suggestion for me? |
|
347bd31 to
3acafe7
Compare
|
Alright, so I've cleaned the commit history as requested. This fix reverts the orientation back to portrait after the video ends by tracking the orientation everytime toggleFullscreen is called. |
7c72104 to
399d541
Compare
What is it?
Description of the changes in your PR
Bug fix with the player in fullscreen landscape. When the video ends, the player now exits fullscreen mode with the correct orientation.
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence