[GStreamer] Pipewire display capture broken

https://bugs.webkit.org/show_bug.cgi?id=239435 Patch by Philippe Normand <pnormand@igalia.com> on 2022-04-28 Reviewed by Xabier Rodriguez-Calvar. The pipewiresrc GStreamer element now needs both the `fd` and `path` properties set in order to open the right Pipewire stream, so we need to get the node ID from the portal Start response and pass it to the GStreamer element. * platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h: * platform/mediastream/gstreamer/GStreamerDisplayCaptureDeviceManager.cpp: (WebCore::GStreamerDisplayCaptureDeviceManager::createDisplayCaptureSource): (WebCore::GStreamerDisplayCaptureDeviceManager::waitResponseSignal): * platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp: (WebCore::GStreamerVideoCaptureSource::createPipewireSource): (WebCore::GStreamerVideoCaptureSource::GStreamerVideoCaptureSource): (WebCore::m_deviceType): (WebCore::GStreamerVideoCaptureSource::~GStreamerVideoCaptureSource): * platform/mediastream/gstreamer/GStreamerVideoCaptureSource.h: * platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp: (WebCore::GStreamerVideoCapturer::createSource): (WebCore::GStreamerVideoCapturer::setSize): (WebCore::GStreamerVideoCapturer::setFrameRate): * platform/mediastream/gstreamer/GStreamerVideoCapturer.h: Canonical link: https://commits.webkit.org/250094@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293580 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 28, 2022 · 034d5e94dd872e50e28155a67436b7cf6e1095af · 034d5e9
1 parent 28e2c3d
commit 034d5e94dd872e50e28155a67436b7cf6e1095af
Showing 7 changed files with 101 additions and 29 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2022-04-28  Philippe Normand  <philn@igalia.com>
+
+        [GStreamer] Pipewire display capture broken
+        https://bugs.webkit.org/show_bug.cgi?id=239435
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        The pipewiresrc GStreamer element now needs both the `fd` and `path` properties set in order
+        to open the right Pipewire stream, so we need to get the node ID from the portal Start
+        response and pass it to the GStreamer element.
+
+        * platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h:
+        * platform/mediastream/gstreamer/GStreamerDisplayCaptureDeviceManager.cpp:
+        (WebCore::GStreamerDisplayCaptureDeviceManager::createDisplayCaptureSource):
+        (WebCore::GStreamerDisplayCaptureDeviceManager::waitResponseSignal):
+        * platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:
+        (WebCore::GStreamerVideoCaptureSource::createPipewireSource):
+        (WebCore::GStreamerVideoCaptureSource::GStreamerVideoCaptureSource):
+        (WebCore::m_deviceType):
+        (WebCore::GStreamerVideoCaptureSource::~GStreamerVideoCaptureSource):
+        * platform/mediastream/gstreamer/GStreamerVideoCaptureSource.h:
+        * platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:
+        (WebCore::GStreamerVideoCapturer::createSource):
+        (WebCore::GStreamerVideoCapturer::setSize):
+        (WebCore::GStreamerVideoCapturer::setFrameRate):
+        * platform/mediastream/gstreamer/GStreamerVideoCapturer.h:
+
 2022-04-28  Simon Fraser  <simon.fraser@apple.com>
 
         DisplayList::Recorder should lazily create the DrawGlyphsRecorder

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h b/Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h
@@ -26,10 +26,13 @@
 #include "DisplayCaptureManager.h"
 #include "GRefPtrGStreamer.h"
 #include "GStreamerCaptureDevice.h"
