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

win32: add an option to change window affinity #11494

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

DeadSix27
Copy link
Contributor

Add an option to control the mpv window's affinity. This will affect how it's displayed on the desktop and in system-level operations like taking screenshots.

@garoto
Copy link
Contributor

garoto commented Mar 24, 2023

Will this affect previous Windows versions like 7 or 8.1?

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Mar 24, 2023

Will this affect previous Windows versions like 7 or 8.1?

It's supported since Windows 7, however the option excludefromcapture will not work on anything below Windows 10, as per Windows' API:

Starting in Windows 10 Version 2004, WDA_EXCLUDEFROMCAPTURE is a supported value. Setting the display affinity to WDA_EXCLUDEFROMCAPTURE on previous version of Windows will behave as if WDA_MONITOR is applied.

WDA_MONITOR being: The window content is displayed only on a monitor. Everywhere else, the window appears with no content.

--
So on Windows 7 and 8 setting the option to excludefromcapture will be the same as setting it to monitor.

See https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowdisplayaffinity for more info.

@garoto
Copy link
Contributor

garoto commented Mar 24, 2023

:default: No change in default Windows behavior

"mpv window" I assume?

--window-affinity=<default|excludefromcapture|monitor>
excludefromcapture is only mentioned here. Is it the same as system in the option explanation?

Probably best to replace all occurrences of The window with mpv window, since "The window" is very ambiguous.

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Mar 24, 2023

EDIT: also added ability to change it at runtime now.

:default: No change in default Windows behavior

"mpv window" I assume?

--window-affinity=<default|excludefromcapture|monitor> excludefromcapture is only mentioned here. Is it the same as system in the option explanation?

Probably best to replace all occurrences of The window with mpv window, since "The window" is very ambiguous.

Fixed those mistakes in the doc. Originally I was referring to "the default windows(as in OS) behavior" to avoid repeating "mpv window" but this works too.

@DeadSix27
Copy link
Contributor Author

i have nothing else to add, feel free to review.

@DeadSix27 DeadSix27 force-pushed the add-window-affinity branch 2 times, most recently from 7b8c03d to 0d71053 Compare April 5, 2023 12:07
@Traneptora
Copy link
Member

Can you please squash this commits? It's much easier to review that way. Also you can always merge master in with git pull upstream master --rebase and it will rebase your own commits atop the existing ones from master.

@DeadSix27
Copy link
Contributor Author

Can you please squash this commits? It's much easier to review that way. Also you can always merge master in with git pull upstream master --rebase and it will rebase your own commits atop the existing ones from master.

Hope I did it right.. never squashed commits.

video/out/w32_common.c Outdated Show resolved Hide resolved
@DeadSix27
Copy link
Contributor Author

How can i fix the merge mistake? I'm using the github desktop app.

@Dudemanguy
Copy link
Member

Easiest way imo is to make a new branch off of master, cherry-pick the commit you want, delete your old one, rename the new branch to the old one, and then force push that.

@DeadSix27
Copy link
Contributor Author

Thanks, works now, I also added an #ifdef to m_sub_options for the affinity option.
Mind reviewing it fully now?

@DeadSix27 DeadSix27 force-pushed the add-window-affinity branch 4 times, most recently from 2521a86 to 6c24de5 Compare September 13, 2023 17:59
options/options.c Outdated Show resolved Hide resolved
options/options.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
@DeadSix27 DeadSix27 force-pushed the add-window-affinity branch 4 times, most recently from eba8e88 to dee750e Compare September 13, 2023 20:57
Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments.

options/options.c Outdated Show resolved Hide resolved
options/options.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
@DeadSix27 DeadSix27 force-pushed the add-window-affinity branch 2 times, most recently from 1c781a8 to ad4d269 Compare September 13, 2023 21:37
Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't test but LGTM.

@DeadSix27
Copy link
Contributor Author

I only tested it on Windows11, I have no 10 or 7 Machine.
On Linux I just checked if the code hampers compiling, anything else shouldn't matter code wise.

@github-actions
Copy link

github-actions bot commented Sep 21, 2023

Download the artifacts for this pull request:

Windows

@Dudemanguy
Copy link
Member

Dudemanguy commented Sep 21, 2023

Can you get rid of the merge commit?

Edit: Looks like the fit_border option snuck back in there somehow (just removed it).

@DeadSix27
Copy link
Contributor Author

Cleaned it up, should be fine now I hope, I accidentally ran git reset on 1 of my other repos which gave me headache...

options/options.c Outdated Show resolved Hide resolved
options/options.h Outdated Show resolved Hide resolved
@DeadSix27 DeadSix27 force-pushed the add-window-affinity branch 3 times, most recently from ec471ed to 0eb3a28 Compare September 21, 2023 22:00
@Dudemanguy Dudemanguy merged commit 2c738ca into mpv-player:master Sep 21, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants