Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thumbnails #43

Closed
FichteFoll opened this issue Dec 5, 2017 · 8 comments · Fixed by #62
Closed

Thumbnails #43

FichteFoll opened this issue Dec 5, 2017 · 8 comments · Fixed by #62

Comments

@FichteFoll
Copy link

FichteFoll commented Dec 5, 2017

There is a thumbnail script out: https://github.com/TheAMM/mpv_thumbnail_script

From an earlier discussion in some IRC channel, I thought you'd like to take a look into it for how much work it might be to add this here. Currently, it replaces the OSC with a custom one since it needs to accurately determine the hover regions of the hover bars, or something.

[12-06][00:17:47] <FichteFoll> so, I assume this need to be added to torque-progressbar manually?
[12-06][00:18:53] <@AMM> I looked at it
[12-06][00:19:09] <@AMM> It'll be a fucking pain to integrate it properly
@TheAMM
Copy link

TheAMM commented Dec 5, 2017

<AMM> The pain comes from my stuff not being in moonscript, and from him optimizing his ASS creation

A dirty way would be to just throw stuff after the compiled .lua, setup mouse listeners based on the options, which seems the most likely choice - but it's dirty, hacky.
For reference, with the original OSC I piggyback on the slider renderer and before/after the actual render optimize the overlay a bit and hide the thumbnail (which neatly happens on timeout or mouse-out).

@FichteFoll
Copy link
Author

FichteFoll commented Dec 6, 2017

Maybe a protocol for inter-script communication would suffice?

I mean, the progressbar knows where it is being hovered, so it should be simple enough to just forward these events to the thumbnail script along with some parameters (i.e. (relative) position in the slider, position on-screen for determining position to render the thumbnail at, and ofc when the thumbnail should be hidden again).

@torque
Copy link
Owner

torque commented Dec 6, 2017

Taking a look at how you patched it into the default OSC, it should be trivial to add support, though I don't understand the purpose of all the ass generation that draw_thumbnail does. Because most of the heavy lifting is in the actual generation of thumbnails, I think for my use case, it would be ideal if the thumbnail script provided an API to take an x and y position and a percentage and return an object that can be be used to call mpv.command_native (maybe even with draw and erase methods in its metatable, though I don't know if that will somehow break mpv.command_native).

Also, as a side note, you've got thubmnail_template sprinkled throughout thumbnailer_shared.lua.

@TheAMM
Copy link

TheAMM commented Dec 6, 2017

though I don't understand the purpose of all the ass generation that draw_thumbnail does

https://youtu.be/a9cmt176WDI?t=23

I'll ponder about the API later. Could be neat.

Oh, right, I forgot about that typo. I was aware of it, but because the iteration back then had inter-file references I didn't bother to stop and fix it, and it faded away...

@TheAMM
Copy link

TheAMM commented Dec 9, 2017

Okay, I've had a thought or two. Some notes.

  1. mpv.command_native cannot be hooked by scripts for... "native" commands (which would react immediately and possibly return a value). Best we get is mpv.command_native("script-message", message_name, ...) which only supports string arguments
  2. script-message would be fine, but there is some inherent delay involved - you preferably do the overlay-add during the same tick - as sending an event and waiting for the api host to overlay the thumbnail may be a tad slow - especially since all the events happen in the same queue (for example, [at least on the default OSC] start wildy dragging on the seekbar, then release mouse but keep moving the mouse around - the drag will stick for a moment, because the event doesn't get processed).

So I'm going for something wild - the host will send a piece of Lua which the client-wannabe will loadstring() into an api client, which will automatically listen to state updates (eg. finished thumbnails) from the api host. The api client will have appropriate methods for displaying the thumbnail for a timestamp at a given position, with alignment etc. which means the overlay can be done on the tick, by the client itself.
Does this sound... reasonable?
I'm not one to extensively plan ahead, so I'm not sure I'll have the most robust api client available at first, but I think this should be neat enough, in both meanings of the word.

The user would have three relevant files in their scripts directory: mpv_thumbnail_script_server.lua, mpv_thumbnail_script_apihost.lua and a progressbar.lua with support for script-messaging the apihost.

@torque
Copy link
Owner

torque commented Dec 11, 2017

I'm ideologically opposed to usage of loadstring, so here's my counter that I believe will behave identically without needing it.

The filesystem:

~/.config/mpv/scripts
├── mpv_thumbnail_script_server_1.lua
├── mpv_thumbnail_script_server_2.lua
├── mpv_thumbnail_script_api
│   └── thumbnailer.lua (not autoloaded by mpv because it is in a nested directory)
└── progressbar.lua

Then, within progressbar.lua, I should be able to do something like

local Thumbnailer = require( 'mpv_thumbnail_script_api.thumbnailer' )
-- connect to the server via mpv's script_message ipc mechanism
Thumbnailer:init( ) 
-- run the command to draw the thumbnail for the given timestamp at the given coordinates.
Thumbnailer:drawThumbnail( x, y, timestamp )

@rohitghali
Copy link

Any progress with this? I use both of these scripts. It'd be kick ass to have them work together.

@tmpm697
Copy link

tmpm697 commented Jul 4, 2022

any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants