linux-capture: proper DMA-BUF capability negotiation via PipeWire #5131

columbarius · 2021-08-16T02:03:50Z

Description

This MR implements the negotiation for sharing of DMA-BUFs via PipeWire as described in https://docs.pipewire.org/page_dma_buf.html.
This MR follows up on #5221 by adding capabilities to query all modifiers supported by the renderer.
This query was implemented for EGL based renderers.

Motivation and Context

For proper DMA-BUF capability negotiation it is important, that both clients announce there capabilities by the means of a list of supported modifiers for a given format. Since PipeWire 0.3.33 we have the possibility to announce such a list per format and assure that it will not be mixed up with the capabilities of SHM buffer transport, which is distinguished by not announcing the modifier key.
This MR is required for obs to be able to use DMA-BUF transport via PipeWire with Wayland compositors after the mentioned MRs in the tested section have been merged.

How Has This Been Tested?

This can/will (be able to) be tested with:

the next kwin release containing https://invent.kde.org/plasma/kwin/-/merge_requests/1210
a mutter build containing https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1939
a build of xdg-desktop-portal-wlr containing Dmabuf modifierless emersion/xdg-desktop-portal-wlr#159

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Documentation (a change to documentation pages)

Checklist:

My code has been run through clang-format.
I have read the contributing document.
My code is not on the master branch.
The code has been tested.
All commit messages are properly formatted and commits squashed where appropriate.
I have included updates to all appropriate documentation.

Requires: #5221
Requires: #5585
Requires: PipeWire 0.3.33
Split into 3 Feature sets wrt. PipeWire version:

x < 0.3.24: SHM only
0.3.24 <= x < 0.3.40: Old style Dma-Buf negotiation
x >= 0.3.40: Proper modifier negotiation

plugins/linux-capture/pipewire.c

columbarius · 2021-09-16T22:11:04Z

Updated this MR with simpler EGL extension checks and dropping modifiers when we can't import them (issue: PipeWire won't trigger renegotiation on the compositor side, test https://github.com/columbarius/obs-studio/tree/egl-modifiers-vN-failure)

columbarius · 2021-09-17T21:42:00Z

Removed the last commit, since removing a EnumFormat didn't retrigger the format negotiation. This is expected to work as good as pre MR, since the importing wasn't touched, while negotiation from our side is more correct.

Should I open a pipewire-dmabuf-next MR Draft, or just link the development branch?

columbarius · 2021-09-17T22:12:25Z

cc: @GeorgesStavracas
Would be nice to have a flatpak build of this branch for easier testing. Tried it, but the build failed somewhere unrelated to this changes. See flathub/com.obsproject.Studio#148

columbarius · 2021-09-19T13:42:29Z

Updated the flatpak build and now it worked. This MR can be tested with flathub/com.obsproject.Studio#148

columbarius · 2021-11-13T11:19:45Z

Rebased onto #5394 including some cleanup commits from @GeorgesStavracas.
I included them especially for 1b2a715.

aleixpol · 2021-11-16T17:27:28Z

+1 Tested it with Plasma 5.23 and it's working seemingly okay and using the right path (unlike stable which is not going through dmabuf because of changes in pipewire handling of dmabuf).

jadahl · 2021-11-26T10:56:38Z

libobs-opengl/gl-egl-common.c

+		query_dmabuf_modifiers(egl_display, drm_format, &modifier_list);
+
+	brealloc(modifier_list, (num_modifiers + 1) * sizeof(uint64_t));
+	modifier_list[num_modifiers] = DRM_FORMAT_MOD_INVALID;


This feels somewhat like a thing that should be handled in another layer, e.g. in build_format() in linux-capture: Announce supported modifiers via PipeWire.

It can't be done in build_format(). The plugin doesn't have any clue which renderer is used. If obs would add a vulkan renderer in the future adding the token for implicit modifier would be false. So we either add it at the end of the list returned by query_modifiers or we need another call to the renderer to query if implicit modifiers are supported.

