[Video Capture Service] Multi-client: Add support for mixed buffer types

Add support for having some clients require buffers of type
kSharedMemoryViaRawFileDescriptor while others are using kSharedMemory
while sharing the same device. This is done by having class
BroadcastingReceiver convert the buffer type as needed when distributing
buffer handles to the clients.

Design Doc: https://docs.google.com/document/d/1mYnsZfLBRmbsDpUtfb6C7dzhfw2Kcxg_-uiG_6MnWVQ/edit?usp=sharing

Test: content_browsertests --gtest_filter=WebRtcVideoCaptureSharedDeviceBrowserTest.*
Bug: 783442
Change-Id: Ia636157dfddba3eec09476c4b2be7750ff931fd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1413022
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Luke Sorenson <lasoren@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638231}

content/browser/webrtc/webrtc_video_capture_shared_device_browsertest.cc

@@ -5,6 +5,7 @@
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
+#include "build/build_config.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_manager_connection.h"
@@ -22,6 +23,7 @@
 
 using testing::_;
 using testing::AtLeast;
+using testing::Invoke;
 using testing::InvokeWithoutArgs;
 using testing::Return;
 
@@ -49,7 +51,9 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest : public ContentBrowserTest {
 
   ~WebRtcVideoCaptureSharedDeviceBrowserTest() override {}
 
-  void OpenDeviceViaService(base::OnceClosure done_cb) {
+  void OpenDeviceViaService(
+      media::VideoCaptureBufferType buffer_type_to_request,
+      base::OnceClosure done_cb) {
     connector_->BindInterface(video_capture::mojom::kServiceName,
                               &device_factory_provider_);
     device_factory_provider_->ConnectToVideoSourceProvider(
@@ -57,7 +61,8 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest : public ContentBrowserTest {
 
     video_source_provider_->GetSourceInfos(base::BindOnce(
         &WebRtcVideoCaptureSharedDeviceBrowserTest::OnSourceInfosReceived,
-        weak_factory_.GetWeakPtr(), std::move(done_cb)));
+        weak_factory_.GetWeakPtr(), buffer_type_to_request,
+        std::move(done_cb)));
   }
 
   void OpenDeviceInRendererAndWaitForPlaying() {
@@ -108,6 +113,7 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest : public ContentBrowserTest {
 
  private:
   void OnSourceInfosReceived(
+      media::VideoCaptureBufferType buffer_type_to_request,
       base::OnceClosure done_cb,
       const std::vector<media::VideoCaptureDeviceInfo>& infos) {
     ASSERT_FALSE(infos.empty());
@@ -118,6 +124,7 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest : public ContentBrowserTest {
     ASSERT_FALSE(infos[0].supported_formats.empty());
     requestable_settings.requested_format = infos[0].supported_formats[0];
     requestable_settings.requested_format.frame_size = kVideoSize;
+    requestable_settings.buffer_type = buffer_type_to_request;
 
     video_capture::mojom::PushVideoStreamSubscriptionPtr subscription;
     video_source_->CreatePushSubscription(
@@ -133,8 +140,7 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest : public ContentBrowserTest {
       base::OnceClosure done_cb,
       video_capture::mojom::CreatePushSubscriptionResultCode result_code,
       const media::VideoCaptureParams& params) {
-    ASSERT_EQ(video_capture::mojom::CreatePushSubscriptionResultCode::
-                  kCreatedWithRequestedSettings,
+    ASSERT_NE(video_capture::mojom::CreatePushSubscriptionResultCode::kFailed,
               result_code);
     subscription_->Activate();
     std::move(done_cb).Run();
@@ -151,11 +157,22 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest : public ContentBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(WebRtcVideoCaptureSharedDeviceBrowserTest);
 };
 
-IN_PROC_BROWSER_TEST_F(WebRtcVideoCaptureSharedDeviceBrowserTest,
-                       ReceiveFrameFromServiceAndInRenderer) {
+// Tests that a single fake video capture device can be opened via JavaScript
+// by the Renderer while it is already in use by a direct client of the
+// video capture service.
+IN_PROC_BROWSER_TEST_F(
+    WebRtcVideoCaptureSharedDeviceBrowserTest,
+    ReceiveFrameInRendererWhileDeviceAlreadyInUseViaDirectServiceClient) {
   Initialize();
 
   base::RunLoop receive_frame_from_service_wait_loop;
+  ON_CALL(*mock_receiver_, DoOnNewBuffer(_, _))
+      .WillByDefault(Invoke(
+          [](int32_t, media::mojom::VideoBufferHandlePtr* buffer_handle) {
+            ASSERT_EQ(
+                media::mojom::VideoBufferHandle::Tag::SHARED_BUFFER_HANDLE,
+                (*buffer_handle)->which());
+          }));
   EXPECT_CALL(*mock_receiver_, DoOnFrameReadyInBuffer(_, _, _, _))
       .WillOnce(InvokeWithoutArgs([&receive_frame_from_service_wait_loop]() {
         receive_frame_from_service_wait_loop.Quit();
@@ -163,7 +180,8 @@ IN_PROC_BROWSER_TEST_F(WebRtcVideoCaptureSharedDeviceBrowserTest,
       .WillRepeatedly(Return());
 
   base::RunLoop open_device_via_service_run_loop;
-  OpenDeviceViaService(open_device_via_service_run_loop.QuitClosure());
+  OpenDeviceViaService(media::VideoCaptureBufferType::kSharedMemory,
+                       open_device_via_service_run_loop.QuitClosure());
   open_device_via_service_run_loop.Run();
 
   OpenDeviceInRendererAndWaitForPlaying();
@@ -171,4 +189,74 @@ IN_PROC_BROWSER_TEST_F(WebRtcVideoCaptureSharedDeviceBrowserTest,
   receive_frame_from_service_wait_loop.Run();
 }
 
+#if defined(OS_LINUX)
+
+// Tests that a single fake video capture device can be opened via JavaScript
+// by the Renderer while it is already in use by a direct client of the
+// video capture service that requested to get buffers as raw file handles.
+IN_PROC_BROWSER_TEST_F(
+    WebRtcVideoCaptureSharedDeviceBrowserTest,
+    ReceiveFrameInRendererWhileDeviceAlreadyInUseUsingRawFileHandleBuffers) {
+  Initialize();
+
+  base::RunLoop receive_frame_from_service_wait_loop;
+  ON_CALL(*mock_receiver_, DoOnNewBuffer(_, _))
+      .WillByDefault(Invoke(
+          [](int32_t, media::mojom::VideoBufferHandlePtr* buffer_handle) {
+            ASSERT_EQ(media::mojom::VideoBufferHandle::Tag::
+                          SHARED_MEMORY_VIA_RAW_FILE_DESCRIPTOR,
+                      (*buffer_handle)->which());
+          }));
+  EXPECT_CALL(*mock_receiver_, DoOnFrameReadyInBuffer(_, _, _, _))
+      .WillOnce(InvokeWithoutArgs([&receive_frame_from_service_wait_loop]() {
+        receive_frame_from_service_wait_loop.Quit();
+      }))
+      .WillRepeatedly(Return());
+
+  base::RunLoop open_device_via_service_run_loop;
+  OpenDeviceViaService(
+      media::VideoCaptureBufferType::kSharedMemoryViaRawFileDescriptor,
+      open_device_via_service_run_loop.QuitClosure());
+  open_device_via_service_run_loop.Run();
+
+  OpenDeviceInRendererAndWaitForPlaying();
+
+  receive_frame_from_service_wait_loop.Run();
+}
+
+// Tests that a single fake video capture device can be opened by a direct
+// client of the video capture service that requests to get buffers as raw
+// file handles while it is already in use via JavaScript by the Renderer.
+IN_PROC_BROWSER_TEST_F(
+    WebRtcVideoCaptureSharedDeviceBrowserTest,
+    ReceiveFrameFromServiceViaRawFileHandlesWhileDeviceAlreadyInUseByRenderer) {
+  Initialize();
+
+  OpenDeviceInRendererAndWaitForPlaying();
+
+  base::RunLoop receive_frame_from_service_wait_loop;
+  ON_CALL(*mock_receiver_, DoOnNewBuffer(_, _))
+      .WillByDefault(Invoke(
+          [](int32_t, media::mojom::VideoBufferHandlePtr* buffer_handle) {
+            ASSERT_EQ(media::mojom::VideoBufferHandle::Tag::
+                          SHARED_MEMORY_VIA_RAW_FILE_DESCRIPTOR,
+                      (*buffer_handle)->which());
+          }));
+  EXPECT_CALL(*mock_receiver_, DoOnFrameReadyInBuffer(_, _, _, _))
+      .WillOnce(InvokeWithoutArgs([&receive_frame_from_service_wait_loop]() {
+        receive_frame_from_service_wait_loop.Quit();
+      }))
+      .WillRepeatedly(Return());
+
+  base::RunLoop open_device_via_service_run_loop;
+  OpenDeviceViaService(
+      media::VideoCaptureBufferType::kSharedMemoryViaRawFileDescriptor,
+      open_device_via_service_run_loop.QuitClosure());
+  open_device_via_service_run_loop.Run();
+
+  receive_frame_from_service_wait_loop.Run();
+}
+
+#endif  // defined(OS_LINUX)
+
 }  // namespace content

services/video_capture/broadcasting_receiver.cc

@@ -5,6 +5,8 @@
 #include "services/video_capture/broadcasting_receiver.h"
 
 #include "base/bind.h"
+#include "build/build_config.h"
+#include "media/capture/video/shared_memory_handle_provider.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "services/video_capture/public/mojom/scoped_access_permission.mojom.h"
 
@@ -22,10 +24,46 @@ class ConsumerAccessPermission : public mojom::ScopedAccessPermission {
   base::OnceClosure destruction_cb_;
 };
 
+void CloneSharedBufferHandle(const mojo::ScopedSharedBufferHandle& source,
+                             media::mojom::VideoBufferHandlePtr* target) {
+  // Special behavior here: If the handle was already read-only, the
+  // Clone() call here will maintain that read-only permission. If it was
+  // read-write, the cloned handle will have read-write permission.
+  //
+  // TODO(crbug.com/797470): We should be able to demote read-write to
+  // read-only permissions when Clone()'ing handles. Currently, this
+  // causes a crash.
+  (*target)->set_shared_buffer_handle(
+      source->Clone(mojo::SharedBufferHandle::AccessMode::READ_WRITE));
+}
+
+void CloneSharedBufferToRawFileDescriptorHandle(
+    const mojo::ScopedSharedBufferHandle& source,
+    media::mojom::VideoBufferHandlePtr* target) {
+#if defined(OS_LINUX)
+  media::SharedMemoryHandleProvider provider;
+  provider.InitFromMojoHandle(
+      source->Clone(mojo::SharedBufferHandle::AccessMode::READ_WRITE));
+  auto sub_struct = media::mojom::SharedMemoryViaRawFileDescriptor::New();
+  sub_struct->shared_memory_size_in_bytes = provider.GetMemorySizeInBytes();
+  sub_struct->file_descriptor_handle = mojo::WrapPlatformFile(
+      base::SharedMemory::DuplicateHandle(
+          provider.GetNonOwnedSharedMemoryHandleForLegacyIPC())
+          .GetHandle());
+  (*target)->set_shared_memory_via_raw_file_descriptor(std::move(sub_struct));
+#else
+  NOTREACHED() << "Cannot convert buffer handle to "
+                  "kSharedMemoryViaRawFileDescriptor on non-Linux platform.";
+#endif
+}
+
 }  // anonymous namespace
 
-BroadcastingReceiver::ClientContext::ClientContext(mojom::ReceiverPtr client)
+BroadcastingReceiver::ClientContext::ClientContext(
+    mojom::ReceiverPtr client,
+    media::VideoCaptureBufferType target_buffer_type)
     : client_(std::move(client)),
+      target_buffer_type_(target_buffer_type),
       is_suspended_(false),
       on_started_has_been_called_(false),
       on_started_using_gpu_decode_has_been_called_(false) {}
@@ -83,39 +121,70 @@ bool BroadcastingReceiver::BufferContext::IsStillBeingConsumed() const {
 }
 
 media::mojom::VideoBufferHandlePtr
-BroadcastingReceiver::BufferContext::CloneBufferHandle() {
-  // Unable to use buffer_handle_->Clone(), because shared_buffer does not
-  // support the copy constructor.
+BroadcastingReceiver::BufferContext::CloneBufferHandle(
+    media::VideoCaptureBufferType target_buffer_type) {
   media::mojom::VideoBufferHandlePtr result =
       media::mojom::VideoBufferHandle::New();
-  if (buffer_handle_->is_shared_buffer_handle()) {
-    // Special behavior here: If the handle was already read-only, the Clone()
-    // call here will maintain that read-only permission. If it was read-write,
-    // the cloned handle will have read-write permission.
-    //
-    // TODO(crbug.com/797470): We should be able to demote read-write to
-    // read-only permissions when Clone()'ing handles. Currently, this causes a
-    // crash.
-    result->set_shared_buffer_handle(
-        buffer_handle_->get_shared_buffer_handle()->Clone(
-            mojo::SharedBufferHandle::AccessMode::READ_WRITE));
-  } else if (buffer_handle_->is_mailbox_handles()) {
+
+  // If the source uses mailbox hanldes, i.e. textures, we pass those through
+  // without conversion, no matter what clients requested.
+  if (buffer_handle_->is_mailbox_handles()) {
     result->set_mailbox_handles(buffer_handle_->get_mailbox_handles()->Clone());
-  } else if (buffer_handle_->is_shared_memory_via_raw_file_descriptor()) {
-    auto sub_struct = media::mojom::SharedMemoryViaRawFileDescriptor::New();
-    sub_struct->shared_memory_size_in_bytes =
-        buffer_handle_->get_shared_memory_via_raw_file_descriptor()
-            ->shared_memory_size_in_bytes;
-    sub_struct->file_descriptor_handle = mojo::ScopedHandle(
-        buffer_handle_->get_shared_memory_via_raw_file_descriptor()
-            ->file_descriptor_handle.get());
-    result->set_shared_memory_via_raw_file_descriptor(std::move(sub_struct));
-  } else {
-    NOTREACHED() << "Unexpected video buffer handle type";
+    return result;
+  }
+
+  switch (target_buffer_type) {
+    case media::VideoCaptureBufferType::kMailboxHolder:
+      NOTREACHED() << "Cannot convert buffer type to kMailboxHolder from "
+                      "handle type other than mailbox handles.";
+      break;
+    case media::VideoCaptureBufferType::kSharedMemory:
+      if (buffer_handle_->is_shared_buffer_handle()) {
+        CloneSharedBufferHandle(buffer_handle_->get_shared_buffer_handle(),
+                                &result);
+      } else if (buffer_handle_->is_shared_memory_via_raw_file_descriptor()) {
+        ConvertRawFileDescriptorToSharedBuffer();
+        CloneSharedBufferHandle(buffer_handle_->get_shared_buffer_handle(),
+                                &result);
+      } else {
+        NOTREACHED() << "Unexpected video buffer handle type";
+      }
+      break;
+    case media::VideoCaptureBufferType::kSharedMemoryViaRawFileDescriptor:
+      if (buffer_handle_->is_shared_buffer_handle()) {
+        CloneSharedBufferToRawFileDescriptorHandle(
+            buffer_handle_->get_shared_buffer_handle(), &result);
+      } else if (buffer_handle_->is_shared_memory_via_raw_file_descriptor()) {
+        ConvertRawFileDescriptorToSharedBuffer();
+        CloneSharedBufferToRawFileDescriptorHandle(
+            buffer_handle_->get_shared_buffer_handle(), &result);
+      } else {
+        NOTREACHED() << "Unexpected video buffer handle type";
+      }
+      break;
   }
   return result;
 }
 
+void BroadcastingReceiver::BufferContext::
+    ConvertRawFileDescriptorToSharedBuffer() {
+  DCHECK(buffer_handle_->is_shared_memory_via_raw_file_descriptor());
+
+#if defined(OS_LINUX)
+  media::SharedMemoryHandleProvider provider;
+  provider.InitAsReadOnlyFromRawFileDescriptor(
+      std::move(buffer_handle_->get_shared_memory_via_raw_file_descriptor()
+                    ->file_descriptor_handle),
+      buffer_handle_->get_shared_memory_via_raw_file_descriptor()
+          ->shared_memory_size_in_bytes);
+  buffer_handle_->set_shared_buffer_handle(
+      provider.GetHandleForInterProcessTransit(true /*read_only*/));
+#else
+  NOTREACHED() << "Unable to consume buffer handle of type "
+                  "kSharedMemoryViaRawFileDescriptor on non-Linux platform.";
+#endif
+}
+
 BroadcastingReceiver::BroadcastingReceiver()
     : status_(Status::kOnStartedHasNotYetBeenCalled),
       error_(media::VideoCaptureError::kNone),
@@ -139,10 +208,12 @@ void BroadcastingReceiver::SetOnStoppedHandler(
   on_stopped_handler_ = std::move(on_stopped_handler);
 }
 
-int32_t BroadcastingReceiver::AddClient(mojom::ReceiverPtr client) {
+int32_t BroadcastingReceiver::AddClient(
+    mojom::ReceiverPtr client,
+    media::VideoCaptureBufferType target_buffer_type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto client_id = next_client_id_++;
-  ClientContext context(std::move(client));
+  ClientContext context(std::move(client), target_buffer_type);
   auto& added_client_context =
       clients_.insert(std::make_pair(client_id, std::move(context)))
           .first->second;
@@ -163,7 +234,9 @@ int32_t BroadcastingReceiver::AddClient(mojom::ReceiverPtr client) {
 
   for (auto& buffer_context : buffer_contexts_) {
     added_client_context.client()->OnNewBuffer(
-        buffer_context.buffer_id(), buffer_context.CloneBufferHandle());
+        buffer_context.buffer_id(),
+        buffer_context.CloneBufferHandle(
+            added_client_context.target_buffer_type()));
   }
   return client_id;
 }
@@ -190,8 +263,9 @@ void BroadcastingReceiver::OnNewBuffer(
   buffer_contexts_.emplace_back(buffer_id, std::move(buffer_handle));
   auto& buffer_context = buffer_contexts_.back();
   for (auto& client : clients_) {
-    client.second.client()->OnNewBuffer(buffer_context.buffer_id(),
-                                        buffer_context.CloneBufferHandle());
+    client.second.client()->OnNewBuffer(
+        buffer_context.buffer_id(),
+        buffer_context.CloneBufferHandle(client.second.target_buffer_type()));
   }
 }
 

services/video_capture/broadcasting_receiver.h

@@ -10,6 +10,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
 #include "media/capture/video/video_frame_receiver.h"
+#include "media/capture/video_capture_types.h"
 #include "services/video_capture/public/mojom/receiver.mojom.h"
 #include "services/video_capture/public/mojom/scoped_access_permission.mojom.h"
 
@@ -34,7 +35,8 @@ class BroadcastingReceiver : public mojom::Receiver {
   void SetOnStoppedHandler(base::OnceClosure on_stopped_handler);
 
   // Returns a client_id that can be used for a call to Suspend/Resume/Remove.
-  int32_t AddClient(mojom::ReceiverPtr client);
+  int32_t AddClient(mojom::ReceiverPtr client,
+                    media::VideoCaptureBufferType target_buffer_type);
   void SuspendClient(int32_t client_id);
   void ResumeClient(int32_t client_id);
   // Returns ownership of the client back to the caller.
@@ -71,7 +73,8 @@ class BroadcastingReceiver : public mojom::Receiver {
   // a client is suspended.
   class ClientContext {
    public:
-    explicit ClientContext(mojom::ReceiverPtr client);
+    ClientContext(mojom::ReceiverPtr client,
+                  media::VideoCaptureBufferType target_buffer_type);
     ~ClientContext();
     ClientContext(ClientContext&& other);
     ClientContext& operator=(ClientContext&& other);
@@ -79,11 +82,15 @@ class BroadcastingReceiver : public mojom::Receiver {
     void OnStartedUsingGpuDecode();
 
     mojom::ReceiverPtr& client() { return client_; }
+    media::VideoCaptureBufferType target_buffer_type() {
+      return target_buffer_type_;
+    }
     void set_is_suspended(bool suspended) { is_suspended_ = suspended; }
     bool is_suspended() const { return is_suspended_; }
 
    private:
     mojom::ReceiverPtr client_;
+    media::VideoCaptureBufferType target_buffer_type_;
     bool is_suspended_;
     bool on_started_has_been_called_;
     bool on_started_using_gpu_decode_has_been_called_;
@@ -104,9 +111,16 @@ class BroadcastingReceiver : public mojom::Receiver {
     void IncreaseConsumerCount();
     void DecreaseConsumerCount();
     bool IsStillBeingConsumed() const;
-    media::mojom::VideoBufferHandlePtr CloneBufferHandle();
+    media::mojom::VideoBufferHandlePtr CloneBufferHandle(
+        media::VideoCaptureBufferType target_buffer_type);
 
    private:
+    // If the source handle is shared_memory_via_raw_file_descriptor, we first
+    // have to unwrap it before we can clone it. Instead of unwrapping, cloning,
+    // and than wrapping back each time we need to clone it, we convert it to
+    // a regular shared memory and keep it in this form.
+    void ConvertRawFileDescriptorToSharedBuffer();
+
     int32_t buffer_id_;
     media::mojom::VideoBufferHandlePtr buffer_handle_;
     // Indicates how many consumers are currently relying on

services/video_capture/push_video_stream_subscription_impl.cc

@@ -70,7 +70,8 @@ void PushVideoStreamSubscriptionImpl::OnDeviceStartFailed() {
 void PushVideoStreamSubscriptionImpl::Activate() {
   if (status_ != Status::kNotYetActivated)
     return;
-  broadcaster_client_id_ = broadcaster_->AddClient(std::move(subscriber_));
+  broadcaster_client_id_ = broadcaster_->AddClient(
+      std::move(subscriber_), requested_settings_.buffer_type);
   status_ = Status::kActive;
 }