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, wayland: add clipboard support #15355

Merged
merged 14 commits into from
Nov 27, 2024
Merged

Conversation

na-na-hi
Copy link
Contributor

Alternative to #13837.

This adds a general internal clipboard API with support for different formats, targets, and autoprobing backends, and implements 2 clipboard backends (win32and VO) and Wayland support for the VO backend. The clipboard can be interacted publicly with the clipboard property, with change notification support. The internal API provides a polling interface for clipboard backends which use polling for notification. Other backends monitors changes in the backgrounds and notify the property when changes are detected.

Currently, only text format and clipboard target is implemented, but image formats and primary selection target can be added later.

console.lua is changed to use the clipboard property when supported.

Fixes: #14373
Partially fixes (currently Windows, other platforms can be added later): #7361

DOCS/man/options.rst Outdated Show resolved Hide resolved
@guidocella
Copy link
Contributor

guidocella commented Nov 22, 2024

This completely freezes mpv on startup on sway for me. If compiling with ASan, I get

AddressSanitizer:DEADLYSIGNAL
=================================================================
==5622==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7b71fdb22d24 bp 0x7b71cd73b950 sp 0x7b71cd73b928 T15)
==5622==The signal is caused by a READ memory access.
==5622==Hint: address points to the zero page.
    #0 0x7b71fdb22d24 in wl_proxy_get_version (/usr/lib/libwayland-client.so.0+0x4d24) (BuildId: de7a5d36bc86167ed7de538a6f63cdc526014ace)
    #1 0x585b878085a0 in wl_data_offer_receive /usr/include/wayland-client-protocol.h:2369
    #2 0x585b8781476e in data_device_handle_selection ../../opt/mpv/video/out/wayland_common.c:860
    #3 0x7b71eef04595  (/usr/lib/libffi.so.8+0x7595) (BuildId: eecfa567f01d70c2ca4b60a1f7931e5634e41eea)
    #4 0x7b71eef0100d  (/usr/lib/libffi.so.8+0x400d) (BuildId: eecfa567f01d70c2ca4b60a1f7931e5634e41eea)
    #5 0x7b71eef03bd2 in ffi_call (/usr/lib/libffi.so.8+0x6bd2) (BuildId: eecfa567f01d70c2ca4b60a1f7931e5634e41eea)
    #6 0x7b71fdb228af  (/usr/lib/libwayland-client.so.0+0x48af) (BuildId: de7a5d36bc86167ed7de538a6f63cdc526014ace)
    #7 0x7b71fdb23138  (/usr/lib/libwayland-client.so.0+0x5138) (BuildId: de7a5d36bc86167ed7de538a6f63cdc526014ace)
    #8 0x7b71fdb23552 in wl_display_dispatch_queue_pending (/usr/lib/libwayland-client.so.0+0x5552) (BuildId: de7a5d36bc86167ed7de538a6f63cdc526014ace)
    #9 0x585b87839133 in wayland_dispatch_events ../../opt/mpv/video/out/wayland_common.c:2827
    #10 0x585b87847d39 in vo_wayland_wait_frame ../../opt/mpv/video/out/wayland_common.c:3511
    #11 0x585b878c38a8 in wayland_egl_swap_buffers ../../opt/mpv/video/out/opengl/context_wayland.c:73
    #12 0x585b878d6d4f in ra_gl_ctx_swap_buffers ../../opt/mpv/video/out/opengl/context.c:285
    #13 0x585b8763db8d in flip_page ../../opt/mpv/video/out/vo_gpu.c:98
    #14 0x585b87633175 in render_frame ../../opt/mpv/video/out/vo.c:981
    #15 0x585b87635976 in vo_thread ../../opt/mpv/video/out/vo.c:1105
    #16 0x7b720145d109 in asan_thread_start /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:234
    #17 0x7b71fd0a339c  (/usr/lib/libc.so.6+0x9439c) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #18 0x7b71fd12849b  (/usr/lib/libc.so.6+0x11949b) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/usr/lib/libwayland-client.so.0+0x4d24) (BuildId: de7a5d36bc86167ed7de538a6f63cdc526014ace) in wl_proxy_get_version
Thread T15 created by T0 here:
    #0 0x7b72014f468b in pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:245
    #1 0x585b87624964 in vo_create ../../opt/mpv/video/out/vo.c:317
    #2 0x585b8762511b in init_best_video_out ../../opt/mpv/video/out/vo.c:354
    #3 0x585b87417e85 in reinit_video_chain_src ../../opt/mpv/player/video.c:235
    #4 0x585b87417895 in reinit_video_chain ../../opt/mpv/player/video.c:211
    #5 0x585b873c9bfc in play_current_file ../../opt/mpv/player/loadfile.c:1728
    #6 0x585b873ceeca in mp_play_files ../../opt/mpv/player/loadfile.c:1990
    #7 0x585b873d520c in mpv_main ../../opt/mpv/player/main.c:446
    #8 0x585b879a7b80 in main ../../opt/mpv/osdep/main-fn-unix.c:5
    #9 0x7b71fd034e07  (/usr/lib/libc.so.6+0x25e07) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #10 0x7b71fd034ecb in __libc_start_main (/usr/lib/libc.so.6+0x25ecb) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #11 0x585b86fad424 in _start (/tmp/mpv/mpv+0xeec424) (BuildId: 1938173f87f386fa0f49ae165b39c7e45484d97c)

==5622==ABORTING

On dwl + wlroots-git I can use mpv and read the clipboard, but writing to clipboard/text fails.

Same behavior on Mesa and Nvidia.

Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

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

