use pipewire buffers

[columbarius]

This MR aims to implement screencast driven by pipewire as explained in #137.

Resolves: #137

Depends: #150 Depends: #148 Depends: #46 Depends: #61

[columbarius]

Update: works on my machine!

Please take a look if it's going in the right direction. Performance is about doubled (at least from the cpu reading in obs).

I have to look into the shutdown procedure, if there is sth. to improve.

Sry for the last commit. The changes with using pipewire to allocate buffers, importing them and restructuring the wlroots screencopy handlers were so connected, that i couldn't find a good spot, which didn't produce a memory leak or segfault in an intermediate step, so I added it as once.

[columbarius]

If you want to inspect the "new" flow how we export the buffers please start with the screencopy handlers here: https://github.com/columbarius/xdg-desktop-portal-wlr/blob/screencast-pw-driven/src/screencast/wlr_screencast.c#L62

[columbarius]

Changing screenresolution can work, but it can fail, if pipewire let's us delete a buffer in the middle of processing (eg. after the copy request and before the buffer_ready, which segfaults in export_buffer).

We probably need a check if the buffer we are exporting still exists.

Further todo: investigate if we can just take any buffer as invalid and cleanedup after pipewire calls remove_buffer and we don't have to requeue them.

[danshick]

@emersion

We need to push a bugfix release before this lands (which may be sooner than I expected, thanks columbarius). People without configs who also don't have a default chooser installed are experiencing errors.

[columbarius]

This zwlr_screencopy_frame_v1@12: error 1: invalid buffer stride error appears on resolution change.

2021/05/28 10:30:10 [TRACE] - ********************
2021/05/28 10:30:10 [TRACE] - wlroots: frame destroyed
2021/05/28 10:30:10 [TRACE] - wlroots: callbacks registered
2021/05/28 10:30:10 [TRACE] - event-loop: got wayland event
2021/05/28 10:30:10 [TRACE] - wlroots: buffer event handler
2021/05/28 10:30:10 [TRACE] - pipewire: importing buffer
2021/05/28 10:30:10 [TRACE] - wlroots: linux_dmabuf event handler
2021/05/28 10:30:10 [TRACE] - wlroots: buffer_done event handler
2021/05/28 10:30:10 [TRACE] - wlroots: shm buffer exists
2021/05/28 10:30:10 [TRACE] - wlroots: frame copied
2021/05/28 10:30:10 [TRACE] - event-loop: got wayland event
2021/05/28 10:30:11 [TRACE] - event-loop: got wayland event
2021/05/28 10:30:11 [TRACE] - wlroots: flags event handler
2021/05/28 10:30:11 [TRACE] - wlroots: damage event handler
2021/05/28 10:30:11 [TRACE] - wlroots: ready event handler
2021/05/28 10:30:11 [TRACE] - pipewire: exporting buffer
2021/05/28 10:30:11 [TRACE] - ********************
2021/05/28 10:30:11 [TRACE] - pipewire: pointer 0x6070001207c0
2021/05/28 10:30:11 [TRACE] - pipewire: size 7680000
2021/05/28 10:30:11 [TRACE] - pipewire: stride 6400
2021/05/28 10:30:11 [TRACE] - pipewire: width 1600
2021/05/28 10:30:11 [TRACE] - pipewire: height 1200
2021/05/28 10:30:11 [TRACE] - pipewire: y_invert 0
2021/05/28 10:30:11 [TRACE] - ********************
2021/05/28 10:30:11 [TRACE] - wlroots: frame destroyed
2021/05/28 10:30:11 [TRACE] - wlroots: callbacks registered
2021/05/28 10:30:11 [TRACE] - event-loop: got wayland event
2021/05/28 10:30:11 [TRACE] - wlroots: buffer event handler
2021/05/28 10:30:11 [TRACE] - pipewire: importing buffer
2021/05/28 10:30:11 [TRACE] - wlroots: buffer properties changed
2021/05/28 10:30:11 [DEBUG] - wlroots: reset buffer
2021/05/28 10:30:11 [TRACE] - wlroots: linux_dmabuf event handler
2021/05/28 10:30:11 [TRACE] - wlroots: buffer_done event handler
2021/05/28 10:30:11 [TRACE] - pipewire: exporting buffer
2021/05/28 10:30:11 [TRACE] - ********************
2021/05/28 10:30:11 [TRACE] - pipewire: pointer 0x6070001207c0
2021/05/28 10:30:11 [TRACE] - pipewire: size 7680000
2021/05/28 10:30:11 [TRACE] - pipewire: stride 6400
2021/05/28 10:30:11 [TRACE] - pipewire: width 1920
2021/05/28 10:30:11 [TRACE] - pipewire: height 1080
2021/05/28 10:30:11 [TRACE] - pipewire: y_invert 0
2021/05/28 10:30:11 [TRACE] - ********************
2021/05/28 10:30:11 [TRACE] - pipewire: stream update parameters
2021/05/28 10:30:11 [TRACE] - wlroots: frame destroyed
2021/05/28 10:30:11 [TRACE] - wlroots: callbacks registered
2021/05/28 10:30:11 [TRACE] - event-loop: got pipewire event
2021/05/28 10:30:11 [TRACE] - event-loop: got wayland event
2021/05/28 10:30:11 [TRACE] - wlroots: buffer event handler
2021/05/28 10:30:11 [TRACE] - pipewire: importing buffer
2021/05/28 10:30:11 [TRACE] - wlroots: linux_dmabuf event handler
2021/05/28 10:30:11 [TRACE] - wlroots: buffer_done event handler
2021/05/28 10:30:11 [TRACE] - wlroots: shm buffer exists
2021/05/28 10:30:11 [TRACE] - wlroots: frame copied
2021/05/28 10:30:11 [TRACE] - event-loop: got pipewire event
2021/05/28 10:30:11 [TRACE] - pipewire: remove buffer event handle
2021/05/28 10:30:11 [TRACE] - pipewire: can't remove buffer. still in use
2021/05/28 10:30:11 [TRACE] - pipewire: stream parameters changed
2021/05/28 10:30:11 [TRACE] - event-loop: got wayland event
zwlr_screencopy_frame_v1@12: error 1: invalid buffer stride
2021/05/28 10:30:11 [ERROR] - wl_display_dispatch failed: Protokollfehler

