Allow padding in WrapVideoFrameInCVPixelBuffer

media/base/mac/video_frame_mac_unittests.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/mac/video_frame_mac.h"
 
 #include <stddef.h>
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "media/base/mac/corevideo_glue.h"
 #include "media/base/video_frame.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace media {
 
 namespace {
 
 const int kWidth = 64;
 const int kHeight = 48;
+const int kVisibleRectOffset = 4;
 const base::TimeDelta kTimestamp = base::TimeDelta::FromMicroseconds(1337);
 
 struct FormatPair {
   VideoPixelFormat chrome;
   OSType corevideo;
 };
 
 void Increment(int* i) {
   ++(*i);
 }
 
 }  // namespace
 
 TEST(VideoFrameMac, CheckBasicAttributes) {
   gfx::Size size(kWidth, kHeight);
   auto frame = VideoFrame::CreateFrame(PIXEL_FORMAT_I420, size, gfx::Rect(size),
                                        size, kTimestamp);
   ASSERT_TRUE(frame.get());
 
   auto pb = WrapVideoFrameInCVPixelBuffer(*frame);
   ASSERT_TRUE(pb.get());
 
-  gfx::Size coded_size = frame->coded_size();
-  VideoPixelFormat format = frame->format();
+  const gfx::Size coded_size = frame->coded_size();
+  const VideoPixelFormat format = frame->format();
 
   EXPECT_EQ(coded_size.width(), static_cast<int>(CVPixelBufferGetWidth(pb)));
   EXPECT_EQ(coded_size.height(), static_cast<int>(CVPixelBufferGetHeight(pb)));
   EXPECT_EQ(VideoFrame::NumPlanes(format), CVPixelBufferGetPlaneCount(pb));
 
   CVPixelBufferLockBaseAddress(pb, 0);
   for (size_t i = 0; i < VideoFrame::NumPlanes(format); ++i) {
-    gfx::Size plane_size = VideoFrame::PlaneSize(format, i, coded_size);
+    const gfx::Size plane_size = VideoFrame::PlaneSize(format, i, coded_size);
     EXPECT_EQ(plane_size.width(),
               static_cast<int>(CVPixelBufferGetWidthOfPlane(pb, i)));
     EXPECT_EQ(plane_size.height(),
               static_cast<int>(CVPixelBufferGetHeightOfPlane(pb, i)));
     EXPECT_EQ(frame->data(i), CVPixelBufferGetBaseAddressOfPlane(pb, i));
   }
   CVPixelBufferUnlockBaseAddress(pb, 0);
 }
 
 TEST(VideoFrameMac, CheckFormats) {
@@ skipping 42 matching lines @@
   EXPECT_EQ(1, instances_destroyed);
 }
 
 TEST(VideoFrameMac, CheckWrapperFrame) {
   const FormatPair format_pairs[] = {
       {PIXEL_FORMAT_I420, kCVPixelFormatType_420YpCbCr8Planar},
       {PIXEL_FORMAT_NV12,
        CoreVideoGlue::kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange},
   };
 
-  gfx::Size size(kWidth, kHeight);
+  const gfx::Size size(kWidth, kHeight);
   for (const auto& format_pair : format_pairs) {
     base::ScopedCFTypeRef<CVPixelBufferRef> pb;
     CVPixelBufferCreate(nullptr, kWidth, kHeight, format_pair.corevideo,
                         nullptr, pb.InitializeInto());
     ASSERT_TRUE(pb.get());
 
     auto frame = VideoFrame::WrapCVPixelBuffer(pb.get(), kTimestamp);
     ASSERT_TRUE(frame.get());
     EXPECT_EQ(pb.get(), frame->cv_pixel_buffer());
     EXPECT_EQ(format_pair.chrome, frame->format());
 
     frame = nullptr;
     EXPECT_EQ(1, CFGetRetainCount(pb.get()));
   }
 }
 
+TEST(VideoFrameMac, CheckFrameWithVisibleRect) {

[miu, 2016/03/24 20:29:00]

nit: Please rename to something more meaningful.  Suggestion:
CorrectlyWrapsFramesWithPadding

[emircan, 2016/03/25 07:43:22]

Done.

+  const gfx::Size size(kWidth, kHeight);

[miu, 2016/03/24 20:29:00]

nit: Please name this coded_size for clarity.

[emircan, 2016/03/25 07:43:22]

Done.

+  const gfx::Rect visible_rect(kVisibleRectOffset, kVisibleRectOffset,
+                               kWidth - 2 * kVisibleRectOffset,
+                               kHeight - 2 * kVisibleRectOffset);
+  auto frame = VideoFrame::CreateFrame(PIXEL_FORMAT_I420, size, visible_rect,
+                                       size, kTimestamp);
+  ASSERT_TRUE(frame.get());
+  auto pb = WrapVideoFrameInCVPixelBuffer(*frame);
+  ASSERT_TRUE(pb.get());
+  EXPECT_EQ(kCVPixelFormatType_420YpCbCr8Planar,
+            CVPixelBufferGetPixelFormatType(pb));
+  EXPECT_EQ(visible_rect.width(), static_cast<int>(CVPixelBufferGetWidth(pb)));
+  EXPECT_EQ(visible_rect.height(),
+            static_cast<int>(CVPixelBufferGetHeight(pb)));
+
+  CVPixelBufferLockBaseAddress(pb, 0);
+  for (size_t i = 0; i < VideoFrame::NumPlanes(frame->format()); ++i) {
+    const gfx::Size plane_size =
+        VideoFrame::PlaneSize(frame->format(), i, visible_rect.size());
+    EXPECT_EQ(plane_size.width(),
+              static_cast<int>(CVPixelBufferGetWidthOfPlane(pb, i)));
+    EXPECT_EQ(plane_size.height(),
+              static_cast<int>(CVPixelBufferGetHeightOfPlane(pb, i)));
+    EXPECT_EQ(frame->visible_data(i),
+              CVPixelBufferGetBaseAddressOfPlane(pb, i));
+  }

[miu, 2016/03/24 20:29:00]

Also, you should check the value returned by
CVPixelBufferGetBytesPerRowOfPlane() matches the coded size (for the Y plane;
1/2 size for U and V).

[miu, 2016/03/24 20:29:00]

Per earlier discussion, I think it would be good to read the bytes in the plane,
checking for an expected pattern.

[emircan, 2016/03/25 07:43:22]

Ok, that should equals stride as well.

[emircan, 2016/03/25 07:43:22]

Done.

+  CVPixelBufferUnlockBaseAddress(pb, 0);
+}
+
 }  // namespace media