Fix preemptive mapping of DXGI buffers

Previously, a shared memory region was created and filled with pixel
data during capture if requested by a client. But that doesn't work with
current video capture service design: first, a GMB handle is passed once
and then the client is notified when a frame is available in that handle.

To make preemptive mapping work with that design, a shared memory region
is now always created together with the DXGI GMB handle in the capture
service. Then, when a frame is captured and preemptive mapping is needed,
the region is filled with data and a special flag is passed to the
consumer in VideoFrameInfo. Based on that flag a previously received
shared memory region is passed to the frame buffer.

Bug: 1120900
Change-Id: I6f55be94bab76e9f2d8f5ec0e3b7f7e8badb8fa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2905782
Commit-Queue: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#886340}

components/mirroring/browser/single_client_video_capture_host_unittest.cc

@@ -22,6 +22,8 @@ namespace mirroring {
 
 namespace {
 
+constexpr bool kNotPremapped = false;
+
 class MockVideoCaptureDevice final
     : public content::LaunchedVideoCaptureDevice {
  public:
@@ -164,8 +166,8 @@ class MockVideoCaptureObserver final
 media::mojom::VideoFrameInfoPtr GetVideoFrameInfo() {
   return media::mojom::VideoFrameInfo::New(
       base::TimeDelta(), media::VideoFrameMetadata(), media::PIXEL_FORMAT_I420,
-      gfx::Size(320, 180), gfx::Rect(320, 180), gfx::ColorSpace::CreateREC709(),
-      nullptr);
+      gfx::Size(320, 180), gfx::Rect(320, 180), kNotPremapped,
+      gfx::ColorSpace::CreateREC709(), nullptr);
 }
 
 }  // namespace

components/mirroring/service/fake_video_capture_host.cc

@@ -10,6 +10,13 @@
 
 namespace mirroring {
 
+namespace {
+
+// Video buffer parameters.
+constexpr bool kNotPremapped = false;
+
+}  // namespace
+
 FakeVideoCaptureHost::FakeVideoCaptureHost(
     mojo::PendingReceiver<media::mojom::VideoCaptureHost> receiver)
     : receiver_(this, std::move(receiver)) {}
@@ -50,7 +57,8 @@ void FakeVideoCaptureHost::SendOneFrame(const gfx::Size& size,
   media::mojom::ReadyBufferPtr buffer = media::mojom::ReadyBuffer::New(
       0, media::mojom::VideoFrameInfo::New(
              base::TimeDelta(), metadata, media::PIXEL_FORMAT_I420, size,
-             gfx::Rect(size), gfx::ColorSpace::CreateREC709(), nullptr));
+             gfx::Rect(size), kNotPremapped, gfx::ColorSpace::CreateREC709(),
+             nullptr));
   observer_->OnBufferReady(std::move(buffer), {});
 }
 

components/mirroring/service/video_capture_client_unittest.cc

@@ -26,13 +26,15 @@ namespace {
 
 const media::VideoCaptureFeedback kFeedback(0.6, 30.0, 1000);
 
+constexpr bool kNotPremapped = false;
+
 media::mojom::VideoFrameInfoPtr GetVideoFrameInfo(const gfx::Size& size) {
   media::VideoFrameMetadata metadata;
   metadata.frame_rate = 30;
   metadata.reference_time = base::TimeTicks();
   return media::mojom::VideoFrameInfo::New(
       base::TimeDelta(), metadata, media::PIXEL_FORMAT_I420, size,
-      gfx::Rect(size), gfx::ColorSpace::CreateREC709(), nullptr);
+      gfx::Rect(size), kNotPremapped, gfx::ColorSpace::CreateREC709(), nullptr);
 }
 
 }  // namespace

content/browser/devtools/devtools_video_consumer_unittest.cc

