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

screencast: dmabuf support #9

Closed
danshick opened this issue Jan 15, 2020 · 19 comments · Fixed by #152
Closed

screencast: dmabuf support #9

danshick opened this issue Jan 15, 2020 · 19 comments · Fixed by #152
Labels
enhancement New feature or request

Comments

@danshick
Copy link
Collaborator

danshick commented Jan 15, 2020

Support passing a dmabuf fd to pipewire instead of using the screencopy protocol. There are three things pending before this can (or should) move forward.

  1. Waiting on Pipewire 0.3 to be released:
    https://gitlab.freedesktop.org/pipewire/pipewire

  2. Waiting on xdg-desktop-portal PR to support Pipewire 0.3:
    PipeWire: update to 0.3 API flatpak/xdg-desktop-portal#436

  3. Waiting on downstream ecosystem to support drm_fourcc modifiers:
    Discussion of downstream dependencies that need to support modifiers.
    resolved by newer screencopy protocol

Here is a very early, broken demo

@ghost
Copy link

ghost commented Feb 24, 2020

Pipewire 0.3 is out now. Hope that will help clear up the issues.

@danshick
Copy link
Collaborator Author

danshick commented Mar 1, 2020

That certainly means we can complete dmabuf style screensharing, but it'll be mostly useless unless and until downstream apps support various modifiers. Still worth building out, although it'll likely get hidden behind an argument.

@emersion emersion added the enhancement New feature or request label Mar 13, 2020
@danshick danshick changed the title dmabuf support for screencasting screencast: dmabuf support Mar 13, 2020
@danshick
Copy link
Collaborator Author

danshick commented Apr 1, 2020

Maybe we could use glBlitFramebuffer to copy frames on GPU before sharing. This could avoid the concern about format modifiers. I need to research and experiment with this approach further.

@emersion
Copy link
Owner

emersion commented Jun 8, 2020

swaywm/wlroots#2133 allows blitting DMA-BUFs on the compositor-side.

@danshick
Copy link
Collaborator Author

danshick commented Jun 8, 2020

I've been subscribed to this PR since any1 opened it. Looks like the example can be used almost verbatim for our use case as long as we tell pipewire to map a dmabuf that we provide. Do you think this should be implemented under a CLI flag?

I would say we could fall back gracefully on our side if the compositor doesn't support the protocol version, but I think older pipewire 0.2 consumers (WebRTC especially) will fail on dmabuf (or even memfd) sources. Not sure, would have to test it.

@emersion
Copy link
Owner

emersion commented Jun 8, 2020

Do you think this should be implemented under a CLI flag?

Ideally it should be enabled by default. We should offer support for both DMA-BUF and good old shm, and clients should pick one.

@danshick
Copy link
Collaborator Author

danshick commented Jun 8, 2020

I totally forgot that Mutter already does this (dmabuf producer for xdg-desktop-portal-gtk, consumed by WebRTC). Basically, this should work fine in all cases.

We can't let clients pick one because there is no provision in the portal API for that negotiation to happen, but luckily, I don't think we have to.

The only fallback that would be necessary is if the screencopy protocol version is insufficient on the compositor side.

@emersion
Copy link
Owner

FWIW, https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1442

I think GNOME also blits to LINEAR because all clients ignore the modifier hint and end up with garbage.

Pipewire is really missing a better API for this. Giving DMA-BUFs to a client expecting shm is not going to end well.

@columbarius
Copy link
Collaborator

I guess currently dmabuf support isn't fully implemented in pipewire at the moment. I guess if a client ca't work with a fd, pipewire will copy it to it's data pointer itself: https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/src/pipewire/stream.c#L548
It's done the same way for memfd and dmabuf without awareness of modifiers. I guess creating a separate path using gbm would probably solve some of the issues. Will ask at #pipewire for confirmation.

@danshick
Copy link
Collaborator Author

danshick commented Sep 16, 2020

I guess if a client ca't work with a fd, pipewire will copy it to it's data pointer itself: https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/src/pipewire/stream.c#L548

Pipewire always maps buffers for the client. It isn't a question of client support. The good news is that mutter being broken will definitely help to make the case for a better dmabuf handling story.

