[Fuchsia] Handle sysmem errors in VideoCaptureDeviceFuchsia

VideCaptureDeviceFuchsia wasn't handling sysmem buffer collection
allocation failures. Fixed it to handle them properly. Also added a
unittest.

Bug: 1111761
Change-Id: Ic56f570dfd8826a3663c5752db15487289bff234
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335651
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798265}

media/capture/video/fuchsia/video_capture_device_fuchsia.cc

@@ -268,6 +268,11 @@ void VideoCaptureDeviceFuchsia::OnBufferCollectionCreated(
     std::unique_ptr<SysmemBufferPool> collection) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  // Buffer collection allocation has failed. This case is not treated as an
+  // error because the camera may create a new collection.
+  if (!collection)
+    return;
+
   buffer_collection_ = std::move(collection);
   buffer_collection_->CreateReader(
       base::BindOnce(&VideoCaptureDeviceFuchsia::OnBufferReaderCreated,
@@ -278,6 +283,13 @@ void VideoCaptureDeviceFuchsia::OnBufferReaderCreated(
     std::unique_ptr<SysmemBufferReader> reader) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  // Buffer collection allocation has failed. This case is not treated as an
+  // error because the camera may create a new collection.
+  if (!reader) {
+    buffer_collection_.reset();
+    return;
+  }
+
   buffer_reader_ = std::move(reader);
   if (!buffer_reader_->buffer_settings().has_image_format_constraints) {
     OnError(FROM_HERE, VideoCaptureError::kFuchsiaSysmemDidNotSetImageFormat,

media/capture/video/fuchsia/video_capture_device_fuchsia_test.cc

@@ -233,6 +233,19 @@ class VideoCaptureDeviceFuchsiaTest : public testing::Test {
     EXPECT_TRUE(fake_device_watcher_.stream()->WaitBuffersAllocated());
   }
 
+  void ProduceAndCaptureFrame() {
+    const uint8_t kFrameSalt = 1;
+
+    auto frame_timestamp = base::TimeTicks::Now();
+    fake_device_watcher_.stream()->ProduceFrame(frame_timestamp, kFrameSalt);
+    client_->WaitFrame();
+
+    ASSERT_EQ(client_->received_frames().size(), 1U);
+    EXPECT_EQ(client_->received_frames()[0].reference_time, frame_timestamp);
+    ValidateReceivedFrame(client_->received_frames()[0],
+                          FakeCameraStream::kDefaultFrameSize, kFrameSalt);
+  }
+
  protected:
   base::test::SingleThreadTaskEnvironment task_environment_{
       base::test::SingleThreadTaskEnvironment::MainThreadType::IO};
@@ -252,15 +265,27 @@ TEST_F(VideoCaptureDeviceFuchsiaTest, Initialize) {
 
 TEST_F(VideoCaptureDeviceFuchsiaTest, SendFrame) {
   StartCapturer();
+  ProduceAndCaptureFrame();
+}
 
-  auto frame_timestamp = base::TimeTicks::Now();
-  fake_device_watcher_.stream()->ProduceFrame(frame_timestamp, 1);
-  client_->WaitFrame();
+// Verifies that VideoCaptureDevice can recover from failed Sync() on the sysmem
+// buffer collection.
+TEST_F(VideoCaptureDeviceFuchsiaTest, FailBufferCollectionSync) {
+  fake_device_watcher_.stream()->SetFirstBufferCollectionFailMode(
+      FakeCameraStream::SysmemFailMode::kFailSync);
 
-  ASSERT_EQ(client_->received_frames().size(), 1U);
-  EXPECT_EQ(client_->received_frames()[0].reference_time, frame_timestamp);
-  ValidateReceivedFrame(client_->received_frames()[0],
-                        FakeCameraStream::kDefaultFrameSize, 1);
+  StartCapturer();
+  ProduceAndCaptureFrame();
+}
+
+// Verifies that VideoCaptureDevice can recover from sysmem buffer allocation
+// failures..
+TEST_F(VideoCaptureDeviceFuchsiaTest, FailBufferCollectionAllocation) {
+  fake_device_watcher_.stream()->SetFirstBufferCollectionFailMode(
+      FakeCameraStream::SysmemFailMode::kFailAllocation);
+
+  StartCapturer();
+  ProduceAndCaptureFrame();
 }
 
 TEST_F(VideoCaptureDeviceFuchsiaTest, MultipleFrames) {

media/fuchsia/camera/fake_fuchsia_camera.cc

@@ -149,7 +149,12 @@ struct FakeCameraStream::Buffer {
       release_fence_watch_controller;
 };
 
-FakeCameraStream::FakeCameraStream() : binding_(this) {}
+FakeCameraStream::FakeCameraStream()
+    : binding_(this),
+      sysmem_allocator_(base::ComponentContextForProcess()
+                            ->svc()
+                            ->Connect<fuchsia::sysmem::Allocator>()) {}
+
 FakeCameraStream::~FakeCameraStream() = default;
 
 void FakeCameraStream::Bind(
@@ -157,6 +162,11 @@ void FakeCameraStream::Bind(
   binding_.Bind(std::move(request));
 }
 
+void FakeCameraStream::SetFirstBufferCollectionFailMode(
+    SysmemFailMode fail_mode) {
+  first_buffer_collection_fail_mode_ = fail_mode;
+}
+
 void FakeCameraStream::SetFakeResolution(gfx::Size resolution) {
   resolution_ = resolution;
   resolution_update_ =
@@ -282,6 +292,16 @@ void FakeCameraStream::SetBufferCollection(
       token->Duplicate(/*rights_attenuation_mask=*/0, local_token.NewRequest());
   EXPECT_EQ(status, ZX_OK);
 
+  fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> failed_token;
+  if (first_buffer_collection_fail_mode_ == SysmemFailMode::kFailSync) {
+    // Create an additional token that's dropped before this method returns.
+    // This will cause sysmem to fail the collection, so the future attempt to
+    // Sync() the collection from the production code will fail as well.
+    zx_status_t status = token->Duplicate(/*rights_attenuation_mask=*/0,
+                                          failed_token.NewRequest());
+    EXPECT_EQ(status, ZX_OK);
+  }
+
   status = token->Sync();
   EXPECT_EQ(status, ZX_OK);
 
@@ -290,11 +310,7 @@ void FakeCameraStream::SetBufferCollection(
   SendBufferCollection();
 
   // Initialize the new collection using |local_token|.
-  auto allocator = base::ComponentContextForProcess()
-                       ->svc()
-                       ->Connect<fuchsia::sysmem::Allocator>();
-
-  allocator->BindSharedCollection(std::move(local_token),
+  sysmem_allocator_->BindSharedCollection(std::move(local_token),
                                           buffer_collection_.NewRequest());
   EXPECT_EQ(status, ZX_OK);
 
@@ -321,6 +337,13 @@ void FakeCameraStream::SetBufferCollection(
   constraints.image_format_constraints[0].required_max_coded_height =
       kMaxFrameSize.height();
 
+  if (first_buffer_collection_fail_mode_ == SysmemFailMode::kFailAllocation) {
+    // Set color space to SRGB to trigger sysmem collection failure (SRGB is not
+    // compatible with NV12 pixel type).
+    constraints.image_format_constraints[0].color_space[0].type =
+        fuchsia::sysmem::ColorSpaceType::SRGB;
+  }
+
   buffer_collection_->SetConstraints(/*has_constraints=*/true,
                                      std::move(constraints));
   buffer_collection_->WaitForBuffersAllocated(
@@ -345,6 +368,16 @@ void FakeCameraStream::NotImplemented_(const std::string& name) {
 }
 
 void FakeCameraStream::OnBufferCollectionError(zx_status_t status) {
+  if (first_buffer_collection_fail_mode_ != SysmemFailMode::kNone) {
+    first_buffer_collection_fail_mode_ = SysmemFailMode::kNone;
+
+    // Create a new buffer collection to retry buffer allocation.
+    fuchsia::sysmem::BufferCollectionTokenPtr token;
+    sysmem_allocator_->AllocateSharedCollection(token.NewRequest());
+    SetBufferCollection(std::move(token));
+    return;
+  }
+
   ADD_FAILURE() << "BufferCollection failed.";
   if (wait_buffers_allocated_run_loop_)
     wait_buffers_allocated_run_loop_->Quit();

media/fuchsia/camera/fake_fuchsia_camera.h

@@ -26,6 +26,20 @@ class FakeCameraStream : public fuchsia::camera3::testing::Stream_TestBase,
   static const gfx::Size kMaxFrameSize;
   static const gfx::Size kDefaultFrameSize;
 
+  // Enum used to specify how sysmem collection allocation is expected to fail.
+  enum class SysmemFailMode {
+    // Don't simulate sysmem failure.
+    kNone,
+
+    // Force Sync() failure. Implemented by dropping one of sysmem collection
+    // tokens.
+    kFailSync,
+
+    // Force buffer allocation failure. Implemented by setting incompatible
+    // constraints.
+    kFailAllocation,
+  };
+
   // Verifies that the I420 image stored at |data| matches the frame produced
   // by ProduceFrame().
   static void ValidateFrameData(const uint8_t* data,
@@ -40,6 +54,10 @@ class FakeCameraStream : public fuchsia::camera3::testing::Stream_TestBase,
 
   void Bind(fidl::InterfaceRequest<fuchsia::camera3::Stream> request);
 
+  // Forces the stream to simulate sysmem buffer collection failure for the
+  // first buffer collection.
+  void SetFirstBufferCollectionFailMode(SysmemFailMode fail_mode);
+
   void SetFakeResolution(gfx::Size resolution);
   void SetFakeOrientation(fuchsia::camera3::Orientation orientation);
 
@@ -115,6 +133,7 @@ class FakeCameraStream : public fuchsia::camera3::testing::Stream_TestBase,
   base::Optional<fuchsia::camera3::FrameInfo> next_frame_;
   GetNextFrameCallback get_next_frame_callback_;
 
+  fuchsia::sysmem::AllocatorPtr sysmem_allocator_;
   fuchsia::sysmem::BufferCollectionPtr buffer_collection_;
 
   base::Optional<base::RunLoop> wait_buffers_allocated_run_loop_;
@@ -124,6 +143,8 @@ class FakeCameraStream : public fuchsia::camera3::testing::Stream_TestBase,
   size_t num_used_buffers_ = 0;
 
   size_t frame_counter_ = 0;
+
+  SysmemFailMode first_buffer_collection_fail_mode_ = SysmemFailMode::kNone;
 };
 
 class FakeCameraDevice : public fuchsia::camera3::testing::Device_TestBase {