[Video Capture, UMA] Distinguish out of memory from buffer pool max count exceeded

Currently, failures to obtain a video capture buffer from the buffer pool are
logged all in one bucket. This CL splits the failures into out of memory vs.
buffer pool max count exceeded.

To enable this, the CL changes the interface of media::VideoCaptureBufferPool to
return a result code that breaks out failure cases instead of having a single
failure path.

Bug: 878943
Change-Id: I0a74a8034e936ea02a1ead26a6cb5d916e05e288
Reviewed-on: https://chromium-review.googlesource.com/1220192
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593025}

content/browser/media/capture/desktop_capture_device_unittest.cc

@@ -230,12 +230,13 @@ class DesktopCaptureDeviceTest : public testing::Test {
   std::unique_ptr<media::MockVideoCaptureDeviceClient>
   CreateMockVideoCaptureDeviceClient() {
     auto result = std::make_unique<media::MockVideoCaptureDeviceClient>();
-    ON_CALL(*result, ReserveOutputBuffer(_, _, _))
-        .WillByDefault(
-            Invoke([](const gfx::Size&, media::VideoPixelFormat format, int) {
-              EXPECT_TRUE(format == media::PIXEL_FORMAT_I420);
-              return media::VideoCaptureDevice::Client::Buffer();
-            }));
+    ON_CALL(*result, ReserveOutputBuffer(_, _, _, _))
+        .WillByDefault(Invoke([](const gfx::Size&,
+                                 media::VideoPixelFormat format, int,
+                                 media::VideoCaptureDevice::Client::Buffer*) {
+          EXPECT_TRUE(format == media::PIXEL_FORMAT_I420);
+          return media::VideoCaptureDevice::Client::ReserveResult::kSucceeded;
+        }));
     return result;
   }
 

content/browser/media/capture/screen_capture_device_android_unittest.cc

@@ -45,12 +45,14 @@ class MockDeviceClient : public media::VideoCaptureDevice::Client {
   MOCK_METHOD0(OnStarted, void(void));
 
   // Trampoline methods to workaround GMOCK problems with std::unique_ptr<>.
-  Buffer ReserveOutputBuffer(const gfx::Size& dimensions,
-                             media::VideoPixelFormat format,
-                             int frame_feedback_id) override {
+  media::VideoCaptureDevice::Client::ReserveResult ReserveOutputBuffer(
+      const gfx::Size& dimensions,
+      media::VideoPixelFormat format,
+      int frame_feedback_id,
+      Buffer* buffer) override {
     EXPECT_EQ(media::PIXEL_FORMAT_I420, format);
     DoReserveOutputBuffer();
-    return Buffer();
+    return media::VideoCaptureDevice::Client::ReserveResult::kSucceeded;
   }
   void OnIncomingCapturedBuffer(Buffer buffer,
                                 const media::VideoCaptureFormat& frame_format,

content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc

@@ -77,11 +77,14 @@ class VideoCaptureBufferPoolTest
     DVLOG(1) << media::VideoPixelFormatToString(pixel_format) << " "
              << dimensions.ToString();
     const int arbitrary_frame_feedback_id = 0;
-    const int buffer_id = pool_->ReserveForProducer(
+    int buffer_id = media::VideoCaptureBufferPool::kInvalidId;
+    const auto reserve_result = pool_->ReserveForProducer(
         dimensions, pixel_format, nullptr, arbitrary_frame_feedback_id,
-        &buffer_id_to_drop);
-    if (buffer_id == media::VideoCaptureBufferPool::kInvalidId)
+        &buffer_id, &buffer_id_to_drop);
+    if (reserve_result !=
+        media::VideoCaptureDevice::Client::ReserveResult::kSucceeded) {
       return std::unique_ptr<Buffer>();
+    }
     EXPECT_EQ(expected_dropped_id_, buffer_id_to_drop);
 
     std::unique_ptr<media::VideoCaptureBufferHandle> buffer_handle =

content/browser/renderer_host/media/video_capture_controller_unittest.cc

@@ -317,11 +317,12 @@ TEST_P(VideoCaptureControllerTest, NormalCaptureMultipleClients) {
   uint8_t buffer_no = 1;
   const int arbitrary_frame_feedback_id = 101;
   ASSERT_EQ(0.0, device_client_->GetBufferPoolUtilization());
-  media::VideoCaptureDevice::Client::Buffer buffer =
-      device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                          device_format.pixel_format,
-                                          arbitrary_frame_feedback_id);
-  ASSERT_TRUE(buffer.is_valid());
+  media::VideoCaptureDevice::Client::Buffer buffer;
+  const auto result_code = device_client_->ReserveOutputBuffer(
+      device_format.frame_size, device_format.pixel_format,
+      arbitrary_frame_feedback_id, &buffer);
+  ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+            result_code);
   auto buffer_access = buffer.handle_provider->GetHandleForInProcessAccess();
   ASSERT_EQ(1.0 / kPoolSize, device_client_->GetBufferPoolUtilization());
   memset(buffer_access->data(), buffer_no++, buffer_access->mapped_size());
@@ -363,11 +364,12 @@ TEST_P(VideoCaptureControllerTest, NormalCaptureMultipleClients) {
   // case pretend that the Buffer pointer is held by the device for a long
   // delay. This shouldn't affect anything.
   const int arbitrary_frame_feedback_id_2 = 102;
-  media::VideoCaptureDevice::Client::Buffer buffer2 =
-      device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                          device_format.pixel_format,
-                                          arbitrary_frame_feedback_id_2);
-  ASSERT_TRUE(buffer2.is_valid());
+  media::VideoCaptureDevice::Client::Buffer buffer2;
+  const auto result_code_2 = device_client_->ReserveOutputBuffer(
+      device_format.frame_size, device_format.pixel_format,
+      arbitrary_frame_feedback_id_2, &buffer2);
+  ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+            result_code_2);
   auto buffer2_access = buffer2.handle_provider->GetHandleForInProcessAccess();
   memset(buffer2_access->data(), buffer_no++, buffer2_access->mapped_size());
   client_a_->resource_utilization_ = 0.5;
@@ -400,11 +402,12 @@ TEST_P(VideoCaptureControllerTest, NormalCaptureMultipleClients) {
   // Third, fourth, and fifth buffers. Pretend they all arrive at the same time.
   for (int i = 0; i < kPoolSize; i++) {
     const int arbitrary_frame_feedback_id = 200 + i;
-    media::VideoCaptureDevice::Client::Buffer buffer =
-        device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                            device_format.pixel_format,
-                                            arbitrary_frame_feedback_id);
-    ASSERT_TRUE(buffer.is_valid());
+    media::VideoCaptureDevice::Client::Buffer buffer;
+    const auto result_code = device_client_->ReserveOutputBuffer(
+        device_format.frame_size, device_format.pixel_format,
+        arbitrary_frame_feedback_id, &buffer);
+    ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+              result_code);
     auto buffer_access = buffer.handle_provider->GetHandleForInProcessAccess();
     memset(buffer_access->data(), buffer_no++, buffer_access->mapped_size());
     device_client_->OnIncomingCapturedBuffer(std::move(buffer), device_format,
@@ -412,11 +415,12 @@ TEST_P(VideoCaptureControllerTest, NormalCaptureMultipleClients) {
                                              arbitrary_timestamp_);
   }
   // ReserveOutputBuffer ought to fail now, because the pool is depleted.
-  ASSERT_FALSE(device_client_
-                   ->ReserveOutputBuffer(device_format.frame_size,
-                                         device_format.pixel_format,
-                                         arbitrary_frame_feedback_id)
-                   .is_valid());
+  media::VideoCaptureDevice::Client::Buffer buffer_fail;
+  ASSERT_EQ(
+      media::VideoCaptureDevice::Client::ReserveResult::kMaxBufferCountExceeded,
+      device_client_->ReserveOutputBuffer(
+          device_format.frame_size, device_format.pixel_format,
+          arbitrary_frame_feedback_id, &buffer_fail));
 
   // The new client needs to be notified of the creation of |kPoolSize| buffers;
   // the old clients only |kPoolSize - 1|.
@@ -451,28 +455,30 @@ TEST_P(VideoCaptureControllerTest, NormalCaptureMultipleClients) {
   EXPECT_CALL(*client_b_, DoEnded(client_b_route_1)).Times(1);
   controller_->StopSession(300);
   // Queue up another buffer.
-  media::VideoCaptureDevice::Client::Buffer buffer3 =
-      device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                          device_format.pixel_format,
-                                          arbitrary_frame_feedback_id);
-  ASSERT_TRUE(buffer3.is_valid());
+  media::VideoCaptureDevice::Client::Buffer buffer3;
+  const auto result_code_3 = device_client_->ReserveOutputBuffer(
+      device_format.frame_size, device_format.pixel_format,
+      arbitrary_frame_feedback_id, &buffer3);
+  ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+            result_code_3);
   auto buffer3_access = buffer3.handle_provider->GetHandleForInProcessAccess();
   memset(buffer3_access->data(), buffer_no++, buffer3_access->mapped_size());
   device_client_->OnIncomingCapturedBuffer(std::move(buffer3), device_format,
                                            arbitrary_reference_time_,
                                            arbitrary_timestamp_);
 
-  media::VideoCaptureDevice::Client::Buffer buffer4 =
-      device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                          device_format.pixel_format,
-                                          arbitrary_frame_feedback_id);
+  media::VideoCaptureDevice::Client::Buffer buffer4;
+  const auto result_code_4 = device_client_->ReserveOutputBuffer(
+      device_format.frame_size, device_format.pixel_format,
+      arbitrary_frame_feedback_id, &buffer4);
   {
     // Kill A2 via session close (posts a task to disconnect, but A2 must not
     // be sent either of these two buffers).
     EXPECT_CALL(*client_a_, DoEnded(client_a_route_2)).Times(1);
     controller_->StopSession(200);
   }
