occivink / mpv-image-viewer

Configuration, scripts and tips for using mpv as an image viewer
The Unlicense
284 stars 23 forks source link

osd-dimensions seems to not get loaded in time for align_border #25

Open eugenesvk opened 1 year ago

eugenesvk commented 1 year ago

I've noticed The video-pan-y option is out of range: nan error on launch, which I tracked down to an empty result form this function https://github.com/occivink/mpv-image-viewer/blob/9aab9005219baf99d747d3c275bb05a11805d996/scripts/image-positioning.lua#L205

(this gets invoked from the config script-message align-border "" -1 call on image load)

However, if I map this function to a script keybind and press the key after I've loaded the image, I get correct output (image dimensions)

From this I gather that your script is loaded "too early" or something and not receiving the dimensions, hence it bugs when trying to divide by 0

Do you have an idea what's going on here and how to fix it?

Or maybe it's the next line if not dim then return end that bugs and let's trough dim with 0 values? Then a simple if not dim.w then return end fix would work, but then I'm not sure I understand the logic: is this call supposed to happen when you launch the app (so zero dimensions are a bug) or is it only meant for later image loads (then it's ok for the dim to be empty on launch), but not the first one?

https://github.com/occivink/mpv-image-viewer/blob/9aab9005219baf99d747d3c275bb05a11805d996/scripts/image-positioning.lua#L206

Using mpv 0.35 on a macOS

occivink commented 1 year ago

You're probably right that the if not dim then return end check is not sufficient. There must be a division by 0 happening meaning that the height or width of the window is 0. The problem is that even if we add this check (which would be the correct thing to do), the function will just abort early without doing anything and your image probably won't be aligned as expected. You say that align-border is called on image load, what mechanism do you use for that? Maybe the detect-image.lua script?

eugenesvk commented 1 year ago

You're probably right that the if not dim then return end check is not sufficient. There must be a division by 0 happening meaning that the height or width of the window is 0. The problem is that even if we add this check (which would be the correct thing to do), the function will just abort early without doing anything and your image probably won't be aligned as expected.

Oh, ok, so this must run on the app launch then and the issue is really launching "too early"? No idea how mpv loads everything and how to control that order, though, but hopefuly you know better :)

You say that align-border is called on image load, what mechanism do you use for that? Maybe the detect-image.lua script?

Yes, I'm using your suggested defaults in detect_image.conf, which has command_on_image_loaded that loads script-message align-border "" -1

command_on_first_image_loaded=apply-profile image; enable-section image-viewer; script-message status-line-enable
command_on_image_loaded=no-osd set video-pan-x 0; script-message align-border "" -1
command_on_non_image_loaded=disable-section image-viewer; no-osd set video-pan-x 0; no-osd set video-pan-y 0; no-osd set video-zoom 0; script-message status-line-disable
eugenesvk commented 1 year ago

so I found this

Note that since the script starts execution concurrently with player initialization, some properties may not be populated with meaningful values until the relevant subsystems have initialized.

and

Since mpv 0.6.0, the player will wait until the script is fully loaded before continuing normal operation

from https://mpv.io/manual/stable/#script-location

Maybe there is a way to get some callback after the subsystem is initialized and call the border align again?

occivink commented 1 year ago

Yeah so as far as I know there is no guarantee as to the initialization state of the application when the scripts are loaded. We could work around this in a script by simply doing observe_property("osd-dimensions") and only aligning when its value is valid, but it's not as simple to come up with a configuration that does it without having to write any script. One thing that you can probably do is add force-window=immediate to your mpv.conf, in which case the window should appear earlier. It's still racy so it's not a proper fix

eugenesvk commented 1 year ago

I've fixed this in my "single folder" branch by observing the OSD changes and only launching the alignment function when it exists, https://github.com/occivink/mpv-image-viewer/commit/c517171b6ef03523622ed7dd9a5f1390b6128995

However, there is still one tiny thing missing: for some reason mpv reports valid OSD twice on launch (non-scaled and scaled) and I'm not sure how to only get the latest change (as I don't want to trigger alignmnt on every OSD change since that would break manual user panning by always reverting to the init alignment). Currently I just track the first 2 non-zero OSD changes, but if there is no scaling, then the 2nd might be coming from the user (ideally there should a a timer, so need to track only 2 OSD changes within the first 1 sec, so that's definitely OSD changes at launch)

Have more details at this mpv fr, but maybe you have an idea how to get rid of this wrinkle properly?