My personal choice would be to let the renderer tell whether implicit modifiers are supported, so that the PipeWire bits can deal with this particularity.

I tried minimizing the change in the renderer API, but I'm totally fine with adding a separate call returning a bool for implicit modifier supported, keeping the query function modifier aware only.

Maybe best consult maintainers, I'm just a fly-by reviewer here :P

Generally I would prefer we export with the same semantics as the egl/gl functions we are calling unless we have a good reason to modify them for keeping state consistent in libobs. For these platform specific bits I think theres even more reason to not introduce our own semantics.

While it seems convenient to build the whole list of formats and their modifiers, its probably safest to simply export the QueryFormat and QueryModifier functions separately on the graphics subsystem and build these lists in the pipewire code.

Ok so I'll design the API to be close to the egl functions and add additional bool functions to check for available extensions.

Do we need to provide a way to query extensions? If we return no modifiers when obs detects the extensions are not available then users can fallback to modifierless gracefully without the additional check? Or is there still a need to know if the driver supports the modifier extensions?

Support for implicit modifiers in EGL is defined by the existence of an extension. But I think a bool function _implicit_modifier_available would be better, checking that required extension internally.

Is the new API with 3 functions close enough to the EGL functions or do you want the same signature?

kkartaltepe · 2021-11-26T23:35:41Z

Which commits are specific to this PR? At this point there are so many prs for this in flight im not sure what is associated with what.

kkartaltepe · 2021-11-26T22:51:22Z

plugins/linux-pipewire/pipewire.c

-	uint8_t params_buffer[1024];
+	const struct spa_pod **params = NULL;
+	uint32_t n_params;
+	uint8_t params_buffer[2048];


How many format x modifiers can we fit in this?

plugins/linux-pipewire/pipewire.c

libobs-opengl/gl-egl-common.c

kkartaltepe · 2021-11-26T23:26:59Z

libobs-opengl/gl-egl-common.c

+		query_dmabuf_modifiers(egl_display, drm_format, &modifier_list);
+
+	brealloc(modifier_list, (num_modifiers + 1) * sizeof(uint64_t));
+	modifier_list[num_modifiers] = DRM_FORMAT_MOD_INVALID;


Looking at the previous commits, it seems that modifierless is already handled in the pipewire code separately as suggested. Is the point here to try and merge the modifierless and modifierfull code paths? If so we should remove the modifierless checks in the pipewire code.

libobs-opengl/gl-egl-common.c

GeorgesStavracas

Some more comments. This is shaping up nicely!

libobs-opengl/gl-egl-common.c

plugins/linux-capture/pipewire.c

columbarius · 2021-12-21T12:36:01Z

Note: dbus decided to strike today and every screencast request is ended by a Message recipient disconnected from message bus without replying while to portal implementation doesn't shows any error (The issue might be, that i've installed xdg-desktop-portal-git to get rid of the filemanager deadlock)...
Anyway NOT TESTED

GeorgesStavracas

One more round

libobs-opengl/gl-egl-common.c

plugins/linux-capture/pipewire.c

GeorgesStavracas

There is one question / comment, the rest is just nitpicking.

plugins/linux-capture/pipewire.c

GeorgesStavracas · 2021-12-22T01:25:25Z

One question: with this pull request, what would happen if the host system has PipeWire 0.3.40, but the compositor still negotiates DMA-BUFs the "old" way (i.e. without explicit / implicit modifiers)?

plugins/linux-capture/pipewire.c

columbarius · 2021-12-22T08:53:40Z

One question: with this pull request, what would happen if the host system has PipeWire 0.3.40, but the compositor still negotiates DMA-BUFs the "old" way (i.e. without explicit / implicit modifiers)?