-  ASSERT_TRUE(buffer4.is_valid());
+  ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+            result_code_4);
   auto buffer4_access = buffer4.handle_provider->GetHandleForInProcessAccess();
   memset(buffer4_access->data(), buffer_no++, buffer4_access->mapped_size());
   device_client_->OnIncomingCapturedBuffer(std::move(buffer4), device_format,
@@ -533,10 +539,12 @@ TEST_F(VideoCaptureControllerTest, ErrorBeforeDeviceCreation) {
   media::VideoCaptureFormat device_format(
       capture_resolution, arbitrary_frame_rate_, media::PIXEL_FORMAT_I420);
   const int arbitrary_frame_feedback_id = 101;
-  media::VideoCaptureDevice::Client::Buffer buffer =
-      device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                          device_format.pixel_format,
-                                          arbitrary_frame_feedback_id);
+  media::VideoCaptureDevice::Client::Buffer buffer;
+  const auto reserve_result = device_client_->ReserveOutputBuffer(
+      device_format.frame_size, device_format.pixel_format,
+      arbitrary_frame_feedback_id, &buffer);
+  ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+            reserve_result);
   device_client_->OnIncomingCapturedBuffer(std::move(buffer), device_format,
                                            arbitrary_reference_time_,
                                            arbitrary_timestamp_);
