Mirroring Service: Support shared buffer handles for video capturing.

VideoCaptureClient was recently changed to only support the new
base::ReadOnlySharedMemoryRegion being passed to the client. However,
for desktop capturing, the read-write shared buffer handles are still
passed by media::VideoCaptureDeviceClient. This CL temporally added the
support of shared buffer handles back before the
media::VideoCaptureDeviceClient is migrated to the new read-only API.

Bug: 734672,843117
Change-Id: Ib3f6d58aa2541abfb29f1df014f1a01cfef50ef0
Reviewed-on: https://chromium-review.googlesource.com/1204721Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: Xiangjun Zhang <xjz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589252}

components/mirroring/service/video_capture_client.cc

@@ -92,6 +92,7 @@ void VideoCaptureClient::OnStateChanged(media::mojom::VideoCaptureState state) {
     case media::mojom::VideoCaptureState::STOPPED:
     case media::mojom::VideoCaptureState::ENDED:
       client_buffers_.clear();
+      mapped_buffers_.clear();
       weak_factory_.InvalidateWeakPtrs();
       error_callback_.Reset();
       frame_deliver_callback_.Reset();
@@ -106,12 +107,13 @@ void VideoCaptureClient::OnNewBuffer(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DVLOG(3) << __func__ << ": buffer_id=" << buffer_id;
 
-  if (!buffer_handle->is_read_only_shmem_region()) {
+  if (!buffer_handle->is_read_only_shmem_region() &&
+      !buffer_handle->is_shared_buffer_handle()) {
     NOTIMPLEMENTED();
     return;
   }
-  const auto insert_result = client_buffers_.emplace(std::make_pair(
-      buffer_id, std::move(buffer_handle->get_read_only_shmem_region())));
+  const auto insert_result = client_buffers_.emplace(
+      std::make_pair(buffer_id, std::move(buffer_handle)));
   DCHECK(insert_result.second);
 }
 
@@ -160,29 +162,65 @@ void VideoCaptureClient::OnBufferReady(int32_t buffer_id,
     LOG(DFATAL) << "Ignoring OnBufferReady() for unknown buffer.";
     return;
   }
-
-  base::ReadOnlySharedMemoryMapping mapping = buffer_iter->second.Map();
-  const size_t frame_allocation_size =
-      media::VideoFrame::AllocationSize(info->pixel_format, info->coded_size);
   scoped_refptr<media::VideoFrame> frame;
-  if (mapping.IsValid() && mapping.size() >= frame_allocation_size) {
+  BufferFinishedCallback buffer_finished_callback;
+  if (buffer_iter->second->is_shared_buffer_handle()) {
+    // TODO(https://crbug.com/843117): Remove this case after migrating
+    // media::VideoCaptureDeviceClient to the new shared memory API.
+    auto mapping_iter = mapped_buffers_.find(buffer_id);
+    const size_t buffer_size =
+        media::VideoFrame::AllocationSize(info->pixel_format, info->coded_size);
+    if (mapping_iter != mapped_buffers_.end() &&
+        buffer_size > mapping_iter->second.second) {
+      // Unmaps shared memory for too-small region.
+      mapped_buffers_.erase(mapping_iter);
+      mapping_iter = mapped_buffers_.end();
+    }
+    if (mapping_iter == mapped_buffers_.end()) {
+      mojo::ScopedSharedBufferMapping mapping =
+          buffer_iter->second->get_shared_buffer_handle()->Map(buffer_size);
+      if (!mapping) {
+        video_capture_host_->ReleaseBuffer(kDeviceId, buffer_id, -1.0);
+        return;
+      }
+      mapping_iter =
+          mapped_buffers_
+              .emplace(std::make_pair(
+                  buffer_id, MappingAndSize(std::move(mapping), buffer_size)))
+              .first;
+    }
+    const auto& buffer = mapping_iter->second;
     frame = media::VideoFrame::WrapExternalData(
         info->pixel_format, info->coded_size, info->visible_rect,
         info->visible_rect.size(),
-        const_cast<uint8_t*>(static_cast<const uint8_t*>(mapping.memory())),
-        frame_allocation_size, info->timestamp);
+        reinterpret_cast<uint8_t*>(buffer.first.get()), buffer.second,
+        info->timestamp);
+    buffer_finished_callback = media::BindToCurrentLoop(base::BindOnce(
+        &VideoCaptureClient::OnClientBufferFinished, weak_factory_.GetWeakPtr(),
+        buffer_id, base::ReadOnlySharedMemoryMapping()));
+  } else {
+    base::ReadOnlySharedMemoryMapping mapping =
+        buffer_iter->second->get_read_only_shmem_region().Map();
+    const size_t frame_allocation_size =
+        media::VideoFrame::AllocationSize(info->pixel_format, info->coded_size);
+    if (mapping.IsValid() && mapping.size() >= frame_allocation_size) {
+      frame = media::VideoFrame::WrapExternalData(
+          info->pixel_format, info->coded_size, info->visible_rect,
+          info->visible_rect.size(),
+          const_cast<uint8_t*>(static_cast<const uint8_t*>(mapping.memory())),
+          frame_allocation_size, info->timestamp);
+    }
+    buffer_finished_callback = media::BindToCurrentLoop(base::BindOnce(
+        &VideoCaptureClient::OnClientBufferFinished, weak_factory_.GetWeakPtr(),
+        buffer_id, std::move(mapping)));
   }
+
   if (!frame) {
     LOG(DFATAL) << "Unable to wrap shared memory mapping.";
     video_capture_host_->ReleaseBuffer(kDeviceId, buffer_id, -1.0);
     OnStateChanged(media::mojom::VideoCaptureState::FAILED);
     return;
   }
-
-  BufferFinishedCallback buffer_finished_callback =
-      media::BindToCurrentLoop(base::BindOnce(
-          &VideoCaptureClient::OnClientBufferFinished,
-          weak_factory_.GetWeakPtr(), buffer_id, std::move(mapping)));
   frame->AddDestructionObserver(
       base::BindOnce(&VideoCaptureClient::DidFinishConsumingFrame,
                      frame->metadata(), std::move(buffer_finished_callback)));
@@ -200,6 +238,9 @@ void VideoCaptureClient::OnBufferDestroyed(int32_t buffer_id) {
   const auto& buffer_iter = client_buffers_.find(buffer_id);
   if (buffer_iter != client_buffers_.end())
     client_buffers_.erase(buffer_iter);
+  const auto& mapping_iter = mapped_buffers_.find(buffer_id);
+  if (mapping_iter != mapped_buffers_.end())
+    mapped_buffers_.erase(mapping_iter);
 }
 
 void VideoCaptureClient::OnClientBufferFinished(
@@ -211,6 +252,7 @@ void VideoCaptureClient::OnClientBufferFinished(
 
   // Buffer was already destroyed.
   if (client_buffers_.find(buffer_id) == client_buffers_.end()) {
+    DCHECK(mapped_buffers_.find(buffer_id) == mapped_buffers_.end());
     return;
   }
 

components/mirroring/service/video_capture_client.h

@@ -78,8 +78,11 @@ class COMPONENT_EXPORT(MIRRORING_SERVICE) VideoCaptureClient
 
   mojo::Binding<media::mojom::VideoCaptureObserver> binding_;
 
+  // TODO(https://crbug.com/843117): Store the
+  // base::ReadOnlySharedMemoryRegion instead after migrating the
+  // media::VideoCaptureDeviceClient to the new shared memory API.
   using ClientBufferMap =
-      base::flat_map<int32_t, base::ReadOnlySharedMemoryRegion>;
+      base::flat_map<int32_t, media::mojom::VideoBufferHandlePtr>;
   // Stores the buffer handler on OnBufferCreated(). |buffer_id| is the key.
   ClientBufferMap client_buffers_;
 
@@ -90,6 +93,15 @@ class COMPONENT_EXPORT(MIRRORING_SERVICE) VideoCaptureClient
   // The callback to deliver the received frame.
   FrameDeliverCallback frame_deliver_callback_;
 
+  // TODO(https://crbug.com/843117): Remove the MappingMap after migrating
+  // media::VideoCaptureDeviceClient to the new shared memory API.
+  using MappingAndSize = std::pair<mojo::ScopedSharedBufferMapping, uint32_t>;
+  using MappingMap = base::flat_map<int32_t, MappingAndSize>;
+  // Stores the mapped buffers and their size. Each buffer is added the first
+  // time the mapping is done or a larger size is requested.
+  // |buffer_id| is the key to this map.
+  MappingMap mapped_buffers_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::WeakPtrFactory<VideoCaptureClient> weak_factory_;

components/mirroring/service/video_capture_client_unittest.cc

@@ -23,6 +23,8 @@ namespace mirroring {
 
 namespace {
 
+constexpr double kUtilization = 0.6;
+
 media::mojom::VideoFrameInfoPtr GetVideoFrameInfo(const gfx::Size& size) {
   media::VideoFrameMetadata metadata;
   metadata.SetDouble(media::VideoFrameMetadata::FRAME_RATE, 30);
@@ -36,7 +38,8 @@ media::mojom::VideoFrameInfoPtr GetVideoFrameInfo(const gfx::Size& size) {
 
 }  // namespace
 
-class VideoCaptureClientTest : public ::testing::Test {
+class VideoCaptureClientTest : public ::testing::Test,
+                               public ::testing::WithParamInterface<bool> {
  public:
   VideoCaptureClientTest() {
     media::mojom::VideoCaptureHostPtr host;
@@ -60,72 +63,80 @@ class VideoCaptureClientTest : public ::testing::Test {
   MOCK_METHOD1(OnFrameReceived, void(const gfx::Size&));
   void OnFrameReady(scoped_refptr<media::VideoFrame> video_frame) {
     video_frame->metadata()->SetDouble(
-        media::VideoFrameMetadata::RESOURCE_UTILIZATION, 0.6);
+        media::VideoFrameMetadata::RESOURCE_UTILIZATION, kUtilization);
     OnFrameReceived(video_frame->coded_size());
   }
 
  protected:
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
-  std::unique_ptr<FakeVideoCaptureHost> host_impl_;
-  std::unique_ptr<VideoCaptureClient> client_;
-};
-
-TEST_F(VideoCaptureClientTest, Basic) {
-  base::MockCallback<base::OnceClosure> error_cb;
-  EXPECT_CALL(error_cb, Run()).Times(0);
-  {
+  void StartCapturing() {
+    EXPECT_CALL(error_cb_, Run()).Times(0);
     base::RunLoop run_loop;
     // Expect to call RequestRefreshFrame() after capturing started.
     EXPECT_CALL(*host_impl_, RequestRefreshFrame(_))
         .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
     client_->Start(base::BindRepeating(&VideoCaptureClientTest::OnFrameReady,
                                        base::Unretained(this)),
-                   error_cb.Get());
+                   error_cb_.Get());
     run_loop.Run();
+    scoped_task_environment_.RunUntilIdle();
   }
-  scoped_task_environment_.RunUntilIdle();
 
-  client_->OnNewBuffer(
-      0, media::mojom::VideoBufferHandle::NewReadOnlyShmemRegion(
-             mojo::CreateReadOnlySharedMemoryRegion(100000).region));
-  scoped_task_environment_.RunUntilIdle();
-  {
-    base::RunLoop run_loop;
-    // Expect to receive one frame.
-    EXPECT_CALL(*this, OnFrameReceived(gfx::Size(128, 64))).Times(1);
-    // Expect to return the buffer after the frame is consumed.
-    EXPECT_CALL(*host_impl_, ReleaseBuffer(_, 0, 0.6))
-        .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
-    client_->OnBufferReady(0, GetVideoFrameInfo(gfx::Size(128, 64)));
-    run_loop.Run();
+  void OnNewBuffer(int buffer_id, int buffer_size) {
+    EXPECT_CALL(error_cb_, Run()).Times(0);
+    const bool use_shared_buffer = GetParam();
+    if (use_shared_buffer) {
+      client_->OnNewBuffer(
+          buffer_id, media::mojom::VideoBufferHandle::NewSharedBufferHandle(
+                         mojo::SharedBufferHandle::Create(buffer_size)));
+    } else {
+      client_->OnNewBuffer(
+          buffer_id,
+          media::mojom::VideoBufferHandle::NewReadOnlyShmemRegion(
+              mojo::CreateReadOnlySharedMemoryRegion(buffer_size).region));
+    }
+    scoped_task_environment_.RunUntilIdle();
   }
-  scoped_task_environment_.RunUntilIdle();
 
-  // Received a smaller size video frame in the same buffer.
-  {
+  void OnBufferReady(int buffer_id, const gfx::Size& frame_size) {
+    EXPECT_CALL(error_cb_, Run()).Times(0);
     base::RunLoop run_loop;
-    // Expect to receive one frame.
-    EXPECT_CALL(*this, OnFrameReceived(gfx::Size(64, 32))).Times(1);
-    // Expect to return the buffer after the frame is consumed.
-    EXPECT_CALL(*host_impl_, ReleaseBuffer(_, 0, 0.6))
+    // Expects to receive one frame.
+    EXPECT_CALL(*this, OnFrameReceived(frame_size)).Times(1);
+    // Expects to return the buffer after the frame is consumed.
+    EXPECT_CALL(*host_impl_, ReleaseBuffer(_, 0, kUtilization))
         .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
-    client_->OnBufferReady(0, GetVideoFrameInfo(gfx::Size(64, 32)));
+    client_->OnBufferReady(buffer_id, GetVideoFrameInfo(frame_size));
     run_loop.Run();
+    scoped_task_environment_.RunUntilIdle();
   }
-  scoped_task_environment_.RunUntilIdle();
 
-  // Received a larger size video frame in the same buffer.
-  {
-    base::RunLoop run_loop;
-    // Expect to receive one frame.
-    EXPECT_CALL(*this, OnFrameReceived(gfx::Size(320, 180))).Times(1);
-    // Expect to return the buffer after the frame is consumed.
-    EXPECT_CALL(*host_impl_, ReleaseBuffer(_, 0, 0.6))
-        .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
-    client_->OnBufferReady(0, GetVideoFrameInfo(gfx::Size(320, 180)));
-    run_loop.Run();
-  }
-  scoped_task_environment_.RunUntilIdle();
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  base::MockCallback<base::OnceClosure> error_cb_;
+  std::unique_ptr<FakeVideoCaptureHost> host_impl_;
+  std::unique_ptr<VideoCaptureClient> client_;
+
+  DISALLOW_COPY_AND_ASSIGN(VideoCaptureClientTest);
+};
+
+TEST_P(VideoCaptureClientTest, Basic) {
+  StartCapturing();
+
+  // A new buffer is created.
+  OnNewBuffer(0, 100000);
+
+  // One captured frame is ready. Expects to receive the frame.
+  OnBufferReady(0, gfx::Size(126, 64));
+
+  // A smaller size video frame is received in the same buffer.
+  OnBufferReady(0, gfx::Size(64, 32));
+
+  // A larger size video frame is received in the same buffer.
+  OnBufferReady(0, gfx::Size(320, 180));
 }
 
+INSTANTIATE_TEST_CASE_P(,
+                        VideoCaptureClientTest,
+                        ::testing::Values(true, false));
+
 }  // namespace mirroring