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

Fit / Fill / Zoom bug #5613

Closed
4 tasks done
efb4f5ff-1298-471a-8973-3d47447115dc opened this issue Feb 17, 2021 · 3 comments · Fixed by #5792
Closed
4 tasks done

Fit / Fill / Zoom bug #5613

efb4f5ff-1298-471a-8973-3d47447115dc opened this issue Feb 17, 2021 · 3 comments · Fixed by #5792
Assignees
Labels
bug Issue is related to a bug GUI Issue is related to the graphical user interface player Issues related to any player (main, popup and background)

Comments

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link

efb4f5ff-1298-471a-8973-3d47447115dc commented Feb 17, 2021

Checklist

Steps to reproduce the bug

  1. Open a random video
  2. Watch the video in fullscreen
  3. Choose Fit / Fill or Zoom
  4. Go out of fullscreen
  5. Close the video
  6. Open another random video
  7. watch the video in fullscreen
  8. Notice that the option u chose changed to another one

Actual behaviour

The value doesnt stay the same.
if u choose Zoom then it will default to Fill
if u choose Fill it will default to Fit
if u choose Fit it will default to Zoom

Expected behavior

Value must stay the same!

Screenshots/Screen recordings

signal-2021-02-17-110941.mp4

Logs

Device info

  • Android version/Custom ROM version: Android 11 / Graphene OS
  • Device model: Google Pixel 3a XL
@litetex
Copy link
Member

litetex commented Feb 21, 2021

It think I managed to find the problem by reviewing the code.
However I'm not 100% sure, because I don't have NewPipe setup locally for developing.

The problem is very likely on

player.getPrefs().edit().putInt(
player.getContext().getString(R.string.last_resize_mode), resizeMode).apply();

When the a new resizeMode is set by calling nextResizeModeAndSaveToPrefs (from

void onResizeClicked() {
if (binding != null) {
setResizeMode(nextResizeModeAndSaveToPrefs(this, binding.surfaceView.getResizeMode()));
}
}
), it uses the old resizeMode to get the next/new one.
However it stores the old one in the preferences.

So when switching from ZOOM to FIT, it returns FIT (which is used for the current video), but stores ZOOM in the preferences (used for the next videos)

This was probably introduced by commit f6e2dd1 from @Stypox last month

@triallax triallax added GUI Issue is related to the graphical user interface player Issues related to any player (main, popup and background) labels Feb 28, 2021
@TobiGr TobiGr self-assigned this Mar 7, 2021
TobiGr added a commit that referenced this issue Mar 7, 2021
I think the settings key "last_resize_mode" is ambigous. While it is used to get the recently used resize mode, someone thought while working on the resize mode switcher, that the old (to be replaced) resize mode should be stored. 
Fixes #5613
TobiGr added a commit that referenced this issue Mar 8, 2021
I think the settings key "last_resize_mode" is ambiguous. While it is used to get the recently used resize mode, someone thought while working on the resize mode switcher, that the old (to be replaced) resize mode should be stored. 
Fixes #5613
@TobiGr
Copy link
Contributor

TobiGr commented Mar 9, 2021

Can you test the APK from #5792 and tell me, whether the bug is fixed?

@litetex
Copy link
Member

litetex commented Mar 9, 2021

Stypox pushed a commit to B0pol/NewPipe that referenced this issue Mar 18, 2021
I think the settings key "last_resize_mode" is ambiguous. While it is used to get the recently used resize mode, someone thought while working on the resize mode switcher, that the old (to be replaced) resize mode should be stored. 
Fixes TeamNewPipe#5613
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug GUI Issue is related to the graphical user interface player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants