Reland: "PipeWire capturer: advertise DMA-BUF support when really supported"

We need to check the PipeWire server version in order to be sure we can
advertise DMA-BUF support, because it doesn't mean the version of
PipeWire we built our code against will run against the same PipeWire
version. Also do not announce DMA-BUF support for PipeWire older than
0.3.24 as this will not be working. For DMA-BUF modifiers support we
need the PipeWire version to be at least 0.3.33 on both sides (client
and server). Last but not least minor fix is not to announce
modifier-less DMA-BUF support when we don't have required extension.

Bug: chromium:1233417
Change-Id: If2a0a2328b893ccbeab61cb4039029b8a113a1ab
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246440
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#35699}

modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc

@@ -12,6 +12,7 @@
 
 #include <gio/gunixfdlist.h>
 #include <glib-object.h>
+#include <libdrm/drm_fourcc.h>
 #include <spa/param/format-utils.h>
 #include <spa/param/props.h>
 #include <sys/ioctl.h>
@@ -29,6 +30,7 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/string_encode.h"
+#include "rtc_base/string_to_number.h"
 
 #if defined(WEBRTC_DLOPEN_PIPEWIRE)
 #include "modules/desktop_capture/linux/wayland/pipewire_stubs.h"
@@ -55,6 +57,10 @@ const char kPipeWireLib[] = "libpipewire-0.3.so.0";
 const char kDrmLib[] = "libdrm.so.2";
 #endif
 
+constexpr BaseCapturerPipeWire::PipeWireVersion kDmaBufMinVersion = {0, 3, 24};
+constexpr BaseCapturerPipeWire::PipeWireVersion kDmaBufModifierMinVersion = {
+    0, 3, 33};
+
 #if !PW_CHECK_VERSION(0, 3, 29)
 #define SPA_POD_PROP_FLAG_MANDATORY (1u << 3)
 #endif
@@ -62,31 +68,25 @@ const char kDrmLib[] = "libdrm.so.2";
 #define SPA_POD_PROP_FLAG_DONT_FIXATE (1u << 4)
 #endif
 