+#include "GStreamerVideoCapturer.h"
 #include "RealtimeMediaSourceFactory.h"
 
 namespace WebCore {
 
+using NodeAndFD = GStreamerVideoCapturer::NodeAndFD;
+
 class GStreamerCaptureDeviceManager : public CaptureDeviceManager {
 public:
     ~GStreamerCaptureDeviceManager();
@@ -82,35 +85,37 @@ class GStreamerDisplayCaptureDeviceManager final : public DisplayCaptureManager
     void stopSource(const String& persistentID);
 
 protected:
-    void notifyResponse() { m_currentResponseCallback(); }
+    void notifyResponse(GVariant* parameters) { m_currentResponseCallback(parameters); }
 
 private:
     GStreamerDisplayCaptureDeviceManager();
     ~GStreamerDisplayCaptureDeviceManager();
 
-    void waitResponseSignal(const char* objectPath);
+    using ResponseCallback = CompletionHandler<void(GVariant*)>;
+
+    void waitResponseSignal(const char* objectPath, ResponseCallback&& = [](GVariant*) { });
 
     Vector<CaptureDevice> m_devices;
 
     struct Session {
         WTF_MAKE_STRUCT_FAST_ALLOCATED;
         WTF_MAKE_NONCOPYABLE(Session);
-        Session(int fd, String&& path)
-            : fd(fd)
+        Session(const NodeAndFD& nodeAndFd, String&& path)
+            : nodeAndFd(nodeAndFd)
             , path(WTFMove(path)) { }
 
         ~Session()
         {
-            close(fd);
+            close(nodeAndFd.second);
         }
 
-        int fd;
+        NodeAndFD nodeAndFd;
         String path;
     };
     HashMap<String, std::unique_ptr<Session>> m_sessions;
 
     GRefPtr<GDBusProxy> m_proxy;
-    CompletionHandler<void()> m_currentResponseCallback;
+    ResponseCallback m_currentResponseCallback;
 };
 }
 

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerDisplayCaptureDeviceManager.cpp b/Source/WebCore/platform/mediastream/gstreamer/GStreamerDisplayCaptureDeviceManager.cpp
@@ -64,7 +64,7 @@ CaptureSourceOrError GStreamerDisplayCaptureDeviceManager::createDisplayCaptureS
     const auto it = m_sessions.find(device.persistentId());
     if (it != m_sessions.end()) {
         return GStreamerVideoCaptureSource::createPipewireSource(device.persistentId().isolatedCopy(),
-            it->value->fd, WTFMove(hashSalt), constraints, device.type());
+            it->value->nodeAndFd, WTFMove(hashSalt), constraints, device.type());
     }
 
     GUniqueOutPtr<GError> error;
@@ -138,8 +138,38 @@ CaptureSourceOrError GStreamerDisplayCaptureDeviceManager::createDisplayCaptureS
         return { };
     }
 
+    std::optional<uint32_t> nodeId;
     g_variant_get(result.get(), "(o)", &objectPath.outPtr());
-    waitResponseSignal(objectPath.get());
+    waitResponseSignal(objectPath.get(), [&nodeId](GVariant* parameters) mutable {
+        uint32_t portalResponse;
+        GRefPtr<GVariant> responseData;
+        g_variant_get(parameters, "(u@a{sv})", &portalResponse, &responseData.outPtr());
+
+        if (portalResponse) {
+            WTFLogAlways("User cancelled the Start request or an unknown error happened");
+            return;
+        }
+
+        // The portal interface allows multiple streams but we care only about the first one.
+        GUniqueOutPtr<GVariantIter> iter;
+        if (g_variant_lookup(responseData.get(), "streams", "a(ua{sv})", &iter.outPtr())) {
+            auto variant = adoptGRef(g_variant_iter_next_value(iter.get()));
+            if (!variant) {
+                WTFLogAlways("Stream list is empty");
+                return;
+            }
+
+            uint32_t streamId;
+            GRefPtr<GVariant> options;
+            g_variant_get(variant.get(), "(u@a{sv})", &streamId, &options.outPtr());
+            nodeId = streamId;
+        }
+    });
+
+    if (!nodeId) {
+        WTFLogAlways("Unable to retrieve display capture session data");
+        return { };
+    }
 
     GRefPtr<GUnixFDList> fdList;
     int fd = -1;
@@ -155,9 +185,10 @@ CaptureSourceOrError GStreamerDisplayCaptureDeviceManager::createDisplayCaptureS
     g_variant_get(result.get(), "(h)", &fdOut);
     fd = g_unix_fd_list_get(fdList.get(), fdOut, nullptr);
 
-    auto session = makeUnique<GStreamerDisplayCaptureDeviceManager::Session>(fd, WTFMove(sessionPath));
+    NodeAndFD nodeAndFd = { *nodeId, fd };
+    auto session = makeUnique<GStreamerDisplayCaptureDeviceManager::Session>(nodeAndFd, WTFMove(sessionPath));
     m_sessions.add(device.persistentId(), WTFMove(session));
-    return GStreamerVideoCaptureSource::createPipewireSource(device.persistentId().isolatedCopy(), fd, WTFMove(hashSalt), constraints, device.type());
+    return GStreamerVideoCaptureSource::createPipewireSource(device.persistentId().isolatedCopy(), nodeAndFd, WTFMove(hashSalt), constraints, device.type());
 }
 
 void GStreamerDisplayCaptureDeviceManager::stopSource(const String& persistentID)
