Prevent jumping of the player and wrong padding on devices with cutout #4154

avently · 2020-08-17T17:26:16Z

What is it?

Bug fix (user facing)

Description of the changes in your PR

On devices with cutout there is a "jump" when you are in multiWindow mode or when you're watching vertical video in fullscreen. This PR fixes that jump (the first commit).

On the same devices there is a problem related to top padding on vertical videos in portrait in fullscreen. This is more interesting because a couple hours ago I thought I will not be able to fix it. This problem makes situation when the center of the player is not in the center of the screen. After hours trying I installed the app on my phone and the issue is fixed actually (same Android version but different result). So I have no idea will my method (from the second commit) work on user's devices or not. I think the issue is still here but on devices with non-standard screen ratio. But I don't know how to fix it. You can merge the PR as is or merge only first commit, or drop it.

In short:

in multiwindow mode all videos work great
there is no jump that happened before when a user shows/hides controls in the player
there is a correct padding on top of the player (but I'm sure it isn't on devices with non-standart ratio).

Everyone who has a device with a hole, cutout, whatever check this build in vertical videos in landscape, in portrait, in multi window mode, check non-vertical videos too.

Fixes the following issue(s)

partially fixes #4040

Testing apk

app-release-final.zip

Agreement

I carefully read the contribution guidelines and agree to them.

app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java

blackbox87 · 2020-08-30T14:54:23Z

This problem makes situation when the center of the player is not in the center of the screen. After hours trying I installed the app on my phone and the issue is fixed actually (same Android version but different result). So I have no idea will my method (from the second commit) work on user's devices or not. I think the issue is still here but on devices with non-standard screen ratio. But I don't know how to fix it. You can merge the PR as is or merge only first commit, or drop it.

That commit actually makes the player off-center for me while using a Poco F1. It was perfect before the commit as it was almost identical to the official YouTube app, minus pinch to zoom.

Aligning to the notch means you'll probably lose some of the image. Like in my case it's now off-center and I lose some of the image on the right side of my screen due to the phones rounded corners.

YouTube:

NewPipe v0.19.8:

NewPipe Cutout:

avently · 2020-08-30T16:35:30Z

@blackbox87
Thank you for testing. Actually we need to compare the apk from this PR with the apk from this post: https://github.com/TeamNewPipe/NewPipe/projects/17#card-42918515

You compared with 0.19.8 which doesn't make sense. In 0.19.8 the app uses default mechanism of handling the cutout, everything is done by Android. On the newest version there is no way to give this job to Android because of the need to use Fragment instead of an Activity.

So, just compare with the latest apk from the link. More info about the problem the PR trying to fix can be found in #4040

blackbox87 · 2020-08-30T17:04:17Z

You compared with 0.19.8 which doesn't make sense. In 0.19.8 the app uses default mechanism of handling the cutout, everything is done by Android. On the newest version there is no way to give this job to Android because of the need to use Fragment instead of an Activity.

Sorry about that. I haven't been testing debug versions for a while now, so I didn't think to compare it to a different debug build. But I can confirm that the builds by @B0pol all have the same issue. It's awful.

You're not supposed to align videos to the cutcout while using landscape because cutcouts come in many different sizes. If you do then you'll likely end up with off-centered videos and only having rounded corners on one side of your screen. That's why YouTube, VLC & many other players don't do anything about the notch and always center the videos to fit.

blackbox87 · 2020-09-04T00:55:52Z

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java

+        // Prevent jumping of the player on devices with cutout
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
+            activity.getWindow().getAttributes().layoutInDisplayCutoutMode =
+                    WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_NEVER;
+        }


Change to the following and it fixes landscape too.

// Prevent jumping of the player on devices with cutout if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { if (isLandscape()) { activity.getWindow().getAttributes().layoutInDisplayCutoutMode = WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_SHORT_EDGES; } else { activity.getWindow().getAttributes().layoutInDisplayCutoutMode = WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_NEVER; } }

I can do this but in this case a title and a description will be hidden under possible cutout from top right corner (in landscape orientation it will be in top left corner). I'm not sure about how much devices have such cutout but it's not an ideal choice anyway.

