po5 / thumbfast

High-performance on-the-fly thumbnailer script for mpv
Mozilla Public License 2.0
760 stars 34 forks source link

Exact seek no interval #50

Closed christoph-heinrich closed 1 year ago

christoph-heinrich commented 1 year ago

The changes from #41 are still relevant, but GitHub didn't let me reopen after I pushed so here is a new one.

Limiting the thumbnail count doesn't make sense when they are generated on the fly, and those limits enables us to do an exact seek after the user stops moving the mouse, allowing them to see the exact frame at the requested time.

Initializing things on startup allows us to simplify spawn() a bit, after all there is no benefit in checking every time if things need to be initialized.

With commands being very fast now, we could even remove the seek throttling. That would allow us to simplify the seeking code.

Seeking without throttling ```diff diff --git a/thumbfast.lua b/thumbfast.lua index 5e97de5..77db9e4 100644 --- a/thumbfast.lua +++ b/thumbfast.lua @@ -396,26 +396,14 @@ local function seek(fast) end local seek_period = 3/60 -local seek_requested = false local seek_timer -seek_timer = mp.add_periodic_timer(seek_period, function() - if seek_requested then - seek_requested = false - seek(true) - else - seek_timer:kill() - seek() - end -end) +seek_timer = mp.add_timeout(seek_period, seek) seek_timer:kill() local function request_seek() - if seek_timer:is_enabled() then - seek_requested = true - else - seek_timer:resume() - seek(true) - end + seek_timer:kill() + seek_timer:resume() + seek(true) end local function check_new_thumb() ```
po5 commented 1 year ago

Exact seeks get triggered pretty often when moving over the timeline, especially when the video is unpaused.
This is probably bad for performance.

Could you add something more flexible than the seek_requested boolean? A higher seek_period fixes the issue but makes the whole thumbnail generation slow.
A counter of how many seek_periods have passed, or a delay in seconds, whatever is easier to implement.

I will reintroduce thumb_index and index_time in some form later, as I want to support caching (someone may have an even worse CPU than me) and it will be useful for YouTube support.

christoph-heinrich commented 1 year ago

For me it behaves as expected.

https://user-images.githubusercontent.com/8932183/198852504-1c87e6af-f4dc-4ac6-a46e-70ba0eb7c592.mp4

po5 commented 1 year ago

Perform the same test on an unpaused 24fps video (may have to turn off fancy video-sync?).
It triggers often.

christoph-heinrich commented 1 year ago

24fps, 1x speed, video-sync=audio Still doesn't really happen for me.

https://user-images.githubusercontent.com/8932183/198853719-e007b324-4640-47bb-9df5-179d2e3c4e35.mp4

I've replaced the boolean with a counter. In the version without throttling the time between the last fast seek and the exact seek could be easily configured.

po5 commented 1 year ago

It happens in your video at ~2s. Happens less often with this change, so I'm fine with it.