Gallery icon indicating copy to clipboard operation
Gallery copied to clipboard

Video playback speed control

Open Natolu24 opened this issue 1 year ago • 6 comments

What is it?

  • [ ] Bugfix
  • [x] Feature
  • [ ] Codebase improvement

Description of the changes in your PR

  • Add control to the video playback speed (longpress, then swip up/down to control)
    • It increments by 0.25x, with minimum speed of 0.25x and maximum of 5.0x
  • Add setting to define the default playback speed
    • Possible values : x0.25 / x0.5 / x0.75 / x1.0 / x1.25 / x1.5 / x1.75 / x2.0 / x3.0 / x4.0 / x5.0
  • Add visual indicator to the current playback speed, which appears when the playback speed is changed from the default value
    • And can be clicked to reset to the default speed value

Before/After Screenshots/Screen Record

  • Before: Screenshot_Gallery_debug_Before

  • After: Screenshot_Gallery_debug_After

Fixes the following issue(s)

  • Fixes #116

Acknowledgement

Natolu24 avatar May 09 '24 11:05 Natolu24

Add control to the video playback speed (longpress, then swip up/down to control)

It's counterintuitive. How somebody will know that it's done this way? Also, there's no visual guidance - I don't know when I can start moving my finger to change the speed. I think it should rather be done similarly as it's in Fossify Music Player (and similar to numerous other players like NewPipe or VLC), where we have an icon/button to change the playback speed, and it shows a normal slider.

It increments by 0.25x, with minimum speed of 0.25x and maximum of 5.0x

Why this range? I doubt anybody will need speed higher than 3x. In Fossify Music Player we have range between 0.25x to 3x without limiting to multiplies of 0.25. Same is in NewPipe.

Add visual indicator to the current playback speed, which appears when the playback speed is changed from the default value

Why it displays on left if it's slower and on right if it's faster? It should stay in one place, moving elements are annoying for users.

Add setting to define the default playback speed

I think more intuitive would be to remember last playback speed instead of a separate option in Settings. I'd assume that if I chose 2x in one video, it would be kept until I change it. That's how YouTube works, so that's what people are used to.

Aga-C avatar May 09 '24 13:05 Aga-C

Thanks for the review, it all does indeed makes sense, we'll fix all that !

Natolu24 avatar May 09 '24 15:05 Natolu24

Ok so we somewhat copy pasted the implementation from the Fossify Music Player to here :

Screenshot_20240515_201011_Gallery_debug(1)

Does the location look fine, or the UI itself need some adjustment ? (I guess adding a background to the icon could be nice)

Also, there's still a problem that we have issue finding a solution to : When changing the playback speed, and then changing to a video that has already been started/opened, the playback speed won't be taken into account in the video nor the UI text. I'm thinking that changing the playback speed should then affect all the videos currently started, but I do not really know how exactly it works, and how to implement it right, would someone have an idea / hint where to change things ?

Natolu24 avatar May 15 '24 18:05 Natolu24

  • Background should be added, so it would look similarly to play button. Now the button is invisible on the light theme with horizontal videos. image

  • After checking in settings Always open videos on a separate screen with new horizontal gestures, there's no button for changing speed. Also, it always plays videos with 1x speed in this mode.

  • When you change speed to lower than 1x, the icon changes, what's the same as in Music Player. But when I open a video, there's always an icon with a running man, no matter the speed. Icon should always be adjusted to the speed.

Aga-C avatar May 16 '24 06:05 Aga-C

Background added, and fixed the rest ! Screenshot_20240516_161948_Gallery(1)

Natolu24 avatar May 16 '24 15:05 Natolu24

Seems to work fine, thanks! Now please wait for the final review by Naveen 🙂

Aga-C avatar May 16 '24 16:05 Aga-C

Just made some XML layout changes. Not too happy with the playback speed button placement but I don't have any ideas at the moment either so I guess I'll merge this now and improve it later. Thanks!

naveensingh avatar Sep 07 '24 17:09 naveensingh