components/arc: make CreateGpuMemoryBufferHandle support multiple fds.

Originally, CreateGpuMemoryBufferHandle() function only takes one fd
as argument, and assume each plane of the video frame stores at the
same fd.

This CL overloads CreateGpuMemoryBufferHandle() that takes multiple
fds as arguments. Now we could pass different fd for each plane.

BUG=b:127230761
TEST=autoninja components_unittests

Change-Id: Ie6f132074c7125cf926786924dc30165de90ee24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1826742
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701034}

components/arc/video_accelerator/arc_video_accelerator_util.cc

@@ -36,23 +36,14 @@ bool VerifyGpuMemoryBufferHandle(media::VideoPixelFormat pixel_format,
     return false;
   }
 
-  // Offsets monotonically increase and strides monotonically decrease.
-  // Note: this offset assumption might not be correct if planes are stored in
-  // multiple buffers.
-  // TODO(b/127230761): Remove this offset check once one fd is given per one
-  // plane.
+  // Strides monotonically decrease.
   for (size_t i = 1; i < num_planes; i++) {
-    if (gmb_handle.native_pixmap_handle.planes[i].offset <
-        gmb_handle.native_pixmap_handle.planes[i - 1].offset) {
-      return false;
-    }
     if (gmb_handle.native_pixmap_handle.planes[i - 1].stride <
         gmb_handle.native_pixmap_handle.planes[i].stride) {
       return false;
     }
   }
 
-  size_t prev_buffer_end = 0;
   for (size_t i = 0; i < num_planes; i++) {
     const auto& plane = gmb_handle.native_pixmap_handle.planes[i];
     DVLOGF(4) << "Plane " << i << ", offset: " << plane.offset
@@ -82,15 +73,6 @@ bool VerifyGpuMemoryBufferHandle(media::VideoPixelFormat pixel_format,
       VLOGF(1) << "Invalid strides/offsets";
       return false;
     }
-
-    // The end of the previous plane must not be bigger than the offset of the
-    // current plane.
-    // TODO(b/127230761): Remove this check once one fd is given per one plane.
-    if (prev_buffer_end > base::checked_cast<size_t>(plane.offset)) {
-      VLOGF(1) << "Invalid offset";
-      return false;
-    }
-    prev_buffer_end = min_buffer_size.ValueOrDie();
   }
 
   return true;
@@ -140,27 +122,66 @@ bool GetFileSize(const int fd, size_t* size) {
   return true;
 }
 
+std::vector<base::ScopedFD> DuplicateFD(base::ScopedFD fd, size_t num_fds) {
+  if (!fd.is_valid()) {
+    VLOGF(1) << "Input fd is not valid";
+    return {};
+  }
+
+  std::vector<base::ScopedFD> fds(num_fds);
+  fds[0] = std::move(fd);
+  for (size_t i = 1; i < num_fds; ++i) {
+    base::ScopedFD dup_fd(HANDLE_EINTR(dup(fds[0].get())));
+    if (!dup_fd.is_valid()) {
+      VLOGF(1) << "Failed to duplicate fd";
+      return {};
+    }
+    fds[i] = std::move(dup_fd);
+  }
+
+  return fds;
+}
+
 base::Optional<gfx::GpuMemoryBufferHandle> CreateGpuMemoryBufferHandle(
     media::VideoPixelFormat pixel_format,
     const gfx::Size& coded_size,
-    base::ScopedFD fd,
+    std::vector<base::ScopedFD> scoped_fds,
     const std::vector<VideoFramePlane>& planes) {
+  std::vector<media::ColorPlaneLayout> color_planes;
+  for (size_t i = 0; i < planes.size(); ++i) {
+    int32_t stride = base::checked_cast<int32_t>(planes[i].stride);
+    size_t offset = base::checked_cast<size_t>(planes[i].offset);
+    size_t plane_height =
+        media::VideoFrame::Rows(i, pixel_format, coded_size.height());
+    base::CheckedNumeric<size_t> current_size =
+        base::CheckMul(stride, plane_height);
+    if (!current_size.IsValid()) {
+      VLOGF(1) << "Invalid stride/height";
+      return base::nullopt;
+    }
+
+    color_planes.emplace_back(stride, offset, current_size.ValueOrDie());
+  }
+
+  return CreateGpuMemoryBufferHandle(pixel_format, coded_size,
+                                     std::move(scoped_fds), color_planes);
+}
+
+base::Optional<gfx::GpuMemoryBufferHandle> CreateGpuMemoryBufferHandle(
+    media::VideoPixelFormat pixel_format,
+    const gfx::Size& coded_size,
+    std::vector<base::ScopedFD> scoped_fds,
+    const std::vector<media::ColorPlaneLayout>& planes) {
   const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format);
   if (planes.size() != num_planes || planes.size() == 0) {
     VLOGF(1) << "Invalid number of dmabuf planes passed: " << planes.size()
              << ", expected: " << num_planes;
     return base::nullopt;
   }