@@ -27,6 +27,9 @@ namespace {
 constexpr gfx::Size kResolution = gfx::Size(320, 180);  // Arbitrarily chosen.
 constexpr media::VideoPixelFormat kFormat = media::PIXEL_FORMAT_I420;
 
+// Video buffer parameters.
+constexpr bool kNotPremapped = false;
+
 // A non-zero FrameSinkId to prevent validation errors when
 // DevToolsVideoConsumer::ChangeTarget(viz::FrameSinkId) is called
 // (which eventually fails in FrameSinkVideoCapturerStubDispatch::Accept).
@@ -193,7 +196,8 @@ class DevToolsVideoConsumerTest : public testing::Test {
 
     media::mojom::VideoFrameInfoPtr info = media::mojom::VideoFrameInfo::New(
         base::TimeDelta(), media::VideoFrameMetadata(), kFormat, kResolution,
-        gfx::Rect(kResolution), gfx::ColorSpace::CreateREC709(), nullptr);
+        gfx::Rect(kResolution), kNotPremapped, gfx::ColorSpace::CreateREC709(),
+        nullptr);
 
     consumer_->OnFrameCaptured(std::move(data), std::move(info),
                                gfx::Rect(kResolution),

content/browser/media/capture/frame_sink_video_capture_device_unittest.cc

@@ -70,6 +70,9 @@ constexpr base::TimeDelta kMinCapturePeriod = base::TimeDelta::FromMicroseconds(
     base::Time::kMicrosecondsPerSecond / kMaxFrameRate);
 constexpr media::VideoPixelFormat kFormat = media::PIXEL_FORMAT_I420;
 
+// Video buffer parameters.
+constexpr bool kNotPremapped = false;
+
 // Helper to return the capture parameters packaged in a VideoCaptureParams.
 media::VideoCaptureParams GetCaptureParams() {
   media::VideoCaptureParams params;
@@ -388,7 +391,7 @@ class FrameSinkVideoCaptureDeviceTest : public testing::Test {
               std::move(data),
               media::mojom::VideoFrameInfo::New(
                   kMinCapturePeriod * frame_number, media::VideoFrameMetadata(),
-                  kFormat, kResolution, gfx::Rect(kResolution),
+                  kFormat, kResolution, gfx::Rect(kResolution), kNotPremapped,
                   gfx::ColorSpace::CreateREC709(), nullptr),
               gfx::Rect(kResolution), std::move(callbacks_remote));
         },

media/capture/mojom/video_capture_buffer.mojom

@@ -19,6 +19,11 @@ struct VideoFrameInfo{
   VideoCapturePixelFormat pixel_format;
   gfx.mojom.Size coded_size;
   gfx.mojom.Rect visible_rect;
+  // Some buffer types may be preemtively mapped in the capturer.
+  // In that case a shared memory region is passed to the consumer together
+  // with a GMB handle, and this flag here is passed to notify the consumer
+  // that the region has valid data.
+  bool is_premapped;
   // This field is only optional to work around the issue of native enums
   // not being usable for non-Chromium Mojo clients.
   // TODO(chfremer): Make this non-optional once gfx.mojom.ColorSpace has been

media/capture/video/video_capture_device.h

@@ -137,6 +137,12 @@ class CAPTURE_EXPORT VideoCaptureDevice
       int frame_feedback_id;
       std::unique_ptr<HandleProvider> handle_provider;
       std::unique_ptr<ScopedAccessPermission> access_permission;
+
+      // Some buffer types may be preemptively mapped in the capturer if
+      // requested by the consumer.
+      // This is used to notify the client that a shared memory region
+      // associated with the buffer is valid.
+      bool is_premapped = false;
     };
 
     // Result code for calls to ReserveOutputBuffer()

media/capture/video/video_capture_device_client.cc

@@ -629,6 +629,7 @@ void VideoCaptureDeviceClient::OnIncomingCapturedBufferExt(
   info->coded_size = format.frame_size;
   info->visible_rect = visible_rect;
   info->metadata = metadata;
+  info->is_premapped = buffer.is_premapped;
 
   buffer_pool_->HoldForConsumers(buffer.id, 1);
   receiver_->OnFrameReadyInBuffer(

media/capture/video/win/gpu_memory_buffer_tracker.cc

@@ -101,6 +101,9 @@ bool GpuMemoryBufferTracker::CreateBufferInternal() {
     NOTREACHED() << "Failed to create GPU memory buffer";
     return false;
   }
+
+  region_ = base::UnsafeSharedMemoryRegion::Create(GetMemorySizeInBytes());
+
   return true;
 }
 
@@ -137,10 +140,8 @@ GpuMemoryBufferTracker::DuplicateAsUnsafeRegion() {
   if (!buffer_) {
     return base::UnsafeSharedMemoryRegion();
   }
-  const auto data_size = GetMemorySizeInBytes();
-  if (!region_.IsValid() || region_.GetSize() < data_size) {
-    region_ = base::UnsafeSharedMemoryRegion::Create(data_size);
-  }
+
+  CHECK(region_.IsValid());
 
   if (!gpu::CopyDXGIBufferToShMem(buffer_->GetHandle(), region_.Duplicate(),
                                   d3d_device_.Get(), &staging_texture_)) {
@@ -160,7 +161,9 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferTracker::GetGpuMemoryBufferHandle() {
   if (!EnsureD3DDevice()) {
     return gfx::GpuMemoryBufferHandle();
   }
-  return buffer_->CloneHandle();
+  auto handle = buffer_->CloneHandle();
+  handle.region = region_.Duplicate();
+  return handle;
 }
 
 uint32_t GpuMemoryBufferTracker::GetMemorySizeInBytes() {

media/capture/video/win/video_capture_device_mf_win.cc

@@ -1497,9 +1497,15 @@ HRESULT VideoCaptureDeviceMFWin::DeliverTextureToClient(
   auto gmb_handle = capture_buffer.handle_provider->GetGpuMemoryBufferHandle();
   hr = CopyTextureToGpuMemoryBuffer(texture, gmb_handle.dxgi_handle.Get());
 
+  capture_buffer.is_premapped = false;
   if (last_feedback_.require_mapped_frame) {
-    gmb_handle.region =
-        capture_buffer.handle_provider->DuplicateAsUnsafeRegion();
+    // Only a flag on the Buffer is set here; the region itself isn't passed
+    // anywhere because it was passed when the buffer was created.
+    // Now the flag would tell the consumer that the region contains actual
+    // frame data.
+    if (capture_buffer.handle_provider->DuplicateAsUnsafeRegion().IsValid()) {
+      capture_buffer.is_premapped = true;
+    }
   }
 
   if (FAILED(hr)) {

third_party/blink/renderer/platform/video_capture/video_capture_impl.cc

@@ -382,15 +382,26 @@ bool VideoCaptureImpl::VideoFrameBufferPreparer::Initialize() {
                     video_capture_impl_.gpu_factories_
                         ->GpuMemoryBufferManager(),
                     video_capture_impl_.pool_);
+        // Keep one GpuMemoryBuffer for current GpuMemoryHandle alive,
+        // so that any associated structures are kept alive while this buffer id
+        // is still used (e.g. DMA buf handles for linux/CrOS).
         buffer_context_->SetGpuMemoryBuffer(std::move(gmb));
       }
       CHECK(buffer_context_->GetGpuMemoryBuffer());
 
+      auto buffer_handle = buffer_context_->GetGpuMemoryBuffer()->CloneHandle();
+      if (!frame_info_->is_premapped) {
+        // The buffer may have a region associated with it, which is passed
+        // together with the buffer handle when a new buffer is allocated by the
+        // capturer. However, it will contain actual frame data only if the
+        // |is_premapped| flag is signalled for this frame.
+        buffer_handle.region = base::UnsafeSharedMemoryRegion();
+      }
       // Clone the GpuMemoryBuffer and wrap it in a VideoFrame.
       gpu_memory_buffer_ =
           video_capture_impl_.gpu_memory_buffer_support_
               ->CreateGpuMemoryBufferImplFromHandle(
-                  buffer_context_->GetGpuMemoryBuffer()->CloneHandle(),
+                  std::move(buffer_handle),
                   buffer_context_->GetGpuMemoryBuffer()->GetSize(),
                   buffer_context_->GetGpuMemoryBuffer()->GetFormat(),
                   gfx::BufferUsage::SCANOUT_VEA_CPU_READ, base::DoNothing(),