fix: default quality setting should respect video aspect ratio #5797
Conversation
There was a problem hiding this comment.
When autoplay continues and the next video is a short again, the factor will be (16/9)^2 after your changes because you did fullscreenReolution * 16/9 twice.
And when the next video is a normal one and not a short, it still uses the resolution for shorts.
TLDR: Your code shouldn't change the fullscreenOrientation variables, it should only read them in the function where the resolution is set and set it to 16/9 * resolution ditectly instead of modifying the variable.
|
Now it will set resolution directly without modifing the default variables and when user will change resoultion for Shorts, let say to 1280 then it will save 720 as a default resoultion for current queue. |
|
I think it would be better to do this in |
|
That would work. But, i think it will create some problem when user will select a custom resolution while playing a video. So we can pass a boolean to setPlayerResolution whether it is choosed by user or it's default. |
Yes, sounds good 👍 |
|
Now i had moved the same code to setPlayerResolution. |
app/src/main/java/com/github/libretube/ui/fragments/PlayerFragment.kt
Outdated
Show resolved
Hide resolved
| if (isShort) { | ||
| newResolution = ceil(newResolution * 9.0 / 16.0).toInt() | ||
| } |
There was a problem hiding this comment.
I think this code here should be removed, since we already handle that in setPlayerResolution?
There was a problem hiding this comment.
No, it should not be removed. Because when a user is watching a short in queue and changed resolution to 1280p then all landscape videos will also be played on 1280p which should be played on 720p by respecting the aspect ratio.
There was a problem hiding this comment.
I don't really like/agree on this change, it's not a good idea to modify the newResolution variable, it should always be in the "same format" (normal horizontal video ratio resolution).
There was a problem hiding this comment.
This here makes everything more complicated and I don't think anybody knows what's going on when maintaining that code in the future.
app/src/main/java/com/github/libretube/ui/fragments/PlayerFragment.kt
Outdated
Show resolved
Hide resolved
Bnyro
force-pushed
the
fix-default-quality-setting-for-shorts
branch
from
6f8c9f2
to
7c893cd
Compare
Bnyro
force-pushed
the
fix-default-quality-setting-for-shorts
branch
from
7c893cd
to
12dd526
Compare
Bnyro
force-pushed
the
fix-default-quality-setting-for-shorts
branch
from
12dd526
to
1f218ff
Compare
This fixes the issue #5765 where the default quality settings should respect aspect ratio. Now when we will open the shorts and default quality setting is 720p then it will auto play with 1280p.