components/arc: GAVEA maps video frame with stride and offset provided by Android

GAVEA formerly computes the mapped size of video frame and addresses of planes
from format and coded size. We recently changed code in Android to use minigbm
stack to allocate video frame buffer. It is infeasible to compute them only from
format and coded size. We have to use offset and stride that is provided by
Android.

Bug: b:118544836
Test: CtsVideoTestCases and CtsMediaTestCases
Change-Id: Ic2dca4c3fcbc9116ef6890c3802c1ac8afb97a33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1676941
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682576}

components/arc/video_accelerator/arc_video_accelerator_util.cc

@@ -8,6 +8,8 @@
 #include <unistd.h>
 
 #include "base/files/platform_file.h"
+#include "base/numerics/checked_math.h"
+#include "media/base/video_frame.h"
 #include "media/gpu/macros.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 
@@ -50,4 +52,49 @@ bool GetFileSize(const int fd, size_t* size) {
   return true;
 }
 
+bool VerifyVideoFrame(media::VideoPixelFormat pixel_format,
+                      const gfx::Size& coded_size,
+                      int fd,
+                      const std::vector<VideoFramePlane>& planes) {
+  const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format);
+  if (planes.size() != num_planes || num_planes == 0) {
+    VLOGF(1) << "Invalid number of dmabuf planes passed: " << planes.size()
+             << ", expected: " << num_planes;
+    return false;
+  }
+
+  // We expect offset monotonically increase.
+  for (size_t i = 1; i < num_planes; i++) {
+    if (planes[i].offset < planes[i - 1].offset)
+      return false;
+  }
+
+  size_t file_size_in_bytes;
+  if (!GetFileSize(fd, &file_size_in_bytes))
+    return false;
+
+  for (size_t i = 0; i < planes.size(); ++i) {
+    const auto& plane = planes[i];
+
+    DVLOGF(4) << "Plane " << i << ", offset: " << plane.offset
+              << ", stride: " << plane.stride;
+
+    // Check |offset| + (the size of a plane) on each plane is not larger than
+    // |file_size_in_bytes|. This ensures we don't access out of a buffer
+    // referred by |fd|.
+    size_t row_bytes =
+        media::VideoFrame::RowBytes(i, pixel_format, coded_size.height());
+    base::CheckedNumeric<size_t> current_size(plane.offset);
+    current_size += base::CheckMul(plane.stride, row_bytes);
+
+    if (!current_size.IsValid() ||
+        current_size.ValueOrDie() > file_size_in_bytes) {
+      VLOGF(1) << "Invalid strides/offsets.";
+      return false;
+    }
+  }
+
+  return true;
+}
+
 }  // namespace arc

components/arc/video_accelerator/arc_video_accelerator_util.h

@@ -5,16 +5,28 @@
 #ifndef COMPONENTS_ARC_VIDEO_ACCELERATOR_ARC_VIDEO_ACCELERATOR_UTIL_H_
 #define COMPONENTS_ARC_VIDEO_ACCELERATOR_ARC_VIDEO_ACCELERATOR_UTIL_H_
 
+#include <vector>
+
 #include "base/files/scoped_file.h"
+#include "components/arc/video_accelerator/video_frame_plane.h"
+#include "media/base/video_types.h"
 #include "mojo/public/cpp/system/handle.h"
+#include "ui/gfx/geometry/size.h"
+
 namespace arc {
 
 // Creates ScopedFD from given mojo::ScopedHandle.
 // Returns invalid ScopedFD on failure.
 base::ScopedFD UnwrapFdFromMojoHandle(mojo::ScopedHandle handle);
 
-// Return the file size of |fd|.
+// Return the file size of |fd| in bytes.
 bool GetFileSize(const int fd, size_t* size);
 
+// Return true iff |planes| is valid for a video frame located on |fd|
+// and of |pixel_format| and |coded_size|.
+bool VerifyVideoFrame(media::VideoPixelFormat pixel_format,
+                      const gfx::Size& coded_size,
+                      int fd,
+                      const std::vector<VideoFramePlane>& planes);
 }  // namespace arc
 #endif  // COMPONENTS_ARC_VIDEO_ACCELERATOR_ARC_VIDEO_ACCELERATOR_UTIL_H_

components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc

@@ -68,42 +68,6 @@ arc::mojom::VideoDecodeAccelerator::Result ConvertErrorCode(
   }
 }
 
