po5 / thumbfast

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

Fix two subprocesses with `spawn_first=yes` #45

Closed christoph-heinrich closed 1 year ago

christoph-heinrich commented 1 year ago

There is a problem where spawn_first=yes leads to two subprocesses running at the same time.

The first one got spawned in file_load() and the second one in watch_changes(). A quit command did get sent out between those, however the first process wasn't yet ready to receive commands, so it got ignored.

The solution is to rate limit spawning and let spawn handle sending quit when there is already a subprocess spawned.

A spawn period of 400ms would still sometimes have two subprocesses running while 500ms worked very reliably in my testing. I went with 1s to make sure it's not a problem on slower systems.

po5 commented 1 year ago

Can you reimplement on master if the issue is still present?

christoph-heinrich commented 1 year ago

I've reimplemented it because the issue is still present on master, however it doesn't always work anymore.

The problem is that result.status is often -2 even when the subprocess starts seemingly without a problem (mpv, tail,... is all there), and because of that spawned gets set to false, which means the next time the timeline gets hovered, it spawns another set of processes.

Edit: I can only observe the status being -2 despite spawning fine when using spawn_first=yes, otherwise it never happens. No idea what's going on here.