VideoCapture: Remove deep frame copy in the border to libJingle

media/base/video_frame.cc

Index: media/base/video_frame.cc
diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index 5f0d7282c6e01aedfe0861ee90e55e2cb319b7c1..ea26518e4ab44d41471c448acabd966c64ba6913 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -14,6 +14,7 @@
 #include "gpu/command_buffer/common/mailbox_holder.h"
 #include "media/base/limits.h"
 #include "media/base/video_util.h"
+#include "ui/gfx/point.h"
 
 #if !defined(MEDIA_FOR_CAST_IOS)
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -21,30 +22,84 @@
 
 namespace media {
 
+static bool IsPowerOfTwo(size_t x) {
+  return x != 0 && (x & (x - 1)) == 0;
+}
+
 static inline size_t RoundUp(size_t value, size_t alignment) {
-  // Check that |alignment| is a power of 2.
-  DCHECK((alignment + (alignment - 1)) == (alignment | (alignment - 1)));
+  DCHECK(IsPowerOfTwo(alignment));
   return ((value + (alignment - 1)) & ~(alignment - 1));
 }
 
+static inline size_t RoundDown(size_t value, size_t alignment) {
+  DCHECK(IsPowerOfTwo(alignment));
+  return value & ~(alignment - 1);
+}
+
+// Returns the pixel size per element for given |plane| and |format|. E.g. 2x2
+// for the U-plane in I420.
+static gfx::Size SampleSize(VideoFrame::Format format, size_t plane) {
+  DCHECK(VideoFrame::IsValidPlane(plane, format));
+
+  switch (plane) {
+    case VideoFrame::kYPlane:
+    case VideoFrame::kAPlane:
+      return gfx::Size(1, 1);
+
+    case VideoFrame::kUPlane:  // and VideoFrame::kUVPlane:
+    case VideoFrame::kVPlane:
+      switch (format) {
+        case VideoFrame::YV24:
+          return gfx::Size(1, 1);
+
+        case VideoFrame::YV16:
+          return gfx::Size(2, 1);
+
+        case VideoFrame::YV12:
+        case VideoFrame::YV12J:
+        case VideoFrame::I420:
+        case VideoFrame::YV12A:
+        case VideoFrame::NV12:
+          return gfx::Size(2, 2);
+
+        case VideoFrame::UNKNOWN:
+#if defined(VIDEO_HOLE)
+        case VideoFrame::HOLE:
+#endif  // defined(VIDEO_HOLE)
+        case VideoFrame::NATIVE_TEXTURE:
+          break;
+      }
+  }
+  NOTREACHED();
+  return gfx::Size();
+}
+
+// Return the alignment for the whole frame, calculated as the max of the
+// alignment for each individual plane.
+static gfx::Size CommonAlignment(VideoFrame::Format format) {
+  int max_sample_width = 0;
+  int max_sample_height = 0;
+  for (size_t plane = 0; plane < VideoFrame::NumPlanes(format); ++plane) {
+    const gfx::Size sample_size = SampleSize(format, plane);
+    max_sample_width = std::max(max_sample_width, sample_size.width());
+    max_sample_height = std::max(max_sample_height, sample_size.height());
+  }
+  return gfx::Size(max_sample_width, max_sample_height);
+}
+
+// Returns the number of bytes per element for given |plane| and |format|. E.g.
+// 2 for the UV plane in NV12.
+static int BytesPerElement(VideoFrame::Format format, size_t plane) {
+  DCHECK(VideoFrame::IsValidPlane(plane, format));
+  return (format == VideoFrame::NV12 && plane == VideoFrame::kUVPlane) ? 2 : 1;
+}
+
 // Rounds up |coded_size| if necessary for |format|.
 static gfx::Size AdjustCodedSize(VideoFrame::Format format,
                                  const gfx::Size& coded_size) {
-  gfx::Size new_coded_size(coded_size);
-  switch (format) {
-    case VideoFrame::YV12:
-    case VideoFrame::YV12A:
-    case VideoFrame::I420:
-    case VideoFrame::YV12J:
-      new_coded_size.set_height(RoundUp(new_coded_size.height(), 2));
-    // Fallthrough.
-    case VideoFrame::YV16:
-      new_coded_size.set_width(RoundUp(new_coded_size.width(), 2));
-      break;
-    default:
-      break;
-  }
-  return new_coded_size;
+  const gfx::Size alignment = CommonAlignment(format);
+  return gfx::Size(RoundUp(coded_size.width(), alignment.width()),
+                   RoundUp(coded_size.height(), alignment.height()));
 }
 
 // static
@@ -64,7 +119,7 @@ scoped_refptr<VideoFrame> VideoFrame::CreateFrame(
   // Since we're creating a new YUV frame (and allocating memory for it
   // ourselves), we can pad the requested |coded_size| if necessary if the
   // request does not line up on sample boundaries.
-  gfx::Size new_coded_size = AdjustCodedSize(format, coded_size);
+  const gfx::Size new_coded_size = AdjustCodedSize(format, coded_size);

[scherkus (not reviewing), 2014/10/22 23:01:49]

general tip for the future: try to avoid making these unrelated changed in code
reviews -- it can make things a bit more confusing when reviewing

[magjed_chromium, 2014/10/23 11:44:30]

Yeah, sorry about that. I will keep that in mind in the future.

   DCHECK(IsValidConfig(format, new_coded_size, visible_rect, natural_size));
 
   scoped_refptr<VideoFrame> frame(
@@ -131,35 +186,35 @@ bool VideoFrame::IsValidConfig(VideoFrame::Format format,
     case VideoFrame::UNKNOWN:
       return (coded_size.IsEmpty() && visible_rect.IsEmpty() &&
               natural_size.IsEmpty());
+
+    // NATIVE_TEXTURE and HOLE have no software-allocated buffers and are
+    // allowed to skip the below check.
+    case VideoFrame::NATIVE_TEXTURE:
+#if defined(VIDEO_HOLE)
+    case VideoFrame::HOLE:
+#endif  // defined(VIDEO_HOLE)
+      return true;
+
     case VideoFrame::YV24:
-      break;
     case VideoFrame::YV12:
     case VideoFrame::YV12J:
     case VideoFrame::I420:
     case VideoFrame::YV12A:
     case VideoFrame::NV12:
-      // Subsampled YUV formats have width/height requirements.
-      if (static_cast<size_t>(coded_size.height()) <
-          RoundUp(visible_rect.bottom(), 2))
-        return false;
-    // Fallthrough.
     case VideoFrame::YV16:
-      if (static_cast<size_t>(coded_size.width()) <
-          RoundUp(visible_rect.right(), 2))
-        return false;
-      break;
-    case VideoFrame::NATIVE_TEXTURE:
-#if defined(VIDEO_HOLE)
-    case VideoFrame::HOLE:
-#endif  // defined(VIDEO_HOLE)
-      // NATIVE_TEXTURE and HOLE have no software-allocated buffers and are
-      // allowed to skip the below check and be empty.
-      return true;
+      // Check that software-allocated buffer formats are aligned correctly and
+      // not empty.
+      const gfx::Size alignment = CommonAlignment(format);
+      return RoundUp(visible_rect.right(), alignment.width()) <=
+                 static_cast<size_t>(coded_size.width()) &&
+             RoundUp(visible_rect.bottom(), alignment.height()) <=
+                 static_cast<size_t>(coded_size.height()) &&
+             !coded_size.IsEmpty() && !visible_rect.IsEmpty() &&
+             !natural_size.IsEmpty();
   }
 
-  // Check that software-allocated buffer formats are not empty.
-  return (!coded_size.IsEmpty() && !visible_rect.IsEmpty() &&
-          !natural_size.IsEmpty());
+  NOTREACHED();
+  return false;
 }
 
 // static
@@ -203,7 +258,7 @@ scoped_refptr<VideoFrame> VideoFrame::WrapExternalPackedMemory(
     base::SharedMemoryHandle handle,
     base::TimeDelta timestamp,
     const base::Closure& no_longer_needed_cb) {
-  gfx::Size new_coded_size = AdjustCodedSize(format, coded_size);
+  const gfx::Size new_coded_size = AdjustCodedSize(format, coded_size);
 
   if (!IsValidConfig(format, new_coded_size, visible_rect, natural_size))
     return NULL;
@@ -291,7 +346,7 @@ scoped_refptr<VideoFrame> VideoFrame::WrapCVPixelBuffer(
   DCHECK(cv_pixel_buffer);
   DCHECK(CFGetTypeID(cv_pixel_buffer) == CVPixelBufferGetTypeID());
 
-  OSType cv_format = CVPixelBufferGetPixelFormatType(cv_pixel_buffer);
+  const OSType cv_format = CVPixelBufferGetPixelFormatType(cv_pixel_buffer);
   Format format;
   // There are very few compatible CV pixel formats, so just check each.
   if (cv_format == kCVPixelFormatType_420YpCbCr8Planar) {
@@ -307,9 +362,9 @@ scoped_refptr<VideoFrame> VideoFrame::WrapCVPixelBuffer(
     return NULL;
   }
 
-  gfx::Size coded_size(CVImageBufferGetEncodedSize(cv_pixel_buffer));
-  gfx::Rect visible_rect(CVImageBufferGetCleanRect(cv_pixel_buffer));
-  gfx::Size natural_size(CVImageBufferGetDisplaySize(cv_pixel_buffer));
+  const gfx::Size coded_size(CVImageBufferGetEncodedSize(cv_pixel_buffer));
+  const gfx::Rect visible_rect(CVImageBufferGetCleanRect(cv_pixel_buffer));
+  const gfx::Size natural_size(CVImageBufferGetDisplaySize(cv_pixel_buffer));
 
   if (!IsValidConfig(format, coded_size, visible_rect, natural_size))
     return NULL;
@@ -342,7 +397,7 @@ scoped_refptr<VideoFrame> VideoFrame::WrapExternalYuvData(
     uint8* v_data,
     base::TimeDelta timestamp,
     const base::Closure& no_longer_needed_cb) {
-  gfx::Size new_coded_size = AdjustCodedSize(format, coded_size);
+  const gfx::Size new_coded_size = AdjustCodedSize(format, coded_size);
   CHECK(IsValidConfig(format, new_coded_size, visible_rect, natural_size));
 
   scoped_refptr<VideoFrame> frame(
@@ -497,78 +552,18 @@ size_t VideoFrame::AllocationSize(Format format, const gfx::Size& coded_size) {
 gfx::Size VideoFrame::PlaneSize(Format format,
                                 size_t plane,
                                 const gfx::Size& coded_size) {
+  DCHECK(IsValidPlane(plane, format));
+
   // Align to multiple-of-two size overall. This ensures that non-subsampled
   // planes can be addressed by pixel with the same scaling as the subsampled
   // planes.
   const int width = RoundUp(coded_size.width(), 2);
   const int height = RoundUp(coded_size.height(), 2);
-  switch (format) {
-    case VideoFrame::YV24:
-      switch (plane) {
-        case VideoFrame::kYPlane:
-        case VideoFrame::kUPlane:
-        case VideoFrame::kVPlane:
-          return gfx::Size(width, height);
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12:
-    case VideoFrame::YV12J:
-    case VideoFrame::I420:
-      switch (plane) {
-        case VideoFrame::kYPlane:
-          return gfx::Size(width, height);
-        case VideoFrame::kUPlane:
-        case VideoFrame::kVPlane:
-          return gfx::Size(width / 2, height / 2);
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12A:
-      switch (plane) {
-        case VideoFrame::kYPlane:
-        case VideoFrame::kAPlane:
-          return gfx::Size(width, height);
-        case VideoFrame::kUPlane:
-        case VideoFrame::kVPlane:
-          return gfx::Size(width / 2, height / 2);
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV16:
-      switch (plane) {
-        case VideoFrame::kYPlane:
-          return gfx::Size(width, height);
-        case VideoFrame::kUPlane:
-        case VideoFrame::kVPlane:
-          return gfx::Size(width / 2, height);
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::NV12:
-      switch (plane) {
-        case VideoFrame::kYPlane:
-          return gfx::Size(width, height);
-        case VideoFrame::kUVPlane:
-          return gfx::Size(width, height / 2);
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::UNKNOWN:
-    case VideoFrame::NATIVE_TEXTURE:
-#if defined(VIDEO_HOLE)
-    case VideoFrame::HOLE:
-#endif  // defined(VIDEO_HOLE)
-      break;
-  }
-  NOTREACHED() << "Unsupported video frame format/plane: "
-               << format << "/" << plane;
-  return gfx::Size();
+
+  const gfx::Size subsample = SampleSize(format, plane);
+  DCHECK(width % subsample.width() == 0 && height % subsample.height() == 0);

[scherkus (not reviewing), 2014/10/22 23:01:49]

if this fails we won't know which condition failed

you should either split into two consecutive DCHECKs or you can emit a string
containing the information

e.g., DCHECK(...) << "blah blah blah" << width;

[magjed_chromium, 2014/10/23 11:44:31]

Done.

+  return gfx::Size(BytesPerElement(format, plane) * width / subsample.width(),
+                   height / subsample.height());
 }
 
 size_t VideoFrame::PlaneAllocationSize(Format format,
@@ -580,63 +575,11 @@ size_t VideoFrame::PlaneAllocationSize(Format format,
 
 // static
 int VideoFrame::PlaneHorizontalBitsPerPixel(Format format, size_t plane) {
-  switch (format) {
-    case VideoFrame::YV24:
-      switch (plane) {
-        case kYPlane:
-        case kUPlane:
-        case kVPlane:
-          return 8;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12:
-    case VideoFrame::YV16:
-    case VideoFrame::I420:
-    case VideoFrame::YV12J:
-      switch (plane) {
-        case kYPlane:
-          return 8;
-        case kUPlane:
-        case kVPlane:
-          return 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12A:
-      switch (plane) {
-        case kYPlane:
-        case kAPlane:
-          return 8;
-        case kUPlane:
-        case kVPlane:
-          return 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::NV12:
-      switch (plane) {
-        case kYPlane:
-          return 8;
-        case kUVPlane:
-          return 4;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::UNKNOWN:
-#if defined(VIDEO_HOLE)
-    case VideoFrame::HOLE:
-#endif  // defined(VIDEO_HOLE)
-    case VideoFrame::NATIVE_TEXTURE:
-      break;
-  }
-  NOTREACHED() << "Unsupported video frame format/plane: "
-               << format << "/" << plane;
-  return 0;
+  DCHECK(IsValidPlane(plane, format));
+  const int bits_per_element = 8 * BytesPerElement(format, plane);
+  const int pixels_per_element = SampleSize(format, plane).GetArea();
+  DCHECK(bits_per_element % pixels_per_element == 0);
+  return bits_per_element / pixels_per_element;
 }
 
 // Release data allocated by AllocateYUV().
@@ -646,60 +589,40 @@ static void ReleaseData(uint8* data) {
 }
 
 void VideoFrame::AllocateYUV() {
-  DCHECK(format_ == VideoFrame::YV12 || format_ == VideoFrame::YV16 ||
-         format_ == VideoFrame::YV12A || format_ == VideoFrame::I420 ||
-         format_ == VideoFrame::YV12J || format_ == VideoFrame::YV24);
-  // Align Y rows at least at 16 byte boundaries.  The stride for both
-  // YV12 and YV16 is 1/2 of the stride of Y.  For YV12, every row of bytes for
-  // U and V applies to two rows of Y (one byte of UV for 4 bytes of Y), so in
-  // the case of YV12 the strides are identical for the same width surface, but
-  // the number of bytes allocated for YV12 is 1/2 the amount for U & V as
-  // YV16. We also round the height of the surface allocated to be an even
-  // number to avoid any potential of faulting by code that attempts to access
-  // the Y values of the final row, but assumes that the last row of U & V
-  // applies to a full two rows of Y. YV12A is the same as YV12, but with an
-  // additional alpha plane that has the same size and alignment as the Y plane.
-  size_t y_stride = RoundUp(row_bytes(VideoFrame::kYPlane),
-                            kFrameSizeAlignment);
-  size_t uv_stride = RoundUp(row_bytes(VideoFrame::kUPlane),
-                             kFrameSizeAlignment);
-
-  // The *2 here is because some formats (e.g. h264) allow interlaced coding,
-  // and then the size needs to be a multiple of two macroblocks (vertically).
-  // See libavcodec/utils.c:avcodec_align_dimensions2().
-  size_t y_height = RoundUp(coded_size_.height(), kFrameSizeAlignment * 2);
-  size_t uv_height =
-      (format_ == VideoFrame::YV12 || format_ == VideoFrame::YV12A ||
-       format_ == VideoFrame::I420)
-          ? y_height / 2
-          : y_height;
-  size_t y_bytes = y_height * y_stride;
-  size_t uv_bytes = uv_height * uv_stride;
-  size_t a_bytes = format_ == VideoFrame::YV12A ? y_bytes : 0;
+  DCHECK(format_ == YV12 || format_ == YV16 || format_ == YV12A ||
+         format_ == I420 || format_ == YV12J || format_ == YV24);
+  COMPILE_ASSERT(0 == kYPlane, y_plane_data_must_be_index_0);
+
+  size_t data_size = 0;
+  size_t offset[kMaxPlanes];
+  for (size_t plane = 0; plane < VideoFrame::NumPlanes(format_); ++plane) {
+    // The *2 in alignment for height is because some formats (e.g. h264) allow
+    // interlaced coding, and then the size needs to be a multiple of two
+    // macroblocks (vertically). See
+    // libavcodec/utils.c:avcodec_align_dimensions2().
+    const size_t height = RoundUp(rows(plane), kFrameSizeAlignment * 2);
+    strides_[plane] = RoundUp(row_bytes(plane), kFrameSizeAlignment);
+    offset[plane] = data_size;
+    data_size += height * strides_[plane];
+  }
 
   // The extra line of UV being allocated is because h264 chroma MC
   // overreads by one line in some cases, see libavcodec/utils.c:
   // avcodec_align_dimensions2() and libavcodec/x86/h264_chromamc.asm:
   // put_h264_chroma_mc4_ssse3().
-  const size_t data_size =
-      y_bytes + (uv_bytes * 2 + uv_stride) + a_bytes + kFrameSizePadding;
-  uint8* data = reinterpret_cast<uint8*>(
-      base::AlignedAlloc(data_size, kFrameAddressAlignment));
+  DCHECK(IsValidPlane(kUPlane, format_));
+  data_size += strides_[kUPlane] + kFrameSizePadding;
+
   // FFmpeg expects the initialize allocation to be zero-initialized.  Failure
   // to do so can lead to unitialized value usage.  See http://crbug.com/390941
+  uint8* data = reinterpret_cast<uint8*>(
+      base::AlignedAlloc(data_size, kFrameAddressAlignment));
   memset(data, 0, data_size);
+
+  for (size_t plane = 0; plane < VideoFrame::NumPlanes(format_); ++plane)
+    data_[plane] = data + offset[plane];
+
   no_longer_needed_cb_ = base::Bind(&ReleaseData, data);
-  COMPILE_ASSERT(0 == VideoFrame::kYPlane, y_plane_data_must_be_index_0);
-  data_[VideoFrame::kYPlane] = data;
-  data_[VideoFrame::kUPlane] = data + y_bytes;
-  data_[VideoFrame::kVPlane] = data + y_bytes + uv_bytes;
-  strides_[VideoFrame::kYPlane] = y_stride;
-  strides_[VideoFrame::kUPlane] = uv_stride;
-  strides_[VideoFrame::kVPlane] = uv_stride;
-  if (format_ == YV12A) {
-    data_[VideoFrame::kAPlane] = data + y_bytes + (2 * uv_bytes);
-    strides_[VideoFrame::kAPlane] = y_stride;
-  }
 }
 
 VideoFrame::VideoFrame(VideoFrame::Format format,
@@ -750,65 +673,11 @@ int VideoFrame::stride(size_t plane) const {
 }
 
 // static
-size_t VideoFrame::RowBytes(size_t plane, VideoFrame::Format format,
+size_t VideoFrame::RowBytes(size_t plane,
+                            VideoFrame::Format format,
                             int width) {
   DCHECK(IsValidPlane(plane, format));
-  switch (format) {
-    case VideoFrame::YV24:
-      switch (plane) {
-        case kYPlane:
-        case kUPlane:
-        case kVPlane:
-          return width;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12:
-    case VideoFrame::YV16:
-    case VideoFrame::I420:
-    case VideoFrame::YV12J:
-      switch (plane) {
-        case kYPlane:
-          return width;
-        case kUPlane:
-        case kVPlane:
-          return RoundUp(width, 2) / 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12A:
-      switch (plane) {
-        case kYPlane:
-        case kAPlane:
-          return width;
-        case kUPlane:
-        case kVPlane:
-          return RoundUp(width, 2) / 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::NV12:
-      switch (plane) {
-        case kYPlane:
-        case kUVPlane:
-          return width;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::UNKNOWN:
-#if defined(VIDEO_HOLE)
-    case VideoFrame::HOLE:
-#endif  // defined(VIDEO_HOLE)
-    case VideoFrame::NATIVE_TEXTURE:
-      break;
-  }
-  NOTREACHED() << "Unsupported video frame format/plane: " << format << "/"
-               << plane;
-  return 0;
+  return BytesPerElement(format, plane) * Columns(plane, format, width);
 }
 
 int VideoFrame::row_bytes(size_t plane) const {
@@ -818,74 +687,53 @@ int VideoFrame::row_bytes(size_t plane) const {
 // static
 size_t VideoFrame::Rows(size_t plane, VideoFrame::Format format, int height) {
   DCHECK(IsValidPlane(plane, format));
-  switch (format) {
-    case VideoFrame::YV24:
-    case VideoFrame::YV16:
-      switch (plane) {
-        case kYPlane:
-        case kUPlane:
-        case kVPlane:
-          return height;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12:
-    case VideoFrame::YV12J:
-    case VideoFrame::I420:
-      switch (plane) {
-        case kYPlane:
-          return height;
-        case kUPlane:
-        case kVPlane:
-          return RoundUp(height, 2) / 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::YV12A:
-      switch (plane) {
-        case kYPlane:
-        case kAPlane:
-          return height;
-        case kUPlane:
-        case kVPlane:
-          return RoundUp(height, 2) / 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::NV12:
-      switch (plane) {
-        case kYPlane:
-          return height;
-        case kUVPlane:
-          return RoundUp(height, 2) / 2;
-        default:
-          break;
-      }
-      break;
-    case VideoFrame::UNKNOWN:
-#if defined(VIDEO_HOLE)
-    case VideoFrame::HOLE:
-#endif  // defined(VIDEO_HOLE)
-    case VideoFrame::NATIVE_TEXTURE:
-      break;
-  }
-  NOTREACHED() << "Unsupported video frame format/plane: " << format << "/"
-               << plane;
-  return 0;
+  const int sample_height = SampleSize(format, plane).height();
+  return RoundUp(height, sample_height) / sample_height;
+}
+
+// static
+size_t VideoFrame::Columns(size_t plane, Format format, int width) {
+  DCHECK(IsValidPlane(plane, format));
+  const int sample_width = SampleSize(format, plane).width();
+  return RoundUp(width, sample_width) / sample_width;
 }
 
 int VideoFrame::rows(size_t plane) const {
   return Rows(plane, format_, coded_size_.height());
 }
 
-uint8* VideoFrame::data(size_t plane) const {
+const uint8* VideoFrame::data(size_t plane) const {
   DCHECK(IsValidPlane(plane, format_));
   return data_[plane];
 }
 
+uint8* VideoFrame::data(size_t plane) {
+  DCHECK(IsValidPlane(plane, format_));
+  return data_[plane];
+}
+
+const uint8* VideoFrame::visible_data(size_t plane) const {
+  DCHECK(IsValidPlane(plane, format_));
+
+  // Calculate an offset that is properly aligned for all planes.
+  const gfx::Size alignment = CommonAlignment(format_);
+  const gfx::Point offset(RoundDown(visible_rect_.x(), alignment.width()),
+                          RoundDown(visible_rect_.y(), alignment.height()));
+
+  const gfx::Size subsample = SampleSize(format_, plane);
+  DCHECK(offset.x() % subsample.width() == 0 &&
+         offset.y() % subsample.height() == 0);

[scherkus (not reviewing), 2014/10/22 23:01:50]

ditto for DCHECK failure

[magjed_chromium, 2014/10/23 11:44:31]

Done.

+  return data(plane) +
+         stride(plane) * (offset.y() / subsample.height()) +  // Row offset.
+         BytesPerElement(format_, plane) *                    // Column offset.
+             (offset.x() / subsample.width());
+}
+
+uint8* VideoFrame::visible_data(size_t plane) {
+  return const_cast<uint8*>(
+      static_cast<const VideoFrame*>(this)->visible_data(plane));
+}
+
 const gpu::MailboxHolder* VideoFrame::mailbox_holder() const {
   DCHECK_EQ(format_, NATIVE_TEXTURE);
   return mailbox_holder_.get();
@@ -919,9 +767,7 @@ CVPixelBufferRef VideoFrame::cv_pixel_buffer() const {
 #endif
 
 void VideoFrame::HashFrameForTesting(base::MD5Context* context) {
-  for (int plane = 0; plane < kMaxPlanes; ++plane) {
-    if (!IsValidPlane(plane, format_))
-      break;
+  for (size_t plane = 0; plane < NumPlanes(format_); ++plane) {
     for (int row = 0; row < rows(plane); ++row) {
       base::MD5Update(context, base::StringPiece(
           reinterpret_cast<char*>(data(plane) + stride(plane) * row),