[GStreamer][GL] Enable DMABuf sink #10588

philn · 2023-02-23T15:59:20Z

`e962336`

[GStreamer][GL] Enable DMABuf sink
https://bugs.webkit.org/show_bug.cgi?id=252826

Reviewed by Žan Doberšek and Xabier Rodriguez-Calvar.

Unconditionally enable the DMABuf sink when running with GStreamer 1.22 and newer. The
WEBKIT_GST_DMABUF_SINK_DISABLED environment variable can be set to fallback to the GL sink.

* Source/WebCore/platform/graphics/gstreamer/DMABufVideoSinkGStreamer.cpp:
(webKitDMABufVideoSinkGetProperty):
(webKitDMABufVideoSinkIsEnabled):
(webKitDMABufVideoSinkProbePlatform):
* Tools/Scripts/webkitpy/port/glib.py:
(GLibPort.setup_environ_for_server):

Canonical link: https://commits.webkit.org/260881@main

bd1b6f2

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
✅ 🧪 webkitpy	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-02-23T16:13:23Z

EWS run on previous version of this PR (hash e9832c9)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 wincairo~~
~~🧪 bindings~~	~~🛠 ios-sim~~	~~🛠 mac-AS-debug~~	~~🛠 gtk~~
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 api-gtk~~
	~~🛠 tv~~	~~🧪 mac-wk2~~
	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~
	~~🛠 watch~~	~~🧪 mac-wk2-stress~~
	~~🛠 watch-sim~~

zdobersek · 2023-02-23T16:13:35Z

Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp

-            if (webKitDMABufVideoSinkIsEnabled() && webKitDMABufVideoSinkProbePlatform())
+            if (webKitDMABufVideoSinkProbePlatform())


These two conditions should match conditions in createVideoSinkDMABuf(). Otherwise things can get weird with using TextureMapperPlatformLayerProxyDMABuf along with a GL sink.

So I would recommend keeping webKitDMABufVideoSinkIsEnabled(), and using WEBKIT_GST_DISABLE_DMABUF_SINK env to disable it. That way behavior between DMABuf sink and GL sink can be examined by selectively disabling one or the other, as long as they coexist.

Makes sense! I'll update the PR, thanks for the review.

Maybe we can make this work with GST_PLUGIN_FEATURE_RANK=webkitdmabufsink:0 too. I can try that, I've been wondering lately about avoid new env vars when we can...

GST_PLUGIN_FEATURE_RANK works only during gst_init_check() so for our custom elements registered afterwards we'd need to re-parse the env var... Not sure it's worth.

webkit-early-warning-system · 2023-02-23T16:13:45Z

EWS run on previous version of this PR (hash 5a20129)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	❌ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

calvaris

Other than what @zdobersek said, I think we would be good to go.

webkit-early-warning-system · 2023-02-24T17:11:26Z

Starting EWS tests for 681cc1e. Live statuses available at the PR page, #10588

webkit-early-warning-system · 2023-02-24T17:11:27Z

EWS run on previous version of this PR (hash 681cc1e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 gtk-wk2
✅ 🧪 webkitpy	✅ 🧪 api-ios	✅ 🧪 mac-wk1	❌ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

philn · 2023-02-24T18:31:36Z

The test bots doesn't like this PR at all. Somehow the dmabuf sink is misbehaving there, is that supposed to work in XvFB and similar antique setups?

clopez · 2023-02-24T18:42:27Z

Check https://ews-build.webkit.org/#/builders/86/builds/389
The WPE EWS for layout tests (still on testing) have detected more than 500 crashes caused by this patch. Check here the crash logs: https://ews-build.s3-us-west-2.amazonaws.com/WPE-WK2-Tests-EWS/5a201291-389/results.html

And the GTK EWS for layout tests seems to be detecting something similar.
https://ews-build.webkit.org/#/builders/35/builds/43165

philn · 2023-02-24T19:15:28Z

Yeah I noticed the previous version of the patch was already flagged... Crash is in gbm_bo_create()... I guess those bots don't have whatever GBM requires, like a GPU?

clopez · 2023-02-24T19:25:14Z

Yeah I noticed the previous version of the patch was already flagged... Crash is in gbm_bo_create()... I guess those bots don't have whatever GBM requires, like a GPU?

That is right: the bots don't have a GPU. There are no /dev/dri/* devices on them.

philn · 2023-02-24T20:22:02Z

@zdobersek wdyt about making webKitDMABufVideoSinkProbePlatform() check /dev/dri/whatever char devices? Would it make sense?

clopez · 2023-02-24T23:36:37Z

@zdobersek wdyt about making webKitDMABufVideoSinkProbePlatform() check /dev/dri/whatever char devices? Would it make sense?

Sounds a bit hackish. Maybe an idea is checking that we pass a valid device when calling gbm_bo_create() at GBMBufferSwapchain.cpp ?

I guess (not checked) the call to drmGetDevices2() at GBMDevice.cpp is returning zero devices so device.device() returns nullptr causing the crash when gbm_bo_create() is called with nullptr as first argument

philn · 2023-02-25T09:00:23Z

We want to fallback to the GL sink when dmabuf sink is not usable, so doing the check in GBMBufferSwapchain seems a bit late, no?

webkit-early-warning-system · 2023-02-25T10:21:21Z

Starting EWS tests for bd1b6f2. Live statuses available at the PR page, #10588

webkit-early-warning-system · 2023-02-25T10:21:21Z

EWS run on current version of this PR (hash bd1b6f2)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
✅ 🧪 webkitpy	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge	✅ 🛠 watch-sim

clopez · 2023-02-25T12:08:06Z

bd1b6f2 worked 🎉 -> https://ews-build.webkit.org/#/builders/86/builds/590
Great solution!

https://bugs.webkit.org/show_bug.cgi?id=252826 Reviewed by Žan Doberšek and Xabier Rodriguez-Calvar. Unconditionally enable the DMABuf sink when running with GStreamer 1.22 and newer. The WEBKIT_GST_DMABUF_SINK_DISABLED environment variable can be set to fallback to the GL sink. * Source/WebCore/platform/graphics/gstreamer/DMABufVideoSinkGStreamer.cpp: (webKitDMABufVideoSinkGetProperty): (webKitDMABufVideoSinkIsEnabled): (webKitDMABufVideoSinkProbePlatform): * Tools/Scripts/webkitpy/port/glib.py: (GLibPort.setup_environ_for_server): Canonical link: https://commits.webkit.org/260881@main

webkit-commit-queue · 2023-02-27T14:24:23Z

Committed 260881@main (e962336): https://commits.webkit.org/260881@main

Reviewed commits have been landed. Closing PR #10588 and removing active labels.

philn self-assigned this Feb 23, 2023

philn added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label Feb 23, 2023

philn force-pushed the eng/252826 branch from e9832c9 to 5a20129 Compare February 23, 2023 16:00

philn requested review from magomez and calvaris February 23, 2023 16:05

zdobersek reviewed Feb 23, 2023

View reviewed changes

calvaris approved these changes Feb 24, 2023

View reviewed changes

philn force-pushed the eng/252826 branch from 5a20129 to 681cc1e Compare February 24, 2023 17:11

philn requested a review from JonWBedard as a code owner February 24, 2023 17:11

philn marked this pull request as draft February 24, 2023 17:25

clopez mentioned this pull request Feb 24, 2023

[WPE] Add EWS WPE WK2 layout tests status-bubbles to PRs #10606

Merged

philn force-pushed the eng/252826 branch from 681cc1e to bd1b6f2 Compare February 25, 2023 10:21

philn requested a review from zdobersek February 25, 2023 12:40

philn marked this pull request as ready for review February 25, 2023 12:40

zdobersek approved these changes Feb 27, 2023

View reviewed changes

philn added the merge-queue Applied to send a pull request to merge-queue label Feb 27, 2023

webkit-early-warning-system force-pushed the eng/252826 branch from bd1b6f2 to e962336 Compare February 27, 2023 14:24

webkit-early-warning-system merged commit e962336 into WebKit:main Feb 27, 2023

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 27, 2023

philn deleted the eng/252826 branch February 27, 2023 14:37

		if (webKitDMABufVideoSinkIsEnabled() && webKitDMABufVideoSinkProbePlatform())
		if (webKitDMABufVideoSinkProbePlatform())

[GStreamer][GL] Enable DMABuf sink #10588

[GStreamer][GL] Enable DMABuf sink #10588

Uh oh!

Conversation

philn commented Feb 23, 2023 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

e962336

Uh oh!

webkit-early-warning-system commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zdobersek Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

philn Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

philn Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

philn Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calvaris left a comment

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Feb 24, 2023

Uh oh!

webkit-early-warning-system commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philn commented Feb 24, 2023

Uh oh!

clopez commented Feb 24, 2023

Uh oh!

philn commented Feb 24, 2023

Uh oh!

clopez commented Feb 24, 2023

Uh oh!

philn commented Feb 24, 2023

Uh oh!

clopez commented Feb 24, 2023

Uh oh!

philn commented Feb 25, 2023

Uh oh!

webkit-early-warning-system commented Feb 25, 2023

Uh oh!

webkit-early-warning-system commented Feb 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clopez commented Feb 25, 2023

Uh oh!

webkit-commit-queue commented Feb 27, 2023

Uh oh!

Uh oh!

philn commented Feb 23, 2023 •

edited by webkit-early-warning-system

Loading

`e962336`

webkit-early-warning-system commented Feb 23, 2023 •

edited

Loading

webkit-early-warning-system commented Feb 23, 2023 •

edited

Loading

webkit-early-warning-system commented Feb 24, 2023 •

edited

Loading

webkit-early-warning-system commented Feb 25, 2023 •

edited

Loading