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

Auto resize issue with video size equal or bigger than screen resolution #2935

Closed
maniak1349 opened this issue Mar 12, 2016 · 4 comments
Closed
Labels

Comments

@maniak1349
Copy link
Contributor

This behavior has been introduced the same time mpv started to shrink videos bigger than the screen resolution to fit screen. The thing is mpv is trying to fit not only the video, but the whole window into screen including its borders. That results in slight downscaling (and as a result slight quality drop) in the case when video matches screen width (and/or height) exactly i.e. 1280x768 video on 1280x1024 screen.
Things I tried to fix this behavior:

  1. --autofit and --geomtry will not resize window to a size bigger than the screen
  2. --no-border probably should do the trick but
    a) I still want to have window title bar so I can minimize it with the button
    b) For some reason --no-border makes window size 1 pixel short from maximum possible value (1280->1279) and this results in a downscale
  3. Manual resize of the window with border pulling
    a) I should do it every time I'm starting playing video with aforementioned format which is not convenient
    b) For some reason mpv allows me to resize the video that way more than it probably should (3 or 4 pixels in my case up to 1284 instead of 1280). This will lead to a pixel hunting on every resize attempt to match exact size, which is not convenient at all.
    All of the above was tested on windows 7 x64 with and without aero.

So is mpv trying to fit whole window into screen an issue or a feature? If it is a feature, then I believe there should be an option to make mpv to try to fit video into screen and not the window.
Also, I believe behavior described in 2.b and 3.b is a bug. Is it a bug and should I just leave it here or should I make separate issue(s) for that?

@haasn
Copy link
Member

haasn commented Mar 12, 2016

Naively I'd call it an issue, not a feature.

I think mpv's knowledge about its window size should be focused entirely on the window contents. The border is not coming from mpv, ergo mpv should not concern itself with it.

If mpv detects the screen as being 1280 pixels wide, and the media is also 1280 pixels wide, then mpv should create a window where it has a 1280 pixel-wide backbuffer and render 1:1.

(My 2 cents)

@ghost ghost added the os:win label Mar 12, 2016
@ghost
Copy link

ghost commented Mar 12, 2016

Windows doesn't allow a window to be larger than the screen, so mpv tries to make it smaller. Otherwise you get strange issues like mismatching aspect ratio.

@maniak1349
Copy link
Contributor Author

You refer to the case when the system will resize the window when it's out of the screen? It seems to happen only when the window is positioned higher than -title_bar_height (-18 in my case) but does not happen when window exceeds right/left/bottom border of the screen. It also seems not to be related to window size itself, because it happens to smaller windows too.

@maniak1349
Copy link
Contributor Author

maniak1349 commented Apr 19, 2016

--no-border behavior described in 2.b seems to be originating from the line
((r.right - r.left) >= screen_w || (r.bottom - r.top) >= screen_h))
in video/out/w32_common.c. I'm pretty sure it should be > and not >=. AdjustWindowRect called from add_window_borders returns unmodified values for borderless window, and RECT is one pixel bigger than the area described by it, so unless screen_w is higher than the screen width >= is triggered for the exact match.

Maximum window size (described in 3.b) seems to have nothing to do with mpv code. According to MSDN (ptMaxSize description) window size is restricted to width/height of the primary monitor. But on practice values of ptMaxSize sent by WM_GETMINMAXINFO seems to allow width/height = width/height of the primary monitor + ( window border size x 2 ).

While allowing to fit window with borders outside of the screen by width, this however will not allow to fit window with video height == screen height due to window title bar. That's why I think that trying to fit the client area instead of the window on screen should not be made a default behavior, but rather an option.

The solution for the main issue seems to be to check on cr instead of r when (yet to be added) option is passed (same place in video/out/w32_common.c).
ED: Well, and all size and position calculations should be adjusted for this change. So it's not that simple as I thought.

maniak1349 added a commit to maniak1349/mpv that referenced this issue Apr 26, 2016
Substraction of 1 is not necessary due to .right and .bottom values of RECT
struct describing a point one pixel outside of the rectangle.

Fixes mpv-player#2935. (2.b)
maniak1349 added a commit to maniak1349/mpv that referenced this issue Apr 26, 2016
Fit whole window or just a client area in accord with value of --fit-border option.

Fixes mpv-player#2935.
@ghost ghost closed this as completed in 5c90352 Apr 30, 2016
ghost pushed a commit that referenced this issue Apr 30, 2016
Fit whole window or just a client area in accord with value of --fit-border option.

Fixes #2935.
avih added a commit to avih/mpv that referenced this issue Apr 17, 2021
The accurate description of this option was:
- fit-border is enabled by default. When disabled, it adds a bug where
  if the window has borders and mpv shrinks it to fit the desktop, then
  the calculation ignores the borders and adds incorrect video crop.