I think this version is more scalable than #13837 with clipboard backend definitions. Though notably is missing image support, which for me is not a deal breaker for first implementation, but the other PR did have it implemented, so we should align on a way forward how to merge those two implementations, probably #13837 would be rebased as clipboard backend in this architecture.

meson.build Outdated Show resolved Hide resolved
player/clipboard/clipboard-vo.c Outdated Show resolved Hide resolved
player/clipboard/clipboard-vo.c Outdated Show resolved Hide resolved
player/clipboard/clipboard-vo.c Outdated Show resolved Hide resolved
player/clipboard/clipboard-vo.c Outdated Show resolved Hide resolved
player/command.c Show resolved Hide resolved
player/command.c Outdated Show resolved Hide resolved
player/lua/console.lua Show resolved Hide resolved
static void handle_clipboard_updates(struct MPContext *mpctx)
{
if (mp_clipboard_data_changed(mpctx->clipboard))
mp_notify_property(mpctx, "clipboard");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remind me what would be the use-case for monitoring the clipboard in mpv? I always though it would rather be used to past text / image data on demand by user, rather than monitoring it constantly and acting. I would even argue that I don't want my media player to act depending on my clipboard content automatically. That's in no means rejecting the idea, but I would like to know when would that be useful.

Copy link
Contributor Author

@na-na-hi na-na-hi Nov 22, 2024

Choose a reason for hiding this comment

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

It's implemented in the other PR, and properties are generally expected to be observable. Clipboard polling is also disabled by default.

Copy link

@FrozenGalaxy FrozenGalaxy Nov 26, 2024

Choose a reason for hiding this comment

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

Could you remind me what would be the use-case for monitoring the clipboard in mpv?

Could be used for auto playing Links(like YouTube) while copying links or so, some other programs do that too I think
Never personally used it but nice to have I guess. And: and > "properties are generally expected to be observable."

video/out/wayland_common.c Show resolved Hide resolved
@na-na-hi na-na-hi force-pushed the clipboard branch 2 times, most recently from d190cd3 to fe5676b Compare November 22, 2024 17:24
})
if not res.error then
return res.stdout
if clip and mp.get_property('current-vo') and mp.get_property_native('video-osd') then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is those checks required? Wouldn't it be better to check mp.get_property('clipboard/text', nil) ~= nil and fallback to wl-paste?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Wayland clipboard is only updated when the VO window is focused. If the user only focuses on the terminal window while the VO window exists, the clipboard can contain outdated data.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. And it cannot be detected on clipboard side? Well, it seems bit tricky for script users to know this detail.

Also would focused property work in this case? It is implemented by wayland.

Copy link
Contributor

Choose a reason for hiding this comment

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

The clipboard property still works with a VO and just --video-osd=no.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to use focused property instead.

And it cannot be detected on clipboard side? Well, it seems bit tricky for script users to know this detail.

This is a Wayland limitation, nothing I can do about it. I can document this detail if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please. It should be noted in docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to check current-vo if focused is true, it can't be true in the terminal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@guidocella
Copy link
Contributor

I can confirm it no longer freezes on sway. Also I saw that the docs say that writing is not supported on Wayland.

@na-na-hi na-na-hi force-pushed the clipboard branch 5 times, most recently from c3b9457 to 29b75ef Compare November 22, 2024 19:11
This adds a clipboard API with multiple backend and format support.
--clipboard-enable option can be toggled at runtime to turn native
clipboard on and off.
This adds clipboard property which uses the clipboard
API to get and set clipboard contents. Currently only
clipboard text is implemented, but this can be extended in
the future to cover primary selection and other formats.
This adds VOCTRLs needed for VO-based clipboard backend.
This adds a generic VO clipboard backend based on VOCTRLs
to get and set clipboard data.
Clipboard contents are available as selection data offers under
Wayland. The offer can become invalid at any time, so request the
text format content immediately when an offer is received,
and cache the content for later use.
This adds support for VOCTRL_GET_CLIPBOARD which makes the VO
clipboard backend be able to get clipboard content for Wayland.
This adds a Windows clipboard backend. This doesn't depend on
VO, so it can be used even when no video window is created.
This adds a clipboard monitoring API for backends which use polling
to monitor clipboard updates. --clipboard-monitor option can turn
clipboard monitoring on and off at runtime.
This uses GetClipboardSequenceNumber() to detect clipboard
content changes. Clipboard Format Listener is a better way to
do this according to MS docs, but sequence number works and
the listener requires creating a dedicated thread and HWND
for monitoring, so I will save it for a later time.
Since VOCTRL is not suitable for frequent data query
(see 477a0f8 for details),
it's not suitable to be used by the VO clipboard backend.
Instead, since the VO does the clipboard monitoring by itself,
it can notify the player when the clipboard is updated.

This adds an internal notify-property command so that VOs
can notify player when the clipboard is updated, so that clipboard
monitoring works.
Works by notifying property update when clipboard content is
updated.
Since native clipboard is implemented on win32 and wayland,
use the clipboard property instead.

This fixes the problems with commandline implementations of clipboard:
- On win32, the powershell implementation is complex, and it can take
  several seconds to run for the first time.
- On wayland, it requires wl-paste to be installed, which isn't always
  available. It also works poorly on GNOME.
@kasper93
Copy link
Contributor

Thanks, I hope this will be extended in the future.

@kasper93 kasper93 merged commit b97e3b9 into mpv-player:master Nov 27, 2024
23 of 26 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.

Please don't use powershell Get-Clipboard in console.lua. It can take up to 1x seconds on first run
4 participants