=================================================================
==3091==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 320 byte(s) in 4 object(s) allocated from:
    #0 0x7f5630a7c459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f56309b9afb in wl_proxy_marshal_array_constructor_versioned (/usr/lib/libwayland-client.so.0+0x6afb)

[columbarius]

@emersion

We need to push a bugfix release before this lands (which may be sooner than I expected, thanks columbarius). People without configs who also don't have a default chooser installed are experiencing errors.

I second that part with the bugfix release.

I don't see this MR merged soon, since the change in resolution/renegotiation part needs some work, where I'm not sure how to do it (also important to the point, that pipewire doesn't kills our buffers while we are using them) and I would like to make things right in a way, that it is nicely compatible with both shm and dmabuf depending on the clients preferences. I just had some unexpected time and made those changes to a point, where it works if you don't do anything exceptional.

[columbarius]

Ok, i'm restarting the stream after a renegotiation by some kind of refcounting in {add,remove}_buffer. It's not elegant, but it works for now.

Will have to modify obs again to test what's happening, if a client requests param changes. We might have to add a lock to the imported buffer and register the buffer_destroy call on the release of that lock, to prevent the buffer from being destroyed while wlroots is using it.

We might be able to just drop that buffer without the requeueing the buffer to pipewire beforehand.

Next step would be to rebase the/create a new dmabuf MR on top of the new flow. To test it I have a obs-studio branch ready (https://github.com/columbarius/obs-studio/tree/egl-modifiers).

[emersion]

prevent the buffer from being destroyed while wlroots is using it

It should be fine to destroy the wl_buffer while wlroots is writing to it, if we no longer care about the buffer and don't need the result. wlroots ref'counts its buffers.

[columbarius]

It should be fine to destroy the wl_buffer while wlroots is writing to it, if we no longer care about the buffer and don't need the result. wlroots ref'counts its buffers.

@emersion do you have an explanation how this error occures ( https://github.com/emersion/xdg-desktop-portal-wlr/pull/141#issuecomment-850255089 ) if i destroy the buffer (wl_buffer and the underlying shm buffer)?

[emersion]

Can you grab a WAYLAND_DEBUG=1 log?

[columbarius]

2021/06/14 12:10:37 [TRACE] - pipewire: importing buffer
2021/06/14 12:10:37 [TRACE] - wlroots: buffer properties changed
2021/06/14 12:10:37 [DEBUG] - wlroots: reset buffer
[296121.345] zwlr_screencopy_frame_v1@10.linux_dmabuf(875713089, 1600, 1200)
2021/06/14 12:10:37 [TRACE] - wlroots: linux_dmabuf event handler
[296121.386] zwlr_screencopy_frame_v1@10.buffer_done()
2021/06/14 12:10:37 [TRACE] - wlroots: buffer_done event handler
2021/06/14 12:10:37 [TRACE] - pipewire: exporting buffer
2021/06/14 12:10:37 [TRACE] - ********************
2021/06/14 12:10:37 [TRACE] - pipewire: pointer 0x607000018350
2021/06/14 12:10:37 [TRACE] - pipewire: size 8294400
2021/06/14 12:10:37 [TRACE] - pipewire: stride 7680
2021/06/14 12:10:37 [TRACE] - pipewire: width 1600
2021/06/14 12:10:37 [TRACE] - pipewire: height 1200
2021/06/14 12:10:37 [TRACE] - pipewire: y_invert 0
2021/06/14 12:10:37 [TRACE] - ********************
2021/06/14 12:10:37 [TRACE] - pipewire: stream update parameters
[296121.657]  -> zwlr_screencopy_frame_v1@10.destroy()
2021/06/14 12:10:37 [TRACE] - wlroots: frame destroyed
2021/06/14 12:10:37 [TRACE] - fps_limit: elapsed time since the last measurement: 271592476, target 3333333, target not met (ns)
[296121.731]  -> zwlr_screencopy_manager_v1@6.capture_output(new id zwlr_screencopy_frame_v1@3, 1, wl_output@7)
2021/06/14 12:10:37 [TRACE] - wlroots: callbacks registered
2021/06/14 12:10:37 [TRACE] - event-loop: got pipewire event
2021/06/14 12:10:37 [TRACE] - event-loop: got pipewire event
2021/06/14 12:10:37 [TRACE] - pipewire: remove buffer event handle
[296122.453]  -> wl_buffer@9.destroy()
2021/06/14 12:10:37 [TRACE] - pipewire: stream parameters changed
2021/06/14 12:10:37 [TRACE] - event-loop: got wayland event
[296135.786] wl_display@1.delete_id(10)
[296135.817] wl_display@1.delete_id(9)
[296135.835] zwlr_screencopy_frame_v1@3.buffer(0, 1600, 1200, 6400)
2021/06/14 12:10:37 [TRACE] - wlroots: buffer event handler
2021/06/14 12:10:37 [TRACE] - pipewire: importing buffer
2021/06/14 12:10:37 [WARN] - pipewire: out of buffers
[296135.930] zwlr_screencopy_frame_v1@3.linux_dmabuf(875713089, 1600, 1200)
2021/06/14 12:10:37 [TRACE] - wlroots: linux_dmabuf event handler
[296135.967] zwlr_screencopy_frame_v1@3.buffer_done()
2021/06/14 12:10:37 [TRACE] - wlroots: buffer_done event handler
2021/06/14 12:10:37 [TRACE] - wlroots: shm buffer exists
[296136.006]  -> zwlr_screencopy_frame_v1@3.copy_with_damageAddressSanitizer:DEADLYSIGNAL
=================================================================
==17609==ERROR: AddressSanitizer: SEGV on unknown address 0x000067800002 (pc 0x7f079dbe9520 bp 0x55b5f7be72a2 sp 0x7ffeb82a2630 T0)
==17609==The signal is caused by a READ memory access.
    #0 0x7f079dbe9520  (/usr/lib/libwayland-client.so.0+0xa520)
    #1 0x7f079dbe5c18 in wl_proxy_marshal_array_constructor_versioned (/usr/lib/libwayland-client.so.0+0x6c18)
    #2 0x7f079dbe5df2 in wl_proxy_marshal (/usr/lib/libwayland-client.so.0+0x6df2)
    #3 0x55b5f7bd5c80 in zwlr_screencopy_frame_v1_copy_with_damage protocols/wlr-screencopy-unstable-v1-client-protocol.h:487
    #4 0x55b5f7bd67db in wlr_frame_buffer_done ../src/screencast/wlr_screencast.c:133
    #5 0x7f079d4e7acc  (/usr/lib/libffi.so.7+0x6acc)
    #6 0x7f079d4e7039  (/usr/lib/libffi.so.7+0x6039)
    #7 0x7f079dbe8fe3  (/usr/lib/libwayland-client.so.0+0x9fe3)
    #8 0x7f079dbe5562  (/usr/lib/libwayland-client.so.0+0x6562)
    #9 0x7f079dbe6cab in wl_display_dispatch_queue_pending (/usr/lib/libwayland-client.so.0+0x7cab)
    #10 0x55b5f7bccbe8 in main ../src/core/main.c:212
    #11 0x7f079d8b3b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #12 0x55b5f7bcb8cd in _start (/path/to/project/sway-project/xdg-desktop-portal-wlr/build/xdg-desktop-portal-wlr+0xb8cd)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/usr/lib/libwayland-client.so.0+0xa520)
==17609==ABORTING

Now it's killed before by the address Sanitizer, but i noticed, that i'm not handling import_buffers well if the queue is empty. Let me fix that and try another day.

[emersion]

It seems like you're calling zwlr_screencopy_frame_v1_copy_with_damage with a wl_buffer you've previously destroyed? This is a use-after-free in the xdpw code. Once wl_buffer_destroy has been called, the wl_buffer must not be used in any subsequent request.

[emersion]

Maybe destroying the zwlr_screencopy_frame_v1 together with the wl_buffer can fix it?

[columbarius]

It seems like you're calling zwlr_screencopy_frame_v1_copy_with_damage with a wl_buffer you've previously destroyed? This is a use-after-free in the xdpw code. Once wl_buffer_destroy has been called, the wl_buffer must not be used in any subsequent request.

... i forgot to NULL pw_current_buffer and xdpw_simple_buffer.buffer if the import failed .... ~now i wasn't able to crash it without refcounting and all...~ Will investigate that later. Thanks!

Nevermind still same error possible

[columbarius]

2021/06/14 12:30:58 [TRACE] - wlroots: callbacks registered
2021/06/14 12:30:58 [TRACE] - event-loop: got pipewire event
2021/06/14 12:30:58 [TRACE] - event-loop: got wayland event
[1517378.923] wl_display@1.delete_id(9)
[1517378.941] zwlr_screencopy_frame_v1@11.buffer(0, 1600, 1200, 6400)
2021/06/14 12:30:58 [TRACE] - wlroots: buffer event handler
2021/06/14 12:30:58 [TRACE] - pipewire: importing buffer
2021/06/14 12:30:58 [WARN] - pipewire: out of buffers
[1517379.019] zwlr_screencopy_frame_v1@11.linux_dmabuf(875713089, 1600, 1200)
2021/06/14 12:30:58 [TRACE] - wlroots: linux_dmabuf event handler
[1517379.090] zwlr_screencopy_frame_v1@11.buffer_done()
2021/06/14 12:30:58 [TRACE] - wlroots: buffer_done event handler
2021/06/14 12:30:58 [DEBUG] - wlroots: failed to import buffer
[1517379.136]  -> zwlr_screencopy_frame_v1@11.destroy()
2021/06/14 12:30:58 [TRACE] - wlroots: frame destroyed
2021/06/14 12:30:58 [TRACE] - fps_limit: elapsed time since the last measurement: 261801446, target 3333333, target not met (ns)
[1517379.220]  -> zwlr_screencopy_manager_v1@6.capture_output(new id zwlr_screencopy_frame_v1@9, 1, wl_output@7)
2021/06/14 12:30:58 [TRACE] - wlroots: callbacks registered
2021/06/14 12:30:58 [TRACE] - event-loop: got wayland event
[1517379.392] wl_display@1.delete_id(11)
[1517379.427] zwlr_screencopy_frame_v1@9.buffer(0, 1600, 1200, 6400)
2021/06/14 12:30:58 [TRACE] - wlroots: buffer event handler
2021/06/14 12:30:58 [TRACE] - pipewire: importing buffer
[1517379.503] zwlr_screencopy_frame_v1@9.linux_dmabuf(875713089, 1600, 1200)
2021/06/14 12:30:58 [TRACE] - wlroots: linux_dmabuf event handler
[1517379.551] zwlr_screencopy_frame_v1@9.buffer_done()
2021/06/14 12:30:58 [TRACE] - wlroots: buffer_done event handler
2021/06/14 12:30:58 [TRACE] - wlroots: shm buffer exists
[1517379.601]  -> zwlr_screencopy_frame_v1@9.copy_with_damage(wl_buffer@10)
2021/06/14 12:30:58 [TRACE] - wlroots: frame copied
2021/06/14 12:30:58 [TRACE] - event-loop: got pipewire event
2021/06/14 12:30:58 [TRACE] - pipewire: remove buffer event handle
[1517379.769]  -> wl_buffer@10.destroy()
2021/06/14 12:30:58 [TRACE] - pipewire: stream parameters changed
2021/06/14 12:30:58 [TRACE] - event-loop: got wayland event
[1517379.872] wl_display@1.error(zwlr_screencopy_frame_v1@9, 1, "invalid buffer stride")
zwlr_screencopy_frame_v1@9: error 1: invalid buffer stride
2021/06/14 12:30:58 [ERROR] - wl_display_dispatch failed: protocol error

=================================================================
==23795==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 320 byte(s) in 4 object(s) allocated from:
    #0 0x7f5c59d66459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f5c59ca3afb in wl_proxy_marshal_array_constructor_versioned (/usr/lib/libwayland-client.so.0+0x6afb)

Direct leak of 125 byte(s) in 5 object(s) allocated from:
    #0 0x7f5c59d0b3f9 in __interceptor_strdup /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:454
    #1 0x5604fe5b2da5 in wlr_output_handle_geometry ../src/screencast/wlr_screencast.c:208
    #2 0x7f5c595a5acc  (/usr/lib/libffi.so.7+0x6acc)

Still a use after free.

[emersion]

Interesting, there may be a use-after-free in wlroots here. Can you run the compositor with ASan enabled? Maybe print what the stride values are inside wlroots?

[columbarius]

It has to be a use after free, because we destroy the buffer, while copy with damage is in progress. You are sure wlroots can/should handle that? (I'm asking, because that looks like sth. you should never do. Handing out a buffer to someone else and destroying while the other one is processing it.)

[1517379.601]  -> zwlr_screencopy_frame_v1@9.copy_with_damage(wl_buffer@10) <----
2021/06/14 12:30:58 [TRACE] - wlroots: frame copied
2021/06/14 12:30:58 [TRACE] - event-loop: got pipewire event
2021/06/14 12:30:58 [TRACE] - pipewire: remove buffer event handle
[1517379.769]  -> wl_buffer@10.destroy() <-----
2021/06/14 12:30:58 [TRACE] - pipewire: stream parameters changed
2021/06/14 12:30:58 [TRACE] - event-loop: got wayland event
[1517379.872] wl_display@1.error(zwlr_screencopy_frame_v1@9, 1, "invalid buffer stride") <----
zwlr_screencopy_frame_v1@9: error 1: invalid buffer stride
2021/06/14 12:30:58 [ERROR] - wl_display_dispatch failed: protocol error

Interesting, there may be a use-after-free in wlroots here. Can you run the compositor with ASan enabled? Maybe print what the stride values are inside wlroots?

you mean compiling wlroots and sway with asan?

[emersion]

You are sure wlroots can/should handle that?

Yeah, for instance it's valid for a client to wl_surface.attach a buffer and immediately destroy it after.

you mean compiling wlroots and sway with asan?

Yes. https://github.com/swaywm/sway/wiki/Development-Setup#compiling-as-a-subproject

[columbarius]

Ok, i printed the stride of the imported buffer in wlroots and they where different.

I'm going forward with renaming the xdpw_frame to wlr_screencopy_frame (which will only hold the informations given by the wlr_screencopy protocol) and adding a "new" xdpw_frame, which holds the informations about the imported buffer from pipewire. This way we can check if the imported buffer is compatible with the bufferinformation advertised by wlroots. I somehow wanted to postpone this step until we need it for dmabuf screensharing, but it is not worth it.

[danshick]

Thanks for cleaning up my poor naming conventions.

[columbarius]

Thanks for cleaning up my poor naming conventions.

Don't get your hopes too high up. I introduced a new xdpw_frame called simple_frame which is used in the interaction with pipewire, but also holds a wlr_buffer (that's why i didn't called it pipewire frame). It's currently too hot at my place for creativity, but i'm open to suggestions.

[columbarius]

Resolved those.

---

I'm having problems with the fps limter while cleaning the stuff up. I will deactiveate before the flow change and activate it after wards. We might fix that later, or just ignore it in the intermediate commits.

[danshick]

I don't wanna break master, but in this PR is fine.

[columbarius]

I cleaned the patches up and restructured it a bit, so the steps are smaller and I think we are close to review. Screencast should work at every commit, renegotiation will be broken on some.

[danshick]

To be more explicit, I'm fine with broken commits on master for history preservation if they've never been at HEAD in a practical sense. Not sure if @emersion agrees, but if at rebase time, HEAD contains no regressions, I'm happy.

[columbarius]

I'm using this and it seems to work fine. Also adding dmabuf supported doesn't look like we would need changes in the flow, so I'm marking this as ready for review.

[danshick]

... so I'm marking this as ready for review.

I'll build and use this branch today.

[emersion]

In "screencast: pipewire remove uneeded PW_STREAM_FLAG_MAP_BUFFERS flag": why is this flag unneeded?

[emersion]

I've looked at the commits from "screencast: add logging to pwr_handle_stream_param_changed in pipewire_sreencast.c" to "screencast: pipewire change to self allocation of shm buffers".

[columbarius]

In "screencast: pipewire remove uneeded PW_STREAM_FLAG_MAP_BUFFERS flag": why is this flag unneeded?

Maybe unneeded is the wrong word. A commit later do we map our buffers directly after allocating it. PW_STREAM_FLAG_MAP_BUFFERS indicates, that pipewire should mmap any buffer to d[0].data which we don't need then anymore. I think the best solution would be to just squash those two commits together.

[columbarius]

Thanks for the review!

[emersion]

Hm, it seems like I missed #61 and I've reviewed here commits that belong to that other PR. It doesn't seem like #61 has the updated commits. Can you maybe replace #61's commits with the first 5 commits of this PR?

[columbarius]

I think i just did that right now

[Hubro]

As requested by columbarius, here is the TRACE output from xdg-desktop-portal-wlr crashing as soon as I try to start capturing:

➜ ./build/xdg-desktop-portal-wlr -r -l TRACE
2021/07/06 00:48:25 [TRACE] - config: trying config file /home/tomas/.config/xdg-desktop-portal-wlr/sway
2021/07/06 00:48:25 [TRACE] - config: trying config file /home/tomas/.config/xdg-desktop-portal-wlr/config
2021/07/06 00:48:25 [TRACE] - config: trying config file /usr/local/etc/xdg/xdg-desktop-portal-wlr/sway
2021/07/06 00:48:25 [TRACE] - config: trying config file /usr/local/etc/xdg/xdg-desktop-portal-wlr/config
2021/07/06 00:48:25 [ERROR] - config: no config file found
2021/07/06 00:48:25 [DEBUG] - config: outputname:  (null)
2021/07/06 00:48:25 [DEBUG] - config: max_fps:  0.000000
2021/07/06 00:48:25 [DEBUG] - config: exec_before:  (null)
2021/07/06 00:48:25 [DEBUG] - config: exec_after:  (null)
2021/07/06 00:48:25 [DEBUG] - config: chooser_cmd: (null)
2021/07/06 00:48:25 [DEBUG] - config: chooser_type: default
2021/07/06 00:48:25 [DEBUG] - dbus: connected
2021/07/06 00:48:25 [DEBUG] - wlroots: wl_display connected
2021/07/06 00:48:25 [DEBUG] - pipewire: pw_loop created
2021/07/06 00:48:25 [DEBUG] - pipewire: establishing connection to core
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_shm  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: |-- registered to interface wl_shm (Version 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_drm  (Version: 2)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_linux_dmabuf_v1  (Version: 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_compositor  (Version: 4)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_subcompositor  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_data_device_manager  (Version: 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_gamma_control_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zxdg_output_manager_v1  (Version: 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: |-- registered to interface zxdg_output_manager_v1 (Version 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register org_kde_kwin_idle  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_idle_inhibit_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_layer_shell_v1  (Version: 4)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register xdg_wm_base  (Version: 2)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_tablet_manager_v2  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register org_kde_kwin_server_decoration_manager  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zxdg_decoration_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_relative_pointer_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_pointer_constraints_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wp_presentation  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_output_manager_v1  (Version: 2)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_output_power_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_input_method_manager_v2  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_text_input_manager_v3  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_foreign_toplevel_manager_v1  (Version: 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_export_dmabuf_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_screencopy_manager_v1  (Version: 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: |-- registered to interface zwlr_screencopy_manager_v1 (Version 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_data_control_manager_v1  (Version: 2)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_primary_selection_device_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wp_viewporter  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zxdg_exporter_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zxdg_importer_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zxdg_exporter_v2  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zxdg_importer_v2  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_virtual_keyboard_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_virtual_pointer_manager_v1  (Version: 2)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwlr_input_inhibit_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_keyboard_shortcuts_inhibit_manager_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_seat  (Version: 7)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register zwp_pointer_gestures_v1  (Version: 1)
2021/07/06 00:48:25 [DEBUG] - wlroots: interface to register wl_output  (Version: 3)
2021/07/06 00:48:25 [DEBUG] - wlroots: |-- registered to interface wl_output (Version 1)
2021/07/06 00:48:25 [DEBUG] - wayland: registry listeners run
2021/07/06 00:48:25 [DEBUG] - wayland: xdg output listeners run
2021/07/06 00:48:25 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:25 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:25 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:25 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:29 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:31 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:31 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:31 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:31 [INFO] - dbus: create session method invoked
2021/07/06 00:48:31 [INFO] - dbus: request_handle: /org/freedesktop/portal/desktop/request/1_2541/obs1
2021/07/06 00:48:31 [INFO] - dbus: session_handle: /org/freedesktop/portal/desktop/session/1_2541/obs1
2021/07/06 00:48:31 [INFO] - dbus: app_id:
2021/07/06 00:48:31 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:31 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:31 [INFO] - dbus: select sources method invoked
2021/07/06 00:48:31 [INFO] - dbus: request_handle: /org/freedesktop/portal/desktop/request/1_2541/obs2
2021/07/06 00:48:31 [INFO] - dbus: session_handle: /org/freedesktop/portal/desktop/session/1_2541/obs1
2021/07/06 00:48:31 [INFO] - dbus: app_id:
2021/07/06 00:48:31 [INFO] - dbus: option types:1
2021/07/06 00:48:31 [INFO] - dbus: option multiple: 0
2021/07/06 00:48:31 [INFO] - dbus: option cursor_mode:2
2021/07/06 00:48:31 [DEBUG] - dbus: select sources: found matching session /org/freedesktop/portal/desktop/session/1_2541/obs1
2021/07/06 00:48:31 [INFO] - wlroots: capturable output: Samsung Electric Company model: LC49G95T: id: 43 name: DP-1
2021/07/06 00:48:31 [DEBUG] - wlroots: output chooser called
2021/07/06 00:48:31 [DEBUG] - wlroots: output chooser called
2021/07/06 00:48:31 [TRACE] - exec chooser called: cmd slurp -f %o -or, pipe chooser_in (14,15), pipe chooser_out (16,17)
2021/07/06 00:48:32 [TRACE] - wlroots: output chooser slurp -f %o -or selects output DP-1
2021/07/06 00:48:32 [DEBUG] - wlroots: output chooser selects DP-1
2021/07/06 00:48:32 [INFO] - xdpw: screencast instance 0x5613246055f0 has 1 references
2021/07/06 00:48:32 [INFO] - xdpw: 1 active screencast instances
2021/07/06 00:48:32 [INFO] - wlroots: output: DP-1
2021/07/06 00:48:32 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:32 [TRACE] - event-loop: got dbus event
2021/07/06 00:48:32 [INFO] - dbus: start method invoked
2021/07/06 00:48:32 [INFO] - dbus: request_handle: /org/freedesktop/portal/desktop/request/1_2541/obs3
2021/07/06 00:48:32 [INFO] - dbus: session_handle: /org/freedesktop/portal/desktop/session/1_2541/obs1
2021/07/06 00:48:32 [INFO] - dbus: app_id:
2021/07/06 00:48:32 [INFO] - dbus: parent_window:
2021/07/06 00:48:32 [DEBUG] - dbus: start: found matching session /org/freedesktop/portal/desktop/session/1_2541/obs1
2021/07/06 00:48:32 [TRACE] - wlroots: callbacks registered
2021/07/06 00:48:32 [TRACE] - wlroots: buffer event handler
2021/07/06 00:48:32 [TRACE] - wlroots: linux_dmabuf event handler
2021/07/06 00:48:32 [TRACE] - wlroots: buffer_done event handler
2021/07/06 00:48:32 [WARN] - wlroots: failed to import buffer
2021/07/06 00:48:32 [TRACE] - wlroots: frame destroyed
2021/07/06 00:48:32 [INFO] - pipewire: stream state changed to "connecting"
2021/07/06 00:48:32 [INFO] - pipewire: node id is -1
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [INFO] - pipewire: stream state changed to "paused"
2021/07/06 00:48:32 [INFO] - pipewire: node id is 42
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - pipewire: stream parameters changed
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - pipewire: add buffer event handle
2021/07/06 00:48:32 [TRACE] - pipewire: selected buffertype 2
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [INFO] - pipewire: stream state changed to "streaming"
2021/07/06 00:48:32 [INFO] - pipewire: node id is 42
2021/07/06 00:48:32 [TRACE] - wlroots: callbacks registered
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - event-loop: got wayland event
2021/07/06 00:48:32 [TRACE] - wlroots: buffer event handler
2021/07/06 00:48:32 [TRACE] - wlroots: linux_dmabuf event handler
2021/07/06 00:48:32 [TRACE] - wlroots: buffer_done event handler
2021/07/06 00:48:32 [TRACE] - pipewire: importing buffer
2021/07/06 00:48:32 [DEBUG] - wlroots: pipewire and wlroots metadata are incompatible. Renegotiate stream
2021/07/06 00:48:32 [TRACE] - pipewire: exporting buffer
2021/07/06 00:48:32 [TRACE] - ********************
2021/07/06 00:48:32 [TRACE] - pipewire: pointer 0x56132460ab80
2021/07/06 00:48:32 [TRACE] - pipewire: size 29491200
2021/07/06 00:48:32 [TRACE] - pipewire: stride 20480
2021/07/06 00:48:32 [TRACE] - pipewire: width 5120
2021/07/06 00:48:32 [TRACE] - pipewire: height 1440
2021/07/06 00:48:32 [TRACE] - pipewire: y_invert 0
2021/07/06 00:48:32 [TRACE] - pipewire: damage 0,0 (0x0)
2021/07/06 00:48:32 [TRACE] - pipewire: crop 0,0 (0x0)
2021/07/06 00:48:32 [TRACE] - ********************
2021/07/06 00:48:32 [TRACE] - pipewire: stream update parameters
2021/07/06 00:48:32 [TRACE] - wlroots: frame destroyed
xdg-desktop-portal-wlr: ../src/screencast/fps_limit.c:27: fps_limit_measure_end: Assertion `!timespec_is_zero(&state->frame_last_time)' failed.
zsh: abort (core dumped)  ./build/xdg-desktop-portal-wlr -r -l TRACE

[columbarius]

Fixed the pipewire metadata check, so we don't renegotiate when the alpha is striped and i'm only using the fps limiter if a frame was copied before (which means that the measurement was started). We probably want to check for a more reasonable logic here, see https://github.com/emersion/xdg-desktop-portal-wlr/pull/141/commits/a4485f00ed5d994d4836ab8043ea0d51a04656b4.

[columbarius]

@Hubro thanks for your trace. What confuses me is, why the xdpw wants to renegotiate the stream on the first frame:

2021/07/06 00:48:32 [INFO] - pipewire: stream state changed to "streaming"
2021/07/06 00:48:32 [INFO] - pipewire: node id is 42
2021/07/06 00:48:32 [TRACE] - wlroots: callbacks registered
2021/07/06 00:48:32 [TRACE] - event-loop: got pipewire event
2021/07/06 00:48:32 [TRACE] - event-loop: got wayland event
2021/07/06 00:48:32 [TRACE] - wlroots: buffer event handler
2021/07/06 00:48:32 [TRACE] - wlroots: linux_dmabuf event handler
2021/07/06 00:48:32 [TRACE] - wlroots: buffer_done event handler
2021/07/06 00:48:32 [TRACE] - pipewire: importing buffer
2021/07/06 00:48:32 [DEBUG] - wlroots: pipewire and wlroots metadata are incompatible. Renegotiate stream
2021/07/06 00:48:32 [TRACE] - pipewire: exporting buffer
2021/07/06 00:48:32 [TRACE] - ********************

We do announce the format together with the corresponding format without alpha because of a webrtc issue, but obs should be able to handle the version with alpha afaicr. The screencast fails, because it's leaving the loop before the first frame was copied so the fps_limiter was not engaged and fails when it should stop measuring. I added a check if it was engaged so we should not trigger that assume. Could you please try #156 again?

[columbarius]

@emersion @jbeich Is the build failing inside spa (pipewire) on our side or a freebsd issue?

[emersion]

This seems like a FreeBSD issue…

[jbeich]

This seems like a FreeBSD issue…

See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256970

[emersion]

Ah, thanks! (Though it's quite ironic that trying to be portable brings us portability issues…)

[columbarius]

thanks

[columbarius]

improved commit messages

[columbarius]

Thanks for the review!

Fixed the style issues, but didn't added all asserts, since importing a buffer from pipewire before requesting the one from wlroots (fxing #122 ) will change that. Note: The last commit is not finished, we don't receive any pipewire events about new buffers.

[columbarius]

Added both an event and the process callback

[columbarius]

@emersion @danshick may one of you check what i did wrong in comparison with https://github.com/Plagman/gamescope/commit/ef78e2e2707a75b6660809445fab578959a1548e?

Additionally there is a new pw_stream_trigger_process method previously named _drive which could be usefull for that.

[columbarius]

using pipewire events works pretty good until we can use pw_stream_drive.

[columbarius]

Reworked the wlroots screencopy loop, to have a clear start and end point, where the frame is created and the pipewire buffer is de and enqueued. Does this looks more reasonable?

[columbarius]

Updated the MR again to use the pipewire process callback to start the stream.

Ready for a new review round from my side. We probably want to squash some commits if the reworked loop is fine.

[emersion]

Commits up to "screencast: only end the fps measurement when it was started before" look good to me apart from the style issues and nits.

[columbarius]

Rebased onto master, cleaned up the nits and style, removed transforming y-inverted buffers, added dropping the current buffer on remove_buffer and fixed the loop on trigger_process for PipeWire and the frame_state for screencopy.