-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Small fixes of issues with old devices support, brightness, etc #4272
Conversation
brightness levels work ok now👌 |
@avently The seekbar jumps up a pixel in this as well. To be clear, it's not just the seekbar that moves. The two timers and the fullscreen button also move with it. The whole bottom setup also shrinks by a pixel on either side, I just noticed. |
@opusforlife2 it's probably happens because in the app status bar in landscape mode and have one size, then you move out of the app, the status bar in portrait and it's larger, when you move Into the app again, status bar in landscape again. So the whole view will be repositioned on every such situation. And there are more elements like navigation bar make a difference. Anyway:
|
Neve said it was.
It's not worth fixing anyway. I just noticed it when I was comparing padding between blackbox's PR and the latest unified debug, so I asked. |
I never said that you said:) I said it just for future reviewers of the PR. So it means that this PR is fine.
Actually I thought you'll open an issue but you agree with me. Bravo! It was unexpected :) |
🤣🤣🤣 both of you guys are just hilarious especially avently |
I have something interesting today. After I initially made this https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java#L1492 So I found a new (old) way of how player positioning should work. It doesn't need to have all these manual calculations. It just works (almost identical to 0.19.8). I checked in portrait/landscape, vertical/non-verical videos, with/without cutout, on 4.4 (emulator), 9, 11 (emulator), on Android TV (only a little bit), in multiWindow mode. Maybe some bugs still present on real devices, can't say. It's not a As I said, it works like in 0.19.8 but I don't like how shadow covers the player. Shadow should be reimagined too but not from me or not now 100%. For non-cutout the shadow works ok, but for cutouts it isn't as good as I want it to be. Let me know if some bugs are present. @opusforlife2 @TobiGr @Stypox @blackbox87 @wb9688 @B0pol Edit: updated version with a perfect shadow: |
Sorry, it's not exactly clear what you've changed from a user perspective. Can you clarify what to test? |
@avently with that positioning APK on Huawei P9 lite android 7.0, the controls go behind the system ui (hardware buttons and top dropdown menu indicator) instead of being constrained by them |
@opusforlife2 we have a video view that should be located under status bar and navigation bar in landscape, under status bar in multiwindow mode, away of statusbar in portrait. So if you see some button/control is inaccessible or located at a wrong position in the player view it means you find a bug. |
@Stypox but this device doesn't have Can you show a screenshot? |
@Stypox I installed Android 7.0 emulator and everything works fine, how to reproduce the issue you reported? |
Alright. I have a simple display. Hardware buttons and no cutouts/notches. Here's my testing comparing base unified and positioning apks:
This happens with auto-rotate both off and on. That's it! That's the only bug I found. People with cutouts/notches/navigation bars may have other issues, though, so I suggest trying this experiment in a separate Issue, and if it works out, a PR later. |
Yeah, sorry, I meant those buttons. Anyway, I can't reproduce. Either I tested the wrong app (though I am pretty sure I tapped on "Open" after having installed the apk you provided), or who knows. I didn't do anything strange to create the situation I described above, I just tapped on a random video in trending and tapped on the fullscreen button. |
Fiiine. Made a perfect shadow. Shadow of all shadows!
For me with every configuration and on any devices I test the app works perfectly.
|
ok so @avently I tried this apk #4272 (comment) and got the same result as #4272 (comment) ... although the video would go back playing normal if you return the aspect ratio to fit... moreover, normal landscape videos don't suffer from this problem, on the other hand, probably because all three aspect modes have no effect what's so ever... perhaps we can test this if there is a landscape video with a weird aspect ratio.
do you mean by the second issue the brightness issue? p.s: testing device.. galaxy s4 - android 4.4 |
This one: Also let me know how to reproduce the brightness issue. And thank you for the testing. Looks like the only thing I can do is to disable ZOOM mode for KitKat devices... so this mode will act like FIT mode. |
@avently I have been trying for the past week to catch the exact steps to reproduce this bug but failed. however this is the closest I got:
expected behavior: when changing brightness... it would start from the same exact point used on the last video. |
This can happen before this PR and I fixed such situation here. Are you sure you see such behavior in |
- the app will not rotate the screen to portrait after video completes, it will just exit from fullscreen mode - ability to rotate the orientation via fullscreen button from landscape to portrait when device has locked orientation in landscape - ability to enter/exit to/from fullscreen on tablets with unlocked global orientation in portrait mode
In this apk:
More recent version below |
tried final3 #4272 (comment) on android 4.4 and does solve those mentioned problems. |
… on vertical video to portrait after clicking on fullscreen button
Same as #4272 (comment) but with some changes:
|
I'm reporting a small visual bug here which is present in the unified debug, since it probably has a small fix as well: Play a playlist or some other queue, and open the queue from the main player. Swipe down in the middle. The queue will rapidly vibrate up and down. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Thank you for your ongoing efforts ❤️
app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java
Show resolved
Hide resolved
Vibrate? What do you mean? Show a screenrecord, nothing unexpected happens for me |
Give me a minute. |
Wow. Just... wow. The video was large so I scaled it down using ffmpeg from 1080 to 720. Somehow the size went from 43 MiB to under 2 MiB. O.o |
I was able to reproduce this, too. |
Thanks, reproduced.
I use Pixel 3 emulator with Android 11 and no such problem. Are you sure you use the latest code or apk? Or do I need to do something special to reproduce? |
Never mind. Some noob tested the wrong version... |
fixed |
Confirmed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, once again!
What is it?
Description of the changes in your PR
Some very small issues were fixed by this PR.
Fixes the following issue(s)
closes #4271
closes #4253
closes #4245
closes #4230
closes #4277
closes #4143
closes #4294
closes #4292
closes #4322
Testing apk
find the testing apk somewhere in this page because it's changes often
Agreement