-
-  std::array<base::ScopedFD, media::VideoFrame::kMaxPlanes> scoped_fds;
-  DCHECK_LE(num_planes, media::VideoFrame::kMaxPlanes);
-  scoped_fds[0] = std::move(fd);
-  for (size_t i = 1; i < num_planes; ++i) {
-    scoped_fds[i].reset(HANDLE_EINTR(dup(scoped_fds[0].get())));
-    if (!scoped_fds[i].is_valid()) {
-      VLOGF(1) << "Failed to duplicate fd";
-      return base::nullopt;
-    }
+  if (scoped_fds.size() != num_planes) {
+    VLOGF(1) << "Invalid number of fds passed: " << scoped_fds.size()
+             << ", expected: " << num_planes;
+    return base::nullopt;
   }
 
   gfx::GpuMemoryBufferHandle gmb_handle;
@@ -178,17 +199,9 @@ base::Optional<gfx::GpuMemoryBufferHandle> CreateGpuMemoryBufferHandle(
     }
     uint32_t stride = base::checked_cast<uint32_t>(planes[i].stride);
     uint64_t offset = base::checked_cast<uint64_t>(planes[i].offset);
-
-    size_t plane_height =
-        media::VideoFrame::Rows(i, pixel_format, coded_size.height());
-    base::CheckedNumeric<uint64_t> current_size =
-        base::CheckMul(stride, plane_height);
-    if (!current_size.IsValid()) {
-      VLOGF(1) << "Invalid stride/height";
-      return base::nullopt;
-    }
+    uint64_t size = base::checked_cast<uint64_t>(planes[i].size);
     gmb_handle.native_pixmap_handle.planes.emplace_back(
-        stride, offset, current_size.ValueOrDie(), std::move(scoped_fds[i]));
+        stride, offset, size, std::move(scoped_fds[i]));
   }
 
   if (!VerifyGpuMemoryBufferHandle(pixel_format, coded_size, gmb_handle))

components/arc/video_accelerator/arc_video_accelerator_util.h

@@ -11,6 +11,7 @@
 #include "base/files/scoped_file.h"
 #include "base/optional.h"
 #include "components/arc/video_accelerator/video_frame_plane.h"
+#include "media/base/color_plane_layout.h"
 #include "media/base/video_types.h"
 #include "mojo/public/cpp/system/handle.h"
 #include "ui/gfx/geometry/size.h"
@@ -25,14 +26,23 @@ base::ScopedFD UnwrapFdFromMojoHandle(mojo::ScopedHandle handle);
 // Return the file size of |fd| in bytes.
 bool GetFileSize(const int fd, size_t* size);
 
+// Return a list of duplicated |fd|. The size of list is |num_fds|. Return an
+// empty list if duplicatation fails.
+std::vector<base::ScopedFD> DuplicateFD(base::ScopedFD fd, size_t num_fds);
+
 // Return GpuMemoryBufferHandle iff |planes| are valid for a video frame located