That's actually a separate issue that's caused by the status bar height (and navbar height?) not being accounted for while viewing a video in landscape. So I'm currently trying to work out the best way to center the controls.

Good to know that someone cares about it. Not only landscape mode has the problem with the centering but vertical videos too.

blackbox87 · 2020-09-07T01:46:32Z

@avently Check out my commit. It should give you perfectly centered videos and all of the controls will have the correct padding.

And although it's unrelated to this problem I noticed that screen rotation doesn't always behave correctly.

Enable auto-rotate and watch a video in landscape
Disable auto-rotate while watching the video in landscape

Previously NewPipe would return you to your default orientation, but now you'll remain stuck in landscape and NewPipe's fullscreen button won't do anything no matter how many times you press on it. So as it seems buggy I made this change which restores the original behaviour.

avently · 2020-09-07T02:19:25Z

@blackbox87 I didn't look closely on your changes but looks like the changes depend on this PR. And you say now everything is fine. So we can merge this PR and then to wait your PR, right?

About autorotation issue: can you make a PR too?

blackbox87 · 2020-09-07T02:50:14Z

So we can merge this PR and then to wait your PR, right?

It could be done that way or you could just copy my changes into your branch. I'm happy with whatever as long as this gets fixed.

About autorotation issue: can you make a PR too?

I'd like to know if it's the intended behaviour or not first. Maybe @Stypox or @TobiGr can comment?

It's possible that they want NewPipe to remain in landscape mode, but having the button not do anything when you disable auto-rotate seems like a bug either way. It should always be able to toggle between portrait and landscape.

avently · 2020-09-07T03:00:48Z

@blackbox87 I prefer to maintain only my code. So better to have your code inside your PR.

About autorotation: this is my code that makes such situation so if your solution fixes it just make a PR. For me it was like a rare case (when you lock orientation while in landscape) and I just skipped that case

blackbox87 · 2020-09-07T03:25:30Z

@blackbox87 I prefer to maintain only my code. So better to have your code inside your PR.

Fair enough. If someone merges this in then I'll update my fork and create a pull request.

About autorotation: this is my code that makes such situation so if your solution fixes it just make a PR. For me it was like a rare case (when you lock orientation while in landscape) and I just skipped that case

Well there's two ways to fix the issue.

Set the orientation to portrait if auto-rotate is disabled and certain conditions are met.
Fix the fullscreen button so that you can always toggle between portrait and landscape.

I'm not familiar with the code, which is why I went with the first option and made NewPipe behave as it used to. But I think fixing the button might actually be the better solution.

avently · 2020-09-07T03:38:43Z

@blackbox87 second way is not an option at all. So, yeah, the first way is what we need.

Stypox

Code looks good, though I don't have a device to test this on. But if @blackbox87 says he has a fully working solution based on this one I'd merge right away, also to prevent rebasing issues. Thank you @avently ;-)

@blackbox87 thank you for your investigations, as soon as this PR will get merged feel free to create two separate PRs for cutout and rotation. I think your rotation approach is right, obviously having a button doing nothing is a bad idea and the previous behaviour was ok. Thanks :-D

blackbox87 · 2020-09-07T16:50:53Z

@Stypox

I think your rotation approach is right, obviously having a button doing nothing is a bad idea and the previous behaviour was ok.

I've actually figured out how to fix the button, although @avently said that it wouldn't be an option. So we could have NewPipe remain in landscape and only change to portrait when the fullscreen button is pressed again. Personally I think that might be the better solution too.

Stypox · 2020-09-08T17:49:10Z

@blackbox87 yeah, do as you said ;-)

B0pol · 2020-09-08T18:37:26Z

thank you

avently · 2020-09-08T18:39:11Z

thank you

You're welcome. Can wait to see a fix for centering from @blackbox87

avently mentioned this pull request Aug 17, 2020

Content not centered with 18:9 screen -> weird behaviour with navigation bar #4040

Closed

TobiGr reviewed Aug 17, 2020

View reviewed changes

