osc: add property to signal thumbnailer status

[N-R-K]

this was part of the initial pull request (#17518) but was dropped after adding the ass field to the draw request.

however another use case for this is for UI scripts which position their elements based on whether the thumbnailer is active or not (e.g modernZ ¹).

while the docs already mentions that multiple thumbnailer isn't supported in practice you could have two non-conflicting thumbnailer (e.g thumbfast for local files and thumbyt for network/youtube files) installed and they'd have worked fine.

an unfortunate side effect of adding an enabled field is that such configuration will no longer work (reliably) since both scripts will end up setting this field to false without synchronization.

Fixes: #17518 (comment)

Footnotes

1. https://github.com/mpv-player/mpv/pull/17518#discussion_r3114110555 ↩

[N-R-K]

If anyone has any idea on how to make two non-conflicting thumbnailer work while still signaling whether one is active or not to the osc in a simple manner then feel free to share.

(ping: @Samillion @nekoxuee)

[na-na-hi]

however another use case for this is for UI scripts which position their elements based on whether the thumbnailer is active or not

This can simply be user-data/osc/draw-preview/ass-inactive. This avoids the need for this and also sync issues with delay between observing enabled and then change ass.

[CogentRedTester]

If anyone has any idea on how to make two non-conflicting thumbnailer work while still signaling whether one is active or not to the osc in a simple manner then feel free to share.

(ping: @Samillion @nekoxuee)

You could require thumbnailers to set a property like user-data/mpv/thumbnailers/<script-name> (or equivalent):

mp.set_property_bool('user-data/mpv/thumbnailers/'..mp.get_script_name(), true)
mp.del_property('user-data/mpv/thumbnailers/'..mp.get_script_name())

And then UI scripts can test for it like so:

if next(mp.get_property_native('user-data/mpv/thumbnailers', {})) then
    ...
end

[N-R-K]

This can simply be user-data/osc/draw-preview/ass-inactive. This avoids the need for this and also sync issues with delay between observing enabled and then change ass.

I don't see how that solves anything. Unlike preview border, this isn't some binary "show if thumbnail shows" thing. Take a look at the modernZ link:
#17518 (comment)

It's changing the position of the chapter marker based on the thumbnail. So it needs to know whether thumbnail is active or not.

[na-na-hi]

It's changing the position of the chapter marker based on the thumbnail. So it needs to know whether thumbnail is active or not.

The UI script does not need to know. It only needs to set both ass to the content to draw when the thumbnail is active and ass-inactve to the content to draw when the thumbnail is inactive. The thumbnail script then picks which one to draw depending on whether thumbnail is active or not.

[N-R-K]

The thumbnail script then picks which one to draw depending on whether thumbnail is active or not.

And if no thumbnailer is active/installed then nothing will be drawn and the chapter name will get dropped.

[na-na-hi]

And if no thumbnailer is active

Thumbnailer can draw ass-inactve content when it is inactive.

installed

This can be an OSC option. If the user chooses not to install thumbnailer that option can be set so that OSC draws the marker instead.

[N-R-K]

This can be an OSC option. If the user chooses not to install thumbnailer that option can be set so that OSC draws the marker instead.

This feels like an unjustified level of gymnastics being done just to avoid letting the ui know whether a thumbnailer is active or not.

I also feel like that this is also more inconvenient than just adding an if statement with thumbnailer_enabled. But I have not confirmed this since neither osc nor mfpbar needs such setup, so I'll let @Samillion comment on that instead.

[N-R-K]

You could require thumbnailers to set a property like user-data/mpv/thumbnailers/<script-name>

This could work, but this seems to be skirting around the line of thumbnailer selection, which IMO UI scripts should not get into.

One idea I had was to do something like this:

e = mp.get_propertly_bool("user-data/osc/thumbnailer", false);
if not e then
    mp.set_property("user-data/osc/thumbnailer", "bool", true);
end

This way only the thumbnailer that enabled the property would be active and osc/ui would still get a boolean value.

But this is unsound since each script runs in their own thread, so you'd need something like compare_and_exchange primitive for setting property to do this properly.

[Samillion]

Terribly sorry about the delay in response.

This feels like an unjustified level of gymnastics being done just to avoid letting the ui know whether a thumbnailer is active or not.

I also feel like that this is also more inconvenient than just adding an if statement with thumbnailer_enabled. But I have not confirmed this since neither osc nor mfpbar needs such setup, so I'll let @Samillion comment on that instead.

Agreed. To be clear, this isn't something that only ModernZ utilizes, many "modern" based forks use the same implementation.

However, to not simply say "it's not just ModernZ", I have to ask, what's the actual reason for refusing this? The claim is that it's not needed, but it has been stated (in detail) that it's needed and simpler to manage. The phrase "unjustified level of gymnastics" is apt here, to be honest.

If the idea for this API to be only used for osc.lua, then it would be understandable to be strict or wanting it bare minimum. However, my initial thoughts were that this API would be a standard of sort, across relevant scripts, maybe I misunderstood.

Either way, thank you very much for your effort @N-R-K

[na-na-hi]

This feels like an unjustified level of gymnastics being done just to avoid letting the ui know whether a thumbnailer is active or not.

I already mentioned why technically this is flawed. The accusation of "unjustified level of gymnastics" is not a technical argument.

I also feel like that this is also more inconvenient than just adding an if statement with `thumbnailer_enabled`.

Alternatively the thumbnailer can set a `thumbnailer_installed` variable to tell that a thumbnailer is installed so that the OSC can handle by just adding an if statement with `thumbnailer_installed`. Does this fix your concern?

I have to ask, what's the actual reason for refusing this?

You would have to stop attacking the strawman first. I am not refusing this feature. I just disagree with the current implementation of this.

[Samillion]

You would have to stop attacking the strawman first. I am not refusing this feature. I just disagree with the current implementation of this.

I apologize for choosing the word refuse, did not know it would be offensive. You disagree, perfectly clear now. Thank you.

No need to worry about ModernZ's implementation. Carry on, we'll adapt either way.

[N-R-K]

I already mentioned why technically this is flawed. The accusation of "unjustified level of gymnastics" is not a technical argument.

The sync issue is very much theoretical. You have to work very hard to trigger it. So not wanting to add a bunch of conditions to handle a non-issue is a technical argument.

Alternatively the thumbnailer can set a thumbnailer_installed variable to tell that a thumbnailer is installed so that the OSC can handle by just adding an if statement with thumbnailer_installed. Does this fix your concern?

This would allow two non-conflicting thumbnailers to be installed since they'd both only set it to true and no one ever sets it to false. But from the UI side it's still quite a bit more work to deal with this: you need to have two branches based on thumbnailer_installed and in the true branch you have to prepare both ass and ass-inactive.

And now that I'm writing this I'm realizing this still doesn't even solve the non-conflicting thumbnailer case since the inactive one would end up drawing ass-inactive so you'd have two chapter marker on the modernZ example.

[nekoxuee]

I also feel like that this is also more inconvenient than just adding an if statement with thumbnailer_enabled. But I have not confirmed this since neither osc nor mfpbar needs such setup, so I'll let @Samillion comment on that instead.

Agreed. To be clear, this isn't something that only ModernZ utilizes, many "modern" based forks use the same implementation.

I just want to confirm this isn’t a modernz specific case. Both modernz and uosc rely on knowing whether a thumbnail is currently active to adjust layout.

In both scripts the only thing that changes based on thumbnailer state is the stacking order of elements above the timeline. When active: time tooltip > thumbnail > chapter tooltip. When inactive: time tooltip > chapter tooltip. The thumbnail just pushes the chapter tooltip higher to take its original position.

The check is a single boolean at draw time. The ass-inactive approach doesn't fit this because it's not about what to draw, it's about where to draw something else.

Alternatively the thumbnailer can set a thumbnailer_installed variable to tell that a thumbnailer is installed so that the OSC can handle by just adding an if statement with thumbnailer_installed. Does this fix your concern?

thumbnailer_installed would always be true regardless, and the layout would break for any file the thumbnailer can't handle.

[CogentRedTester]

You could require thumbnailers to set a property like user-data/mpv/thumbnailers/<script-name>

This could work, but this seems to be skirting around the line of thumbnailer selection, which IMO UI scripts should not get into.

I don't think I agree; knowing the names of the script/s that are currently drawing thumbnails in no way allows the UI script to choose or control which script draws thumbnails -- the current API makes this completely impossible. All UI scripts will know is that there are multiple thumbnailers that work with the current file; they could choose to use this information to disable thumbnails or send warnings to the user, but I see this as an error handling feature rather than having anything to do with thumbnail selection.

[na-na-hi]

The sync issue is very much theoretical. You have to work very hard to trigger it. So not wanting to add a bunch of conditions to handle a non-issue is a technical argument.

The issue is very real according to #17518 (comment). The whole reason ass field exists is to avoid the sync issue.

This would allow two non-conflicting thumbnailers to be installed since they'd both only set it to true and no one ever sets it to false. But from the UI side it's still quite a bit more work to deal with this: you need to have two branches based on `thumbnailer_installed` and in the true branch you have to prepare both `ass` and `ass-inactive`.

The ship has already sailed for the simple, active thumbnail API: #17518 (comment)

Is this something that we actually want? I was under the impression that it's better to have a single thumbnailer that can pull from different sources.

#17518 (comment)

I don't think it makes much sense to complicate the simple interface any further.

I think we both agree on the limitation of the active thumbnail displaying model, and why the simple active model can be used in limited situations. In other situations, a passive model can be used instead. Multiple thumbnailers as you said before is not one of the limited situations. The API was not designed to do so, and band-aid more additions like this will not solve the fundamental issue.

[N-R-K]

The issue is very real according to #17518 (comment).

The sync issue you mentioned here was this:

"also sync issues with delay between observing enabled and then change ass."

I'm calling this a non-issue because it will self correct. Offloading the border drawing to the thumbnailer is useful because the border is very closely tied to the thumbnail. The chapter marker on the other hand is not.

It seems to be much more acceptable to the people writing these scripts to have the chapter marker appear on top of thumbnail that hasn't arrived yet, than it is to readjust their rendering code to offload it to the thumbnailer (if installed, of course, while still maintaining a regular rendering path for cases where thumbnailer isn't installed).

