-
-
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
Remember adjustment step size for playback controls (speed and pitch) #7728
Remember adjustment step size for playback controls (speed and pitch) #7728
Conversation
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.
I don't think you should pass around the adjustmentStepSize
as a playback parameter. It's not something influencing the playback, it's just a value to save to / read from preferences. You should just read it when the PlaybackParameterDialog
is setup, and save it when the user changes it inside the same dialog.
@Stypox Sorry, I was busy so I didn't have time to look at the code. I'm guessing this should be done similarly to how NewPipe/app/src/main/java/org/schabi/newpipe/player/helper/PlaybackParameterDialog.java Lines 237 to 260 in af80d96
|
@ktprograms yes, I think that's the way to go |
@Stypox Since the step size will now be saved to shared preferences, should I remove the code that saves it to/reads it from outState? |
@ktprograms yes, there is no need to save/restore it there |
- Add adjustment_step_key to settings_keys.xml to be used when saving/loading the step size. - Remove the global stepSize variable and the code that saves it to outState/loads it from savedInstanceState because it's now saved to Shared Preferences. - Move initially setting step size to setupStepSizeSelector to be consistent with the other view setup methods, using the value loaded from Shared Preferences. - Save the step size to Shared Preferences inside setStepSize. Fixes: TeamNewPipe#7031
6d40a63
to
62c0e66
Compare
@Stypox Done, basically rewrote the entire commit, so here's the overview of everything changed:
|
Kudos, SonarCloud Quality Gate passed! |
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! Code looks good to me, I'm going to test it later :-)
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.
I tested and everything seems to work well :-)
Thanks! |
What is it?
Description of the changes in your PR
adjustmentStepSize
to shared preferences, pass it back and forthbetween PlaybackParameterDialog and Player/PlayQueueActivity.
DEFAULT_STEP
in PlaybackParameterDialog public so it can be used asthe default
adjustmentStepSize
value in the Player.Before/After Screenshots/Screen Record
before.mp4
after.mp4
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.
Due diligence