The option was added at commits 70f64f3 and 949247d, in order to
solve an issue (mpv-player#2935) where if mpv wanted to display a video with
size WxH, then w32_common.c incorrectly set the window to WxH, while
down-scaling the video slightly to fit (even with small sizes).

It was addressed with a new option which is enabled by default, but
does the right thing (sets the client area to WxH) only when disabled,
so that everyone who prefers their video slightly downscaled could
keep their default behavior.

(mpv-player#2935 also addressed an off-by-one issue, fixed before fit-border)

While disabling the option did avoid unnecessary downscaling, it also
added a bug when disabled: the borders are no longer taken into
account when the size is too big for the desktop. Most users don't
notice and are unaffected as it's enabled by default.

Shortly later (981048e) the core issue is fixed, and now the client
area is correctly set to WxH instead of the window (and together with
the three following commits which center the video, adds a new bug
where the window title can be outside the display - addressed next).

However, fit-border remained, now without any effect, except that it
still has the same bug when disabled and the window is too big.

Later code changes and refactoring preserved this issue with great
attention to details, and it remained in identical form until now.

Simply rip out fit-border.
avih added a commit to avih/mpv that referenced this issue Apr 17, 2021
The accurate description of this option was:
- fit-border is enabled by default. When disabled, it adds a bug where
  if the window has borders and mpv shrinks it to fit the desktop, then
  the calculation ignores the borders and adds incorrect video crop.

The option was added at commits 70f64f3 and 949247d, in order to
solve an issue (mpv-player#2935) where if mpv wanted to display a video with
size WxH, then w32_common.c incorrectly set the window to WxH, while
down-scaling the video slightly to fit (even with small sizes).

It was addressed with a new option which is enabled by default, but
does the right thing (sets the client area to WxH) only when disabled,
so that everyone who prefers their video slightly downscaled could
keep their default behavior.

(mpv-player#2935 also addressed an off-by-one issue, fixed before fit-border)

While disabling the option did avoid unnecessary downscaling, it also
added a bug when disabled: the borders are no longer taken into
account when the size is too big for the desktop. Most users don't
notice and are unaffected as it's enabled by default.

Shortly later (981048e) the core issue is fixed, and now the client
area is correctly set to WxH instead of the window (and together with
the three following commits which center the video, adds a new bug
where the window title can be outside the display - addressed next).

However, fit-border remained, now without any effect, except that it
still has the same bug when disabled and the window is too big.

Later code changes and refactoring preserved this issue with great
attention to details, and it remained in identical form until now.

Simply rip out fit-border.
avih added a commit that referenced this issue Apr 23, 2021
The accurate description of this option was:
- fit-border is enabled by default. When disabled, it adds a bug where
  if the window has borders and mpv shrinks it to fit the desktop, then
  the calculation ignores the borders and adds incorrect video crop.

The option was added at commits 70f64f3 and 949247d, in order to
solve an issue (#2935) where if mpv wanted to display a video with
size WxH, then w32_common.c incorrectly set the window to WxH, while
down-scaling the video slightly to fit (even with small sizes).

It was addressed with a new option which is enabled by default, but
does the right thing (sets the client area to WxH) only when disabled,
so that everyone who prefers their video slightly downscaled could
keep their default behavior.

(#2935 also addressed an off-by-one issue, fixed before fit-border)

While disabling the option did avoid unnecessary downscaling, it also
added a bug when disabled: the borders are no longer taken into
account when the size is too big for the desktop. Most users don't
notice and are unaffected as it's enabled by default.

Shortly later (981048e) the core issue is fixed, and now the client
area is correctly set to WxH instead of the window (and together with
the three following commits which center the video, adds a new bug
where the window title can be outside the display - addressed next).

However, fit-border remained, now without any effect, except that it
still has the same bug when disabled and the window is too big.

Later code changes and refactoring preserved this issue with great
attention to details, and it remained in identical form until now.

Simply rip out fit-border.
Dudemanguy pushed a commit to Dudemanguy/mpv that referenced this issue May 18, 2021
The accurate description of this option was:
- fit-border is enabled by default. When disabled, it adds a bug where
  if the window has borders and mpv shrinks it to fit the desktop, then
  the calculation ignores the borders and adds incorrect video crop.

The option was added at commits 70f64f3 and 949247d, in order to
solve an issue (mpv-player#2935) where if mpv wanted to display a video with
size WxH, then w32_common.c incorrectly set the window to WxH, while
down-scaling the video slightly to fit (even with small sizes).

It was addressed with a new option which is enabled by default, but
does the right thing (sets the client area to WxH) only when disabled,
so that everyone who prefers their video slightly downscaled could
keep their default behavior.

(mpv-player#2935 also addressed an off-by-one issue, fixed before fit-border)

While disabling the option did avoid unnecessary downscaling, it also
added a bug when disabled: the borders are no longer taken into
account when the size is too big for the desktop. Most users don't
notice and are unaffected as it's enabled by default.

Shortly later (981048e) the core issue is fixed, and now the client
area is correctly set to WxH instead of the window (and together with
the three following commits which center the video, adds a new bug
where the window title can be outside the display - addressed next).

However, fit-border remained, now without any effect, except that it
still has the same bug when disabled and the window is too big.

Later code changes and refactoring preserved this issue with great
attention to details, and it remained in identical form until now.

Simply rip out fit-border.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants