theothernt / AerialViews

A screensaver for Android TV devices including Nvidia Shield, Fire TV, and Chromecast with Google TV. Inspired by Apple TV's video screensaver.
GNU General Public License v3.0
418 stars 32 forks source link

Enhancements to playback speed feature #67

Closed theothernt closed 1 year ago

theothernt commented 1 year ago

A number of additions have already been submitted by @marcopar but I'm recommending a few changes...

  1. Notification when speed is changed?
  2. The left/right key press ability should be off by default
  3. User can only change playback speed every 2 seconds or so - change from the play isn't instant, can cause exceptions
  4. User cannot change speed of a video 3 seconds before it ends
  5. Check video state before playback speed change (isPlaying event state?)

I'm happy to make the above changes, but either way, let me know what you think @marcopar 😅

marcopar commented 1 year ago

Yes, ok for me. And thanks for making the changes,I don't have much spare time and energy lately.

theothernt commented 1 year ago

I've disabled changing playback speed during playback temporarily as I want to get v1.3.1 out to fix a few small issues.

All the changes above should be in the next release 😅

theothernt commented 1 year ago

It won't be in v1.3.2 as I want to get some fixes out - but it will be in v1.3.3 😅

theothernt commented 1 year ago

Ok, all of changes have been made and are merged into the master branch if you'd like to try it sometime?

marcopar commented 1 year ago

Ok, I will install it on my shield in the weekend.

On Fri, 14 Oct 2022, 13:38 Neil Turner, @.***> wrote:

Ok, all of changes have been made and are merged into the master branch if you'd like to try it sometime?

— Reply to this email directly, view it on GitHub https://github.com/theothernt/AerialViews/issues/67#issuecomment-1278889112, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABK7EWGJXOICB32DXK7I6E3WDFAZ5ANCNFSM53YMTKKA . You are receiving this because you were assigned.Message ID: @.***>

theothernt commented 1 year ago

Great, no hurry (as usual).

The speed changing seems to work fine (your code), and I've added those delays + notifications.

I've only done a little testing - most around skipping a video + trying to change the speed as often as possible - so far, no crashes. I'm happy enough with the little notification.

marcopar commented 1 year ago

So far it's working ok.

I would reduce the change speed deadzone to 2 secs.

theothernt commented 1 year ago

Ok, change delay set to 2 seconds :)

theothernt commented 1 year ago

That feature is finally live 🥳

marcopar commented 1 year ago

Great. I noticed some stuttering at certain speeds. I believe it's normal since we change the speed with a percentage. Do you know if it's possible to get the clips FPS and then set the speed in a somewhat more "base FPS" friendly way....like if it's 24 use multiple of 6 or 8...surely not 5...things like this?

On Tue, 21 Feb 2023, 12:43 Neil Turner, @.***> wrote:

That feature is finally live 🥳

— Reply to this email directly, view it on GitHub https://github.com/theothernt/AerialViews/issues/67#issuecomment-1438336844, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABK7EWH5F5DXWRQWYCXZFP3WYSS7JANCNFSM53YMTKKA . You are receiving this because you were assigned.Message ID: @.***>