I was wrong, it usually is the client mapping the buffer. That makes this more difficult to fix.

@emersion
Copy link
Owner

emersion commented Oct 9, 2020

17:03 <emersion> wtay: fyi https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1442#note_930581
17:03 <emersion> i don't know if this is being worked on or anything
17:05 <@wtay> emersion, don't know
17:06 <emersion> can you elaborate?
17:09 <@wtay> a client can set a mask with supported types, if nothing is set (current situation) all is allowed (pipewire sets ~0 ad allowed types), mutter can check the masks and allocate only when the memory mask field is set
17:10 <@wtay> 1) clients need to set the supported types -> don't think any do that
17:10 <@wtay> 2) mutter checks the flags -> don't think it does that
17:11 <@wtay> I guess we first need to find the clients that consume the streams, fix them to set the mask and then use those in mutter
17:13 <emersion> wtay: can't the compositor detect that nothing is set, and use something that will likely work in this case? (no dmabufs)
17:13 <emersion> ie, remove the "pipewire sets ~0 ad allowed types" part
17:14 <emersion> that way, old clients still get shm, new clients that want dmabufs can indicate it
17:14 <@wtay> I think in that case it's better to check for ~0 than 0, no?
17:15 <emersion> in any case, the backwards compat story needs to be better handled
17:15 <emersion> eh
17:15 <@wtay> ~0 being explicitly that nothing was set where 0 is that nothing is possible
17:15 <emersion> so, can you guarantee that ~0 will never mean "everything is supported"?
17:16 <@wtay> I can do that
17:17 <@wtay> I guess it does not matter much 0 or ~0
17:21 <emersion> ok, thx
17:25 <@wtay> mutter should actually not set a dataType mask in the Buffer params or else it will not see the ~0, just its own mask filtered against the client mask
17:26 <emersion> hmmm
17:27 <emersion> can we just set the highest bit there?
17:27 <emersion> 1<<32 | whatever-i-support
17:27 <emersion> or something like that
17:28 <emersion> or is the compositor's dataType mask completely useless?
17:29 <@wtay> I think so, as an allocator you just want to get the client dataTypes and work from there
17:29 <emersion> ok, makes sense

@danshick
Copy link
Collaborator Author

danshick commented Oct 9, 2020

I'll need to look into the code regarding the dataType masks. It should be trivial to use the dmabuf extension to screencopy right now if we can rely on clients to handle it correctly.

As for export dmabuf, I'm guessing there could still be timing issues there if some downstream client is holding a frame fd for too long? I know that was discussed a while back. Was there ever a decision made about how wlroots handles those cases?

@columbarius
Copy link
Collaborator

Thanks. I guess we have to revert c2883a4.

Does ~0 means NULL?

I guess we need some logic here to select the correct buffer type/backend.

@danshick
Copy link
Collaborator Author

~ is the urinary operator for the bitwise compliment, so ~0 would be a binary value of all ones, which varies in length and value depending on the type.

@emersion
Copy link
Owner

As for export dmabuf, I'm guessing there could still be timing issues there if some downstream client is holding a frame fd for too long? I know that was discussed a while back. Was there ever a decision made about how wlroots handles those cases?

This is a theoretical issue, yes. I'd say let's just stick to screencopy for now and leave export-dmabuf for later.

@columbarius
Copy link
Collaborator

columbarius commented May 17, 2021

Some update to dmabufs in pipewire:

Exampleclients to test format merging:
https://gitlab.freedesktop.org/columbarius/pipewire-modifier-test

Obs branch to have a client to test on (WIP):
https://github.com/columbarius/obs-studio/tree/egl-modifiers

Found the funktion in pipewire responsible for merging the formats:
https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/src/pipewire/context.c#L724
The format_filter doesn't look used or implemented, maybe that's sth. to work on?

@columbarius
Copy link
Collaborator

When this is merged, we can reliably distinguish between dmabufs and MemPtr/MemFd

https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/683

This was referenced Jun 14, 2021
@columbarius
Copy link
Collaborator

Update:
https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/683
got merged into pipewire 0.3.29

@columbarius
Copy link
Collaborator

PipeWire 0.3.40 was released with full support for negotiating and dropping single modifiers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants