Skip to content
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

Throttle seeking #40

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Throttle seeking #40

merged 1 commit into from
Oct 6, 2022

Conversation

christoph-heinrich
Copy link
Contributor

@christoph-heinrich christoph-heinrich commented Oct 6, 2022

I don't know why this is such a problem, but sending commands causes the player to stutter for me. Throttling seeking commands doesn't solve the problem but it certainly helps. The last sent seek will be sent up to 100ms later then without, but in practice that doesn't really make a difference. If the last seek was long enough ago, it will send the command immediately without a timer.

before.mp4
after.mp4

@natural-harmonia-gropius
Copy link
Contributor

This problem is because the hardware cannot handle the pressure of decoding, Easy to reproduce with high bitrate video.

@po5
Copy link
Owner

po5 commented Oct 6, 2022

This is needlessly complex, I had that feature implemented without timers in a single line guard in thumb().
I removed it because it's not a solution and is not needed on every video.
Will add it back as an option, I guess some people may want it.

@christoph-heinrich
Copy link
Contributor Author

Decoding is not the problem, I use a 1 second buffer of decoded frames. But I'm sure better hardware would have less of a problem with it.
It also can't really be the cpu usage of the sub process, because when I replace the seek command with set pause yes it still happens.

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Oct 6, 2022

This is needlessly complex.

I doubt it can be implemented much simpler then this without missing out on the last seek. This is the same as rendering is throttled in osc.lua and uosc.

@po5
Copy link
Owner

po5 commented Oct 6, 2022

without missing out on the last seek

You're right, I've left my solution below.
It works because uosc constantly calls thumb(), but that falls apart when the video is paused and uosc only calls thumb() once with the mouse not moving. Better to not rely on how other scripts call the thumbnailer, too.
Replace 0.1 with a ridiculous value like 5 to see a skipped seek.

diff --git a/thumbfast.lua b/thumbfast.lua
index d164ca8fc8..452007c01e 100644
--- a/thumbfast.lua
+++ b/thumbfast.lua
@@ -62,6 +62,7 @@ local last_x = x
 local last_y = y
 
 local last_seek_time = nil
+local last_request = 0
 
 local effective_w = options.max_width
 local effective_h = options.max_height
@@ -408,6 +409,16 @@ local function thumb(time, r_x, r_y, script)
     end
 
     if seek_time == last_seek_time then return end
+
+    local cur_request = mp.get_time()
+
+    if cur_request - last_request < 0.1 then
+        last_seek_time = nil
+        return
+    end
+
+    last_request = cur_request
+
     last_seek_time = seek_time
     if not spawned then spawn(seek_time) end
     run("async seek "..seek_time.." absolute+keyframes")

The seek interval of 0.1 seems fine.
seek() or request_seek() needs a check for last_seek_time == nil, to not crash when switching files.
We may want to clear the timer in clear(), to avoid unnecessary delays in the initial thumb for a big jump. This only really matters for very high seek_period, but I'd like to account for it.

@christoph-heinrich
Copy link
Contributor Author

Done

@po5 po5 merged commit eb21b2e into po5:master Oct 6, 2022
@christoph-heinrich christoph-heinrich mentioned this pull request Oct 7, 2022
@hooke007
Copy link
Contributor

hooke007 commented Oct 7, 2022

I tested different seek intervals.
0.2 seems to be more friendly with my hardware.
0.5 nearly has no effect on whatever I see. But I am not sure if it would introduce another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants