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

clipboard: add --clipboard-backends option #15870

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

na-na-hi
Copy link
Contributor

This allows specifying a priority list of clipboard backends to use. This is mostly useful on Linux where multiple clipboard backends exist.

Copy link

github-actions bot commented Feb 15, 2025

Download the artifacts for this pull request:

Windows
macOS

@llyyr
Copy link
Contributor

llyyr commented Feb 15, 2025

I can't envision any priority list that's more than 2 backends "X,vo", even taking into account future backends for e.g. X11. It would be far simpler to implement a --clipboard-backend=<native|vo> option IMO.

player/clipboard/clipboard.c Outdated Show resolved Hide resolved
player/clipboard/clipboard.c Outdated Show resolved Hide resolved
@na-na-hi na-na-hi force-pushed the clipboard-backend-list branch from c3c3998 to 6dbd1bc Compare February 15, 2025 12:37
@na-na-hi
Copy link
Contributor Author

I can't envision any priority list that's more than 2 backends "X,vo", even taking into account future backends for e.g. X11.

X11 backend won't be VO depedent, which means that mpv with a wayland VO window can use 3 backends: X11, Wayland, and VO.

@na-na-hi na-na-hi force-pushed the clipboard-backend-list branch from 6dbd1bc to 9ee8c3f Compare February 15, 2025 13:04
@llyyr
Copy link
Contributor

llyyr commented Feb 15, 2025

mpv with a wayland VO window can use 3 backends: X11

Elaborate?

@na-na-hi
Copy link
Contributor Author

na-na-hi commented Feb 15, 2025

Elaborate?

X11 backend can be used to access Xwayland clipboard, or the clipboard of some other X session (like from Xephyr/another VT). This can be done regardless if mpv VO window is Wanyland or X11, or even with no VO window at all.

@llyyr
Copy link
Contributor

llyyr commented Feb 15, 2025

I can't think of any good practical reason to allow that. It's just another attack vector if mpv can access the X11 clipboard while having no VO or having a wayland VO active. A vo native clipboard backend should only be possible to be used if mpv has a VO window of the same backend. This is already the case for every backend except X11 due to platform limitations.

@kasper93
Copy link
Contributor

I can't think of any good practical reason to allow that.

It's only an option. I think it is good to have. And having it directly liked to implementation makes sense, instead of creating another list of pseudo-implementation names.

Also this option makes --clipboard-enable redundant. Do you plan to handle that?

@na-na-hi na-na-hi force-pushed the clipboard-backend-list branch from 7e3d92c to 9fd2e8d Compare February 16, 2025 04:48
@na-na-hi
Copy link
Contributor Author

Also this option makes --clipboard-enable redundant. Do you plan to handle that?

It's now removed, and updated --clipboard-backends doc to include how to disable all backends. Better to do this now before it goes into a release.

@na-na-hi na-na-hi force-pushed the clipboard-backend-list branch from 9fd2e8d to 7c13384 Compare February 16, 2025 04:49
@kasper93
Copy link
Contributor

Also this option makes --clipboard-enable redundant. Do you plan to handle that?

It's now removed, and updated --clipboard-backends doc to include how to disable all backends. Better to do this now before it goes into a release.

Thanks. Except one remaining comment, the change looks ok to me.

@na-na-hi na-na-hi force-pushed the clipboard-backend-list branch 2 times, most recently from cefa5f3 to 00f2e0e Compare February 16, 2025 08:23
This allows specifying a priority list of clipboard backends
to use. This is mostly useful on Linux where multiple clipboard
backends exist.
Clipboard can be now disabled with --clipboard-backends-clr so
--clipboard-enable is redundant.
@na-na-hi na-na-hi force-pushed the clipboard-backend-list branch from 00f2e0e to 46fad15 Compare February 16, 2025 08:26
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.

3 participants