Improve buffer size checking in MojoSharedBufferVideoFrame

If the U and V data is interleaved in the buffer, then the last row of data
doesn't require a full stride. So only count RowBytes() for the last row
when computing where the end of the data is.

BUG=829443
TEST=new media_mojo_unittest passes

Change-Id: I186e4dd9d2045f34897bbf2c1d71e04421e199b7
Reviewed-on: https://chromium-review.googlesource.com/1114335
Commit-Queue: John Rummell <jrummell@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571001}

media/mojo/common/mojo_shared_buffer_video_frame.cc

@@ -93,27 +93,37 @@ scoped_refptr<MojoSharedBufferVideoFrame> MojoSharedBufferVideoFrame::Create(
     return nullptr;
   }
 
+  // Compute the number of bytes needed on each row.
+  const size_t y_row_bytes = RowBytes(kYPlane, format, coded_size.width());
+  const size_t u_row_bytes = RowBytes(kUPlane, format, coded_size.width());
+  const size_t v_row_bytes = RowBytes(kVPlane, format, coded_size.width());
+
   // Safe given sizeof(size_t) >= sizeof(int32_t).
   size_t y_stride_size_t = y_stride;
   size_t u_stride_size_t = u_stride;
   size_t v_stride_size_t = v_stride;
-  if (y_stride_size_t < RowBytes(kYPlane, format, coded_size.width()) ||
-      u_stride_size_t < RowBytes(kUPlane, format, coded_size.width()) ||
-      v_stride_size_t < RowBytes(kVPlane, format, coded_size.width())) {
+  if (y_stride_size_t < y_row_bytes || u_stride_size_t < u_row_bytes ||
+      v_stride_size_t < v_row_bytes) {
     DLOG(ERROR) << __func__ << " Invalid stride";
     return nullptr;
   }
 
-  base::CheckedNumeric<size_t> y_rows =
-      Rows(kYPlane, format, coded_size.height());
-  base::CheckedNumeric<size_t> u_rows =
-      Rows(kUPlane, format, coded_size.height());
-  base::CheckedNumeric<size_t> v_rows =
-      Rows(kVPlane, format, coded_size.height());
-
-  base::CheckedNumeric<size_t> y_bound = y_rows * y_stride + y_offset;
-  base::CheckedNumeric<size_t> u_bound = u_rows * u_stride + u_offset;
-  base::CheckedNumeric<size_t> v_bound = v_rows * v_stride + v_offset;
+  const size_t y_rows = Rows(kYPlane, format, coded_size.height());
+  const size_t u_rows = Rows(kUPlane, format, coded_size.height());
+  const size_t v_rows = Rows(kVPlane, format, coded_size.height());
+
+  // The last row only needs RowBytes() and not a full stride. This is to avoid
+  // problems if the U and V data is interleaved (where |stride| is double the
+  // number of bytes actually needed).
+  base::CheckedNumeric<size_t> y_bound = base::CheckAdd(
+      y_offset, base::CheckMul(base::CheckSub(y_rows, 1), y_stride_size_t),
+      y_row_bytes);
+  base::CheckedNumeric<size_t> u_bound = base::CheckAdd(
+      u_offset, base::CheckMul(base::CheckSub(u_rows, 1), u_stride_size_t),
+      u_row_bytes);
+  base::CheckedNumeric<size_t> v_bound = base::CheckAdd(
+      v_offset, base::CheckMul(base::CheckSub(v_rows, 1), v_stride_size_t),
+      v_row_bytes);
 
   if (!y_bound.IsValid() || !u_bound.IsValid() || !v_bound.IsValid() ||
       y_bound.ValueOrDie() > data_size || u_bound.ValueOrDie() > data_size ||

media/mojo/common/mojo_shared_buffer_video_frame_unittest.cc

@@ -175,4 +175,54 @@ TEST(MojoSharedBufferVideoFrameTest, TestDestructionCallback) {
   EXPECT_TRUE(callback_called);
 }
 
+TEST(MojoSharedBufferVideoFrameTest, InterleavedData) {
+  const VideoPixelFormat format = PIXEL_FORMAT_I420;
+  const int kWidth = 32;
+  const int kHeight = 18;
+  const base::TimeDelta kTimestamp = base::TimeDelta::FromMicroseconds(1338);
+  gfx::Size size(kWidth, kHeight);
+  gfx::Rect visible_rect(size);
+
+  // Create interlaced UV data, which are each 1/4 the size of the Y data.
+  const size_t y_offset = 0;
+  const size_t u_offset =
+      VideoFrame::PlaneSize(format, VideoFrame::kYPlane, size).GetArea();
+  const size_t v_offset =
+      u_offset + VideoFrame::RowBytes(VideoFrame::kUPlane, format, kWidth);
+  const int32_t y_stride =
+      VideoFrame::RowBytes(VideoFrame::kYPlane, format, kWidth);
+  const int32_t u_stride = y_stride;
+  const int32_t v_stride = y_stride;
+
+  // Allocate some shared memory.
+  size_t requested_size = VideoFrame::AllocationSize(format, size);
+  mojo::ScopedSharedBufferHandle handle =
+      mojo::SharedBufferHandle::Create(requested_size);
+  ASSERT_TRUE(handle.is_valid());
+
+  // Allocate frame.
+  scoped_refptr<MojoSharedBufferVideoFrame> frame =
+      MojoSharedBufferVideoFrame::Create(format, size, visible_rect, size,
+                                         std::move(handle), requested_size,
+                                         y_offset, u_offset, v_offset, y_stride,
+                                         u_stride, v_stride, kTimestamp);
+  ASSERT_TRUE(frame.get());
+  EXPECT_EQ(frame->format(), format);
+
+  // The offsets should be set appropriately.
+  EXPECT_EQ(frame->PlaneOffset(VideoFrame::kYPlane), y_offset);
+  EXPECT_EQ(frame->PlaneOffset(VideoFrame::kUPlane), u_offset);
+  EXPECT_EQ(frame->PlaneOffset(VideoFrame::kVPlane), v_offset);
+
+  // The strides should be set appropriately.
+  EXPECT_EQ(frame->stride(VideoFrame::kYPlane), y_stride);
+  EXPECT_EQ(frame->stride(VideoFrame::kUPlane), u_stride);
+  EXPECT_EQ(frame->stride(VideoFrame::kVPlane), v_stride);
+
+  // The data pointers for each plane should be set.
+  EXPECT_TRUE(frame->data(VideoFrame::kYPlane));
+  EXPECT_TRUE(frame->data(VideoFrame::kUPlane));
+  EXPECT_TRUE(frame->data(VideoFrame::kVPlane));
+}
+
 }  // namespace media