@@ -567,11 +575,12 @@ TEST_F(VideoCaptureControllerTest, ErrorAfterDeviceCreation) {
   media::VideoCaptureFormat device_format(
       gfx::Size(10, 10), arbitrary_frame_rate_, media::PIXEL_FORMAT_I420);
   const int arbitrary_frame_feedback_id = 101;
-  media::VideoCaptureDevice::Client::Buffer buffer =
-      device_client_->ReserveOutputBuffer(device_format.frame_size,
-                                          device_format.pixel_format,
-                                          arbitrary_frame_feedback_id);
-  ASSERT_TRUE(buffer.is_valid());
+  media::VideoCaptureDevice::Client::Buffer buffer;
+  const auto result_code = device_client_->ReserveOutputBuffer(
+      device_format.frame_size, device_format.pixel_format,
+      arbitrary_frame_feedback_id, &buffer);
+  ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+            result_code);
 
   device_client_->OnError(
       media::VideoCaptureError::kIntentionalErrorRaisedByUnitTest, FROM_HERE,
@@ -635,10 +644,11 @@ TEST_F(VideoCaptureControllerTest, FrameFeedbackIsReportedForSequenceOfFrames) {
     // The frame is expected to arrive at |client_a_|.DoBufferReady(), which
     // automatically notifies |controller_| that it has finished consuming it.
     media::VideoCaptureDevice::Client::Buffer buffer;
-    buffer = device_client_->ReserveOutputBuffer(arbitrary_format.frame_size,
-                                                 arbitrary_format.pixel_format,
-                                                 stub_frame_feedback_id);
-    ASSERT_TRUE(buffer.is_valid());
+    const auto result_code = device_client_->ReserveOutputBuffer(
+        arbitrary_format.frame_size, arbitrary_format.pixel_format,
+        stub_frame_feedback_id, &buffer);
+    ASSERT_EQ(media::VideoCaptureDevice::Client::ReserveResult::kSucceeded,
+              result_code);
     device_client_->OnIncomingCapturedBuffer(
         std::move(buffer), arbitrary_format, arbitrary_reference_time_,
         arbitrary_timestamp_);
@@ -842,8 +852,7 @@ TEST_F(VideoCaptureControllerTest, DroppedFramesGetLoggedInUMA) {
   controller_->OnFrameDropped(
       media::VideoCaptureFrameDropReason::kDeviceClientFrameHasInvalidFormat);
   controller_->OnFrameDropped(
-      media::VideoCaptureFrameDropReason::
-          kDeviceClientFailedToReserveBufferFromBufferPool);
+      media::VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded);
   controller_->OnFrameDropped(
       media::VideoCaptureFrameDropReason::kDeviceClientFrameHasInvalidFormat);
 
@@ -853,9 +862,7 @@ TEST_F(VideoCaptureControllerTest, DroppedFramesGetLoggedInUMA) {
       2);
   histogram_tester.ExpectBucketCount(
       "Media.VideoCapture.FrameDrop.DeviceCapture",
-      media::VideoCaptureFrameDropReason::
-          kDeviceClientFailedToReserveBufferFromBufferPool,
-      1);
+      media::VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded, 1);
 }
 
 // Tests that too many frames dropped for the same reason emits a special UMA
@@ -955,8 +962,7 @@ TEST_F(VideoCaptureControllerTest,
 
   // Drop for a different reason
   controller_->OnFrameDropped(
-      media::VideoCaptureFrameDropReason::
-          kDeviceClientFailedToReserveBufferFromBufferPool);
+      media::VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded);
 
   controller_->OnFrameDropped(
       media::VideoCaptureFrameDropReason::kDeviceClientFrameHasInvalidFormat);
@@ -966,9 +972,7 @@ TEST_F(VideoCaptureControllerTest,
       VideoCaptureController::kMaxConsecutiveFrameDropForSameReasonCount + 1);
   histogram_tester.ExpectBucketCount(
       "Media.VideoCapture.FrameDrop.DeviceCapture",
-      media::VideoCaptureFrameDropReason::
-          kDeviceClientFailedToReserveBufferFromBufferPool,
-      1);
+      media::VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded, 1);
 }
 
 }  // namespace content

media/capture/content/android/thread_safe_capture_oracle.cc

@@ -96,8 +96,9 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     coded_size.SetSize(base::bits::Align(visible_size.width(), 16),
                        base::bits::Align(visible_size.height(), 16));
 
-    output_buffer = client_->ReserveOutputBuffer(
-        coded_size, params_.requested_format.pixel_format, frame_number);
+    const auto result_code = client_->ReserveOutputBuffer(
+        coded_size, params_.requested_format.pixel_format, frame_number,
+        &output_buffer);
 
     // Get the current buffer pool utilization and attenuate it: The utilization
     // reported to the oracle is in terms of a maximum sustainable amount (not
@@ -105,7 +106,7 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     attenuated_utilization = client_->GetBufferPoolUtilization() *
                              (100.0 / kTargetMaxPoolUtilizationPercent);
 