-struct pw_version {
+BaseCapturerPipeWire::PipeWireVersion ParsePipeWireVersion(
-  int major = 0;
+    const char* version) {
-  int minor = 0;
-  int micro = 0;
-};
-
-bool CheckPipeWireVersion(pw_version required_version) {
   std::vector<std::string> parsed_version;
-  std::string version_string = pw_get_library_version();
+  rtc::split(version, '.', &parsed_version);
-  rtc::split(version_string, '.', &parsed_version);
 
   if (parsed_version.size() != 3) {
-    return false;
+    return {};
   }
 
-  pw_version current_version = {std::stoi(parsed_version.at(0)),
+  absl::optional<int> major = rtc::StringToNumber<int>(parsed_version.at(0));
-                                std::stoi(parsed_version.at(1)),
+  absl::optional<int> minor = rtc::StringToNumber<int>(parsed_version.at(1));
-                                std::stoi(parsed_version.at(2))};
+  absl::optional<int> micro = rtc::StringToNumber<int>(parsed_version.at(2));
 
-  return (current_version.major > required_version.major) ||
+  // Return invalid version if we failed to parse it
-         (current_version.major == required_version.major &&
+  if (!major || !minor || !micro) {
-          current_version.minor > required_version.minor) ||
+    return {0, 0, 0};
-         (current_version.major == required_version.major &&
+  }
-          current_version.minor == required_version.minor &&
+
-          current_version.micro >= required_version.micro);
+  return {major.value(), micro.value(), micro.value()};
 }
 
 spa_pod* BuildFormat(spa_pod_builder* builder,
@@ -106,15 +106,9 @@ spa_pod* BuildFormat(spa_pod_builder* builder,
   spa_pod_builder_add(builder, SPA_FORMAT_VIDEO_format, SPA_POD_Id(format), 0);
 
   if (modifiers.size()) {
-    // SPA_POD_PROP_FLAG_DONT_FIXATE can be used with PipeWire >= 0.3.33
+    spa_pod_builder_prop(
-    if (CheckPipeWireVersion(pw_version{0, 3, 33})) {
+        builder, SPA_FORMAT_VIDEO_modifier,
-      spa_pod_builder_prop(
+        SPA_POD_PROP_FLAG_MANDATORY | SPA_POD_PROP_FLAG_DONT_FIXATE);
-          builder, SPA_FORMAT_VIDEO_modifier,
-          SPA_POD_PROP_FLAG_MANDATORY | SPA_POD_PROP_FLAG_DONT_FIXATE);
-    } else {
-      spa_pod_builder_prop(builder, SPA_FORMAT_VIDEO_modifier,
-                           SPA_POD_PROP_FLAG_MANDATORY);
-    }
     spa_pod_builder_push_choice(builder, &frames[1], SPA_CHOICE_Enum, 0);
     // modifiers from the array
     for (int64_t val : modifiers) {
@@ -165,6 +159,17 @@ class ScopedBuf {
   int fd_;
 };
 
+class PipeWireThreadLoopLock {
+ public:
+  explicit PipeWireThreadLoopLock(pw_thread_loop* loop) : loop_(loop) {
+    pw_thread_loop_lock(loop_);
+  }
+  ~PipeWireThreadLoopLock() { pw_thread_loop_unlock(loop_); }
+
+ private:
+  pw_thread_loop* const loop_;
+};
+
 template <class T>
 class Scoped {
  public:
@@ -234,6 +239,34 @@ Scoped<GUnixFDList>::~Scoped() {
   }
 }
 
+bool operator>=(
+    const BaseCapturerPipeWire::PipeWireVersion& current_pw_version,
+    const BaseCapturerPipeWire::PipeWireVersion& required_pw_version) {
+  if (!current_pw_version.major && !current_pw_version.minor &&
+      !current_pw_version.micro) {
+    return false;
+  }
+
+  return std::tie(current_pw_version.major, current_pw_version.minor,
+                  current_pw_version.micro) >=
+         std::tie(required_pw_version.major, required_pw_version.minor,
+                  required_pw_version.micro);
+}
+
+bool operator<=(
+    const BaseCapturerPipeWire::PipeWireVersion& current_pw_version,
+    const BaseCapturerPipeWire::PipeWireVersion& required_pw_version) {
+  if (!current_pw_version.major && !current_pw_version.minor &&
+      !current_pw_version.micro) {
+    return false;
+  }
+
+  return std::tie(current_pw_version.major, current_pw_version.minor,
+                  current_pw_version.micro) <=
+         std::tie(required_pw_version.major, required_pw_version.minor,
+                  required_pw_version.micro);
+}
+
 void BaseCapturerPipeWire::OnCoreError(void* data,
                                        uint32_t id,
                                        int seq,
@@ -245,6 +278,23 @@ void BaseCapturerPipeWire::OnCoreError(void* data,
   RTC_LOG(LS_ERROR) << "PipeWire remote error: " << message;
 }
 
+void BaseCapturerPipeWire::OnCoreInfo(void* data, const pw_core_info* info) {
+  BaseCapturerPipeWire* capturer = static_cast<BaseCapturerPipeWire*>(data);
+  RTC_DCHECK(capturer);
+
+  capturer->pw_server_version_ = ParsePipeWireVersion(info->version);
+}
+
+void BaseCapturerPipeWire::OnCoreDone(void* data, uint32_t id, int seq) {
+  const BaseCapturerPipeWire* capturer =
+      static_cast<BaseCapturerPipeWire*>(data);
+  RTC_DCHECK(capturer);
+
+  if (id == PW_ID_CORE && capturer->server_version_sync_ == seq) {
+    pw_thread_loop_signal(capturer->pw_main_loop_, false);
+  }
+}
+
 // static
 void BaseCapturerPipeWire::OnStreamStateChanged(void* data,
                                                 pw_stream_state old_state,
@@ -286,20 +336,27 @@ void BaseCapturerPipeWire::OnStreamParamChanged(void* data,
   auto size = height * stride;
 
   that->desktop_size_ = DesktopSize(width, height);
-#if PW_CHECK_VERSION(0, 3, 0)
-  that->modifier_ = that->spa_video_format_.modifier;
-#endif
 
   uint8_t buffer[1024] = {};
   auto builder = spa_pod_builder{buffer, sizeof(buffer)};
 
   // Setup buffers and meta header for new format.
+
+  // When SPA_FORMAT_VIDEO_modifier is present we can use DMA-BUFs as
+  // the server announces support for it.
+  // See https://github.com/PipeWire/pipewire/blob/master/doc/dma-buf.dox
+  const bool has_modifier =
+      spa_pod_find_prop(format, nullptr, SPA_FORMAT_VIDEO_modifier);
+  that->modifier_ =
+      has_modifier ? that->spa_video_format_.modifier : DRM_FORMAT_MOD_INVALID;
+
   const struct spa_pod* params[3];
   const int buffer_types =
-      spa_pod_find_prop(format, nullptr, SPA_FORMAT_VIDEO_modifier)
+      has_modifier || (that->pw_server_version_ >= kDmaBufMinVersion)
           ? (1 << SPA_DATA_DmaBuf) | (1 << SPA_DATA_MemFd) |
                 (1 << SPA_DATA_MemPtr)
           : (1 << SPA_DATA_MemFd) | (1 << SPA_DATA_MemPtr);
+
   params[0] = reinterpret_cast<spa_pod*>(spa_pod_builder_add_object(
       &builder, SPA_TYPE_OBJECT_ParamBuffers, SPA_PARAM_Buffers,
       SPA_PARAM_BUFFERS_size, SPA_POD_Int(size), SPA_PARAM_BUFFERS_stride,
@@ -444,8 +501,6 @@ void BaseCapturerPipeWire::Init() {
 
   pw_main_loop_ = pw_thread_loop_new("pipewire-main-loop", nullptr);
 
-  pw_thread_loop_lock(pw_main_loop_);
-
   pw_context_ =
       pw_context_new(pw_thread_loop_get_loop(pw_main_loop_), nullptr, 0);
   if (!pw_context_) {
@@ -453,14 +508,18 @@ void BaseCapturerPipeWire::Init() {
     return;
   }
 
-  pw_core_ = pw_context_connect_fd(pw_context_, pw_fd_, nullptr, 0);
+  if (pw_thread_loop_start(pw_main_loop_) < 0) {
-  if (!pw_core_) {
+    RTC_LOG(LS_ERROR) << "Failed to start main PipeWire loop";
-    RTC_LOG(LS_ERROR) << "Failed to connect PipeWire context";
+    portal_init_failed_ = true;
     return;
   }
 
+  pw_client_version_ = ParsePipeWireVersion(pw_get_library_version());
+
   // Initialize event handlers, remote end and stream-related.
   pw_core_events_.version = PW_VERSION_CORE_EVENTS;
+  pw_core_events_.info = &OnCoreInfo;
+  pw_core_events_.done = &OnCoreDone;
   pw_core_events_.error = &OnCoreError;
 
   pw_stream_events_.version = PW_VERSION_STREAM_EVENTS;
@@ -468,22 +527,32 @@ void BaseCapturerPipeWire::Init() {
   pw_stream_events_.param_changed = &OnStreamParamChanged;
   pw_stream_events_.process = &OnStreamProcess;
 
-  pw_core_add_listener(pw_core_, &spa_core_listener_, &pw_core_events_, this);
+  {
+    PipeWireThreadLoopLock thread_loop_lock(pw_main_loop_);
 
-  pw_stream_ = CreateReceivingStream();
+    pw_core_ = pw_context_connect_fd(pw_context_, pw_fd_, nullptr, 0);
-  if (!pw_stream_) {
+    if (!pw_core_) {
-    RTC_LOG(LS_ERROR) << "Failed to create PipeWire stream";
+      RTC_LOG(LS_ERROR) << "Failed to connect PipeWire context";
-    return;
+      portal_init_failed_ = true;
+      return;
+    }
+
+    pw_core_add_listener(pw_core_, &spa_core_listener_, &pw_core_events_, this);
+
+    server_version_sync_ =
+        pw_core_sync(pw_core_, PW_ID_CORE, server_version_sync_);
+
+    pw_thread_loop_wait(pw_main_loop_);
+
+    pw_stream_ = CreateReceivingStream();
+    if (!pw_stream_) {
+      RTC_LOG(LS_ERROR) << "Failed to create PipeWire stream";
+      portal_init_failed_ = true;
+      return;
+    }
+
+    RTC_LOG(LS_INFO) << "PipeWire remote opened.";
   }
-
-  if (pw_thread_loop_start(pw_main_loop_) < 0) {
-    RTC_LOG(LS_ERROR) << "Failed to start main PipeWire loop";
-    portal_init_failed_ = true;
-  }
-
-  pw_thread_loop_unlock(pw_main_loop_);
-
-  RTC_LOG(LS_INFO) << "PipeWire remote opened.";
 }
 
 pw_stream* BaseCapturerPipeWire::CreateReceivingStream() {
@@ -497,12 +566,14 @@ pw_stream* BaseCapturerPipeWire::CreateReceivingStream() {
   spa_pod_builder builder = spa_pod_builder{buffer, sizeof(buffer)};
 
   std::vector<const spa_pod*> params;
-  const bool has_required_pw_version =
+  const bool has_required_pw_client_version =
-      CheckPipeWireVersion(pw_version{0, 3, 29});
+      pw_client_version_ >= kDmaBufModifierMinVersion;
+  const bool has_required_pw_server_version =
+      pw_server_version_ >= kDmaBufModifierMinVersion;
   for (uint32_t format : {SPA_VIDEO_FORMAT_BGRA, SPA_VIDEO_FORMAT_RGBA,
                           SPA_VIDEO_FORMAT_BGRx, SPA_VIDEO_FORMAT_RGBx}) {
-    // Modifiers can be used with PipeWire >= 0.3.29
+    // Modifiers can be used with PipeWire >= 0.3.33
-    if (has_required_pw_version) {
+    if (has_required_pw_client_version && has_required_pw_server_version) {
       modifiers = egl_dmabuf_->QueryDmaBufModifiers(format);
 
       if (!modifiers.empty()) {

modules/desktop_capture/linux/wayland/base_capturer_pipewire.h

@@ -44,6 +44,12 @@ class BaseCapturerPipeWire : public DesktopCapturer {
     kMetadata = 0b100
   };
 
+  struct PipeWireVersion {
+    int major = 0;
+    int minor = 0;
+    int micro = 0;
+  };
+
   explicit BaseCapturerPipeWire(CaptureSourceType source_type);
   ~BaseCapturerPipeWire() override;
 
@@ -66,6 +72,14 @@ class BaseCapturerPipeWire : public DesktopCapturer {
   spa_hook spa_core_listener_;
   spa_hook spa_stream_listener_;
 
+  // A number used to verify all previous methods and the resulting
+  // events have been handled.
+  int server_version_sync_ = 0;
+  // Version of the running PipeWire server we communicate with
+  PipeWireVersion pw_server_version_;
+  // Version of the library used to run our code
+  PipeWireVersion pw_client_version_;
+
   // event handlers
   pw_core_events pw_core_events_ = {};
   pw_stream_events pw_stream_events_ = {};
@@ -118,6 +132,8 @@ class BaseCapturerPipeWire : public DesktopCapturer {
                           int seq,
                           int res,
                           const char* message);
+  static void OnCoreDone(void* user_data, uint32_t id, int seq);
+  static void OnCoreInfo(void* user_data, const pw_core_info* info);
   static void OnStreamParamChanged(void* data,
                                    uint32_t id,
                                    const struct spa_pod* format);

modules/desktop_capture/linux/wayland/egl_dmabuf.cc

@@ -349,12 +349,11 @@ EglDmaBuf::EglDmaBuf() {
     egl_.extensions.push_back(std::string(extension));
   }
 
-  bool has_image_dma_buf_import_ext = false;
   bool has_image_dma_buf_import_modifiers_ext = false;
 
   for (const auto& extension : egl_.extensions) {
     if (extension == "EGL_EXT_image_dma_buf_import") {
-      has_image_dma_buf_import_ext = true;
+      has_image_dma_buf_import_ext_ = true;
       continue;
     } else if (extension == "EGL_EXT_image_dma_buf_import_modifiers") {
       has_image_dma_buf_import_modifiers_ext = true;
@@ -362,7 +361,7 @@ EglDmaBuf::EglDmaBuf() {
     }
   }
 
-  if (has_image_dma_buf_import_ext && has_image_dma_buf_import_modifiers_ext) {
+  if (has_image_dma_buf_import_ext_ && has_image_dma_buf_import_modifiers_ext) {
     EglQueryDmaBufFormatsEXT = (eglQueryDmaBufFormatsEXT_func)EglGetProcAddress(
         "eglQueryDmaBufFormatsEXT");
     EglQueryDmaBufModifiersEXT =
@@ -501,18 +500,18 @@ std::vector<uint64_t> EglDmaBuf::QueryDmaBufModifiers(uint32_t format) {
     return {};
   }
 
-  // Modifiers not supported, return just DRM_FORMAT_MOD_INVALID as we can still
+  // Explicit modifiers not supported, return just DRM_FORMAT_MOD_INVALID as we
-  // use modifier-less DMA-BUFs
+  // can still use modifier-less DMA-BUFs if we have required extension
   if (EglQueryDmaBufFormatsEXT == nullptr ||
       EglQueryDmaBufModifiersEXT == nullptr) {
-    return {DRM_FORMAT_MOD_INVALID};
+    return has_image_dma_buf_import_ext_
+               ? std::vector<uint64_t>{DRM_FORMAT_MOD_INVALID}
+               : std::vector<uint64_t>{};
   }
 
   uint32_t drm_format = SpaPixelFormatToDrmFormat(format);
-  if (drm_format == DRM_FORMAT_INVALID) {
+  // Should never happen as it's us who controls the list of supported formats
-    RTC_LOG(LS_ERROR) << "Failed to find matching DRM format.";
+  RTC_DCHECK(drm_format != DRM_FORMAT_INVALID);
-    return {DRM_FORMAT_MOD_INVALID};
-  }
 
   EGLint count = 0;
   EGLBoolean success =

modules/desktop_capture/linux/wayland/egl_dmabuf.h

@@ -52,6 +52,7 @@ class EglDmaBuf {
 
  private:
   bool egl_initialized_ = false;
+  bool has_image_dma_buf_import_ext_ = false;
   int32_t drm_fd_ = -1;               // for GBM buffer mmap
   gbm_device* gbm_device_ = nullptr;  // for passed GBM buffer retrieval
 

modules/desktop_capture/linux/wayland/pipewire.sigs

@@ -39,6 +39,8 @@ void pw_thread_loop_stop(pw_thread_loop *loop);
 void pw_thread_loop_lock(pw_thread_loop *loop);
 void pw_thread_loop_unlock(pw_thread_loop *loop);
 pw_loop * pw_thread_loop_get_loop(pw_thread_loop *loop);
+void pw_thread_loop_signal(pw_thread_loop *loop, bool wait_for_accept);
+void pw_thread_loop_wait(pw_thread_loop *loop);
 
 // context.h
 void pw_context_destroy(pw_context *context);