Mac Zero Copy Capture: Allow external GMB reuse

The capture system is built around the assumption that the
VideoCaptureBufferPool allocates buffers and manages their lifetime.
This does not match the behavior on macOS where the macOS capture
APIs manage buffer pools (via a CVPixelBufferPool).

Prior to this change, these "external" buffers were not tracked by the
VideoCaptureBufferPool beyond having an id assigned for them. They
were never "reused" -- every new frame was treated as though it was
a new buffer. This was problematic because it caused these buffers
to be opened and closed as they propagated through three processes
(the capture, renderer, and GPU process), burning lots of CPU. In fact,
the underlying IOSurfaces are indeed reused in the CVPixelBufferPool.

This patch enables VideoCaptureBufferPool to exploit the IOSurface
reuse done by the CVPixelBufferPool. It uses GpuMemoryBufferTrackerMac
to track the externally-provided buffers. When a new IOSurface is
presented in VideoCaptureDeviceClient::OnIncomingCapturedExternalBuffer,
the function VideoCaptureBufferPoolImpl::ReserveIdForExternalBuffer
uses the new IsSameGpuMemoryBuffer function to determine if we are
already tracking the GpuMemoryBuffer.

Of note is that VideoCaptureBufferPoolImpl::ReserveIdForExternalBuffer
uses LRU order to discard old buffers (which matches the
CVPixelBufferPool usage pattern), while the other function that does
this, VideoCaptureBufferPoolImpl::ReserveForProducerInternal, finds
the largest buffer to discard.

To prevent the CVPixelBufferPool from reclaiming the IOSurface before
the consumer is done with it, add a OnHeldByConsumersChanged function
which calls IOSurfaceIncrementUseCount (via gfx::ScopedInUseIOSurface)
to signal to the CVPixelBufferPool that the IOSurface can't be
reclaimed yet. This involves some refactoring of the
VideoCaptureBufferTracker class.

Prior to this patch, the IOSurface's in use count was being updated
by the ScopedAccessPermission argument provided to the
VideoCaptureDeviceClient::OnIncomingCapturedExternalBuffer function.
Just drop that ScopedAccessPermission on the floor for now -- it will
be cleaned up in a follow-on patch (this one is already big).

Update tests to test this.

Bug: 1125879
Change-Id: I9eaf6dd47126103b094f54bf23a3f28cf2318d62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2493089Reviewed-by: Markus Handell <handellm@google.com>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821837}

content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc

@@ -26,6 +26,10 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_MAC)
+#include "ui/gfx/mac/io_surface.h"
+#endif
+
 namespace content {
 
 static const media::VideoPixelFormat kCapturePixelFormats[] = {
@@ -265,46 +269,82 @@ TEST_P(VideoCaptureBufferPoolTest, BufferPool) {
   buffer4.reset();
 }
 
+#if defined(OS_MAC)
+namespace {
+
+gfx::GpuMemoryBufferHandle CreateIOSurfaceHandle() {
+  gfx::GpuMemoryBufferHandle result;
+  result.type = gfx::GpuMemoryBufferType::IO_SURFACE_BUFFER;
+  result.id.id = -1;
+  result.io_surface.reset(
+      gfx::CreateIOSurface(gfx::Size(100, 100), gfx::BufferFormat::BGRA_8888));
+  return result;
+}
+
+}  // namespace
+
 TEST_P(VideoCaptureBufferPoolTest, BufferPoolExternal) {
+  auto handle0 = CreateIOSurfaceHandle();
+  auto handle1 = CreateIOSurfaceHandle();
+  auto handle2 = CreateIOSurfaceHandle();
+
   constexpr int kInvalidId = -1;
-  std::vector<int> buffer_ids_to_drop;
+  int buffer_id_to_drop;
 
-  int buffer_id0 = pool_->ReserveIdForExternalBuffer(&buffer_ids_to_drop);
+  int buffer_id0 =
+      pool_->ReserveIdForExternalBuffer(handle0, &buffer_id_to_drop);
   EXPECT_NE(buffer_id0, kInvalidId);
-  EXPECT_TRUE(buffer_ids_to_drop.empty());
-
-  int buffer_id1 = pool_->ReserveIdForExternalBuffer(&buffer_ids_to_drop);
+  EXPECT_EQ(buffer_id_to_drop, kInvalidId);
+  EXPECT_FALSE(IOSurfaceIsInUse(handle0.io_surface));
+  pool_->HoldForConsumers(buffer_id0, 1);
+  EXPECT_TRUE(IOSurfaceIsInUse(handle0.io_surface));
+  pool_->RelinquishProducerReservation(buffer_id0);
+
+  // We should get a new buffer for handle1.
+  int buffer_id1 =
+      pool_->ReserveIdForExternalBuffer(handle1, &buffer_id_to_drop);
   EXPECT_NE(buffer_id1, kInvalidId);
-  EXPECT_TRUE(buffer_ids_to_drop.empty());
-
-  int buffer_id2 = pool_->ReserveIdForExternalBuffer(&buffer_ids_to_drop);
-  EXPECT_NE(buffer_id2, kInvalidId);
-  EXPECT_TRUE(buffer_ids_to_drop.empty());
-
-  buffer_ids_to_drop.clear();
-  pool_->RelinquishExternalBufferReservation(buffer_id1);
-  int buffer_id3 = pool_->ReserveIdForExternalBuffer(&buffer_ids_to_drop);
-  EXPECT_NE(buffer_id3, kInvalidId);
-  EXPECT_EQ(buffer_ids_to_drop.size(), 1u);
-  EXPECT_EQ(buffer_ids_to_drop[0], buffer_id1);
-
-  buffer_ids_to_drop.clear();
-  pool_->RelinquishExternalBufferReservation(buffer_id0);
-  pool_->RelinquishExternalBufferReservation(buffer_id2);
-  pool_->RelinquishExternalBufferReservation(buffer_id3);
-  int buffer_id4 = pool_->ReserveIdForExternalBuffer(&buffer_ids_to_drop);
-  EXPECT_NE(buffer_id4, kInvalidId);
-  EXPECT_EQ(buffer_ids_to_drop.size(), 3u);
-  auto found0 = std::find(buffer_ids_to_drop.begin(), buffer_ids_to_drop.end(),
-                          buffer_id0);
-  EXPECT_FALSE(found0 == buffer_ids_to_drop.end());
-  auto found2 = std::find(buffer_ids_to_drop.begin(), buffer_ids_to_drop.end(),
-                          buffer_id2);
-  EXPECT_FALSE(found2 == buffer_ids_to_drop.end());
-  auto found3 = std::find(buffer_ids_to_drop.begin(), buffer_ids_to_drop.end(),
-                          buffer_id3);
-  EXPECT_FALSE(found3 == buffer_ids_to_drop.end());
+  EXPECT_EQ(buffer_id_to_drop, kInvalidId);
+  pool_->HoldForConsumers(buffer_id1, 1);
+  pool_->RelinquishProducerReservation(buffer_id1);
+  pool_->RelinquishConsumerHold(buffer_id1, 1);
+
+  // We should reuse handle1's buffer.
+  int buffer_id1_reuse =
+      pool_->ReserveIdForExternalBuffer(handle1, &buffer_id_to_drop);
+  EXPECT_EQ(buffer_id1, buffer_id1_reuse);
+  EXPECT_EQ(buffer_id_to_drop, kInvalidId);
+  pool_->HoldForConsumers(buffer_id1_reuse, 1);
+  pool_->RelinquishProducerReservation(buffer_id1_reuse);
+
+  // If we leave buffer_id1 held for a consumer, then we create a new buffer id
+  // for it.
+  int buffer_id1_new =
+      pool_->ReserveIdForExternalBuffer(handle1, &buffer_id_to_drop);
+  EXPECT_NE(buffer_id1, buffer_id1_new);
+  EXPECT_EQ(buffer_id_to_drop, kInvalidId);
+  pool_->HoldForConsumers(buffer_id1_new, 1);
+  pool_->RelinquishProducerReservation(buffer_id1_new);
+  pool_->RelinquishConsumerHold(buffer_id1_new, 1);
+
+  // We have now reached kTestBufferPoolSize buffers. So our next allocation
+  // will return the LRU buffer, which is buffer_id1_new.
+  pool_->RelinquishConsumerHold(buffer_id1_reuse, 1);
+  int buffer_id2 =
+      pool_->ReserveIdForExternalBuffer(handle2, &buffer_id_to_drop);
+  EXPECT_NE(buffer_id0, buffer_id2);
+  EXPECT_NE(buffer_id1, buffer_id2);
+  EXPECT_NE(buffer_id1_new, buffer_id2);
+  EXPECT_EQ(buffer_id_to_drop, buffer_id1_new);
+
+  // Finally, let's reuse handle0.
+  pool_->RelinquishConsumerHold(buffer_id0, 1);
+  int buffer_id0_reuse =
+      pool_->ReserveIdForExternalBuffer(handle0, &buffer_id_to_drop);
+  EXPECT_EQ(buffer_id0, buffer_id0_reuse);
+  EXPECT_EQ(buffer_id_to_drop, kInvalidId);
 }
+#endif
 
 INSTANTIATE_TEST_SUITE_P(All,
                          VideoCaptureBufferPoolTest,

media/capture/BUILD.gn

@@ -117,6 +117,7 @@ component("capture_lib") {
     "video/video_capture_buffer_pool.h",
     "video/video_capture_buffer_pool_impl.cc",
     "video/video_capture_buffer_pool_impl.h",
+    "video/video_capture_buffer_tracker.cc",
     "video/video_capture_buffer_tracker.h",
     "video/video_capture_buffer_tracker_factory.h",
     "video/video_capture_buffer_tracker_factory_impl.cc",

media/capture/video/mac/gpu_memory_buffer_tracker_mac.cc

@@ -10,11 +10,16 @@ namespace media {
 
 GpuMemoryBufferTrackerMac::GpuMemoryBufferTrackerMac() {}
 
+GpuMemoryBufferTrackerMac::GpuMemoryBufferTrackerMac(
+    base::ScopedCFTypeRef<IOSurfaceRef> io_surface)
+    : is_external_io_surface_(true), io_surface_(std::move(io_surface)) {}
+
 GpuMemoryBufferTrackerMac::~GpuMemoryBufferTrackerMac() {}
 
 bool GpuMemoryBufferTrackerMac::Init(const gfx::Size& dimensions,
                                      VideoPixelFormat format,
                                      const mojom::PlaneStridesPtr& strides) {
+  DCHECK(!io_surface_);
   if (format != PIXEL_FORMAT_NV12) {
     NOTREACHED() << "Unsupported VideoPixelFormat "
                  << VideoPixelFormatToString(format);
@@ -32,10 +37,19 @@ bool GpuMemoryBufferTrackerMac::Init(const gfx::Size& dimensions,
   }
 }
 
+bool GpuMemoryBufferTrackerMac::IsSameGpuMemoryBuffer(
+    const gfx::GpuMemoryBufferHandle& handle) const {
+  if (!is_external_io_surface_)
+    return false;
+  return IOSurfaceGetID(io_surface_) == IOSurfaceGetID(handle.io_surface);
+}
+
 bool GpuMemoryBufferTrackerMac::IsReusableForFormat(
     const gfx::Size& dimensions,
     VideoPixelFormat format,
     const mojom::PlaneStridesPtr& strides) {
+  if (is_external_io_surface_)
+    return false;
   gfx::Size surface_size(IOSurfaceGetWidth(io_surface_),
                          IOSurfaceGetHeight(io_surface_));
   return format == PIXEL_FORMAT_NV12 && dimensions == surface_size;
@@ -73,4 +87,18 @@ GpuMemoryBufferTrackerMac::GetGpuMemoryBufferHandle() {
   return gmb_handle;
 }
 
+void GpuMemoryBufferTrackerMac::OnHeldByConsumersChanged(
+    bool is_held_by_consumers) {
+  if (!is_external_io_surface_)
+    return;
+
+  if (is_held_by_consumers) {
+    DCHECK(!in_use_for_consumers_);
+    in_use_for_consumers_.reset(io_surface_.get(), base::scoped_policy::RETAIN);
+  } else {
+    DCHECK(in_use_for_consumers_);
+    in_use_for_consumers_.reset();
+  }
+}
+
 }  // namespace media

media/capture/video/mac/gpu_memory_buffer_tracker_mac.h

@@ -15,12 +15,16 @@ class CAPTURE_EXPORT GpuMemoryBufferTrackerMac final
     : public VideoCaptureBufferTracker {
  public:
   GpuMemoryBufferTrackerMac();
+  explicit GpuMemoryBufferTrackerMac(
+      base::ScopedCFTypeRef<IOSurfaceRef> io_surface);
   ~GpuMemoryBufferTrackerMac() override;
 
   // VideoCaptureBufferTracker
   bool Init(const gfx::Size& dimensions,
             VideoPixelFormat format,
             const mojom::PlaneStridesPtr& strides) override;
+  bool IsSameGpuMemoryBuffer(
+      const gfx::GpuMemoryBufferHandle& handle) const override;
   bool IsReusableForFormat(const gfx::Size& dimensions,
                            VideoPixelFormat format,
                            const mojom::PlaneStridesPtr& strides) override;
@@ -29,13 +33,21 @@ class CAPTURE_EXPORT GpuMemoryBufferTrackerMac final
   base::UnsafeSharedMemoryRegion DuplicateAsUnsafeRegion() override;
   mojo::ScopedSharedBufferHandle DuplicateAsMojoBuffer() override;
   gfx::GpuMemoryBufferHandle GetGpuMemoryBufferHandle() override;
+  void OnHeldByConsumersChanged(bool is_held_by_consumers) override;
 
  private:
-  base::ScopedCFTypeRef<IOSurfaceRef> io_surface_;
+  bool is_external_io_surface_ = false;
+  gfx::ScopedIOSurface io_surface_;
+
+  // External IOSurfaces come from a CVPixelBufferPool. An IOSurface in a
+  // CVPixelBufferPool will be reused by the pool as soon IOSurfaceIsInUse is
+  // false. To prevent reuse while consumers are accessing the IOSurface, use
+  // |in_use_for_consumers_| to maintain IOSurfaceIsInUse as true.
+  gfx::ScopedInUseIOSurface in_use_for_consumers_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferTrackerMac);
 };
 
 }  // namespace media
 
-#endif  // MEDIA_CAPTURE_VIDEO_MAC_GPU_MEMORY_BUFFER_TRACKER_MAC_H_
\ No newline at end of file
+#endif  // MEDIA_CAPTURE_VIDEO_MAC_GPU_MEMORY_BUFFER_TRACKER_MAC_H_

media/capture/video/video_capture_buffer_pool.h

@@ -88,17 +88,14 @@ class CAPTURE_EXPORT VideoCaptureBufferPool
   // of ReserveForProducer().
   virtual void RelinquishProducerReservation(int buffer_id) = 0;
 
-  // Reserve a buffer id to use for an external buffer (one that isn't in this
-  // pool). This is needed to ensure that ids for external buffers don't
-  // conflict with ids from the pool. This call cannot fail (no allocation is
-  // done). The behavior of |buffer_id_to_drop| is the same as
-  // ReserveForProducer.
+  // Reserve a buffer id to use for a buffer specified by |handle| (which was
+  // allocated by some external source). This call cannot fail (no allocation is
+  // done). It may return a new id, or may reuse an existing id, if the buffer
+  // represented by |handle| is already being tracked. The behavior of
+  // |buffer_id_to_drop| is the same as ReserveForProducer.
   virtual int ReserveIdForExternalBuffer(
-      std::vector<int>* buffer_ids_to_drop) = 0;
-
-  // Notify the pool that a buffer id is no longer in use, and can be returned
-  // via ReserveIdForExternalBuffer.
-  virtual void RelinquishExternalBufferReservation(int buffer_id) = 0;
+      const gfx::GpuMemoryBufferHandle& handle,
+      int* buffer_id_to_drop) = 0;
 
   // Returns a snapshot of the current number of buffers in-use divided by the
   // maximum |count_|.

media/capture/video/video_capture_buffer_pool_impl.cc

@@ -133,33 +133,49 @@ void VideoCaptureBufferPoolImpl::RelinquishProducerReservation(int buffer_id) {
     NOTREACHED() << "Invalid buffer_id.";
     return;
   }
-  DCHECK(tracker->held_by_producer());
-  tracker->set_held_by_producer(false);
+  tracker->SetHeldByProducer(false);
 }
 
 int VideoCaptureBufferPoolImpl::ReserveIdForExternalBuffer(
-    std::vector<int>* buffer_ids_to_drop) {
+    const gfx::GpuMemoryBufferHandle& handle,
+    int* buffer_id_to_drop) {
   base::AutoLock lock(lock_);
-  int buffer_id = next_buffer_id_++;
-  external_buffers_[buffer_id] = false;
-
-  for (auto it = external_buffers_.begin(); it != external_buffers_.end();) {
-    if (it->second) {
-      buffer_ids_to_drop->push_back(it->first);
-      it = external_buffers_.erase(it);
-    } else {
-      ++it;
+
+  // Look for a tracker that matches this buffer and is not in use. While
+  // iterating, find the least recently used tracker.
+  *buffer_id_to_drop = kInvalidId;
+  auto lru_tracker_it = trackers_.end();
+  for (auto it = trackers_.begin(); it != trackers_.end(); ++it) {
+    VideoCaptureBufferTracker* const tracker = it->second.get();
+    if (tracker->IsHeldByProducerOrConsumer())
+      continue;
+
+    if (tracker->IsSameGpuMemoryBuffer(handle)) {
+      tracker->SetHeldByProducer(true);
+      return it->first;
+    }
+
+    if (lru_tracker_it == trackers_.end() ||
+        lru_tracker_it->second->LastCustomerUseSequenceNumber() >
+            tracker->LastCustomerUseSequenceNumber()) {
+      lru_tracker_it = it;
     }
   }
-  return buffer_id;
-}
 
-void VideoCaptureBufferPoolImpl::RelinquishExternalBufferReservation(
-    int buffer_id) {
-  base::AutoLock lock(lock_);
-  auto found = external_buffers_.find(buffer_id);
-  CHECK(found != external_buffers_.end());
-  found->second = true;
+  // Free the least recently used tracker, if needed.
+  if (trackers_.size() >= static_cast<size_t>(count_) &&
+      lru_tracker_it != trackers_.end()) {
+    *buffer_id_to_drop = lru_tracker_it->first;
+    trackers_.erase(lru_tracker_it);
+  }
+
+  // Create the new tracker.
+  const int new_buffer_id = next_buffer_id_++;
+  auto tracker =
+      buffer_tracker_factory_->CreateTrackerForExternalGpuMemoryBuffer(handle);
+  tracker->SetHeldByProducer(true);
+  trackers_[new_buffer_id] = std::move(tracker);
+  return new_buffer_id;
 }
 
 void VideoCaptureBufferPoolImpl::HoldForConsumers(int buffer_id,
@@ -170,11 +186,8 @@ void VideoCaptureBufferPoolImpl::HoldForConsumers(int buffer_id,
     NOTREACHED() << "Invalid buffer_id.";
     return;
   }
-  DCHECK(tracker->held_by_producer());
-  DCHECK(!tracker->consumer_hold_count());
-
-  tracker->set_consumer_hold_count(num_clients);
-  // Note: |held_by_producer()| will stay true until
+  tracker->AddConsumerHolds(num_clients);
+  // Note: The buffer will stay held by the producer until
   // RelinquishProducerReservation() (usually called by destructor of the object
   // wrapping this tracker, e.g. a VideoFrame).
 }
@@ -187,10 +200,7 @@ void VideoCaptureBufferPoolImpl::RelinquishConsumerHold(int buffer_id,
     NOTREACHED() << "Invalid buffer_id.";
     return;
   }
-  DCHECK_GE(tracker->consumer_hold_count(), num_clients);
-
-  tracker->set_consumer_hold_count(tracker->consumer_hold_count() -
-                                   num_clients);
+  tracker->RemoveConsumerHolds(num_clients);
 }
 
 double VideoCaptureBufferPoolImpl::GetBufferPoolUtilization() const {
@@ -198,7 +208,7 @@ double VideoCaptureBufferPoolImpl::GetBufferPoolUtilization() const {
   int num_buffers_held = 0;
   for (const auto& entry : trackers_) {
     VideoCaptureBufferTracker* const tracker = entry.second.get();
-    if (tracker->held_by_producer() || tracker->consumer_hold_count() > 0)
+    if (tracker->IsHeldByProducerOrConsumer())
       ++num_buffers_held;
   }
   return static_cast<double>(num_buffers_held) / count_;
@@ -221,10 +231,10 @@ VideoCaptureBufferPoolImpl::ReserveForProducerInternal(
   auto tracker_to_drop = trackers_.end();
   for (auto it = trackers_.begin(); it != trackers_.end(); ++it) {
     VideoCaptureBufferTracker* const tracker = it->second.get();
-    if (!tracker->consumer_hold_count() && !tracker->held_by_producer()) {
+    if (!tracker->IsHeldByProducerOrConsumer()) {
       if (tracker->IsReusableForFormat(dimensions, pixel_format, strides)) {
         // Reuse this buffer
-        tracker->set_held_by_producer(true);
+        tracker->SetHeldByProducer(true);
         tracker->set_frame_feedback_id(frame_feedback_id);
         *buffer_id = it->first;
         return VideoCaptureDevice::Client::ReserveResult::kSucceeded;
@@ -261,7 +271,7 @@ VideoCaptureBufferPoolImpl::ReserveForProducerInternal(
     return VideoCaptureDevice::Client::ReserveResult::kAllocationFailed;
   }
 
-  tracker->set_held_by_producer(true);
+  tracker->SetHeldByProducer(true);
   tracker->set_frame_feedback_id(frame_feedback_id);
   trackers_[new_buffer_id] = std::move(tracker);
 

media/capture/video/video_capture_buffer_pool_impl.h

@@ -50,8 +50,8 @@ class CAPTURE_EXPORT VideoCaptureBufferPoolImpl
       int* buffer_id,
       int* buffer_id_to_drop) override;
   void RelinquishProducerReservation(int buffer_id) override;
-  int ReserveIdForExternalBuffer(std::vector<int>* buffer_ids_to_drop) override;
-  void RelinquishExternalBufferReservation(int buffer_id) override;
+  int ReserveIdForExternalBuffer(const gfx::GpuMemoryBufferHandle& handle,
+                                 int* buffer_id_to_drop) override;
   double GetBufferPoolUtilization() const override;
   void HoldForConsumers(int buffer_id, int num_clients) override;
   void RelinquishConsumerHold(int buffer_id, int num_clients) override;
@@ -87,10 +87,6 @@ class CAPTURE_EXPORT VideoCaptureBufferPoolImpl
   std::map<int, std::unique_ptr<VideoCaptureBufferTracker>> trackers_
       GUARDED_BY(lock_);
 
-  // The external buffers, indexed by buffer id. The second parameter is whether
-  // or not the the buffer's reservation has been relinquished.
-  std::map<int, bool> external_buffers_ GUARDED_BY(lock_);
-
   const std::unique_ptr<VideoCaptureBufferTrackerFactory>
       buffer_tracker_factory_ GUARDED_BY(lock_);
 

media/capture/video/video_capture_buffer_tracker.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/capture/video/video_capture_buffer_tracker.h"
+
+namespace media {
+
+void VideoCaptureBufferTracker::SetHeldByProducer(bool new_held_by_producer) {
+  DCHECK_NE(held_by_producer_, new_held_by_producer);
+  // The producer can't take hold while a consumer still has hold.
+  if (new_held_by_producer)
+    DCHECK_EQ(consumer_hold_count_, 0);
+  held_by_producer_ = new_held_by_producer;
+}
+
+void VideoCaptureBufferTracker::AddConsumerHolds(int count) {
+  // New consumer holds may only be made while the producer hold is still on.
+  // This is because the buffer may disappear out from under us as soon as
+  // neither producer nor consumers have a hold on it.
+  DCHECK_EQ(consumer_hold_count_, 0);
+  DCHECK(held_by_producer_);
+  consumer_hold_count_ += count;
+  OnHeldByConsumersChanged(true);
+}
+
+void VideoCaptureBufferTracker::RemoveConsumerHolds(int count) {
+  DCHECK_GE(consumer_hold_count_, count);
+  consumer_hold_count_ -= count;
+  if (consumer_hold_count_ == 0) {
+    static uint64_t sequence_number = 0;
+    last_customer_use_sequence_number_ = ++sequence_number;
+    OnHeldByConsumersChanged(false);
+  }
+}
+
+bool VideoCaptureBufferTracker::IsSameGpuMemoryBuffer(
+    const gfx::GpuMemoryBufferHandle& handle) const {
+  return false;
+}
+
+void VideoCaptureBufferTracker::OnHeldByConsumersChanged(
+    bool is_held_by_consumers) {}
+
+}  // namespace media

media/capture/video/video_capture_buffer_tracker.h

@@ -24,25 +24,33 @@ namespace media {
 // for implementations using different kinds of storage.
 class CAPTURE_EXPORT VideoCaptureBufferTracker {
  public:
-  VideoCaptureBufferTracker()
-      : held_by_producer_(false),
-        consumer_hold_count_(0),
-        frame_feedback_id_(0) {}
+  VideoCaptureBufferTracker() = default;
   virtual bool Init(const gfx::Size& dimensions,
                     VideoPixelFormat format,
                     const mojom::PlaneStridesPtr& strides) = 0;
   virtual ~VideoCaptureBufferTracker() {}
 
-  bool held_by_producer() const { return held_by_producer_; }
-  void set_held_by_producer(bool value) { held_by_producer_ = value; }
-  int consumer_hold_count() const { return consumer_hold_count_; }
-  void set_consumer_hold_count(int value) { consumer_hold_count_ = value; }
+  bool IsHeldByProducerOrConsumer() const {
+    return held_by_producer_ || consumer_hold_count_ > 0;
+  }
+  void SetHeldByProducer(bool value);
+  void AddConsumerHolds(int count);
+  void RemoveConsumerHolds(int count);
+
   void set_frame_feedback_id(int value) { frame_feedback_id_ = value; }
   int frame_feedback_id() { return frame_feedback_id_; }
 
+  // Returns true if |handle| refers to the same buffer as |this|. This is used
+  // to reuse buffers that were externally allocated.
+  virtual bool IsSameGpuMemoryBuffer(
+      const gfx::GpuMemoryBufferHandle& handle) const;
+
+  // Returns true if |this| matches the specified parameters. This is used to
+  // reuse buffers that were internally allocated.
   virtual bool IsReusableForFormat(const gfx::Size& dimensions,
                                    VideoPixelFormat format,
                                    const mojom::PlaneStridesPtr& strides) = 0;
+
   virtual uint32_t GetMemorySizeInBytes() = 0;
 
   virtual std::unique_ptr<VideoCaptureBufferHandle> GetMemoryMappedAccess() = 0;
@@ -51,15 +59,28 @@ class CAPTURE_EXPORT VideoCaptureBufferTracker {
   virtual mojo::ScopedSharedBufferHandle DuplicateAsMojoBuffer() = 0;
   virtual gfx::GpuMemoryBufferHandle GetGpuMemoryBufferHandle() = 0;
 
+  // This is called when the number of consumers goes from zero to non-zero (in
+  // which case |is_held_by_consumers| is true) or from non-zero to zero (in
+  // which case |is_held_by_consumers| is false).
+  virtual void OnHeldByConsumersChanged(bool is_held_by_consumers);
+
+  // External buffers are to be freed in least-recently-used order. This
+  // function returns a number which is greater for more recently used buffers.
+  uint64_t LastCustomerUseSequenceNumber() const {
+    return last_customer_use_sequence_number_;
+  }
+
  private:
   // Indicates whether this VideoCaptureBufferTracker is currently referenced by
   // the producer.
-  bool held_by_producer_;
+  bool held_by_producer_ = false;
 
   // Number of consumer processes which hold this VideoCaptureBufferTracker.
-  int consumer_hold_count_;
+  int consumer_hold_count_ = 0;
+
+  int frame_feedback_id_ = 0;
 
-  int frame_feedback_id_;
+  uint64_t last_customer_use_sequence_number_ = 0;
 };
 
 }  // namespace media

media/capture/video/video_capture_buffer_tracker_factory.h

@@ -10,6 +10,10 @@
 #include "media/capture/capture_export.h"
 #include "media/capture/video_capture_types.h"
 
+namespace gfx {
+struct GpuMemoryBufferHandle;
+}  // namespace gfx
+
 namespace media {
 
 class VideoCaptureBufferTracker;
@@ -19,6 +23,9 @@ class CAPTURE_EXPORT VideoCaptureBufferTrackerFactory {
   virtual ~VideoCaptureBufferTrackerFactory() {}
   virtual std::unique_ptr<VideoCaptureBufferTracker> CreateTracker(
       VideoCaptureBufferType buffer_type) = 0;
+  virtual std::unique_ptr<VideoCaptureBufferTracker>
+  CreateTrackerForExternalGpuMemoryBuffer(
+      const gfx::GpuMemoryBufferHandle& handle) = 0;
 };
 
 }  // namespace media

media/capture/video/video_capture_buffer_tracker_factory_impl.cc

@@ -37,4 +37,14 @@ VideoCaptureBufferTrackerFactoryImpl::CreateTracker(
   }
 }
 
+std::unique_ptr<VideoCaptureBufferTracker>
+VideoCaptureBufferTrackerFactoryImpl::CreateTrackerForExternalGpuMemoryBuffer(
+    const gfx::GpuMemoryBufferHandle& handle) {
+#if defined(OS_MAC)
+  return std::make_unique<GpuMemoryBufferTrackerMac>(handle.io_surface);
+#else
+  return nullptr;
+#endif
+}
+
 }  // namespace media

media/capture/video/video_capture_buffer_tracker_factory_impl.h

@@ -17,6 +17,9 @@ class CAPTURE_EXPORT VideoCaptureBufferTrackerFactoryImpl
  public:
   std::unique_ptr<VideoCaptureBufferTracker> CreateTracker(
       VideoCaptureBufferType buffer_type) override;
+  std::unique_ptr<VideoCaptureBufferTracker>
+  CreateTrackerForExternalGpuMemoryBuffer(
+      const gfx::GpuMemoryBufferHandle& handle) override;
 };
 
 }  // namespace media

media/capture/video/video_capture_device_client.cc

@@ -474,11 +474,12 @@ void VideoCaptureDeviceClient::OnIncomingCapturedExternalBuffer(
     base::TimeDelta timestamp) {
   // Reserve an ID for this buffer that will not conflict with any of the IDs
   // used by |buffer_pool_|.
-  std::vector<int> buffer_ids_to_drop;
-  int buffer_id = buffer_pool_->ReserveIdForExternalBuffer(&buffer_ids_to_drop);
+  int buffer_id_to_drop = VideoCaptureBufferPool::kInvalidId;
+  int buffer_id =
+      buffer_pool_->ReserveIdForExternalBuffer(handle, &buffer_id_to_drop);
 
   // If a buffer to retire was specified, retire one.
-  for (int buffer_id_to_drop : buffer_ids_to_drop) {
+  if (buffer_id_to_drop != VideoCaptureBufferPool::kInvalidId) {
     auto entry_iter =
         std::find(buffer_ids_known_by_receiver_.begin(),
                   buffer_ids_known_by_receiver_.end(), buffer_id_to_drop);
@@ -488,8 +489,8 @@ void VideoCaptureDeviceClient::OnIncomingCapturedExternalBuffer(
     }
   }
 
-  // Register the buffer with the receiver.
-  {
+  // Register the buffer with the receiver if it is new.
+  if (!base::Contains(buffer_ids_known_by_receiver_, buffer_id)) {
     media::mojom::VideoBufferHandlePtr buffer_handle =
         media::mojom::VideoBufferHandle::New();
     buffer_handle->set_gpu_memory_buffer_handle(std::move(handle));
@@ -497,25 +498,6 @@ void VideoCaptureDeviceClient::OnIncomingCapturedExternalBuffer(
     buffer_ids_known_by_receiver_.push_back(buffer_id);
   }
 
-  // Wrap |scoped_access_permission_to_wrap| in a ScopedAccessPermission that
-  // will retire |buffer_id| as soon as access ends.
-  std::unique_ptr<VideoCaptureDevice::Client::Buffer::ScopedAccessPermission>
-      scoped_access_permission;
-  {
-    auto callback_lambda =
-        [](int buffer_id, scoped_refptr<VideoCaptureBufferPool> buffer_pool,
-           std::unique_ptr<
-               VideoCaptureDevice::Client::Buffer::ScopedAccessPermission>
-               scoped_access_permission) {
-          buffer_pool->RelinquishExternalBufferReservation(buffer_id);
-        };
-    auto closure = base::BindOnce(callback_lambda, buffer_id, buffer_pool_,
-                                  std::move(scoped_access_permission_to_wrap));
-    scoped_access_permission =
-        std::make_unique<ScopedAccessPermissionEndWithCallback>(
-            std::move(closure));
-  }
-
   // Tell |receiver_| that the frame has been received.
   {
     mojom::VideoFrameInfoPtr info = mojom::VideoFrameInfo::New();
@@ -526,9 +508,14 @@ void VideoCaptureDeviceClient::OnIncomingCapturedExternalBuffer(
     info->visible_rect = gfx::Rect(format.frame_size);
     info->metadata.frame_rate = format.frame_rate;
     info->metadata.reference_time = reference_time;
-    receiver_->OnFrameReadyInBuffer(buffer_id, 0 /* frame_feedback_id */,
-                                    std::move(scoped_access_permission),
-                                    std::move(info));
+
+    buffer_pool_->HoldForConsumers(buffer_id, 1);
+    buffer_pool_->RelinquishProducerReservation(buffer_id);
+    receiver_->OnFrameReadyInBuffer(
+        buffer_id, 0 /* frame_feedback_id */,
+        std::make_unique<ScopedBufferPoolReservation<ConsumerReleaseTraits>>(
+            buffer_pool_, buffer_id),
+        std::move(info));
   }
 }