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..33b00549db270bf5fc9a2d311e6b02414409f23c 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -27,6 +27,12 @@ static inline size_t RoundUp(size_t value, size_t alignment) {
   return ((value + (alignment - 1)) & ~(alignment - 1));
 }
 
+static inline size_t RoundDown(size_t value, size_t alignment) {
+  // Check that |alignment| is a power of 2.
+  DCHECK((alignment + (alignment - 1)) == (alignment | (alignment - 1)));
+  return value & ~(alignment - 1);
+}
+
 // Rounds up |coded_size| if necessary for |format|.
 static gfx::Size AdjustCodedSize(VideoFrame::Format format,
                                  const gfx::Size& coded_size) {
@@ -881,11 +887,40 @@ 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_));
+  const uint8* coded_data = data(plane);
+  const int coded_stride = stride(plane);
+  const int offset_x = RoundDown(visible_rect_.x(), 2);

[scherkus (not reviewing), 2014/10/20 19:30:36]

could you add a comment briefly explaining the math we're doing here? (e.g., why
round down to 2? if x is 1 and I have YV24 content, why will you give me 0
instead of 1?)

[magjed_chromium, 2014/10/22 15:16:39]

This is removed now. See comment below.

+  const int offset_y = RoundDown(visible_rect_.y(), 2);
+  switch (plane) {
+    case kYPlane:
+    case kAPlane:
+      return coded_data + offset_y * coded_stride + offset_x;
+    case kUPlane:
+    case kVPlane:
+      return coded_data + offset_y / 2 * coded_stride + offset_x / 2;

[scherkus (not reviewing), 2014/10/20 19:30:36]

are you sure the division-by-two makes sense even for YV16 and YV24?

[magjed_chromium, 2014/10/22 15:16:39]

You are right, it does not make sense. It is hardcoded for I420 at the moment. 
I didn't want to fix it by adding another huge switch, so I implemented some
helper functions and took the liberty to clean up the rest of the code as well.

+    default:

[scherkus (not reviewing), 2014/10/20 19:30:36]

can you list these cases explicitly? if we ever change the enum the compiler
will whine and the developer will be forced to assess whether this code needs to
be changed

[magjed_chromium, 2014/10/22 15:16:39]

Done. Not applicable anymore.

+      NOTIMPLEMENTED();
+      return NULL;
+  }
+}
+
+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();