Audit/fix use of media::VideoFrame::coded_size()

media/base/video_frame_unittest.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/video_frame.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/format_macros.h"
 #include "base/memory/aligned_memory.h"
@@ skipping 70 matching lines @@
       EXPECT_EQ(expect_rgb_color, rgb_row_data[col]);
     }
   }
 
   base::AlignedFree(rgb_data);
 }
 
 // Fill each plane to its reported extents and verify accessors report non
 // zero values.  Additionally, for the first plane verify the rows and
 // row_bytes values are correct.
-void ExpectFrameExtents(VideoFrame::Format format, int planes,
-                        int bytes_per_pixel, const char* expected_hash) {
+void ExpectFrameExtents(VideoFrame::Format format,
+                        int planes,
+                        const char* expected_hash) {
   const unsigned char kFillByte = 0x80;
   const int kWidth = 61;
   const int kHeight = 31;
   const base::TimeDelta kTimestamp = base::TimeDelta::FromMicroseconds(1337);
 
   gfx::Size size(kWidth, kHeight);
   scoped_refptr<VideoFrame> frame = VideoFrame::CreateFrame(
       format, size, gfx::Rect(size), size, kTimestamp);
   ASSERT_TRUE(frame.get());
 
   for(int plane = 0; plane < planes; plane++) {
     SCOPED_TRACE(base::StringPrintf("Checking plane %d", plane));
     EXPECT_TRUE(frame->data(plane));
     EXPECT_TRUE(frame->stride(plane));
     EXPECT_TRUE(frame->rows(plane));
     EXPECT_TRUE(frame->row_bytes(plane));
 
-    if (plane == 0) {

[Ami GONE FROM CHROMIUM, 2014/02/28 23:17:32]

Why this change?

[sheu, 2014/03/01 01:56:57]

VideoFrame::CreateFrame() can pad the coded_size of the frame, which makes
frame->rows(plane) not necessarily correspond to kHeight; similarly with
frame->row_bytes(plane).

-      EXPECT_EQ(frame->rows(plane), kHeight);
-      EXPECT_EQ(frame->row_bytes(plane), kWidth * bytes_per_pixel);
-    }
-
     memset(frame->data(plane), kFillByte,
            frame->stride(plane) * frame->rows(plane));
   }
 
   base::MD5Context context;
   base::MD5Init(&context);
   frame->HashFrameForTesting(&context);
   base::MD5Digest digest;
   base::MD5Final(&digest, &context);
   EXPECT_EQ(MD5DigestToBase16(digest), expected_hash);
@@ skipping 74 matching lines @@
     v_plane += frame->stride(VideoFrame::kVPlane);
   }
 }
 
 // Ensure each frame is properly sized and allocated.  Will trigger OOB reads
 // and writes as well as incorrect frame hashes otherwise.
 TEST(VideoFrame, CheckFrameExtents) {
   // Each call consists of a VideoFrame::Format, # of planes, bytes per pixel,
   // and the expected hash of all planes if filled with kFillByte (defined in
   // ExpectFrameExtents).
-  ExpectFrameExtents(
-      VideoFrame::YV12,   3, 1, "71113bdfd4c0de6cf62f48fb74f7a0b1");
-  ExpectFrameExtents(
-      VideoFrame::YV16,   3, 1, "9bb99ac3ff350644ebff4d28dc01b461");
+  ExpectFrameExtents(VideoFrame::YV12, 3, "8e5d54cb23cd0edca111dd35ffb6ff05");
+  ExpectFrameExtents(VideoFrame::YV16, 3, "cce408a044b212db42a10dfec304b3ef");

[Ami GONE FROM CHROMIUM, 2014/02/28 23:17:32]

why these changes?
(does this mean a bug was hiding in the checksum before?)

[sheu, 2014/03/01 01:56:57]

Nope; we're padding out coded_size now so there's extra bytes getting
checksummed.  (The extra bytes are getting filled properly, so there's no
indeterminism here.)

 }
 
 static void TextureCallback(uint32* called_sync_point,
                             scoped_ptr<gpu::MailboxHolder> mailbox_holder) {
   *called_sync_point = mailbox_holder->sync_point;
 }
 
 // Verify the gpu::MailboxHolder::ReleaseCallback is called when VideoFrame is
 // destroyed with the original sync point.
 TEST(VideoFrame, TextureNoLongerNeededCallbackIsCalled) {
@@ skipping 42 matching lines @@
     EXPECT_EQ(sync_point, mailbox_holder->sync_point);
 
     // Finish using the mailbox_holder and drop our reference.
     sync_point = 10;
     mailbox_holder->sync_point = sync_point;
   }
   EXPECT_EQ(sync_point, called_sync_point);
 }
 
 }  // namespace media