Add base::ReadOnlySharedMemoryRegion to media::mojom::VideoBufferHandle

This is the first in a series of changes to migrate the video capture
stack over to the new Chromium shmem APIs. This change adds the use of
base::ReadOnlySharedMemoryRegion as an alternative to mojo shared_buffer
handles.

Bug: 797470,843117
Change-Id: Ifa51a4984f48376a5da4b437618d301669f1ae1e
Reviewed-on: https://chromium-review.googlesource.com/1164637Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581347}

content/browser/renderer_host/media/video_capture_controller.cc

@@ -174,6 +174,9 @@ VideoCaptureController::BufferContext::CloneBufferHandle() {
     result->set_shared_buffer_handle(
         buffer_handle_->get_shared_buffer_handle()->Clone(
             mojo::SharedBufferHandle::AccessMode::READ_WRITE));
+  } else if (buffer_handle_->is_read_only_shmem_region()) {
+    result->set_read_only_shmem_region(
+        buffer_handle_->get_read_only_shmem_region().Duplicate());
   } else if (buffer_handle_->is_mailbox_handles()) {
     result->set_mailbox_handles(buffer_handle_->get_mailbox_handles()->Clone());
   } else {

content/renderer/media/video_capture_impl.cc

@@ -41,6 +41,10 @@ struct VideoCaptureImpl::BufferContext
         InitializeFromSharedMemory(
             std::move(buffer_handle->get_shared_buffer_handle()));
         break;
+      case VideoFrameBufferHandleType::READ_ONLY_SHMEM_REGION:
+        InitializeFromReadOnlyShmemRegion(
+            std::move(buffer_handle->get_read_only_shmem_region()));
+        break;
       case VideoFrameBufferHandleType::SHARED_MEMORY_VIA_RAW_FILE_DESCRIPTOR:
         NOTREACHED();
         break;
@@ -51,8 +55,10 @@ struct VideoCaptureImpl::BufferContext
   }
 
   VideoFrameBufferHandleType buffer_type() const { return buffer_type_; }
-  base::SharedMemory* shared_memory() { return shared_memory_.get(); }
+  base::SharedMemory* shared_memory() const { return shared_memory_.get(); }
   size_t shared_memory_size() const { return shared_memory_size_; }
+  const void* read_only_shmem() const { return read_only_mapping_.memory(); }
+  size_t read_only_shmem_size() const { return read_only_mapping_.size(); }
   const std::vector<gpu::MailboxHolder>& mailbox_holders() const {
     return mailbox_holders_;
   }
@@ -79,6 +85,13 @@ struct VideoCaptureImpl::BufferContext
     }
   }
 