app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java Outdated Show resolved Hide resolved

TobiGr added bug Issue is related to a bug player Issues related to any player (main, popup and background) labels Aug 17, 2020

B0pol added this to the 0.20.0 milestone Aug 17, 2020

avently added 3 commits September 3, 2020 15:48

Prevent jumping of the player on devices with cutout

8030312

Fixed wrong padding on devices with cutout on vertical videos

e833d41

Moved device-specific code into DeviceUtils

b8a35e9

Stypox mentioned this pull request Sep 3, 2020

fullscreen mode too small #4194

Closed

avently force-pushed the video-placement branch from fe90638 to b8a35e9 Compare September 3, 2020 18:41

blackbox87 reviewed Sep 4, 2020

View reviewed changes

Stypox approved these changes Sep 7, 2020

View reviewed changes

wiidev approved these changes Sep 8, 2020

View reviewed changes

Stypox merged commit 530f745 into TeamNewPipe:dev Sep 8, 2020

blackbox87 mentioned this pull request Sep 8, 2020

Center videos correctly & fix padding for controls #4255

Closed

5 tasks

Stypox mentioned this pull request Sep 11, 2020

version 530f745 2020-09-08 crush when i click on video #4267

Closed

ZeroPucks mentioned this pull request Sep 13, 2020

Unified Player seekbar not appearing on Android 11 #4277

Closed

blackbox87 mentioned this pull request Sep 16, 2020

Small fixes of issues with old devices support, brightness, etc #4272

Merged

2 tasks

blackbox87 mentioned this pull request Sep 26, 2020

Fix rotation when auto-rotate is disabled #4254

Closed

5 tasks

This was referenced Sep 27, 2020

Release 0.20.0 #4332

Closed

Release 0.20.0 #4333

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent jumping of the player and wrong padding on devices with cutout #4154

Prevent jumping of the player and wrong padding on devices with cutout #4154

avently commented Aug 17, 2020 •

edited

Loading

blackbox87 commented Aug 30, 2020

avently commented Aug 30, 2020

blackbox87 commented Aug 30, 2020

blackbox87 Sep 4, 2020

avently Sep 4, 2020

blackbox87 Sep 4, 2020

avently Sep 4, 2020

blackbox87 commented Sep 7, 2020 •

edited

Loading

avently commented Sep 7, 2020

blackbox87 commented Sep 7, 2020

avently commented Sep 7, 2020

blackbox87 commented Sep 7, 2020 •

edited

Loading

avently commented Sep 7, 2020

Stypox left a comment

blackbox87 commented Sep 7, 2020

Stypox commented Sep 8, 2020

B0pol commented Sep 8, 2020

avently commented Sep 8, 2020

Prevent jumping of the player and wrong padding on devices with cutout #4154

Prevent jumping of the player and wrong padding on devices with cutout #4154

Conversation

avently commented Aug 17, 2020 • edited Loading

What is it?

Description of the changes in your PR

Fixes the following issue(s)

Testing apk

Agreement

blackbox87 commented Aug 30, 2020

avently commented Aug 30, 2020

blackbox87 commented Aug 30, 2020

blackbox87 Sep 4, 2020

Choose a reason for hiding this comment

avently Sep 4, 2020

Choose a reason for hiding this comment

blackbox87 Sep 4, 2020

Choose a reason for hiding this comment

avently Sep 4, 2020

Choose a reason for hiding this comment

blackbox87 commented Sep 7, 2020 • edited Loading

avently commented Sep 7, 2020

blackbox87 commented Sep 7, 2020

avently commented Sep 7, 2020

blackbox87 commented Sep 7, 2020 • edited Loading

avently commented Sep 7, 2020

Stypox left a comment

Choose a reason for hiding this comment

blackbox87 commented Sep 7, 2020

Stypox commented Sep 8, 2020

B0pol commented Sep 8, 2020

avently commented Sep 8, 2020

avently commented Aug 17, 2020 •

edited

Loading

blackbox87 commented Sep 7, 2020 •

edited

Loading

blackbox87 commented Sep 7, 2020 •

edited

Loading