[macOS] PixelBufferPool: Test retaining IOSurface, not CVPixelBuffer.

Pixel buffers originating from a CVPixelBufferPool are recycled when
no longer in use, previously only tested by releasing the last pixel
buffer reference and not touching the IOSurface.

This CL tests what happens when we are retaining not references to the
pixel buffer, but references to a pixel buffer's IOSurface or
incrementing its use count.

Outcome:
- Retaining references (base::ScopedCFTypeRef<IOSurfaceRef>) to an
  IOSurface does NOT prevent recycling.
- Retaining use count (IOSurfaceIncrementUseCount) to an IOSurface DOES
  prevent recycling.

This behavior is now documented in pixel_buffer_pool_mac.h.

Bug: chromium:1132299
Change-Id: Ibdcd3752a4467bc495eba4b543ecd73da1b2f41f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2520829
Commit-Queue: Markus Handell <handellm@google.com>
Reviewed-by: Markus Handell <handellm@google.com>
Cr-Commit-Position: refs/heads/master@{#824814}

media/capture/video/mac/pixel_buffer_pool_mac.h

@@ -35,6 +35,15 @@ class CAPTURE_EXPORT PixelBufferPool {
   // Freeing all buffer references returns the underlying buffer to the pool. In
   // order to free memory, you must both release all buffers and call Flush() or
   // delete the pool. It is safe for a buffer to outlive its pool.
+  //
+  // Retaining a pixel buffer and preventing it from returning to the pool can
+  // be done either by keeping a reference directly to the CVPixelBuffer, e.g.
+  // with a base::ScopedCFTypeRef<CVPixelBufferRef>, or by incrementing the use
+  // count of the IOSurface, i.e. with IOSurfaceIncrementUseCount().
+  //
+  // WARNING: Retaining references to the pixel buffer's IOSurface (e.g. with
+  // base::ScopedCFTypeRef<IOSurfaceRef>) without incrementing its use count
+  // does NOT prevent it from being recycled!
   base::ScopedCFTypeRef<CVPixelBufferRef> CreateBuffer();
 
   // Frees the memory of any released buffers returned to the pool.

media/capture/video/mac/pixel_buffer_pool_mac_unittest.mm

@@ -43,9 +43,10 @@ TEST(PixelBufferPoolTest, CreatedBufferHasIOSurface) {
   EXPECT_TRUE(CVPixelBufferGetIOSurface(buffer));
 }
 
-TEST(PixelBufferPoolTest, CannotExceedMaxBuffers) {
-  std::unique_ptr<PixelBufferPool> pool =
-      PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 2);
+TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenHoldingOnToPixelBuffer) {
+  constexpr size_t kPoolMaxBuffers = 2;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
   base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
   EXPECT_TRUE(first_buffer);
   base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
@@ -54,6 +55,25 @@ TEST(PixelBufferPoolTest, CannotExceedMaxBuffers) {
   EXPECT_FALSE(third_buffer);
 }
 
+TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenIOSurfaceIsInUse) {
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
+  base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(first_buffer);
+  IOSurfaceRef io_surface = CVPixelBufferGetIOSurface(first_buffer);
+  EXPECT_TRUE(io_surface);
+  // Incremet use count of raw ptr IOSurface reference while releasing the pixel
+  // buffer's only reference.
+  IOSurfaceIncrementUseCount(io_surface);
+  first_buffer.reset();
+  // The pixel buffer has not been returned to the pool.
+  base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
+  EXPECT_FALSE(second_buffer);
+  // Cleanup.
+  IOSurfaceDecrementUseCount(io_surface);
+}
+
 TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) {
   std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
       kPixelFormatNv12, kVgaWidth, kVgaHeight, base::nullopt);
@@ -66,14 +86,58 @@ TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) {
 }
 
 TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousBufferIsReleased) {
-  std::unique_ptr<PixelBufferPool> pool =
-      PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1);
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
   base::ScopedCFTypeRef<CVPixelBufferRef> buffer = pool->CreateBuffer();
   buffer.reset();
   buffer = pool->CreateBuffer();
   EXPECT_TRUE(buffer);
 }
 
+TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousIOSurfaceIsNoLongerUsed) {
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
+  base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(first_buffer);
+  IOSurfaceRef io_surface = CVPixelBufferGetIOSurface(first_buffer);
+  EXPECT_TRUE(io_surface);
+  IOSurfaceIncrementUseCount(io_surface);
+  first_buffer.reset();
+  // Decrementing the use count when there are no pixel buffer references
+  // returns it to the pool.
+  IOSurfaceDecrementUseCount(io_surface);
+  base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(second_buffer);
+}
+
+TEST(PixelBufferPoolTest,
+     SimplyReferencingAnIOSurfaceDoesNotPreventItReturningToThePool) {
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
+  base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(first_buffer);
+  base::ScopedCFTypeRef<IOSurfaceRef> first_buffer_io_surface(
+      CVPixelBufferGetIOSurface(first_buffer), base::scoped_policy::RETAIN);
+  EXPECT_TRUE(first_buffer_io_surface);
+  CVPixelBufferRef first_buffer_unretaind_ptr = first_buffer;
+  // Releasing the first buffer returns it to the pool, despite the IOSurface
+  // still being referenced by |first_buffer_io_surface|.
+  first_buffer.reset();
+
+  base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(second_buffer);
+  base::ScopedCFTypeRef<IOSurfaceRef> second_buffer_io_surface(
+      CVPixelBufferGetIOSurface(second_buffer), base::scoped_policy::RETAIN);
+  EXPECT_TRUE(second_buffer_io_surface);
+
+  // Because this is a recycled pixel buffer, the IOSurface is also recycled.
+  EXPECT_EQ(first_buffer_unretaind_ptr, second_buffer);
+  EXPECT_EQ(first_buffer_io_surface, second_buffer_io_surface);
+}
+
 TEST(PixelBufferPoolTest, BuffersCanOutliveThePool) {
   std::unique_ptr<PixelBufferPool> pool =
       PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1);