-// on |fd| and of |pixel_format| and |coded_size|. Otherwise returns
-// base::nullopt.
+// on |scoped_fds| and of |pixel_format| and |coded_size|. Otherwise
+// returns base::nullopt.
 base::Optional<gfx::GpuMemoryBufferHandle> CreateGpuMemoryBufferHandle(
     media::VideoPixelFormat pixel_format,
     const gfx::Size& coded_size,
-    base::ScopedFD fd,
+    std::vector<base::ScopedFD> scoped_fds,
     const std::vector<VideoFramePlane>& planes);
+base::Optional<gfx::GpuMemoryBufferHandle> CreateGpuMemoryBufferHandle(
+    media::VideoPixelFormat pixel_format,
+    const gfx::Size& coded_size,
+    std::vector<base::ScopedFD> scoped_fds,
+    const std::vector<media::ColorPlaneLayout>& planes);
 
 // Create a temp file and write |data| into the file.
 base::ScopedFD CreateTempFileForTesting(const std::string& data);

components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc

@@ -486,8 +486,17 @@ void GpuArcVideoDecodeAccelerator::ImportBufferForPicture(
     }
     gmb_handle.native_pixmap_handle = std::move(protected_native_pixmap);
   } else {
+    std::vector<base::ScopedFD> handle_fds =
+        DuplicateFD(std::move(handle_fd), planes.size());
+    if (handle_fds.empty()) {
+      VLOGF(1) << "Failed to duplicate fd";
+      client_->NotifyError(
+          mojom::VideoDecodeAccelerator::Result::INVALID_ARGUMENT);
+      return;
+    }
+
     auto handle = CreateGpuMemoryBufferHandle(pixel_format, coded_size_,
-                                              std::move(handle_fd), planes);
+                                              std::move(handle_fds), planes);
     if (!handle) {
       VLOGF(1) << "Failed to create GpuMemoryBufferHandle";
       client_->NotifyError(

components/arc/video_accelerator/gpu_arc_video_encode_accelerator.cc

@@ -182,8 +182,14 @@ void GpuArcVideoEncodeAccelerator::EncodeDmabuf(
     return;
   }
 
+  std::vector<base::ScopedFD> fds = DuplicateFD(std::move(fd), planes.size());
+  if (fds.empty()) {
+    DLOG(ERROR) << "Failed to duplicate fd";
+    client_->NotifyError(Error::kInvalidArgumentError);
+    return;
+  }
   auto gmb_handle =
-      CreateGpuMemoryBufferHandle(format, coded_size_, std::move(fd), planes);
+      CreateGpuMemoryBufferHandle(format, coded_size_, std::move(fds), planes);
   if (!gmb_handle) {
     DLOG(ERROR) << "Failed to create GpuMemoryBufferHandle";
     client_->NotifyError(Error::kInvalidArgumentError);
@@ -232,8 +238,17 @@ void GpuArcVideoEncodeAccelerator::EncodeSharedMemory(
     return;
   }
 
-  auto gmb_handle = CreateGpuMemoryBufferHandle(
-      format, coded_size_, base::ScopedFD(HANDLE_EINTR(dup(fd.get()))), planes);
+  base::ScopedFD dup_fd(HANDLE_EINTR(dup(fd.get())));
+  std::vector<base::ScopedFD> fds =
+      DuplicateFD(std::move(dup_fd), planes.size());
+  if (fds.empty()) {
+    DLOG(ERROR) << "Failed to duplicate fd";
+    client_->NotifyError(Error::kInvalidArgumentError);
+    return;
+  }
+
+  auto gmb_handle =
+      CreateGpuMemoryBufferHandle(format, coded_size_, std::move(fds), planes);
   if (!gmb_handle) {
     DLOG(ERROR) << "Failed to create GpuMemoryBufferHandle";
     client_->NotifyError(Error::kInvalidArgumentError);