Add variable of display resolution to the VapourSynth filter

[CrendKing]

Expected behavior of the wanted feature

In mpv, the VapourSynth filter has display_fps. Often I come across use cases where I would like to resize the video up or down to the display size in VapourSynth and further process the scaled video before sending it back to mpv. Currently there is no way to get the information.

It looks like mpv is calling this get_display_fps() to get the display FPS, which internally calls vo_control(p->vo, VOCTRL_GET_DISPLAY_FPS, &res). It seems there is already a VOCTRL_GET_DISPLAY_RES. Maybe this is easy to implement?

Alternative behavior of the wanted feature

Currently I have to hard code a resolution in the script, which only works for me. This makes it difficult to share the script without telling everyone to change those numbers.

Log file

I'll provide log if needed.

[Dudemanguy]

Yes, actually this should be pretty trivial.

[Dudemanguy]

@CrendKing: I added a commit that in theory should do this in #12132. I know nothing about actually using vapoursynth though. Can you verify if it works? I had to move the display res logic to a different place.

[CrendKing]

I'm sorry I tried hard to make a build that works with VapourSynth, but unable to do so. The build under MSYS just unable to initialize VSScript for some reason. And shinchiro's script just don't work for me.

Maybe you commit that, and I'll check shinchiro's daily build ASAP? If anything is wrong, you can still revert.

Alternatively, if you have a Windows environment, I can tell you steps to test this in VapourSynth yourself.

[Dudemanguy]

Do you have like a nice minimal example script I could try out that would process frame by frame? The variables are display_res[0] and display_res[1]. They aren't immediately available though, but I assume that's okay because it's supposed to be updated later.

[CrendKing]

video_in.text.Text(f'display_res[0]: {display_res[0]}, display_res[1]: {display_res[1]}').set_output()

[Dudemanguy]

Hmm okay, I understand a little better now. So there aren't any runtime changes; what you get during initialization is all that is there. Quite unfortunately, X11 and Wayland don't get the information from the backend fast enough. The vapoursynth filter initializes before the windowing/display server gives us information (for the record, display_fps doesn't work on wayland either) so this is always just 0.

However, peaking at the windows code it appears to get the default monitor back which is better than nothing so it shouldn't be totally useless. Anyways, this will let me simplify that commit. Thanks.

[CrendKing]

it appears to get the default monitor back

So like you said, this always returns the resolution of the "default" (first) monitor, even if mpv is for example started with --screen=1? It's certainly useful, and it's understandable to make this static. It would be nicer if it returns the monitor mpv started on (e.g. second monitor if --screen=1). Of course, it that requires huge effort, then forget about it.

[Dudemanguy]

Yes, I'm thinking having some kind of fallback for these properties is useful when we're in the pre-initialization phase, I just pushed a commit for X11/Wayland on that PR.

It would be nicer if it returns the monitor mpv started on (e.g. second monitor if --screen=1). Of course, it that requires huge effort, then forget about it.

On a second look, it appears that this is exactly how it should work on Windows. get_default_monitor is influenced by the --screen=1, so VOCTRL_GET_DISPLAY_RES should return that one if it exists. I actually like the windows behavior here; probably X11 and Wayland could match it.

[Dudemanguy]

I merged it now. Should hopefully return actually meaningful values on windows.

[CrendKing]

@Dudemanguy, I got the nightly build to test this now. It works perfectly if the initial display is the main one (i.e. --screen=0). However, it's wrong on secondary display. For example, suppose my main display is 1920x1080, second is 2560x1440, and I have them horizontally placed:

-------  ------------------
|     |  |                |
|  0  |  |                |
|     |  |        1       |
-------  |                |
         |                |
         ------------------

When I start mpv with --screen=1, I get 4480x1440, where 4480 = 1920 + 2560. If I place them vertically:

-------
|     |
|  0  |
|     |
-------
------------------
|                |
|                |
|        1       |
|                |
|                |
------------------

I get 2560x2520, where 2520 = 1080 + 1440.

The reason is that in get_screen_area() it returns the RECT, which is fine since it has and x and y information. However, in the code handling VOCTRL_GET_DISPLAY_RES, the x and y are dropped. I think that returned data is simply not what the API is supposed to return, and we should change it, even it means breaking some backward compatibility. I'm wondering why no one has complained yet. Does that mean no one uses this API?

Also, what about the implementation of the same API on other systems? For example, drm probably got it right, but wayland seems to be wrong: it's the coordinate of the bottom right corner, not width and height. Maybe the whole thing warrants a separate issue and PR?

[Dudemanguy]

If you check the display-width and display-height properties on windows afterwards on the secondary display (like with the console), does it display the right coordinates? If so, then it has something to do with initialization. If not, then indeed it's just plain wrong and no one ever noticed.

Wayland is right. I tested that myself. There's no global rectangular layout exposed to clients in wayland. You just get the width/height for every output, so it is indeed x1 and y1.

[CrendKing]

The display-width and display-height are the exact same value as the new display_res, so they are also wrong. Looks like it comes from the same source.

Sorry about misunderstanding Wayland. I didn't know that. I also glanced at common.swift and x11_common.c and both seem correct. Hopefully w32 is the only one incorrect.

Do you want me to make an issue to track this?

[Dudemanguy]

Wayland is different from literally everything else for better or worse, so not surprising that someone would misunderstand. And yeah that's a legitimate bug then and worth an issue.

[CrendKing]

#12172