@@ -178,15 +209,15 @@ void GStreamerDisplayCaptureDeviceManager::stopSource(const String& persistentID
         WTFLogAlways("Portal session could not be closed: %s", error->message);
 }
 
-void GStreamerDisplayCaptureDeviceManager::waitResponseSignal(const char* objectPath)
+void GStreamerDisplayCaptureDeviceManager::waitResponseSignal(const char* objectPath, ResponseCallback&& callback)
 {
     RELEASE_ASSERT(!m_currentResponseCallback);
-    m_currentResponseCallback = [] { };
+    m_currentResponseCallback = WTFMove(callback);
     auto* connection = g_dbus_proxy_get_connection(m_proxy.get());
     auto signalId = g_dbus_connection_signal_subscribe(connection, "org.freedesktop.portal.Desktop", "org.freedesktop.portal.Request",
-        "Response", objectPath, nullptr, G_DBUS_SIGNAL_FLAGS_NO_MATCH_RULE, reinterpret_cast<GDBusSignalCallback>(+[](GDBusConnection*, const char* /* senderName */, const char* /* objectPath */, const char* /* interfaceName */, const char* /* signalName */, GVariant* /* parameters */, gpointer userData) {
+        "Response", objectPath, nullptr, G_DBUS_SIGNAL_FLAGS_NO_MATCH_RULE, reinterpret_cast<GDBusSignalCallback>(+[](GDBusConnection*, const char* /* senderName */, const char* /* objectPath */, const char* /* interfaceName */, const char* /* signalName */, GVariant* parameters, gpointer userData) {
             auto& manager = *reinterpret_cast<GStreamerDisplayCaptureDeviceManager*>(userData);
-            manager.notifyResponse();
+            manager.notifyResponse(parameters);
         }), this, nullptr);
 
     while (m_currentResponseCallback)

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp b/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp
@@ -96,9 +96,9 @@ CaptureSourceOrError GStreamerVideoCaptureSource::create(String&& deviceID, Stri
     return CaptureSourceOrError(WTFMove(source));
 }
 
-CaptureSourceOrError GStreamerVideoCaptureSource::createPipewireSource(String&& deviceID, int fd, String&& hashSalt, const MediaConstraints* constraints, CaptureDevice::DeviceType deviceType)
+CaptureSourceOrError GStreamerVideoCaptureSource::createPipewireSource(String&& deviceID, const NodeAndFD& nodeAndFd, String&& hashSalt, const MediaConstraints* constraints, CaptureDevice::DeviceType deviceType)
 {
-    auto source = adoptRef(*new GStreamerVideoCaptureSource(WTFMove(deviceID), { }, WTFMove(hashSalt), "pipewiresrc", deviceType, fd));
+    auto source = adoptRef(*new GStreamerVideoCaptureSource(WTFMove(deviceID), { }, WTFMove(hashSalt), "pipewiresrc", deviceType, nodeAndFd));
     if (constraints) {
         if (auto result = source->applyConstraints(*constraints))
             return WTFMove(result->badConstraint);
@@ -118,13 +118,13 @@ DisplayCaptureFactory& GStreamerVideoCaptureSource::displayFactory()
     return factory.get();
 }
 
-GStreamerVideoCaptureSource::GStreamerVideoCaptureSource(String&& deviceID, String&& name, String&& hashSalt, const gchar* sourceFactory, CaptureDevice::DeviceType deviceType, int fd)
+GStreamerVideoCaptureSource::GStreamerVideoCaptureSource(String&& deviceID, String&& name, String&& hashSalt, const gchar* sourceFactory, CaptureDevice::DeviceType deviceType, const NodeAndFD& nodeAndFd)
     : RealtimeVideoCaptureSource(WTFMove(name), WTFMove(deviceID), WTFMove(hashSalt), { })
     , m_capturer(makeUnique<GStreamerVideoCapturer>(sourceFactory, deviceType))
     , m_deviceType(deviceType)
 {
     initializeDebugCategory();
-    m_capturer->setPipewireFD(fd);
+    m_capturer->setPipewireNodeAndFD(nodeAndFd);
     m_capturer->addObserver(*this);
 }
 
@@ -145,7 +145,7 @@ GStreamerVideoCaptureSource::~GStreamerVideoCaptureSource()
     g_signal_handlers_disconnect_by_func(m_capturer->sink(), reinterpret_cast<gpointer>(newSampleCallback), this);
     m_capturer->stop();
 
-    if (auto fd = m_capturer->pipewireFD()) {
+    if (m_capturer->feedingFromPipewire()) {
         auto& manager = GStreamerDisplayCaptureDeviceManager::singleton();
         manager.stopSource(persistentID());
     }

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.h b/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.h
@@ -30,10 +30,12 @@
 
 namespace WebCore {
 
+using NodeAndFD = GStreamerVideoCapturer::NodeAndFD;
+
 class GStreamerVideoCaptureSource : public RealtimeVideoCaptureSource, GStreamerCapturer::Observer {
 public:
     static CaptureSourceOrError create(String&& deviceID, String&& hashSalt, const MediaConstraints*);
-    static CaptureSourceOrError createPipewireSource(String&& deviceID, int fd, String&& hashSalt, const MediaConstraints*, CaptureDevice::DeviceType);
+    static CaptureSourceOrError createPipewireSource(String&& deviceID, const NodeAndFD&, String&& hashSalt, const MediaConstraints*, CaptureDevice::DeviceType);
 
     WEBCORE_EXPORT static VideoCaptureFactory& factory();
 
@@ -49,7 +51,7 @@ class GStreamerVideoCaptureSource : public RealtimeVideoCaptureSource, GStreamer
     void sourceCapsChanged(const GstCaps*) final;
 
 protected:
-    GStreamerVideoCaptureSource(String&& deviceID, String&& name, String&& hashSalt, const gchar* source_factory, CaptureDevice::DeviceType, int fd);
+    GStreamerVideoCaptureSource(String&& deviceID, String&& name, String&& hashSalt, const gchar* source_factory, CaptureDevice::DeviceType, const NodeAndFD&);
     GStreamerVideoCaptureSource(GStreamerCaptureDevice, String&& hashSalt);
     virtual ~GStreamerVideoCaptureSource();
     void startProducingData() override;

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp b/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp
@@ -55,8 +55,13 @@ GStreamerVideoCapturer::GStreamerVideoCapturer(const char* sourceFactory, Captur
 GstElement* GStreamerVideoCapturer::createSource()
 {
     auto* src = GStreamerCapturer::createSource();
-    if (m_fd)
-        g_object_set(m_src.get(), "fd", *m_fd, nullptr);
+    if (m_nodeAndFd) {
+        auto& [node, fd] = *m_nodeAndFd;
+        auto path = AtomString::number(node);
+        // FIXME: The path property is deprecated in favor of target-object but the portal doesn't expose this object.
+        g_object_set(m_src.get(), "path", path.string().ascii().data(), nullptr);
+        g_object_set(m_src.get(), "fd", fd, nullptr);
+    }
     return src;
 }
 
@@ -77,7 +82,7 @@ GstVideoInfo GStreamerVideoCapturer::getBestFormat()
 
 bool GStreamerVideoCapturer::setSize(int width, int height)
 {
-    if (m_fd.has_value()) {
+    if (feedingFromPipewire()) {
         // Pipewiresrc doesn't seem to support caps re-negotiation and framerate configuration properly.
         GST_FIXME_OBJECT(m_pipeline.get(), "Resizing disabled on display capture source");
         return true;
@@ -105,7 +110,7 @@ bool GStreamerVideoCapturer::setSize(int width, int height)
 
 bool GStreamerVideoCapturer::setFrameRate(double frameRate)
 {
-    if (m_fd.has_value()) {
+    if (feedingFromPipewire()) {
         // Pipewiresrc doesn't seem to support caps re-negotiation and framerate configuration properly.
         GST_FIXME_OBJECT(m_pipeline.get(), "Framerate override disabled on display capture source");
         return true;

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.h b/Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.h
@@ -43,11 +43,13 @@ class GStreamerVideoCapturer final : public GStreamerCapturer {
     bool setFrameRate(double);
     GstVideoInfo getBestFormat();
 
-    void setPipewireFD(int fd) { m_fd = fd; }
-    std::optional<int> pipewireFD() const { return m_fd; }
+    using NodeAndFD = std::pair<uint32_t, int>;
+
+    void setPipewireNodeAndFD(const NodeAndFD& nodeAndFd) { m_nodeAndFd = nodeAndFd; }
+    bool feedingFromPipewire() const { return m_nodeAndFd.has_value(); }
 
 private:
-    std::optional<int> m_fd;
+    std::optional<NodeAndFD> m_nodeAndFd;
 };
 
 } // namespace WebCore