And this doesn't even solve the sync issue either, consider case where a thumbnailer is active but the thumbnail is taking a while to arrive. You still want the chapter marker to show, even if thumbnail isn't showing. So do you want another ass-immediate field which renders immediately?

If you don't want to accept the fact that the simple/active api will have inherit sync issue, then you're going to have to end up inventing something approximating a distributed system where ui logic will need to be split into both ui and thumbnailer script.

Multiple thumbnailers as you said before is not one of the limited situations. The API was not designed to do so

Right, and that's fine by me, as mentioned in the OP and commit message. I asked if someone has any idea on how to make it work while still being simple, but it's not a priority.

[AvidMex]

well, a newbie suggestion, using mpv for a long time, i have 1 issue with this- instead of setting a property with every script, can’t you make it script-opts in osc.conf, if i want thumbnail i can just set there and work with multiple scripts or i need to create a 3rd script just to manage this so one script doesn’t block functionality for others

[na-na-hi]

The chapter marker on the other hand is not.

It is. The position and style of the marker directly depends on whether the thumbnail is rendered or not.

And this doesn't even solve the sync issue either, consider case where a thumbnailer is active but the thumbnail is taking a while to arrive. You still want the chapter marker to show, even if thumbnail isn't showing. So do you want another ass-immediate field which renders immediately?

The UI script does not need to care when the thumbnailer is "active" or if the thumbnail has "arrived". They can be cleanly handled by the thumbnailer, because thumbnailer is the one that knows these information. If the thumbnail is rendered, then it uses the normal ass, else it uses the inactive ass, regardless of the reason the thumbnail is not rendered.

If you don't want to accept the fact that the simple/active api will have inherit sync issue

It has sync issue if the thumbnail and ass are drawn by different scripts. There is no sync issue if they are drawn by the same script. The simple/active api will not have sync issue if they are both drawn by the thumbnailer script.