osc: define and add support for "thumbnailer" api#17518
osc: define and add support for "thumbnailer" api#17518N-R-K wants to merge 2 commits intompv-player:masterfrom
Conversation
|
TODO: add docs. To test this PR using thumbfast, use this glue script-- glue code to make thumbfast work with the "thumbnailer" api
-- ideally should be merged into thumbfast once finalized.
local msg = require('mp.msg')
local utils = require('mp.utils')
local thumbfast = {
width = 0, height = 0,
enabled = false, available = false,
overlay_id = -1,
}
local state = {
showing = false
}
local script_name = mp.get_script_name()
local function thumb_clear()
if (state.showing) then
mp.commandv("script-message-to", "thumbfast", "clear")
mp.command_native({"overlay-remove", thumbfast.overlay_id})
state.showing = false
end
end
mp.register_script_message("thumbfast-info", function(json)
local data = utils.parse_json(json)
if (type(data) ~= "table" or not data.width or not data.height) then
msg.error("thumbfast-info: received json didn't produce a table with thumbnail information")
else
thumbfast = data
mp.set_property_native("user-data/thumbnailer", {
enabled = data.available and not data.disabled
})
end
end)
mp.observe_property('user-data/osc/thumbnailer', 'native', function(kind, t)
if t == nil then
thumb_clear()
elseif t.hover_sec and t.x and t.y and t.w and t.h then
thumb_clear()
mp.commandv(
"script-message-to", "thumbfast", "thumb",
t.hover_sec, "", "", script_name
)
local show_thumb = thumbfast.thumbnail
if show_thumb then
mp.command_native_async({
"overlay-add",
thumbfast.overlay_id, t.x, t.y,
thumbfast.thumbnail..".bgra", 0, "bgra",
thumbfast.width, thumbfast.height, 4*thumbfast.width,
t.w, t.h
}, function() end)
state.showing = true
end
else
msg.error("user-data/osc/thumbnailer: malformed property")
end
end)(glue code for the older version of the proposed api)-- glue code to make thumbfast work with the "thumbnailer" api
-- ideally should be merged into thumbfast once finalized.
local msg = require('mp.msg')
local utils = require('mp.utils')
mp.register_script_message("thumbfast-info", function(json)
local data = utils.parse_json(json)
if (type(data) ~= "table" or not data.width or not data.height) then
msg.error("thumbfast-info: received json didn't produce a table with thumbnail information")
else
mp.set_property_native("user-data/thumbnailer", { width = data.width, height = data.height })
end
end)
mp.observe_property('user-data/osc/thumbnailer', 'native', function(kind, t)
if t == nil then
mp.commandv("script-message-to", "thumbfast", "clear")
elseif t.hover_sec and t.x and t.y then
mp.commandv(
"script-message-to", "thumbfast", "thumb",
t.hover_sec, t.x, t.y
)
else
msg.error("user-data/osc/thumbnailer: malformed property")
end
end) |
|
come on bro, that is exactly what i was doing in my pull request #17510 i am creating if this is not hypocrisy this i don't know what it is you are depending on you are making a osc depend on 3rd party script, my code is way better than yours, as it reveals the hover-time so custom script can do everything on their own but your PR will be merged mine not, good luck in advance |
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer` property with the width and height of the thumbnail. UI scripts can observe this property to know the thumbnail w/h along with whether thumbnailer is active or not. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x` and `y` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
width and height should be decided by the UI script, not thumbnailer. |
This was chosen based on current script convention (I've checked |
No matter what the convention is, special
This can be done by the UI script to advertise the desired size in advance. The thumbnailer then observes the property and pregenerate thumbnail whenever it changes. This also allows the UI script to change thumbnail size dynamically at runtime. |
That is true, but I'm also thinking of cases where thumbnailer might be getting pregenerated thumbnails (e.g from disk cache) or from web sources. So it'd need to do resizing (not impossible but will require a tangible amount of change in the existing thumbnailer scripts). Also if a thumbnailer does store disk cache, the width/height will directly affect the size of the cache. So from a user perspective, doesn't it make more sense for me to define these values via the thumbnailer config instead? (EDIT: also thumbnail width/height value in osc user's config will just become no-op if user doesn't install a 3rd party thumbnailer. So won't that also cause confusion for users?) I don't oppose adding width/height to osc on a theoretical basis though. But it will require more work, both on osc side and on thumbnailer scripts. I'll wait before making any more drastic changes since @po5 might provide more insight on this. |
This can be solved with script-opts. I agree with @na-na-hi that w/h request should come from UI script. It can ignore the request, it should be treated as a hint, and UI script should be able to handle resizing on its own, well this is already done because overlay-add will scale if needed, but it's better to not generate 4k images to scale it to 200x200 with bilinear, because it will look really bad. |
The UI script in principle only cares about controlling display size. The actual size of the image and the displayed size can be different in
They only need to documented to require a compatible thumbnailer.
There will be no much more work on osc side, only user config option for thumbnail size. In both cases UI script needs to calculate x/y from the thumbnail size. On thumbnailer side it can do the minimum required by changing the |
|
Unless I'm missing something, this API is basically that a 3rd party thumbnail script sets a thumbnail size it wants to render, and in return the OSC updates the mouse-hover-timepos when needed and X/Y of where the script should render at. I think it can be useful, but it puts burden on the OSC to communicate configurability which the user might want. For instance, currently the user cannot configure how far the thumbnail is from the osc box, etc, or anything else really, other than the size - which they can configure with the thumbnail script. Here's an alternative suggesiton: Instead of the OSC telling the script where to put the image, and over time also other user options which the OSC would have to communicate to the script, how about the OSC just giving the script the data which is enough to makes the decision where to draw, and leave the actual decision making and user-configurability to the script? If I read the code correctly, the only input which the OSC takes into account, other than the thumbnail size, is the layout. It has fixed relative position per layout, and that's it. Right? So relay the layout to the script, and let them make a configurable decision themselves, and deal with the user options to control that? But they don't even need that. They can already know what the OSC layout is - it's a property they could read themselves. It's nicer is the OSC could report it though, so I have no problem with that. But if there's one thing which might be harder for the script to know, it's the bounding box of the OSC. So I think a cleaner API would be that instead of advertizing hover-timepos and X/Y, to advertize hover-timepos and OSC layout name and OSC bounding box. EDIT: and remove the requirement from the script to send its desired size, because the OSC doesn't care anymore about that. I think it's plenty info for any 3rd party script to make a very informed decision, while taking away the burden from the OSC to maintain all the thumbnail user configurability and act as a mediator between the user wants and that script. |
It is simpler said than done. What information need to relayed? Seek bar geometry, other elements near seek bar, anything that the UI does not want to be obscured? I think it is pretty clear that only the osc has enough knowledge to know where to best position the thumbnail, and for a third party thumbnailer to make decision it has to know basically all geometry information of individual osc UI elements, including all of the "avoid zones". Different UI scripts can have completely different UI paradigm, which makes it difficult to generalize them into a generic "layout" information to relay to thumbnailer scripts. For example, it might want to show thumbnail at a fixed position instead of following mouse on seek bar. |
|
Exactly. The "thumbnailer" script should be UI agnostic, else it's basically tied to the specific implementation of OSC/UI. UI script may decide, to put thumbnail in any place, any z-order, add some deadzones, fade-in/fade-out, don't even track mouse. Why do we expect thumbnailer to have all this knowledge? If you are tinkering with your own script, you can make all sort of assumptions and maybe mouse position is the only information you need. Upstream solution however should be extensible and designed in a way that is reusable. If we design and implement thumbnaling interface that makes sense, it will be adopted by other scripts. If we hack around because it works, everyone will ignore it and keep forking osc. |
|
It may be worthwhile to consider what po5 wrote in tomasklaen/uosc#353 (comment) (and the whole thread in general). Seems like a lot of effort is required to make sure that the thumbnailer and osc are synchronized on everything but the proposed API makes sense. |
Theoretically, absolutely. If we're defining an interface between two arbitrary entities which don't know anything about eachother, of course it's not enough, and of course the UI part should specify both size and position. The "service" side should have absolutely no say at all. Not even the image size, because how can it know what are the minimum and maximum sizes (and aspect ratios) which will play nicely with the UI? The existing interface in this PR makes exactly the same assumption (of some existing non-communicated knowledge), just on a smaller scale. It allows the script to request arbitrary image size and aspect, and in some cases the OSC will have no useful reply as to where it wants the script to place the image so that it ends up useful to the user (think too small or too big).. But in practice, the mpv interface doesn't change every two weeks or years and probably also decades, In the context of mpv, the bounding box (yes, geometry - a rect) would cover all those needs ever since mpv have came to be, and that's also unlikely to change in the forseeable future. Please don't say windows title (and if you insist, then advertize the title geometry as well). mpv (but not mplayer) had rectangular-ish UI forever. And with rectangular geometry assumption it's a trivial task to cover all needs. Also, it doesn't have to be either or. The UI side could still advertise a hint about the preferred size and position, but still let the script side together with the user go wild. So every such script could have an option "respect OSC size/position hint", which would work even if mpv changes the OSC shape, layout and geometry every minutes. But the rest of the time, geometry alone will be enough for the script to support many wild configurations which might be useful for some users, but have no place in a world where the OSC is the sole mediator between the user needs and the 3rd party script. |
I agree, but I already addressed this in #17518 (comment).
The PR is designing a generic interface between UI scripts and thumbnailers. Even if the layout of osc.lua does not change much, the same cannot be said for third party scripts. Even under the assumption that all osc UIs are "rectangular", the "layout" proposal still needs to advertise at least a bounding box (forming an "allowed zone") to the thumbnailer. Compared to my proposal where only a defined destination rectangle is sent, the API is not simpler. Meanwhile, it makes user config more complicated: since the UI script defines the "allowed zone" and the thumbnailer decides where to draw in the "allowed zone", now the user has to configure both the UI script and the thumbnailer and understand the interaction between these scripts to achieve a desired placement. While in my proposal, all configuration can be done in the UI script. It also makes the responsibility unclear between the UI and thumbnailer scripts, which eventually complicates the implementation. For example, the
The responsibility is unclear, as all situations are valid. Now consider one OSC uses first approach, while another OSC uses the second approach. To make the thumbnailer work with both scripts, it has to contain the implementation of both, which increases code complexity, making it difficult for the thumbnailer to work with all OSCs which have this loose interface. A cleaner, stricter interface would eliminate this complexity.
What is the advantage of ignoring OSC size/position hint here? And in practice, the UI script can restrict the "allowed zone" to exactly the same as the hint, so it can force the thumbnailer to respect the hint anyway. |
I was referring to how the PR looks now, where the external script specifies the size. If you agree that it's not ideal from a perspective of "OSC should be in control", then great! :)
Well, more like forming an "exclusion zone" (the OSC bounding box - because the OSC could be in the middle, which leaves two "allowed zones" - above or below it), but sure, similar enough semantics.
Not sure I'm seeing this proposal. Do you mean where the OSC should specify X/Y/W/H (exact position and size) instead of only X/Y ? Also, I never made claims about the simplicity of the API, although I obviously prefer simpler over more complex. See next about what I claim and towards which goals.
Not exactly, because if we take this approach, then it's not about how the OSC can lie to the script. It's about coordinating in a way which is beneficial to both sides - where the OSC advertizes where its located on screen, so that the script can respect that and draw the thumbnail without occluding the OSC. As for "what is the advantage", quite a lot IMO. The way I see it, the beauty of mpv with regards to extensions is that it allows 3rd party scripts to go wild. Use their imagination, and come up with new and useful things to do. It's a symbiosys where mpv gives 3rd party scripts a lot of tools at their disposal (all the client API commands/properties, and builtin scripting support), and also giving them a very long leash about what they're allowed or not allowed to do, without being too opinionated about it, while trying to balance that with not too many obvious foot-guns. The clients/scripts authors side is to use their imagination to come up with useful and beautiful mpv extensions. This has worked great so far, and many of the things we have today in mpv started as 3rd party scripts or inspired by such, like the builtin console, or the context menu, and even the OSC itself started its life an an external 3rd party script, and much of its original code and structure is still in The reason it could happen is because the line of responsiblity is largely well defined: mpv focuses on exposing quality info and tools, and clients use that to do useful things which the mpv maintainers never even considered possible. Everyone benefits. It's relatively easy for the mpv devs to expose good info and tools, 3rd party authors are happy to extend mpv imaginatively, and the rest of the users also benefit from this symbiosys. So back to the original question of "What is the advantage of ignoring OSC size/position hint here?" The potential value in that is that 3rd party authors can be more imaginative than mpv devs about where, how, and when to display the thumbnail, or anything else really. Hopefully they have enough info to not obstruct the OSC itself, because we're all in this boat together - mpv devs and 3rd party authors. We're not against eachother. We want to complement eachother. And in general, this separation of concerns also means less "dirty" maintenance load on mpv devs. It's very easy to expose the OSC position and maybe thumbnail size and position hints, while letting 3rd party scripts take it from there and go wild. It's much less fun to keep a tight leash over what the script can or can't do. First, because if it's too tight then 3rd party authors will try to circumvent that. But more importanly, because it means that the responsibility of managing every aspect of it, including user configuration, extensibility, etc, is now on the mpv-devs shoulders. The more control mpv asserts, the higher the burden of responsibility which comes with it, and it's an ongoing maintenance burden. mpv devs want to control thumbnail size and position? now it also has the responsibility to expose it as configurable. This includes size, relative position to something (to what?), how to scale it with the window size, whether to disable it under some conditions, etc, etc etc. Just see how many options the OSC has to realize that making UI configurable is a never-ending task, and a never-ending stream of dealing with users about it. So instead of trying to control everything as tightly as possible, I'm suggesting to figure out what is the line which reduces the maintenance burden on the mpv side in the long term, while still allowing scripts to be well informed and responsible so that they don't accidentally clash with things which are controlled by mpv - specifically the OSC. It's all about this balance and symbiosys. mpv focuses on providing good tools and info with a long leash, and script authors try to use this leash imaginatively but also responsibly. |
Here's one example. A 3rd party author decides to use this API to display a box which contains the subtitles/lyrics/conversation/chat which refers to the hover-timepos over the seek bar. They don't want it to move with the mouse. They want it to be on the top right, or top left, or bottom left. However possible and configurable but without obstructing the OSC itself, because that would suck for the user. So they only need the hover timepos and OSC bounding box so that they can play nice, but they don't care about the size/pos hint. Don't limit the functionality in advance only to what you can imagine now. Provide good tools/info, and let 3rd party authors take it from there. |
thumbnailer may not have an ability to scale thumbnails, when they are sourced externally or whatever. While UI most certainly have that ability. I don't know why this topic has spurn so heated discussion. Drawing thumbnails on top of existing UI in external scripts is a workaround forced by the OSC limitations. All scripts run in own event loops, they are not synchronized, nor they can block another script (well they can, but it's not the design we want). This naturally causes UI elements to be drawn at different ticks. Unless you actively wait for Thumbnail generator should be passive module, not active. It should wait for image request. It can preload if possible. Most common solution is just to quantize timeline in N ranges and generate thumbs for them. Not for every millisecond of movie, but I digress. It's the UI script that draws, decorate, animate the image. It can move the image, fade it and so on, to provide smooth interface, without asking for thumbnailer script permission or update. It makes the communication layer simple as only the image request/response is made, not dozen of parameters how it should be drawn or where. Of course nothing stands in way of making UI modular, where the thumbnail UI handler is separate from main UI script, but this modularity is more of internal design choice, not the external API interface, as again the UI thumbnail handler needs to have all sorts of internal information about the UI itself. |
Yes.
This is flawed reasoning. The same goal can also be achieved when OSC decides where to draw, so it will obviously not occlude the OSC. Not only the OSC has more information on this factor so the placement end result is better, but the thumbnailer script also does not need to worry about placement which makes it simpler to implement. The benefit over your proposal is clear.
The mpv core is not opinionated on this matter. The command and property interface is as generic as possible, and does not need to care too much about what clients do, because they are not the direct interface between user and player for normal usage. OSC on the other hand is not part of the mpv core. It is a mpv client which uses the above mechanism to achieve a specific goal. This goal IS opinionated: the goal of OSC is to provide the user a good UI to interact with the player. It needs to ensure consistency of UI elements that are part of it.
Yes, the generic mechanism provided by the mpv core works great so far. It says little about the interplay between scripts, which is the topic here.
Yes, a 3rd party OSC script can also be more imaginative than mpv devs about where, how, and when to display the thumbnail, or anything else really. The interface I proposed still allows 3rd party OSC scripts to "go wild" and do whatever they want about the thumbnail placement. mpv devs can also selectively implement the good ideas imagined by them into osc.lua. Whether the placement is done by the OSC or by the thumbnailer is not important here compared to the ultimate goal, which is to serve user needs. Thus, the real benefit analysis hinges on the technical aspects between the two solutions, which I argue that placement by OSC is superior.
I consider this the only technical argument against the placement by OSC solution, that it increases the burden of mpv developers if osc.lua has to implement lots of configurable options. However, keep in mind that 3rd party OSC scripts can "go wild" and implement what osc.lua is not supporting. This, as you said, "has worked great so far", with them doing "useful things which the mpv maintainers never even considered possible". The goal of osc.lua is to provide a good out-of-box experience for most users, without needing to satisify ALL user needs. The 3rd party scripts exist to fill the gap.
Here's one example. A 3rd party script A decides to use this API to display a box which contains the subtitles which refers to the hover-timepos over the seek bar. They don't want it to move with the mouse. They want it to be on the bottom right. Now a user installs these 3 scripts, hoping to see this information presented nicely. The user moves mouse to near the end of seekbar. What is shown is that the information from 3 scripts displaying on top of each other, which is not the user wants. This happens because all of these scripts compete within the same "allowed zone" without knowledge of each other. Instead, consider when the position is controlled by the OSC. The user specifies that contents from script A and B placed at bottom right, while C follows mouse. The OSC can then smartly allocate the zones for the scripts so that they are placed according to the placement instruction without obscuring each other. |
I have already answered this in #17518 (comment):
I think this is OK in concept for the UI script to handle drawing, but it requires image data sharing between the thumbnailer and the UI script, which is where this approach faces practical difficulty. Assuming the scripts are written in lua without calling native C functions, they would not be able to share This also requires the UI script to do manual resource management to free the resource when it is not needed, instead of being handled entirely by the thumbnailer. If the thumbnailer allocates the resources using native function calls, it is also responsible to keep them alive as needed, and the UI script is also reliant on using script message to the thumbnailer for freeing resources. All of these make the communication layer more complicated. |
|
Since we want the OSC to control the rendering has anyone considered just making the OSC itself generate the thumbnails through a builtin module (that other OSCs can import)? It shouldn't be slower with async commands, it would be easy to pass data to and from the module without sync issues and would be easy to enable for users without finding an external script.
Note that we don't want to downscale with |
I don't think it's heated at all. I think it's a great discussion. The comments are respectful and on topic, and there are different opinions which IMO is great for everyone. We didn't yet find something which everyone is happy with, but I still think it's a great discussion.
Correct, but this has other disadvantages, as described later in that post.
It's correct, but I disagree with this premise. My goal here is to allow 3rd party script to be imaginative with the builtin OSC, without having to provide their own OSC - which is exactly what they have to do now if they want thumbnails. This PR and discussion is about allowing third party scripts to do that without bringing their OSC with them.
I obviously agree. But I don't think this contradicts my suggestion in any way. Exposing the hover timestamp, osc bounding box, and if you really want to - also thumbnail pos/size hint already does all that, without meaningful additional effort currently or in the future, while still allowing 3rd paty scripts to go wild either with the builtin OSC or to bring their own OSC. The only difference from your proposal on a technical level, if I get it right, is whether we document the size/pos data as requirement or hint, and whether we also advertise the OSC bounding box, correct? If yes, do you consider it big enough difference to fight over it (insist on not advertising the box, and insist on calling size/pos a requirement) instead of satisfying both of us trivially? "strict" 3rd party thumnail scripts can still ignore the osc box and just use the size/info hint, but it gives so much more possiblities for other scripts to do really nice things responsibly with the builtin OSC. And on a non-technical level, I already explained my philosophy and goal. Good info which allows scripts to extend mpv responsibly. Without the osc box exposed, it makes it a lot harder for scripts to be responsible IMO. And yes, "bring your own OSC" is always an option, which is completely irrelevant to this discussion, because the sole goal of this PR is to allow 3rd parties to avoid exactly that.
You could use this kind of argument regardless of which interface you think is best, and regardless of this PR. There's also a responsibility of the user here. They should not enable two thumbnail scripts, or any two other scripts which end up degrading their experience. This is the value and price of freedom. Users can experiment and pick whatever works best for them, but also shoot their foot if they insist. mpv cannot control everything tightly enough to ensure that no two scripts can ever interfere with eachother. So I just don't think it's a valid argument. |
This is not inter-script communication issue, this is
This is normal software development. At least you don't depend of UI color implementation details. It is simple as giving two entry point, to get image and to free image. (same as create_osd_overlay() has remove()) This is also extensible to any number of solutions, because you can implement native cplugin that just gives memory address for you to use. The expectation from thumbnailer would be to prepare payload for use with overlay-add, but its UI script decision to call it.
I don't see it. Depending on the design, it can be as simple as |
Yes.
This PR is about making a generic interface between any OSC and thumbnailer scripts, and not specific to osc.lua. My argument in #17518 (comment) is still valid. The "OSC bounding box" interface implies an assumption on how the UI is laid out, which cannot serve as a generic interface for arbitrary UI scripts. I think what can be done is that for OSC scripts that satisify this "OSC bounding box" model, the box can be advertised, but it is purely optional, and for any script that does not advertise a bounding box the size/pos data becomes a requirement. And this is only after an investigation of all popular 3rd party OSC scripts that all of them fit into this model to warrant this becoming a generic interface.
All of them can be implemented in the OSC so the user can shoot their foot if they insist. It does not have to live in the thumbnailer. |
By drawing thumbnail in the thumbnailer script, this issue is avoided. It does not matter whether the issue is "inter-script communication issue" or not if it causes drawbacks.
It requires a bidirectional communication flow of request data -> receive data -> free data, 3 communication per thumbnail change, while drawing thumbnail in the thumbnailer script allows for one-way communication, 1 per thumbnail change. The inter-script communication creates delay as pointed out by tomasklaen/uosc#353 (comment). I agree that this solution is better when properly implemented, but this requires higher implemention complexity for both UI and thumbnailer scripts. |
I completely disagree. It's very specifically only about the builtin osc.lua. Anyone can bring their own osc with their own interface to a thumbnail or any other script. I see this PR as wanting to allow 3rd party thumbnailers to work with the builtin osc, without any claims or suggestions what other osc scripts should or shouldn't do. Obviously, if it proves useful, then third party OSCs might want to adopt it, but it's not mpv's job to tell them what to do.
We already agreed on that, and we can also agree that the builtin osc and all the others which we (I) know of are all rectangular, and always have been, and that's not very likely to change. Additionally, once there's a non-rectanrular osc, it can simply not advertise its bounding box. Or maybe yes advertise a bounding box even if it's a circle or a triangle or whatever else. It can be documented as "The OSC may advertise its bounding box if possible, to allow 3rd party scripts to integrate better if they want to add more content on screen. At this time, and probably as long as the builtin osc is rectangular, it does advertise it.". And this comment applies equally also to your point of view that this PR is about general interface between OSCs to whatever and not specific to our osc.lua at all. It's about being practical. I think it's silly to avoid something which is likely to be useful in a lot of contexts, including mpv itself in the past and likely future too, just because there might be an OSC which cannot or don't want to do it. If it can't, then it can't. It ends there. It's simply not suitable to this kind of integration, which is absolutely fine. |
Single direction communication will never be flexible. It's just a quick poc, see mum I can display image on my timeline. Also see, even if thumbnailer script is handling drawing on request, it still needs to be told when to call None of the above solutions are able to be synchronized correctly. Even if you make a script that basically wraps However, if the UI script has the data at hand, it can show/hide it at full framerate as it wants it, without any synchronization considerations. And only delay is that it needs to get the data from script (2 ticks), which I think shouldn't be longer than the extraction itself, so we already have inherit delay. If this is really a problem, thumbnailer script shouldn't be based on script-messages and instead have native interface. Maybe osc.lua should directly call helper binary from subprocess... |
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer/enabled` property to true. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x`, `y`, `w` and `h` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in and `w` and `h` are width and height of the size to draw the thumbnail at (the actual backing thumbnail size may be different). x,y,w,h must be positive integers. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer/enabled` property to true. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x`, `y`, `w` and `h` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in and `w` and `h` are width and height of the size to draw the thumbnail at (the actual backing thumbnail size may be different). x,y,w,h must be positive integers. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
|
On a technical level, as far as I can tell, the new code block is inside So if the mouse leaves the slider then this code would not enter, and nothing would set |
Oops. I misread the indentation. The set to So no problem here. |
Looks like there's some discussion around it in po5/thumbfast#84 (comment). TLDR: trying to use screenshot with |
We later concluded that you can avoid creating a window with |
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer/enabled` property to true. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x`, `y`, `w` and `h` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in and `w` and `h` are width and height of the size to draw the thumbnail at (the actual backing thumbnail size may be different). x,y,w,h must be positive integers. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer/enabled` property to true. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x`, `y`, `w` and `h` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in and `w` and `h` are width and height of the size to draw the thumbnail at (the actual backing thumbnail size may be different). x,y,w,h must be positive integers. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
|
Since it seemed a bit weird to add an api which no script in mpv repo supports, I added
Question: is this something we want to add in theory (leave the current implementation details aside). If yes, then is there any major objection to the current implementation? (If yes, I will open a separate PR to move the discussion there to avoid cluttering this PR). cc: @na-na-hi |
20a2470 to
d53ef30
Compare
IMO it is ok as an addition to TOOLS, but I do not consider this too useful. The current thumbnailer API only allows one thumbnailer enabled at a time, and this thumbnailer only supports pre-generated thumbnails. This means it is impossible to use it with a 3rd party thumbnailer that supports both pre-generated and on demand thumbnails. And in this case it might be better to use the more complete, mature, polished 3rd party thumbnailer instead. I think a better alternative is to open a PR in one of the 3rd party thumbnailer to implement this API, and users can test by applying patches on both sides. |
IMO it is ok as an addition to TOOLS, but I do not consider this too useful. I think the whole exercise of external API for thumbnailing is to allow external script to work. If we were to implement 1st party solution, it would rather be correctly implemented in C to avoid all the limitations of lua solution. |
This doesn't prevent 3rd party thumbnailers from working. And unlike
These are not mutually exclusive. The point was to have something that can be used to test the thumbnailer in the main repo. And getting pre-generated thumbnail from youtube seemed easier than to manage persistent on disk cache (mpv_thumbnail_script) or to do on-demand thumbnailing on a separate mpv process (thumbfast). I can also license the script under public domain/unlicense so that it can serve as an example for 3rd party script writers on how to do youtube-dl storyboard thumbnailing. |
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer/enabled` property to true. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x`, `y`, `w` and `h` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in and `w` and `h` are width and height of the size to draw the thumbnail at (the actual backing thumbnail size may be different). x,y,w,h must be positive integers. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
46c66e3 to
0e5237e
Compare
| if state.platform == "windows" then | ||
| cmd = {"rmdir", "/s", "/q", state.tmpdir} | ||
| end |
There was a problem hiding this comment.
Windows is entirely untested.
If the 3rd party thumbnailer sets
Yes, thus the question about how necessary this is. This means maintaining another script that would be the largest lua script in TOOLS. |
I mean that there wouldn't be two (or more) thumbnailer scripts active to begin with (for the same reasons you wouldn't have OSC and some other 3rd party UI script active at the same time). That can't really work, even with the "passive" model (i.e thumbnailer gives back thumbnail instead of drawing it) without some sort of mechanism for thumbnailers to communicate within themselves to figure out which one accepted the request. This I consider out of scope for this PR. |
this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api [1]. the api works as follows: * If a thumbnailer script is active, it will set `user-data/thumbnailer/enabled` property to true. * To issue a thumbnail draw command, the UI script will set the property `user-data/osc/thumbnailer` with `hover_sec`, `x`, `y`, `w` and `h` field set. hover_sec is the position in seconds where the user is hovering. x and y are top-left coordinates to draw the thumbnail in and `w` and `h` are width and height of the size to draw the thumbnail at (the actual backing thumbnail size may be different). x,y,w,h must be positive integers. * To clear the thumbnail, the UI script will set the previously mentioned property to `nil`. a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together. this change has been tested with work with thumbfast (using the "thumbfast-glue" script [4]). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch [3]. the code to determine thumbnail x,y is based on the osc fork inside of thumbfast [2]. [1]: https://github.com/po5/thumbfast?tab=readme-ov-file#for-ui-developers-how-to-add-thumbfast-support-to-your-script [2]: https://github.com/po5/thumbfast/tree/vanilla-osc [3]: https://codeberg.org/NRK/mpv-toolbox/src/branch/thumbnailer/mfpbar [4]: mpv-player#17518 (comment)
this adds a thumbnailer script that uses pre-generated youtube-dl thumbnails. not a fully featured thumbnailer, but useful for testing and as an example. released into public domain so that 3rd party script writers can use/copy it for their (hopefully fully featured) script. can also be used in case user only wants web thumbnail (since local files can just be seeked fast).
Also counterpoint, local file seeks are usually fast. Whereas seeking/scrubbing through web content might not be feasible depending on your demuxer cache and network bandwidth. Youtube can also sometimes just stop playing the video if you do seek (related issue #8779). So getting a preview without actually seeking can mitigate (even if not solve) the issue a bit. And so the user-case for a web only thumbnailer might not be zero. Even if most would prefer a complete solution (myself included, I plan to use thumbfast which has a pending PR for youtube-dl storyboard). |

this adds a "standard" api for ui scripts and thumbnailers to communicate with each other, based on the simple thumbfast api 1.
the api works as follows:
user-data/thumbnailerproperty with the width and height of the thumbnail. UI scripts can observe this property to know the thumbnail w/h along with whether thumbnailer is active or not.user-data/osc/thumbnailerwithhover_sec,xandyfield set.nil.a more ideal api would make it so that the thumbnailer script only generates the thumbnail and doesn't need to draw at all. but this is a decent enough api that allows arbitrary thumbnailers and ui scripts to communicate between each other and work together.
this change has been tested with work with thumbfast (using the "thumbfast-glue" script below). and for demonstration that this api can be useful outside of osc, it has also been tested to work on mfpbar's thumbnailer branch 3.
the code to determine thumbnail x,y is based on the osc fork inside of thumbfast 2.