+  void InitializeFromReadOnlyShmemRegion(
+      base::ReadOnlySharedMemoryRegion region) {
+    DCHECK(region.IsValid());
+    read_only_mapping_ = region.Map();
+    DCHECK(read_only_mapping_.IsValid());
+  }
+
   void InitializeFromMailbox(
       media::mojom::MailboxBufferHandleSetPtr mailbox_handles) {
     DCHECK_EQ(media::VideoFrame::kMaxPlanes,
@@ -91,11 +104,14 @@ struct VideoCaptureImpl::BufferContext
 
   VideoFrameBufferHandleType buffer_type_;
 
-  // Only valid for |buffer_type_ == kSharedMemory|.
+  // Only valid for |buffer_type_ == SHARED_BUFFER_HANDLE|.
   std::unique_ptr<base::SharedMemory> shared_memory_;
   size_t shared_memory_size_;
 
-  // Only valid for |buffer_type_ == kMailboxHolder|.
+  // Only valid for |buffer_type_ == READ_ONLY_SHMEM_REGION|.
+  base::ReadOnlySharedMemoryMapping read_only_mapping_;
+
+  // Only valid for |buffer_type_ == MAILBOX_HANDLES|.
   std::vector<gpu::MailboxHolder> mailbox_holders_;
 
   DISALLOW_COPY_AND_ASSIGN(BufferContext);
@@ -349,6 +365,14 @@ void VideoCaptureImpl::OnBufferReady(int32_t buffer_id,
           buffer_context->shared_memory()->handle(),
           0 /* shared_memory_offset */, info->timestamp);
       break;
+    case VideoFrameBufferHandleType::READ_ONLY_SHMEM_REGION:
+      frame = media::VideoFrame::WrapExternalData(
+          static_cast<media::VideoPixelFormat>(info->pixel_format),
+          info->coded_size, info->visible_rect, info->visible_rect.size(),
+          const_cast<uint8_t*>(
+              static_cast<const uint8_t*>(buffer_context->read_only_shmem())),
+          buffer_context->read_only_shmem_size(), info->timestamp);
+      break;
     case VideoFrameBufferHandleType::SHARED_MEMORY_VIA_RAW_FILE_DESCRIPTOR:
       NOTREACHED();
       break;
@@ -402,7 +426,7 @@ void VideoCaptureImpl::OnAllClientsFinishedConsumingFrame(
     double consumer_resource_utilization) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
 
-// Subtle race note: It's important that the |buffer| argument be
+// Subtle race note: It's important that the |buffer_context| argument be
 // std::move()'ed to this method and never copied. This is so that the caller,
 // DidFinishConsumingFrame(), does not implicitly retain a reference while it
 // is running the trampoline callback on another thread. This is necessary to

content/renderer/media/video_capture_impl_unittest.cc

@@ -5,6 +5,7 @@
 #include <stddef.h>
 
 #include "base/macros.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/shared_memory.h"
 #include "base/test/scoped_task_environment.h"
 #include "content/child/child_process.h"
@@ -146,12 +147,19 @@ class VideoCaptureImplTest : public ::testing::Test {
   }
 
   void SimulateOnBufferCreated(int buffer_id, const base::SharedMemory& shm) {
-    media::mojom::VideoBufferHandlePtr buffer_handle =
-        media::mojom::VideoBufferHandle::New();
-    buffer_handle->set_shared_buffer_handle(mojo::WrapSharedMemoryHandle(
-        shm.GetReadOnlyHandle(), shm.mapped_size(),
-        mojo::UnwrappedSharedMemoryHandleProtection::kReadOnly));
-    video_capture_impl_->OnNewBuffer(buffer_id, std::move(buffer_handle));
+    video_capture_impl_->OnNewBuffer(
+        buffer_id,
+        media::mojom::VideoBufferHandle::NewSharedBufferHandle(
+            mojo::WrapSharedMemoryHandle(
+                shm.GetReadOnlyHandle(), shm.mapped_size(),
+                mojo::UnwrappedSharedMemoryHandleProtection::kReadOnly)));
+  }
+
+  void SimulateReadOnlyBufferCreated(int buffer_id,
+                                     base::ReadOnlySharedMemoryRegion region) {
+    video_capture_impl_->OnNewBuffer(
+        buffer_id, media::mojom::VideoBufferHandle::NewReadOnlyShmemRegion(
+                       std::move(region)));
   }
 
   void SimulateBufferReceived(int buffer_id, const gfx::Size& size) {
@@ -284,7 +292,7 @@ TEST_F(VideoCaptureImplTest, GetDeviceFormatsInUse) {
 }
 
 TEST_F(VideoCaptureImplTest, BufferReceived) {
-  const int kBufferId = 11;
+  const int kArbitraryBufferId = 11;
 
   base::SharedMemory shm;
   const size_t frame_size = media::VideoFrame::AllocationSize(
@@ -296,20 +304,48 @@ TEST_F(VideoCaptureImplTest, BufferReceived) {
   EXPECT_CALL(*this, OnFrameReady(_, _));
   EXPECT_CALL(mock_video_capture_host_, DoStart(_, kSessionId, params_small_));
   EXPECT_CALL(mock_video_capture_host_, Stop(_));
-  EXPECT_CALL(mock_video_capture_host_, ReleaseBuffer(_, kBufferId, _))
+  EXPECT_CALL(mock_video_capture_host_, ReleaseBuffer(_, kArbitraryBufferId, _))
+      .Times(0);
+
+  StartCapture(0, params_small_);
+  SimulateOnBufferCreated(kArbitraryBufferId, shm);
+  SimulateBufferReceived(kArbitraryBufferId,
+                         params_small_.requested_format.frame_size);
+  StopCapture(0);
+  SimulateBufferDestroyed(kArbitraryBufferId);
+
+  EXPECT_EQ(mock_video_capture_host_.released_buffer_count(), 0);
+}
+
+TEST_F(VideoCaptureImplTest, BufferReceived_ReadOnlyShmemRegion) {
+  const int kArbitraryBufferId = 11;
+
+  const size_t frame_size = media::VideoFrame::AllocationSize(
+      media::PIXEL_FORMAT_I420, params_small_.requested_format.frame_size);
+  base::MappedReadOnlyRegion shm =
+      base::ReadOnlySharedMemoryRegion::Create(frame_size);
+  ASSERT_TRUE(shm.IsValid());
+
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STOPPED));
+  EXPECT_CALL(*this, OnFrameReady(_, _));
+  EXPECT_CALL(mock_video_capture_host_, DoStart(_, kSessionId, params_small_));
+  EXPECT_CALL(mock_video_capture_host_, Stop(_));
+  EXPECT_CALL(mock_video_capture_host_, ReleaseBuffer(_, kArbitraryBufferId, _))
       .Times(0);
 
   StartCapture(0, params_small_);
-  SimulateOnBufferCreated(kBufferId, shm);
-  SimulateBufferReceived(kBufferId, params_small_.requested_format.frame_size);
+  SimulateReadOnlyBufferCreated(kArbitraryBufferId, std::move(shm.region));
+  SimulateBufferReceived(kArbitraryBufferId,
+                         params_small_.requested_format.frame_size);
   StopCapture(0);
-  SimulateBufferDestroyed(kBufferId);
+  SimulateBufferDestroyed(kArbitraryBufferId);
 
   EXPECT_EQ(mock_video_capture_host_.released_buffer_count(), 0);
 }
 
 TEST_F(VideoCaptureImplTest, BufferReceivedAfterStop) {
-  const int kBufferId = 12;
+  const int kArbitraryBufferId = 12;
 
   base::SharedMemory shm;
   const size_t frame_size = media::VideoFrame::AllocationSize(
@@ -321,14 +357,44 @@ TEST_F(VideoCaptureImplTest, BufferReceivedAfterStop) {
   EXPECT_CALL(*this, OnFrameReady(_, _)).Times(0);
   EXPECT_CALL(mock_video_capture_host_, DoStart(_, kSessionId, params_large_));
   EXPECT_CALL(mock_video_capture_host_, Stop(_));
-  EXPECT_CALL(mock_video_capture_host_, ReleaseBuffer(_, kBufferId, _));
+  EXPECT_CALL(mock_video_capture_host_,
+              ReleaseBuffer(_, kArbitraryBufferId, _));
 
   StartCapture(0, params_large_);
-  SimulateOnBufferCreated(kBufferId, shm);
+  SimulateOnBufferCreated(kArbitraryBufferId, shm);
   StopCapture(0);
   // A buffer received after StopCapture() triggers an instant ReleaseBuffer().
-  SimulateBufferReceived(kBufferId, params_large_.requested_format.frame_size);
-  SimulateBufferDestroyed(kBufferId);
+  SimulateBufferReceived(kArbitraryBufferId,
+                         params_large_.requested_format.frame_size);
+  SimulateBufferDestroyed(kArbitraryBufferId);
+
+  EXPECT_EQ(mock_video_capture_host_.released_buffer_count(), 1);
+}
+
+TEST_F(VideoCaptureImplTest, BufferReceivedAfterStop_ReadOnlyShmemRegion) {
+  const int kArbitraryBufferId = 12;
+
+  const size_t frame_size = media::VideoFrame::AllocationSize(
+      media::PIXEL_FORMAT_I420, params_large_.requested_format.frame_size);
+  base::MappedReadOnlyRegion shm =
+      base::ReadOnlySharedMemoryRegion::Create(frame_size);
+  ASSERT_TRUE(shm.IsValid());
+
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STOPPED));
+  EXPECT_CALL(*this, OnFrameReady(_, _)).Times(0);
+  EXPECT_CALL(mock_video_capture_host_, DoStart(_, kSessionId, params_large_));
+  EXPECT_CALL(mock_video_capture_host_, Stop(_));
+  EXPECT_CALL(mock_video_capture_host_,
+              ReleaseBuffer(_, kArbitraryBufferId, _));
+
+  StartCapture(0, params_large_);
+  SimulateReadOnlyBufferCreated(kArbitraryBufferId, std::move(shm.region));
+  StopCapture(0);
+  // A buffer received after StopCapture() triggers an instant ReleaseBuffer().
+  SimulateBufferReceived(kArbitraryBufferId,
+                         params_large_.requested_format.frame_size);
+  SimulateBufferDestroyed(kArbitraryBufferId);
 
   EXPECT_EQ(mock_video_capture_host_.released_buffer_count(), 1);
 }
@@ -377,7 +443,7 @@ TEST_F(VideoCaptureImplTest, ErrorBeforeStop) {
 }
 
 TEST_F(VideoCaptureImplTest, BufferReceivedBeforeOnStarted) {
-  const int kBufferId = 16;
+  const int kArbitraryBufferId = 16;
 
   base::SharedMemory shm;
   const size_t frame_size = media::VideoFrame::AllocationSize(
@@ -387,10 +453,46 @@ TEST_F(VideoCaptureImplTest, BufferReceivedBeforeOnStarted) {
   InSequence s;
   EXPECT_CALL(mock_video_capture_host_, DoStart(_, kSessionId, params_small_))
       .WillOnce(DoNothing());
-  EXPECT_CALL(mock_video_capture_host_, ReleaseBuffer(_, kBufferId, _));
+  EXPECT_CALL(mock_video_capture_host_,
+              ReleaseBuffer(_, kArbitraryBufferId, _));
+  StartCapture(0, params_small_);
+  SimulateOnBufferCreated(kArbitraryBufferId, shm);
+  SimulateBufferReceived(kArbitraryBufferId,
+                         params_small_.requested_format.frame_size);
+
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
+  EXPECT_CALL(mock_video_capture_host_, RequestRefreshFrame(_));
+  video_capture_impl_->OnStateChanged(media::mojom::VideoCaptureState::STARTED);
+
+  // Additional STARTED will cause RequestRefreshFrame a second time.
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
+  EXPECT_CALL(mock_video_capture_host_, RequestRefreshFrame(_));
+  video_capture_impl_->OnStateChanged(media::mojom::VideoCaptureState::STARTED);
+
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STOPPED));
+  EXPECT_CALL(mock_video_capture_host_, Stop(_));
+  StopCapture(0);
+}
+
+TEST_F(VideoCaptureImplTest,
+       BufferReceivedBeforeOnStarted_ReadOnlyShmemRegion) {
+  const int kArbitraryBufferId = 16;
+
+  const size_t frame_size = media::VideoFrame::AllocationSize(
+      media::PIXEL_FORMAT_I420, params_small_.requested_format.frame_size);
+  base::MappedReadOnlyRegion shm =
+      base::ReadOnlySharedMemoryRegion::Create(frame_size);
+  ASSERT_TRUE(shm.IsValid());
+
+  InSequence s;
+  EXPECT_CALL(mock_video_capture_host_, DoStart(_, kSessionId, params_small_))
+      .WillOnce(DoNothing());
+  EXPECT_CALL(mock_video_capture_host_,
+              ReleaseBuffer(_, kArbitraryBufferId, _));
   StartCapture(0, params_small_);
-  SimulateOnBufferCreated(kBufferId, shm);
-  SimulateBufferReceived(kBufferId, params_small_.requested_format.frame_size);
+  SimulateReadOnlyBufferCreated(kArbitraryBufferId, std::move(shm.region));
+  SimulateBufferReceived(kArbitraryBufferId,
+                         params_small_.requested_format.frame_size);
 
   EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
   EXPECT_CALL(mock_video_capture_host_, RequestRefreshFrame(_));

media/capture/mojom/video_capture_types.mojom

@@ -5,6 +5,7 @@
 module media.mojom;
 
 import "gpu/ipc/common/mailbox_holder.mojom";
+import "mojo/public/mojom/base/shared_memory.mojom";
 import "mojo/public/mojom/base/time.mojom";
 import "mojo/public/mojom/base/values.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
@@ -144,6 +145,7 @@ struct SharedMemoryViaRawFileDescriptor {
 
 union VideoBufferHandle {
   handle<shared_buffer> shared_buffer_handle;
+  mojo_base.mojom.ReadOnlySharedMemoryRegion read_only_shmem_region;
   SharedMemoryViaRawFileDescriptor shared_memory_via_raw_file_descriptor;
   MailboxBufferHandleSet mailbox_handles;
 };