-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Change pitch by semitones #7565
Conversation
Woohoo! I think this is a really good idea. Thanks for thinking of it and working on it! |
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.
Good work! I will probably use this feature. Code mostly looks good, thank you :-)
Note for maintainers (nothing wrong with this PR): PlaybackParameterDialog
should use ConstraintLayout
and view binding (if the latter is not possible it should at least be completely refactored) in a future PR
app/src/main/java/org/schabi/newpipe/player/helper/PlaybackParameterDialog.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/player/helper/PlaybackParameterDialog.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/player/helper/PlaybackParameterDialog.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/player/helper/PlaybackParameterDialog.java
Outdated
Show resolved
Hide resolved
@haklc Instead of the dev branch, please name your branch something meaningful: https://github.com/TeamNewPipe/NewPipe/blob/dev/.github/CONTRIBUTING.md#creating-a-pull-request-pr |
Will do. |
@opusforlife2 @haklc now it's too late to change the branch's name... Let's keep it |
@Stypox I've resolved your comments, the code should now be good to go :) |
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 :-D
Code looks good, and I tested again and found no issues. Will be merged next release :-)
Awesome! :D |
Keep in mind that this will negate the optimizations done in #7363. I also think that the class needs an overhaul, however this is not part of this issue. |
What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
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