-    if (!output_buffer.is_valid()) {
+    if (result_code != VideoCaptureDevice::Client::ReserveResult::kSucceeded) {
       TRACE_EVENT_INSTANT2(
           "gpu.capture", "PipelineLimited", TRACE_EVENT_SCOPE_THREAD, "trigger",
           VideoCaptureOracle::EventAsString(event), "atten_util_percent",

media/capture/mojom/video_capture_types.mojom

@@ -211,7 +211,6 @@ enum VideoCaptureError {
 enum VideoCaptureFrameDropReason {
   kNone,
   kDeviceClientFrameHasInvalidFormat,
-  kDeviceClientFailedToReserveBufferFromBufferPool,
   kDeviceClientLibyuvConvertToI420Failed,
   kV4L2BufferErrorFlagWasSet,
   kV4L2InvalidNumberOfBytesInBuffer,
@@ -223,7 +222,9 @@ enum VideoCaptureFrameDropReason {
   kWinDirectShowFailedToGetMemoryPointerFromMediaSample,
   kWinMediaFoundationReceivedSampleIsNull,
   kWinMediaFoundationLockingBufferDelieveredNullptr,
-  kWinMediaFoundationGetBufferByIndexReturnedNull
+  kWinMediaFoundationGetBufferByIndexReturnedNull,
+  kBufferPoolMaxBufferCountExceeded,
+  kBufferPoolBufferAllocationFailed
 };
 
 struct VideoCaptureFormat {

media/capture/mojom/video_capture_types_mojom_traits.cc

@@ -1192,10 +1192,6 @@ EnumTraits<media::mojom::VideoCaptureFrameDropReason,
     case media::VideoCaptureFrameDropReason::kDeviceClientFrameHasInvalidFormat:
       return media::mojom::VideoCaptureFrameDropReason::
           kDeviceClientFrameHasInvalidFormat;
-    case media::VideoCaptureFrameDropReason::
-        kDeviceClientFailedToReserveBufferFromBufferPool:
-      return media::mojom::VideoCaptureFrameDropReason::
-          kDeviceClientFailedToReserveBufferFromBufferPool;
     case media::VideoCaptureFrameDropReason::
         kDeviceClientLibyuvConvertToI420Failed:
       return media::mojom::VideoCaptureFrameDropReason::
@@ -1237,6 +1233,12 @@ EnumTraits<media::mojom::VideoCaptureFrameDropReason,
         kWinMediaFoundationGetBufferByIndexReturnedNull:
       return media::mojom::VideoCaptureFrameDropReason::
           kWinMediaFoundationGetBufferByIndexReturnedNull;
+    case media::VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded:
+      return media::mojom::VideoCaptureFrameDropReason::
+          kBufferPoolMaxBufferCountExceeded;
+    case media::VideoCaptureFrameDropReason::kBufferPoolBufferAllocationFailed:
+      return media::mojom::VideoCaptureFrameDropReason::
+          kBufferPoolBufferAllocationFailed;
   }
   NOTREACHED();
   return media::mojom::VideoCaptureFrameDropReason::kNone;
@@ -1256,11 +1258,6 @@ bool EnumTraits<media::mojom::VideoCaptureFrameDropReason,
       *output = media::VideoCaptureFrameDropReason::
           kDeviceClientFrameHasInvalidFormat;
       return true;
-    case media::mojom::VideoCaptureFrameDropReason::
-        kDeviceClientFailedToReserveBufferFromBufferPool:
-      *output = media::VideoCaptureFrameDropReason::
-          kDeviceClientFailedToReserveBufferFromBufferPool;
-      return true;
     case media::mojom::VideoCaptureFrameDropReason::
         kDeviceClientLibyuvConvertToI420Failed:
       *output = media::VideoCaptureFrameDropReason::
@@ -1317,6 +1314,16 @@ bool EnumTraits<media::mojom::VideoCaptureFrameDropReason,
       *output = media::VideoCaptureFrameDropReason::
           kWinMediaFoundationGetBufferByIndexReturnedNull;
       return true;
+    case media::mojom::VideoCaptureFrameDropReason::
+        kBufferPoolMaxBufferCountExceeded:
+      *output =
+          media::VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded;
+      return true;
+    case media::mojom::VideoCaptureFrameDropReason::
+        kBufferPoolBufferAllocationFailed:
+      *output =
+          media::VideoCaptureFrameDropReason::kBufferPoolBufferAllocationFailed;
+      return true;
   }
   NOTREACHED();
   return false;

media/capture/video/chromeos/mock_video_capture_client.cc

@@ -68,13 +68,15 @@ void MockVideoCaptureClient::OnIncomingCapturedGfxBuffer(
 }
 
 // Trampoline methods to workaround GMOCK problems with std::unique_ptr<>.
-VideoCaptureDevice::Client::Buffer MockVideoCaptureClient::ReserveOutputBuffer(
+VideoCaptureDevice::Client::ReserveResult
+MockVideoCaptureClient::ReserveOutputBuffer(
     const gfx::Size& dimensions,
     VideoPixelFormat format,
-    int frame_feedback_id) {
+    int frame_feedback_id,
+    VideoCaptureDevice::Client::Buffer* buffer) {
   DoReserveOutputBuffer();
   NOTREACHED() << "This should never be called";
-  return Buffer();
+  return ReserveResult::kSucceeded;
 }
 
 void MockVideoCaptureClient::OnIncomingCapturedBuffer(

media/capture/video/chromeos/mock_video_capture_client.h

@@ -53,9 +53,10 @@ class MockVideoCaptureClient : public VideoCaptureDevice::Client {
                                    base::TimeDelta timestamp,
                                    int frame_feedback_id = 0) override;
   // Trampoline methods to workaround GMOCK problems with std::unique_ptr<>.
-  Buffer ReserveOutputBuffer(const gfx::Size& dimensions,
-                             VideoPixelFormat format,
-                             int frame_feedback_id) override;
+  ReserveResult ReserveOutputBuffer(const gfx::Size& dimensions,
+                                    VideoPixelFormat format,
+                                    int frame_feedback_id,
+                                    Buffer* buffer) override;
   void OnIncomingCapturedBuffer(Buffer buffer,
                                 const VideoCaptureFormat& format,
                                 base::TimeTicks reference_time,

media/capture/video/fake_video_capture_device.cc

@@ -574,11 +574,15 @@ void ClientBufferFrameDeliverer::PaintAndDeliverNextFrame(
     return;
 
   const int arbitrary_frame_feedback_id = 0;
-  auto capture_buffer = client()->ReserveOutputBuffer(
+  VideoCaptureDevice::Client::Buffer capture_buffer;
+  const auto reserve_result = client()->ReserveOutputBuffer(
       device_state()->format.frame_size, device_state()->format.pixel_format,
-      arbitrary_frame_feedback_id);
-  DLOG_IF(ERROR, !capture_buffer.is_valid())
-      << "Couldn't allocate Capture Buffer";
+      arbitrary_frame_feedback_id, &capture_buffer);
+  if (reserve_result != VideoCaptureDevice::Client::ReserveResult::kSucceeded) {
+    client()->OnFrameDropped(
+        ConvertReservationFailureToFrameDropReason(reserve_result));
+    return;
+  }
   auto buffer_access =
       capture_buffer.handle_provider->GetHandleForInProcessAccess();
   DCHECK(buffer_access->data()) << "Buffer has NO backing memory";

media/capture/video/fake_video_capture_device_unittest.cc

@@ -148,12 +148,14 @@ class FakeVideoCaptureDeviceTestBase : public ::testing::Test {
 
   std::unique_ptr<MockVideoCaptureDeviceClient> CreateClient() {
     auto result = std::make_unique<MockVideoCaptureDeviceClient>();
-    ON_CALL(*result, ReserveOutputBuffer(_, _, _))
-        .WillByDefault(Invoke(
-            [](const gfx::Size& dimensions, VideoPixelFormat format, int) {
+    ON_CALL(*result, ReserveOutputBuffer(_, _, _, _))
+        .WillByDefault(
+            Invoke([](const gfx::Size& dimensions, VideoPixelFormat format, int,
+                      VideoCaptureDevice::Client::Buffer* buffer) {
               EXPECT_GT(dimensions.GetArea(), 0);
               const VideoCaptureFormat frame_format(dimensions, 0.0, format);
-              return CreateStubBuffer(0, frame_format.ImageAllocationSize());
+              *buffer = CreateStubBuffer(0, frame_format.ImageAllocationSize());
+              return VideoCaptureDevice::Client::ReserveResult::kSucceeded;
             }));
     ON_CALL(*result, OnIncomingCapturedData(_, _, _, _, _, _, _))
         .WillByDefault(

media/capture/video/mock_video_capture_device_client.h

@@ -30,8 +30,8 @@ class MockVideoCaptureDeviceClient : public VideoCaptureDevice::Client {
                     base::TimeTicks reference_time,
                     base::TimeDelta timestamp,
                     int frame_feedback_id));
-  MOCK_METHOD3(ReserveOutputBuffer,
-               Buffer(const gfx::Size&, VideoPixelFormat, int));
+  MOCK_METHOD4(ReserveOutputBuffer,
+               ReserveResult(const gfx::Size&, VideoPixelFormat, int, Buffer*));
   MOCK_METHOD3(OnError,
                void(media::VideoCaptureError error,
                     const base::Location& from_here,

media/capture/video/video_capture_buffer_pool.h

@@ -8,6 +8,7 @@
 #include "base/memory/ref_counted.h"
 #include "media/capture/capture_export.h"
 #include "media/capture/mojom/video_capture_types.mojom.h"
+#include "media/capture/video/video_capture_device.h"
 #include "media/capture/video_capture_types.h"
 #include "mojo/public/cpp/system/buffer.h"
 #include "ui/gfx/geometry/size.h"
@@ -59,9 +60,9 @@ class CAPTURE_EXPORT VideoCaptureBufferPool
       int buffer_id) = 0;
 
   // Reserve or allocate a buffer to support a packed frame of |dimensions| of
-  // pixel |format| and return its id. This will fail (returning kInvalidId) if
-  // the pool already is at its |count| limit of the number of allocations, and
-  // all allocated buffers are in use by the producer and/or consumers.
+  // pixel |format| and return its id. If the pool is already at maximum
+  // capacity, this will return kMaxBufferCountExceeded and set |buffer_id| to
+  // |kInvalidId|.
   //
   // If successful, the reserved buffer remains reserved (and writable by the
   // producer) until ownership is transferred either to the consumer via
@@ -71,11 +72,13 @@ class CAPTURE_EXPORT VideoCaptureBufferPool
   // On occasion, this call will decide to free an old buffer to make room for a
   // new allocation at a larger size. If so, the ID of the destroyed buffer is
   // returned via |buffer_id_to_drop|.
-  virtual int ReserveForProducer(const gfx::Size& dimensions,
-                                 VideoPixelFormat format,
-                                 const mojom::PlaneStridesPtr& strides,
-                                 int frame_feedback_id,
-                                 int* buffer_id_to_drop) = 0;
+  virtual VideoCaptureDevice::Client::ReserveResult ReserveForProducer(
+      const gfx::Size& dimensions,
+      VideoPixelFormat format,
+      const mojom::PlaneStridesPtr& strides,
+      int frame_feedback_id,
+      int* buffer_id,
+      int* buffer_id_to_drop) = 0;
 
   // Indicate that a buffer held for the producer should be returned back to the
   // pool without passing on to the consumer. This effectively is the opposite

media/capture/video/video_capture_buffer_pool_impl.cc

@@ -93,15 +93,18 @@ VideoCaptureBufferPoolImpl::GetHandleForInProcessAccess(int buffer_id) {
   return tracker->GetMemoryMappedAccess();
 }
 
-int VideoCaptureBufferPoolImpl::ReserveForProducer(
+VideoCaptureDevice::Client::ReserveResult
+VideoCaptureBufferPoolImpl::ReserveForProducer(
     const gfx::Size& dimensions,
     VideoPixelFormat format,
     const mojom::PlaneStridesPtr& strides,
     int frame_feedback_id,
+    int* buffer_id,
     int* buffer_id_to_drop) {
   base::AutoLock lock(lock_);
   return ReserveForProducerInternal(dimensions, format, strides,
-                                    frame_feedback_id, buffer_id_to_drop);
+                                    frame_feedback_id, buffer_id,
+                                    buffer_id_to_drop);
 }
 
 void VideoCaptureBufferPoolImpl::RelinquishProducerReservation(int buffer_id) {
@@ -157,11 +160,13 @@ double VideoCaptureBufferPoolImpl::GetBufferPoolUtilization() const {
   return static_cast<double>(num_buffers_held) / count_;
 }
 
-int VideoCaptureBufferPoolImpl::ReserveForProducerInternal(
+VideoCaptureDevice::Client::ReserveResult
+VideoCaptureBufferPoolImpl::ReserveForProducerInternal(
     const gfx::Size& dimensions,
     VideoPixelFormat pixel_format,
     const mojom::PlaneStridesPtr& strides,
     int frame_feedback_id,
+    int* buffer_id,
     int* buffer_id_to_drop) {
   lock_.AssertAcquired();
 
@@ -177,7 +182,8 @@ int VideoCaptureBufferPoolImpl::ReserveForProducerInternal(
         // Reuse this buffer
         tracker->set_held_by_producer(true);
         tracker->set_frame_feedback_id(frame_feedback_id);
-        return it->first;
+        *buffer_id = it->first;
+        return VideoCaptureDevice::Client::ReserveResult::kSucceeded;
       }
       if (tracker->GetMemorySizeInBytes() > largest_memory_size_in_bytes) {
         largest_memory_size_in_bytes = tracker->GetMemorySizeInBytes();
@@ -191,27 +197,30 @@ int VideoCaptureBufferPoolImpl::ReserveForProducerInternal(
   if (trackers_.size() == static_cast<size_t>(count_)) {
     if (tracker_to_drop == trackers_.end()) {
       // We're out of space, and can't find an unused tracker to reallocate.
-      return kInvalidId;
+      *buffer_id = kInvalidId;
+      return VideoCaptureDevice::Client::ReserveResult::kMaxBufferCountExceeded;
     }
     *buffer_id_to_drop = tracker_to_drop->first;
     trackers_.erase(tracker_to_drop);
   }
 
   // Create the new tracker.
-  const int buffer_id = next_buffer_id_++;
+  const int new_buffer_id = next_buffer_id_++;
 
   std::unique_ptr<VideoCaptureBufferTracker> tracker =
       buffer_tracker_factory_->CreateTracker();
   if (!tracker->Init(dimensions, pixel_format, strides)) {
     DLOG(ERROR) << "Error initializing VideoCaptureBufferTracker";
-    return kInvalidId;
+    *buffer_id = kInvalidId;
+    return VideoCaptureDevice::Client::ReserveResult::kAllocationFailed;
   }
 
   tracker->set_held_by_producer(true);
   tracker->set_frame_feedback_id(frame_feedback_id);
-  trackers_[buffer_id] = std::move(tracker);
+  trackers_[new_buffer_id] = std::move(tracker);
 
-  return buffer_id;
+  *buffer_id = new_buffer_id;
+  return VideoCaptureDevice::Client::ReserveResult::kSucceeded;
 }
 
 VideoCaptureBufferTracker* VideoCaptureBufferPoolImpl::GetTracker(

media/capture/video/video_capture_buffer_pool_impl.h

@@ -44,11 +44,13 @@ class CAPTURE_EXPORT VideoCaptureBufferPoolImpl
   CreateSharedMemoryViaRawFileDescriptorStruct(int buffer_id) override;
   std::unique_ptr<VideoCaptureBufferHandle> GetHandleForInProcessAccess(
       int buffer_id) override;
-  int ReserveForProducer(const gfx::Size& dimensions,
-                         VideoPixelFormat format,
-                         const mojom::PlaneStridesPtr& strides,
-                         int frame_feedback_id,
-                         int* buffer_id_to_drop) override;
+  VideoCaptureDevice::Client::ReserveResult ReserveForProducer(
+      const gfx::Size& dimensions,
+      VideoPixelFormat format,
+      const mojom::PlaneStridesPtr& strides,
+      int frame_feedback_id,
+      int* buffer_id,
+      int* buffer_id_to_drop) override;
   void RelinquishProducerReservation(int buffer_id) override;
   double GetBufferPoolUtilization() const override;
   void HoldForConsumers(int buffer_id, int num_clients) override;
@@ -58,11 +60,13 @@ class CAPTURE_EXPORT VideoCaptureBufferPoolImpl
   friend class base::RefCountedThreadSafe<VideoCaptureBufferPoolImpl>;
   ~VideoCaptureBufferPoolImpl() override;
 
-  int ReserveForProducerInternal(const gfx::Size& dimensions,
-                                 VideoPixelFormat format,
-                                 const mojom::PlaneStridesPtr& strides,
-                                 int frame_feedback_id,
-                                 int* tracker_id_to_drop);
+  VideoCaptureDevice::Client::ReserveResult ReserveForProducerInternal(
+      const gfx::Size& dimensions,
+      VideoPixelFormat format,
+      const mojom::PlaneStridesPtr& strides,
+      int frame_feedback_id,
+      int* buffer_id,
+      int* tracker_id_to_drop);
 
   VideoCaptureBufferTracker* GetTracker(int buffer_id);
 

media/capture/video/video_capture_device.cc

@@ -74,4 +74,16 @@ PowerLineFrequency VideoCaptureDevice::GetPowerLineFrequency(
   }
 }
 
+VideoCaptureFrameDropReason ConvertReservationFailureToFrameDropReason(
+    VideoCaptureDevice::Client::ReserveResult reserve_result) {
+  switch (reserve_result) {
+    case VideoCaptureDevice::Client::ReserveResult::kSucceeded:
+      return VideoCaptureFrameDropReason::kNone;
+    case VideoCaptureDevice::Client::ReserveResult::kMaxBufferCountExceeded:
+      return VideoCaptureFrameDropReason::kBufferPoolMaxBufferCountExceeded;
+    case VideoCaptureDevice::Client::ReserveResult::kAllocationFailed:
+      return VideoCaptureFrameDropReason::kBufferPoolBufferAllocationFailed;
+  }
+}
+
 }  // namespace media

media/capture/video/video_capture_device.h

@@ -112,14 +112,19 @@ class CAPTURE_EXPORT VideoCaptureDevice
       Buffer(Buffer&& other);
       Buffer& operator=(Buffer&& other);
 
-      bool is_valid() const { return handle_provider != nullptr; }
-
       int id;
       int frame_feedback_id;
       std::unique_ptr<HandleProvider> handle_provider;
       std::unique_ptr<ScopedAccessPermission> access_permission;
     };
 
+    // Result code for calls to ReserveOutputBuffer()
+    enum class ReserveResult {
+      kSucceeded,
+      kMaxBufferCountExceeded,
+      kAllocationFailed
+    };
+
     virtual ~Client() {}
 
     // Captured a new video frame, data for which is pointed to by |data|.
@@ -164,18 +169,20 @@ class CAPTURE_EXPORT VideoCaptureDevice
         int frame_feedback_id = 0) = 0;
 
     // Reserve an output buffer into which contents can be captured directly.
-    // The returned Buffer will always be allocated with a memory size suitable
-    // for holding a packed video frame with pixels of |format| format, of
-    // |dimensions| frame dimensions. It is permissible for |dimensions| to be
-    // zero; in which case the returned Buffer does not guarantee memory
+    // The returned |buffer| will always be allocated with a memory size
+    // suitable for holding a packed video frame with pixels of |format| format,
+    // of |dimensions| frame dimensions. It is permissible for |dimensions| to
+    // be zero; in which case the returned Buffer does not guarantee memory
     // backing, but functions as a reservation for external input for the
     // purposes of buffer throttling.
     //
     // The buffer stays reserved for use by the caller as long as it
     // holds on to the contained |buffer_read_write_permission|.
-    virtual Buffer ReserveOutputBuffer(const gfx::Size& dimensions,
-                                       VideoPixelFormat format,
-                                       int frame_feedback_id) = 0;
+    virtual ReserveResult ReserveOutputBuffer(const gfx::Size& dimensions,
+                                              VideoPixelFormat format,
+                                              int frame_feedback_id,
+                                              Buffer* buffer)
+        WARN_UNUSED_RESULT = 0;
 
     // Provides VCD::Client with a populated Buffer containing the content of
     // the next video frame. The |buffer| must originate from an earlier call to
@@ -309,6 +316,9 @@ class CAPTURE_EXPORT VideoCaptureDevice
   PowerLineFrequency GetPowerLineFrequencyForLocation() const;
 };
 
+VideoCaptureFrameDropReason ConvertReservationFailureToFrameDropReason(
+    VideoCaptureDevice::Client::ReserveResult reserve_result);
+
 }  // namespace media
 
 #endif  // MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_H_

media/capture/video/video_capture_device_client.cc

@@ -183,13 +183,12 @@ void VideoCaptureDeviceClient::OnIncomingCapturedData(
   libyuv::RotationMode rotation_mode = TranslateRotation(rotation);
 
   const gfx::Size dimensions(destination_width, destination_height);
-  Buffer buffer =
-      ReserveOutputBuffer(dimensions, PIXEL_FORMAT_I420, frame_feedback_id);
-  // Failed to reserve I420 output buffer, so drop the frame.
-  if (!buffer.is_valid()) {
+  Buffer buffer;
+  auto reservation_result_code = ReserveOutputBuffer(
+      dimensions, PIXEL_FORMAT_I420, frame_feedback_id, &buffer);
+  if (reservation_result_code != ReserveResult::kSucceeded) {
     receiver_->OnFrameDropped(
-        VideoCaptureFrameDropReason::
-            kDeviceClientFailedToReserveBufferFromBufferPool);
+        ConvertReservationFailureToFrameDropReason(reservation_result_code));
     return;
   }
 
@@ -338,14 +337,14 @@ void VideoCaptureDeviceClient::OnIncomingCapturedGfxBuffer(
   libyuv::RotationMode rotation_mode = TranslateRotation(clockwise_rotation);
 
   const gfx::Size dimensions(destination_width, destination_height);
-  auto output_buffer =
-      ReserveOutputBuffer(dimensions, PIXEL_FORMAT_I420, frame_feedback_id);
+  Buffer output_buffer;
+  const auto reservation_result_code = ReserveOutputBuffer(
+      dimensions, PIXEL_FORMAT_I420, frame_feedback_id, &output_buffer);
 
   // Failed to reserve I420 output buffer, so drop the frame.
-  if (!output_buffer.is_valid()) {
+  if (reservation_result_code != ReserveResult::kSucceeded) {
     receiver_->OnFrameDropped(
-        VideoCaptureFrameDropReason::
-            kDeviceClientFailedToReserveBufferFromBufferPool);
+        ConvertReservationFailureToFrameDropReason(reservation_result_code));
     return;
   }
 
@@ -383,18 +382,21 @@ void VideoCaptureDeviceClient::OnIncomingCapturedGfxBuffer(
                            reference_time, timestamp);
 }
 
-VideoCaptureDevice::Client::Buffer
+VideoCaptureDevice::Client::ReserveResult
 VideoCaptureDeviceClient::ReserveOutputBuffer(const gfx::Size& frame_size,
                                               VideoPixelFormat pixel_format,
-                                              int frame_feedback_id) {
+                                              int frame_feedback_id,
+                                              Buffer* buffer) {
   DFAKE_SCOPED_RECURSIVE_LOCK(call_from_producer_);
   DCHECK_GT(frame_size.width(), 0);
   DCHECK_GT(frame_size.height(), 0);
   DCHECK(IsFormatSupported(pixel_format));
 
   int buffer_id_to_drop = VideoCaptureBufferPool::kInvalidId;
-  const int buffer_id = buffer_pool_->ReserveForProducer(
-      frame_size, pixel_format, nullptr, frame_feedback_id, &buffer_id_to_drop);
+  int buffer_id = VideoCaptureBufferPool::kInvalidId;
+  auto reservation_result_code = buffer_pool_->ReserveForProducer(
+      frame_size, pixel_format, nullptr, frame_feedback_id, &buffer_id,
+      &buffer_id_to_drop);
   if (buffer_id_to_drop != VideoCaptureBufferPool::kInvalidId) {
     // |buffer_pool_| has decided to release a buffer. Notify receiver in case
     // the buffer has already been shared with it.
@@ -406,8 +408,10 @@ VideoCaptureDeviceClient::ReserveOutputBuffer(const gfx::Size& frame_size,
       receiver_->OnBufferRetired(buffer_id_to_drop);
     }
   }
-  if (buffer_id == VideoCaptureBufferPool::kInvalidId)
-    return Buffer();
+  if (reservation_result_code != ReserveResult::kSucceeded)
+    return reservation_result_code;
+
+  DCHECK_NE(VideoCaptureBufferPool::kInvalidId, buffer_id);
 
   if (!base::ContainsValue(buffer_ids_known_by_receiver_, buffer_id)) {
     media::mojom::VideoBufferHandlePtr buffer_handle =
@@ -431,7 +435,8 @@ VideoCaptureDeviceClient::ReserveOutputBuffer(const gfx::Size& frame_size,
     buffer_ids_known_by_receiver_.push_back(buffer_id);
   }
 
-  return MakeBufferStruct(buffer_pool_, buffer_id, frame_feedback_id);
+  *buffer = MakeBufferStruct(buffer_pool_, buffer_id, frame_feedback_id);
+  return ReserveResult::kSucceeded;
 }
 
 void VideoCaptureDeviceClient::OnIncomingCapturedBuffer(
@@ -511,16 +516,16 @@ void VideoCaptureDeviceClient::OnIncomingCapturedY16Data(
     base::TimeTicks reference_time,
     base::TimeDelta timestamp,
     int frame_feedback_id) {
-  Buffer buffer = ReserveOutputBuffer(format.frame_size, PIXEL_FORMAT_Y16,
-                                      frame_feedback_id);
+  Buffer buffer;
+  const auto reservation_result_code = ReserveOutputBuffer(
+      format.frame_size, PIXEL_FORMAT_Y16, frame_feedback_id, &buffer);
   // The input |length| can be greater than the required buffer size because of
   // paddings and/or alignments, but it cannot be smaller.
   DCHECK_GE(static_cast<size_t>(length), format.ImageAllocationSize());
   // Failed to reserve output buffer, so drop the frame.
-  if (!buffer.is_valid()) {
+  if (reservation_result_code != ReserveResult::kSucceeded) {
     receiver_->OnFrameDropped(
-        VideoCaptureFrameDropReason::
-            kDeviceClientFailedToReserveBufferFromBufferPool);
+        ConvertReservationFailureToFrameDropReason(reservation_result_code));
     return;
   }
   auto buffer_access = buffer.handle_provider->GetHandleForInProcessAccess();

media/capture/video/video_capture_device_client.h

@@ -69,9 +69,10 @@ class CAPTURE_EXPORT VideoCaptureDeviceClient
                                    base::TimeTicks reference_time,
                                    base::TimeDelta timestamp,
                                    int frame_feedback_id = 0) override;
-  Buffer ReserveOutputBuffer(const gfx::Size& dimensions,
-                             VideoPixelFormat format,
-                             int frame_feedback_id) override;
+  ReserveResult ReserveOutputBuffer(const gfx::Size& dimensions,
+                                    VideoPixelFormat format,
+                                    int frame_feedback_id,
+                                    Buffer* buffer) override;
   void OnIncomingCapturedBuffer(Buffer buffer,
                                 const VideoCaptureFormat& format,
                                 base::TimeTicks reference_time,

media/capture/video/video_capture_device_unittest.cc

@@ -257,7 +257,7 @@ class VideoCaptureDeviceTest
   std::unique_ptr<MockVideoCaptureDeviceClient> CreateDeviceClient() {
     auto result = std::make_unique<MockVideoCaptureDeviceClient>();
     ON_CALL(*result, OnError(_, _, _)).WillByDefault(Invoke(DumpError));
-    EXPECT_CALL(*result, ReserveOutputBuffer(_, _, _)).Times(0);
+    EXPECT_CALL(*result, ReserveOutputBuffer(_, _, _, _)).Times(0);
     EXPECT_CALL(*result, DoOnIncomingCapturedBuffer(_, _, _, _)).Times(0);
     EXPECT_CALL(*result, DoOnIncomingCapturedBufferExt(_, _, _, _, _, _))
         .Times(0);

media/capture/video/win/video_capture_device_mf_win_unittest.cc

@@ -41,8 +41,8 @@ class MockClient : public VideoCaptureDevice::Client {
                                    base::TimeDelta timestamp,
                                    int frame_feedback_id = 0) override {}
 
-  MOCK_METHOD3(ReserveOutputBuffer,
-               Buffer(const gfx::Size&, VideoPixelFormat, int));
+  MOCK_METHOD4(ReserveOutputBuffer,
+               ReserveResult(const gfx::Size&, VideoPixelFormat, int, Buffer*));
 
   void OnIncomingCapturedBuffer(Buffer buffer,
                                 const VideoCaptureFormat& format,

media/capture/video_capture_types.h

@@ -186,7 +186,6 @@ enum class VideoCaptureError {
 enum class VideoCaptureFrameDropReason {
   kNone = 0,
   kDeviceClientFrameHasInvalidFormat = 1,
-  kDeviceClientFailedToReserveBufferFromBufferPool = 2,
   kDeviceClientLibyuvConvertToI420Failed = 3,
   kV4L2BufferErrorFlagWasSet = 4,
   kV4L2InvalidNumberOfBytesInBuffer = 5,
@@ -199,7 +198,9 @@ enum class VideoCaptureFrameDropReason {
   kWinMediaFoundationReceivedSampleIsNull = 12,
   kWinMediaFoundationLockingBufferDelieveredNullptr = 13,
   kWinMediaFoundationGetBufferByIndexReturnedNull = 14,
-  kMaxValue = 14
+  kBufferPoolMaxBufferCountExceeded = 15,
+  kBufferPoolBufferAllocationFailed = 16,
+  kMaxValue = 16
 };
 
 // Assert that the int:frequency mapping is correct.

services/video_capture/shared_memory_virtual_device_mojo_adapter.cc

@@ -60,8 +60,9 @@ void SharedMemoryVirtualDeviceMojoAdapter::RequestFrameBuffer(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   int buffer_id_to_drop = media::VideoCaptureBufferPool::kInvalidId;
-  const int buffer_id = buffer_pool_->ReserveForProducer(
-      dimension, pixel_format, strides, 0 /* frame_feedback_id */,
+  int buffer_id = media::VideoCaptureBufferPool::kInvalidId;
+  const auto reserve_result = buffer_pool_->ReserveForProducer(
+      dimension, pixel_format, strides, 0 /* frame_feedback_id */, &buffer_id,
       &buffer_id_to_drop);
 
   // Remove dropped buffer if there is one.
@@ -77,8 +78,8 @@ void SharedMemoryVirtualDeviceMojoAdapter::RequestFrameBuffer(
     }
   }
 
-  // No buffer available.
-  if (buffer_id == media::VideoCaptureBufferPool::kInvalidId) {
+  if (reserve_result !=
+      media::VideoCaptureDevice::Client::ReserveResult::kSucceeded) {
     std::move(callback).Run(mojom::kInvalidBufferId);
     return;
   }

tools/metrics/histograms/enums.xml

@@ -50309,6 +50309,8 @@ Full version information for the fingerprint enum values:
   <int value="12" label="kWinMediaFoundationReceivedSampleIsNull"/>
   <int value="13" label="kWinMediaFoundationLockingBufferDelieveredNullptr"/>
   <int value="14" label="kWinMediaFoundationGetBufferByIndexReturnedNull"/>
+  <int value="15" label="kBufferPoolMaxBufferCountExceeded"/>
+  <int value="16" label="kBufferPoolBufferAllocationFailed"/>
 </enum>
 
 <enum name="VideoCaptureServiceEvent">