Seek by file

[christoph-heinrich]

Just like in my previous PR #41 I'm removing the interval and thumbnail count things because since we're not generating thumbnails in advance, they don't really make sense. Also removing them enables us to do exact seeks at the end of dragging.

Instead of seeking via commands sent through the socket, seek requests are now written to a file that is then read by the sub process. Not going though the socket has massive performance benefits. Look how smooth it is now and compare it to the videos in #40.

https://user-images.githubusercontent.com/8932183/194593907-ab308510-011f-4b13-aeef-aeb8878d0d31.mp4

[po5]

Isn't this a roundabout way to say spawning a subprocess is inefficient?
Writing to the socket directly (#35) should have better performance than this.
I believe this PR still runs into the issue of blocking lua io, and fail to see the improvement over #35.
Haven't tested either yet, but it's my first impression.

[christoph-heinrich]

The videos should be proof enough :wink:. Try it out. For me there is 0 stutter now, where as before, every time a seek command gets sent, it stutters a little.

I don't know how it is on windows, as I'm on linux and so #35 also makes no difference to me. But I have tried #37 and it still stutters.

https://user-images.githubusercontent.com/8932183/194621412-1ebce5cd-61d0-4e49-86ed-0a54159f7baf.mp4

[christoph-heinrich]

Isn't this a roundabout way to say spawning a subprocess is inefficient?

How is it saying that? It still spawns the subprocess, the only difference is how the subprocess receives it's seek commands.

As I have pointed out in https://github.com/po5/thumbfast/pull/40#issuecomment-1270440742, sending via socket is what's causing the stuttering, so I found a different way of doing it.

[dyphire]

Isn't this a roundabout way to say spawning a subprocess is inefficient? Writing to the socket directly (#35) should have better performance than this. I believe this PR still runs into the issue of blocking lua io, and fail to see the improvement over #35. Haven't tested either yet, but it's my first impression.

It can be confirmed that #35 is much better on Windows.

[po5]

The issue isn't sockets it's that we spawn another process to send anything to it.

[christoph-heinrich]

This would be a performant cross platform solution. Since I can't test #35 I don't know if it works as well as this, but even if it does, it's still windows only.

[qwerty12]

~~I realise talk's cheap so I'll put up a quick video if really desired, but from the video, #35 + thumbfast HEAD is faster than this on Windows. But it's not a fair comparison because I'm going outside of standard Lua there. I wouldn't go back to using io.open though; being able to put the pipe handle into non-blocking mode is where the advantage is gained. As per MSDN: "WriteFile always [returns] immediately". The advantage to this over #35 is that it's standard Lua and is more or less consistent on all platforms, which actually is a big advantage over #35 maintenance-wise. That said, I'd probably end up trying to maintain a personal fork that keeps the socket-based approach, because Windows is known for its slower FS performance compared to Linux and because I like that the thumbfast mpv process can wait until it receives data instead of having to use a timer to check.~~

I would've read Beej's guide and provided an equivalent of #35 for Linux, but it's the LuaJIT availability that's a blocker there. On Windows, I don't think I'm exaggerating when I say it's a near-guarantee that users are using mpv with LuaJIT because that's what shinchiro cooks up, but with Linux I can't say the same. Arch uses mpv linked to LuaJIT; however, with Fedora, RPMFusion links to PUC-Lua 5.1, so that's already a fair amount of people who can't take advantage. *Though on Linux, LuaRocks isn't a pain to use and luaposix/luasocket can be installed and used by mpv...

Sorry

[christoph-heinrich]

Currently it only checks for seeks every 100ms, that shouldn't be a problem even on windows. And it stops looking for updates if the file hasn't been written to in 100ms. The timer gets activated again when the title gets set via the socket (wake up signal), so it only checks while you're moving around and then stops pretty much immediately afterwards.

[christoph-heinrich]

but from the video, #35 + thumbfast HEAD is faster than this on Windows.

I only now realized that you probably mean the thumbnail generation is faster. That is not what this is about at all. If I don't cripple my cpu by locking it to it's lowest frequency, I also get faster generation :laughing:.

https://user-images.githubusercontent.com/8932183/194678849-cec7abec-0f40-473e-a634-069b48863e19.mp4

And if that's still too slow for you, reducing the 100ms is also an option. We could even make it configurable.

https://user-images.githubusercontent.com/8932183/194678984-090084be-9c28-42e2-bf6e-fac1c2d6bed5.mp4

But this PR is about the stuttering of the video playback while seeking around with thumbnails and not about the speed of thumbnail generation.

[qwerty12]

I only now realized that you probably mean the thumbnail generation is faster.

Guilty as charged. Having actually tried thumbfast-seek-by-file.lua myself instead of relying on the initial video, I have to say I don't think #35 is any faster when it comes to thumbnail generation. I don't get any stuttering with #35 either when rapidly seeking for a longish amount of time on a 4k video but, yeah, Windows only. Sorry, I only felt compelled to comment because this being merged would've meant I wouldn't be able to use #35 but now I guess it doesn't matter.

[christoph-heinrich]

While this improves the situation while moving around, it still relies on the socket for the wake up signal and for setting properties, so #37 and #35 would still be beneficial imo.

[christoph-heinrich]

After playing around with some small fast to decode files, there was some benefit to halving the file check rate from 100ms to 50ms (3/60s), but lower then that gives no practical benefit even for files with near instant seeking. But with recent complaints about cpu usage, maybe we should make it configurable?

Also I've encountered cases where the timer got killed prematurely, so I gave it one additional period of slack to make sure that doesn't happen.

[po5]

Will revisit this to allow seeking on mpv <0.33.0 on Linux without socat installed.
It would also bring Mac support to 0.29.0, unifying the version support as was always intended.
Because these constraints mean no direct communication with the thumbnailer process, we'd have to use a simple unconditional check interval. It's a compatibility fallback, anyway.

[po5]

Note for myself, this way of getting the script path likely doesn't work with mpv <0.35.0 (before this commit https://github.com/mpv-player/mpv/commit/84821dbcb6d9e16b8f11da2135208e4f3e66fcd0).
We should use debug.getinfo(1, "S").source:gsub("^@", "") instead of debug.getinfo(1, "S").source:sub(2), to only remove the at-sign when it's there.