mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.2k stars 2.89k forks source link

Stats overlay lingers when keeping key down #6412

Open occivink opened 5 years ago

occivink commented 5 years ago

mpv version and platform

mpv 0.29.1 Copyright © 2000-2018 mpv/MPlayer/mplayer2 projects
 built on Sun Dec  9 16:21:27 CET 2018
ffmpeg library versions:
   libavutil       56.22.100
   libavcodec      58.35.100
   libavformat     58.20.100
   libswscale      5.3.100
   libavfilter     7.40.101
   libswresample   3.3.100
ffmpeg version: n4.1

Reproduction steps

Play a file and keep i pressed down for some time. Release the key.

Expected behavior

After releasing i, the stats overlay stays visible for the duration script-option of stats.lua.

Actual behavior

The stats overlay stays visible for duration plus some time depending on how long the key was kept down. This is probably happening because key repetition events happen faster than the redraw of the stats, so events get backed up. Drawing in an idle handler could solve this.

Log file

https://0x0.st/snlY.json

Argon- commented 5 years ago

It used to be a bit more like what you want back when there was no "continuous" mode. People wanted it to change.

Nevertheless, the "keep pressing and after the last keypress it still holds on for duration" is in accordance with other OSD commands' behavior. Deviating from this "standard behavior" would certainly be unexpected and worsen UX.

occivink commented 5 years ago

I think you misunderstand, my point is that it holds for (sometimes much) longer than duration. I have a 45ms repeat rate, and if I hold i down for 10s, the overlay stays visible for about ~15s after I release the key, even though duration is set to 4

Argon- commented 5 years ago

Ah, now I get it. But your suggestion with an idle handler would no longer make it update "fast". E.g. right now keeping the button pressed will update as fast as it's possible for the script. Using an idle handler will delay that until your key presses are processed. If you are not interested in fast updates but still want to have stats up longer than duration then I is what you are looking for.

occivink commented 5 years ago

The only difference it should make in update speed is when processing time > repeat rate, in which case the updates will be aligned to multiples of the repeat rate. I think that's better, especially when you consider that the script becomes unresponsive as it's processing the events that have been accumulated (e.g. you can't press I until right after holding i for some time).

Argon- commented 5 years ago

A quick test with mp.register_idle(fn) only executed fn after all key events got processed. Therefore, in my case, as long as I kept i pressed, fn was never executed, resulting in the undesired behavior I described above. Is this different for you? Or did you have something else in mind?

occivink commented 5 years ago

I had something like this snippet in mind:

do_stuff = false

mp.register_idle(function()
    if not do_stuff then return end
    --os.execute("sleep " .. tonumber(0.1))
    print("idle")
    do_stuff=false
end)

mp.add_key_binding("j", "test", function()
    print("binding")
    do_stuff=true
end, {repeatable=true})

Normally "idle" and "binding" are printed at the same time periodically when you keep j down. If you uncomment the sleep call (to simulate slow drawing or low repeat rate), you should get multiple "binding" prints for one "idle" but the script stays responsive and stops as soon as your release the key.

Argon- commented 5 years ago

I get what you mean, it didn't work like that in my quick test though. I'll make a more thorough attempt and recheck, might've done something wrong.

I'm not a big fan of it though. For one, your snippet triggers "idle" multiple times for a single key press. That means I'd have to introduce more (global/script-local) state to work around that because I need to make sure which idle run I have to ignore and which one to use for drawing. And what about the recorders for performance data in toggled mode, when the script observes properties and triggers every tick? I made sure to cut down the logic evaluated during those runs to minimize impact on performance (lua is not fast after all).

If possible I'd prefer a different solution... not sure if it exists though. Maybe block registering new key bindings while the stats are being drawn by removing and re-adding the bindings (not ideal as it would kill bindings in input.conf). Also there must be a queue somewhere where the key press events are stored. Decreasing this queue's depth would be a quick fix.

ghost commented 4 years ago

Uh, you can just use key down/up events and ignore auto-repeated ones.