-// Return true iff |planes| is valid for a video frame located on |dmabuf_fd|
-// and of |pixel_format|.
-bool VerifyDmabuf(media::VideoPixelFormat pixel_format,
-                  const gfx::Size& coded_size,
-                  int dmabuf_fd,
-                  const std::vector<arc::VideoFramePlane>& planes) {
-  const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format);
-  if (planes.size() != num_planes || num_planes == 0) {
-    VLOGF(1) << "Invalid number of dmabuf planes passed: " << planes.size()
-             << ", expected: " << num_planes;
-    return false;
-  }
-
-  size_t size;
-  if (!arc::GetFileSize(dmabuf_fd, &size))
-    return false;
-
-  for (size_t i = 0; i < planes.size(); ++i) {
-    const auto& plane = planes[i];
-
-    DVLOGF(4) << "Plane " << i << ", offset: " << plane.offset
-              << ", stride: " << plane.stride;
-
-    size_t rows = media::VideoFrame::Rows(i, pixel_format, coded_size.height());
-    base::CheckedNumeric<size_t> current_size(plane.offset);
-    current_size += base::CheckMul(plane.stride, rows);
-
-    if (!current_size.IsValid() || current_size.ValueOrDie() > size) {
-      VLOGF(1) << "Invalid strides/offsets.";
-      return false;
-    }
-  }
-
-  return true;
-}
-
 }  // namespace
 
 namespace arc {
@@ -527,7 +491,7 @@ void GpuArcVideoDecodeAccelerator::ImportBufferForPicture(
     }
     gmb_handle.native_pixmap_handle = std::move(protected_native_pixmap);
   } else {
-    if (!VerifyDmabuf(pixel_format, coded_size_, handle_fd.get(), planes)) {
+    if (!VerifyVideoFrame(pixel_format, coded_size_, handle_fd.get(), planes)) {
       VLOGF(1) << "Failed verifying dmabuf";
       client_->NotifyError(
           mojom::VideoDecodeAccelerator::Result::INVALID_ARGUMENT);

components/arc/video_accelerator/gpu_arc_video_encode_accelerator.cc

@@ -153,8 +153,45 @@ void GpuArcVideoEncodeAccelerator::EncodeSharedMemory(
     return;
   }
 
-  size_t allocation_size =
-      media::VideoFrame::AllocationSize(format, coded_size_);
+  if (!VerifyVideoFrame(format, coded_size_, fd.get(), planes)) {
+    client_->NotifyError(Error::kInvalidArgumentError);
+    return;
+  }
+
+  const size_t num_planes = media::VideoFrame::NumPlanes(format);
+  // This is guaranteed because format is I420 here.
+  DCHECK_EQ(num_planes, 3u);
+  std::vector<media::VideoFrameLayout::Plane> layout_planes(num_planes);
+  for (size_t i = 0; i < num_planes; i++) {
+    layout_planes[i].stride = planes[i].stride;
+    layout_planes[i].offset = planes[i].offset;
+    if (i != num_planes - 1) {
+      layout_planes[i].size = planes[i + 1].offset - planes[i].offset;
+    } else {
+      layout_planes[i].size =
+          media::VideoFrame::Rows(i, format, visible_size_.height()) *
+          planes[i].stride;
+    }
+  }
+
+  auto layout = media::VideoFrameLayout::CreateWithPlanes(
+      format, coded_size_, std::move(layout_planes));
+  if (!layout) {
+    DLOG(ERROR) << "Failed to create VideoFrameLayout.";
+    client_->NotifyError(Error::kInvalidArgumentError);
+    return;
+  }
+
+  base::CheckedNumeric<size_t> map_size = planes[0].offset;
+  for (size_t i = 0; i < num_planes; i++) {
+    map_size += layout->planes()[i].size;
+  }
+  if (!map_size.IsValid()) {
+    DLOG(ERROR) << "Invalid map_size";
+    client_->NotifyError(Error::kInvalidArgumentError);
+    return;
+  }
+
   // TODO(rockot): Pass GUIDs through Mojo. https://crbug.com/713763.
   // TODO(rockot): This fd comes from a mojo::ScopedHandle in
   // GpuArcVideoService::BindSharedMemory. That should be passed through,
@@ -164,24 +201,11 @@ void GpuArcVideoEncodeAccelerator::EncodeSharedMemory(
       base::subtle::PlatformSharedMemoryRegion::Take(
           std::move(fd),
           base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
-          allocation_size, guid);
+          map_size.ValueOrDie(), guid);
   base::UnsafeSharedMemoryRegion shared_region =
       base::UnsafeSharedMemoryRegion::Deserialize(std::move(platform_region));
-
-  base::CheckedNumeric<off_t> map_offset = planes[0].offset;
-  base::CheckedNumeric<size_t> map_size = allocation_size;
-  const uint32_t aligned_offset =
-      planes[0].offset % base::SysInfo::VMAllocationGranularity();
-  map_offset -= aligned_offset;
-  map_size += aligned_offset;
-
-  if (!map_offset.IsValid() || !map_size.IsValid()) {
-    DLOG(ERROR) << "Invalid map_offset or map_size";
-    client_->NotifyError(Error::kInvalidArgumentError);
-    return;
-  }
   base::WritableSharedMemoryMapping mapping =
-      shared_region.MapAt(map_offset.ValueOrDie(), map_size.ValueOrDie());
+      shared_region.MapAt(0u, map_size.ValueOrDie());
   if (!mapping.IsValid()) {
     DLOG(ERROR) << "Failed to map memory.";
     client_->NotifyError(Error::kPlatformFailureError);
@@ -189,12 +213,23 @@ void GpuArcVideoEncodeAccelerator::EncodeSharedMemory(
   }
 
   uint8_t* shm_memory = mapping.GetMemoryAsSpan<uint8_t>().data();
-  auto frame = media::VideoFrame::WrapExternalData(
-      format, coded_size_, gfx::Rect(visible_size_), visible_size_,
-      shm_memory + aligned_offset, allocation_size,
+  DCHECK_EQ(layout->planes().size(), num_planes);
+  auto frame = media::VideoFrame::WrapExternalYuvDataWithLayout(
+      *layout, gfx::Rect(visible_size_), visible_size_,
+      shm_memory + layout->planes()[0].offset,
+      shm_memory + layout->planes()[1].offset,
+      shm_memory + layout->planes()[2].offset,
       base::TimeDelta::FromMicroseconds(timestamp));
-  frame->BackWithOwnedSharedMemory(std::move(shared_region), std::move(mapping),
-                                   planes[0].offset);
+  if (!frame) {
+    DLOG(ERROR) << "Failed to create VideoFrame";
+    client_->NotifyError(Error::kInvalidArgumentError);
+    return;
+  }
+  frame->BackWithOwnedSharedMemory(std::move(shared_region),
+                                   std::move(mapping));
+  // Add the function to |callback| to |frame|'s  destruction observer. When the
+  // |frame| goes out of scope, it executes |callback|.
+  frame->AddDestructionObserver(std::move(callback));
   accelerator_->Encode(frame, force_keyframe);
 }