The old way should still work as before. I removed the strict checking at some point and I would like to reintroduce it again and force everyone to do it correctly (insert evil laughter), but unless it creates problems, we should just announce both. (See https://github.com/obsproject/obs-studio/pull/5131/files#diff-ede22eaf772c418b60991eed2e7a76f8dcb41ee8400f188d8ca1cb5fb0de7a6fR783)

It was naive to add these defines here to avoid a direct include when the entire platform already depends on the DRM subsystem. Just include it and let it provide the image formats.

When sharing DMA-BUFs it is required the announce the underlying hardware capabilities via supported modifiers. Add new device_query_dmabuf_capabilities vfunc to gs_exports and connect it to the egl implementation stubs in the supported render platforms. Add a new public method gs_query_dmabuf_capabilities() that calls the vfunc above. Add new device_query_dmabuf_modifiers vfunc to gs_exports and connect it to the egl implementation in the supported render platforms. Add a new public method gs_query_dmabuf_modifiers() that calls the vfunc above.

Implement device_query_dmabuf_capabilities and device_query_dmabuf_modifiers for EGL/Wayland and EGL/X11.

We require PipeWire 0.3.33 or later to make use of the introduces flags `SPA_POD_PROP_FLAG_MANDATORY` and `SPA_POD_PROP_FLAG_DONT_FIXATE`, which are required for the negotiation process introduced in the following commits.

Sharing DMA-BUFs via PipeWire requires the client to announce all formats together with their supported modifiers. [1] [1] https://docs.pipewire.org/page_dma_buf.html

We require PipeWire 0.3.40 or later in preparation for proper DMA-BUF handling, which includes negotiation of modifier lists and dropping of single modifiers. This commit should be reverted as soon as the Freedesktop Flatpak SDK contains this or a newer PipeWire version.

Importing a DMA-BUF can fail even if the renderer announces support for the used format modifier pair. This can be caused by a number of reasons specific to the underlying hardware and api [1]. In that case we want to remove that modifier from our list of supported ones and renegotiate. This feature was added in PipeWire 0.3.40. On previous versions the best we can do is drop all modifiers an fallback to DMA-BUF with another format or directly to the SHM buffer transport. This mechanism is demonstrated in [2]. [1] https://xdc2020.x.org/event/9/contributions/615/attachments/704/1301/XDC_2020__Allocation_Constraints.pdf [2] https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/src/examples/video-play-fixate.c

Proper DMA-BUF format and modifier negotiation is possible with PipeWire 0.3.40. This commit adds checks for older versions and allows to build against them. These are classified as follows: * PipeWire server older than 0.3.24: Restrict to SHM only * PipeWire server between 0.3.24 (incl.) and 0.3.40: Announce modifiers along with the old method. On failed import drop all modifiers. * PipeWire server 0.3.40 and newer: Announce modifiers along with the old method. On failed import drop only a single modifier.

Ubuntu 21.10 provides PipeWire 0.3.32 which is missing the `SPA_POD_PROP_FLAG_DONT_FIXATE` required for proper DMA-BUF negotiation. Since this isn't implemented in the DE's of this Ubuntu version just defining this flag won't have any impact. Revert after support for Ubuntu 21.10 ended.

GeorgesStavracas

Took the liberty to push a small tweak, and I think this is good to go now

columbarius · 2021-12-22T16:19:44Z

Took the liberty to push a small tweak, and I think this is good to go now

Fine with that!

We are currently using the PipeWire default value and it seamed to work, so the thing underneath might just behave fine.

One last thing: could you please check what the "default" value for the modifier (as received in the old negotiation style) is? https://github.com/obsproject/obs-studio/pull/5131/files#diff-ede22eaf772c418b60991eed2e7a76f8dcb41ee8400f188d8ca1cb5fb0de7a6fL572

For the old negotiation style we should force that value to be either linear or implicit modifier.

aleixpol · 2021-12-23T00:12:25Z

~~FWIW, just tried it and now it's not working on Plasma anymore. Has anything changed that we need to be aware of?~~

My bad, I see there was a stable xdp release and it's working fine 💪

columbarius force-pushed the egl-modifiers branch from c3b4375 to 3ca3f28 Compare August 16, 2021 02:12

WizardCM added Documentation Enhancement Improvement to existing functionality Linux Affects Linux labels Aug 16, 2021

columbarius force-pushed the egl-modifiers branch from 3ca3f28 to cb1a709 Compare August 30, 2021 12:16

This was referenced Sep 1, 2021

libobs-opengl: Load required EGL extensions #5221

Merged

Egl modifiers part 1 (implicit modifiers) #5121

Closed

kkartaltepe reviewed Sep 1, 2021

View reviewed changes

plugins/linux-capture/pipewire.c Outdated Show resolved Hide resolved

columbarius force-pushed the egl-modifiers branch 2 times, most recently from ff618eb to 43b0718 Compare September 16, 2021 21:46

columbarius force-pushed the egl-modifiers branch from 43b0718 to cc4f248 Compare September 16, 2021 22:16

columbarius marked this pull request as ready for review September 16, 2021 22:16

columbarius force-pushed the egl-modifiers branch from cc4f248 to cdcbf60 Compare September 17, 2021 21:33

columbarius force-pushed the egl-modifiers branch from cdcbf60 to fe5f6c6 Compare September 17, 2021 21:43

columbarius changed the title ~~Egl modifiers part 2 (explicit modifiers)~~ PipeWire: proper DMA-BUF capability negotiation Sep 17, 2021

columbarius force-pushed the egl-modifiers branch from fe5f6c6 to d1cb804 Compare September 18, 2021 21:44

WizardCM requested a review from GeorgesStavracas September 18, 2021 22:28

columbarius force-pushed the egl-modifiers branch from d1cb804 to e22bc69 Compare September 19, 2021 00:48

columbarius force-pushed the egl-modifiers branch from e22bc69 to b862a30 Compare October 14, 2021 19:12

columbarius force-pushed the egl-modifiers branch from b862a30 to 0264081 Compare November 13, 2021 00:50

columbarius mentioned this pull request Nov 25, 2021

Dmabuf modifierless emersion/xdg-desktop-portal-wlr#159

Merged

jadahl reviewed Nov 26, 2021

View reviewed changes

kkartaltepe reviewed Nov 26, 2021

View reviewed changes

columbarius force-pushed the egl-modifiers branch 2 times, most recently from c861537 to a059e50 Compare December 19, 2021 14:19

columbarius requested a review from GeorgesStavracas December 19, 2021 14:45

GeorgesStavracas requested changes Dec 20, 2021

View reviewed changes

columbarius force-pushed the egl-modifiers branch from a059e50 to 4997142 Compare December 21, 2021 12:28

GeorgesStavracas requested changes Dec 21, 2021

View reviewed changes

columbarius force-pushed the egl-modifiers branch from 4997142 to c3b702a Compare December 21, 2021 23:05

GeorgesStavracas requested changes Dec 22, 2021

View reviewed changes

plugins/linux-capture/pipewire.c Outdated Show resolved Hide resolved

plugins/linux-capture/pipewire.c Outdated Show resolved Hide resolved

plugins/linux-capture/pipewire.c Outdated Show resolved Hide resolved

plugins/linux-capture/pipewire.c Show resolved Hide resolved

GeorgesStavracas reviewed Dec 22, 2021

View reviewed changes

plugins/linux-capture/pipewire.c Outdated Show resolved Hide resolved

columbarius force-pushed the egl-modifiers branch from c3b702a to d029d53 Compare December 22, 2021 10:04

GeorgesStavracas and others added 9 commits December 22, 2021 12:44

linux-pipewire: Use DRM fourcc defines directly

4a7df82

It was naive to add these defines here to avoid a direct include when the entire platform already depends on the DRM subsystem. Just include it and let it provide the image formats.

libobs-opengl: Implement DMA-BUF query functions for EGL renderer

b174d26

Implement device_query_dmabuf_capabilities and device_query_dmabuf_modifiers for EGL/Wayland and EGL/X11.

linux-capture: Bump minimal PipeWire version to 0.3.33

0ac6be9

We require PipeWire 0.3.33 or later to make use of the introduces flags `SPA_POD_PROP_FLAG_MANDATORY` and `SPA_POD_PROP_FLAG_DONT_FIXATE`, which are required for the negotiation process introduced in the following commits.

linux-capture: Announce supported modifiers via PipeWire

292346d

Sharing DMA-BUFs via PipeWire requires the client to announce all formats together with their supported modifiers. [1] [1] https://docs.pipewire.org/page_dma_buf.html

GeorgesStavracas force-pushed the egl-modifiers branch from d029d53 to 4e587a0 Compare December 22, 2021 15:48

GeorgesStavracas approved these changes Dec 22, 2021

View reviewed changes

GeorgesStavracas merged commit 0532a5c into obsproject:master Dec 22, 2021

WizardCM added this to the OBS Studio 27.2 milestone Dec 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linux-capture: proper DMA-BUF capability negotiation via PipeWire #5131

linux-capture: proper DMA-BUF capability negotiation via PipeWire #5131

columbarius commented Aug 16, 2021 •

edited

Loading

columbarius commented Sep 16, 2021

columbarius commented Sep 17, 2021 •

edited

Loading

columbarius commented Sep 17, 2021 •

edited

Loading

columbarius commented Sep 19, 2021

columbarius commented Nov 13, 2021

aleixpol commented Nov 16, 2021

jadahl Nov 26, 2021

columbarius Nov 26, 2021

jadahl Nov 26, 2021

columbarius Nov 26, 2021

jadahl Nov 26, 2021

kkartaltepe Nov 27, 2021 •

edited

Loading

columbarius Nov 27, 2021

kkartaltepe Nov 27, 2021

columbarius Nov 27, 2021

columbarius Nov 27, 2021

kkartaltepe commented Nov 26, 2021

kkartaltepe Nov 26, 2021

kkartaltepe Nov 26, 2021

GeorgesStavracas left a comment

columbarius commented Dec 21, 2021

GeorgesStavracas left a comment

GeorgesStavracas left a comment

GeorgesStavracas commented Dec 22, 2021

columbarius commented Dec 22, 2021

GeorgesStavracas left a comment

columbarius commented Dec 22, 2021 •

edited

Loading

aleixpol commented Dec 23, 2021 •

edited

Loading

linux-capture: proper DMA-BUF capability negotiation via PipeWire #5131

linux-capture: proper DMA-BUF capability negotiation via PipeWire #5131

Conversation

columbarius commented Aug 16, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

columbarius commented Sep 16, 2021

columbarius commented Sep 17, 2021 • edited Loading

columbarius commented Sep 17, 2021 • edited Loading

columbarius commented Sep 19, 2021

columbarius commented Nov 13, 2021

aleixpol commented Nov 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkartaltepe Nov 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkartaltepe commented Nov 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeorgesStavracas left a comment

Choose a reason for hiding this comment

columbarius commented Dec 21, 2021

GeorgesStavracas left a comment

Choose a reason for hiding this comment

GeorgesStavracas left a comment

Choose a reason for hiding this comment

GeorgesStavracas commented Dec 22, 2021

columbarius commented Dec 22, 2021

GeorgesStavracas left a comment

Choose a reason for hiding this comment

columbarius commented Dec 22, 2021 • edited Loading

aleixpol commented Dec 23, 2021 • edited Loading

columbarius commented Aug 16, 2021 •

edited

Loading

columbarius commented Sep 17, 2021 •

edited

Loading

columbarius commented Sep 17, 2021 •

edited

Loading

kkartaltepe Nov 27, 2021 •

edited

Loading

columbarius commented Dec 22, 2021 •

edited

Loading

aleixpol commented Dec 23, 2021 •

edited

Loading