-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat: Add hold-to-fast-forward (2x speed) feature #12843
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
base: refactor
Are you sure you want to change the base?
Conversation
|
please fill out the template properly and add screen recordings |
I'm not sure what the create clones sections means Or relies on the following changes? That's my entire pr so I'd be restating my pr |
|
In the section "Description of the changes in your PR" you should replace the bullet points with your own (The ones you wrote at the very top). "Relies on the following changes" is only relevant when you need a different PR to be merged first, so you can leave it empty. |
This should be completed properly now
Thank you, appreciate the clarification Appreciate the patience |
|
I don't know if some of these tests failing means this can't be merged; I'm not sure what's going wrong. If it would prevent a merge, I will investigate further |
|
The PlayerService.kt lint errors were not from my changes. They came from when I rebased on PR #12838 to work around the JitPack dependency issue (Could not find com.github.TeamNewPipe:NewPipeExtractor:0023b22...). I've now rebased back to the current upstream/refactor. The CI will likely fail due to the JitPack issue, but my actual code changes don't touch PlayerService.kt at all. Once PR #12838 is merged, I can rebase on the updated refactor branch and CI should pass. |
|
I fixed this in 40caeb4 so a rebase should make the checks pass. |
- Add tap and hold gesture to increase playback speed to 2x while held - Add settings toggle in Video & Audio settings to enable/disable the feature - Add visual indicator overlay showing '2x Speed' when feature is active - Feature is enabled by default and respects user preference
Great, thank you! |
is there anything stopping this from being merged? |
| originalPlaybackSpeed = player.playbackSpeed | ||
|
|
||
| // Set playback speed to 2x | ||
| player.setPlaybackSpeed(fastForwardSpeed) |
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.
Wouldn't it be more generally helpful if it didn't set the speed to 2.0, but to twice the originalPlaybackSpeed? I'm thinking about cases like this:
I'm playing a video on 2.5 speed and then use this feature. I would be very confused if this led to a slower temporary playback speed of 2.0.
For the majority of people only ever watching on 1.0 speed, it would be the exact same behavior as your current PR has.
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.
This is just a copy paste from how the YouTube functionality works and other video services. I think there is some benefit to doing a 2x original source but may be too fast if you're watching at 1.5x and hold to get 3x instead of 2x.
I think just going with the industry standard would be fine imo
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.
On YouTube, the case I mentioned can't occur, though, because their max speed is 2.0.
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.
They just recently added higher speed for premium users. It makes sense too, wanting to increase speed momentarily while above 2x Speed would be exceedingly rare. There is value too in having a consistent known value too I know I will always get 2x speed regardless of which speed I'm currently watching at
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.
So you're saying that YouTube actually slows down when you do this while being on 3x speed? 😮
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 have not verified if it slows down or does nothing but I think this is a pretty extreme edge case

What is it?
refactorbranchDescription of the changes in your PR
Before/After Screenshots/Screen Record
Fixes the following issue(s)
Relies on the following changes
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. You can find more info and a video demonstration on this wiki page.
Due diligence