Make VEA test support videos with different coded size and visible size

content/common/gpu/media/video_encode_accelerator_unittest.cc

Index: content/common/gpu/media/video_encode_accelerator_unittest.cc
diff --git a/content/common/gpu/media/video_encode_accelerator_unittest.cc b/content/common/gpu/media/video_encode_accelerator_unittest.cc
index b07211d0867f59b8fa97b11b70c4f9f3a2b74512..2a9c28741cd4d1592e1be13e72e1597727bcd3af 100644
--- a/content/common/gpu/media/video_encode_accelerator_unittest.cc
+++ b/content/common/gpu/media/video_encode_accelerator_unittest.cc
@@ -33,6 +33,8 @@
 #error The VideoEncodeAcceleratorUnittest is not supported on this platform.
 #endif
 
+#define ALIGN_64_BYTES(x) (((x) + 63) & ~63)
+
 using media::VideoEncodeAccelerator;
 
 namespace content {
@@ -112,6 +114,29 @@ struct TestStream {
   unsigned int requested_subsequent_framerate;
 };
 
+static void PrepareAlignedTempFile(

[Pawel Osciak, 2014/08/01 00:48:32]

Plenty of things may fail here, need to return value and check it.

+    const base::FilePath::StringType& filename,
+    const size_t input_buffer_size) {
+  base::MemoryMappedFile input_file;
+  base::File *file = new base::File(base::FilePath("temp" + filename),

[Pawel Osciak, 2014/08/01 00:48:32]

base::CreateAndOpenTemporaryFile()

[henryhsu, 2014/08/01 04:55:55]

Done.

+      base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+  input_file.Initialize(base::FilePath(filename));

[Pawel Osciak, 2014/08/01 00:48:32]

Please check the return value.

[henryhsu, 2014/08/01 04:55:55]

Done.

+
+  size_t position = 0, frame_offset = 0;

[Pawel Osciak, 2014/08/01 00:48:32]

off_t

[henryhsu, 2014/08/01 04:55:55]

Done.

+  size_t aligned_frame_size = ALIGN_64_BYTES(input_buffer_size);
+  char*dummy_buffer = new char[aligned_frame_size - input_buffer_size];
+  while (position < input_file.length()) {
+    char *buffer = reinterpret_cast<char*>(
+        const_cast<uint8*>(input_file.data() + position));
+    file->Write(frame_offset, buffer, input_buffer_size);

[Pawel Osciak, 2014/08/01 00:48:32]

This function may not write all data. It returns the number of bytes written,
you should check it.

+    file->Write(frame_offset + input_buffer_size, dummy_buffer,

[Pawel Osciak, 2014/08/01 00:48:32]

Why not use AppendToFile? You don't need to manage the offset then and smaller
chance of error?

+        aligned_frame_size - input_buffer_size);
+    frame_offset += aligned_frame_size;
+    position += input_buffer_size;
+  }
+  file->Close();

[Pawel Osciak, 2014/08/01 00:48:32]

Leaking file?

[henryhsu, 2014/08/01 04:55:55]

Done.

+}

[Pawel Osciak, 2014/08/01 00:48:32]

And leaking dummy_buffer.

[henryhsu, 2014/08/01 04:55:55]

Done.

+
 // Parse |data| into its constituent parts, set the various output fields
 // accordingly, read in video stream, and store them to |test_streams|.
 static void ParseAndReadTestStreamData(const base::FilePath::StringType& data,
@@ -161,7 +186,16 @@ static void ParseAndReadTestStreamData(const base::FilePath::StringType& data,
                                &test_stream->requested_subsequent_framerate));
     }
 
-    CHECK(test_stream->input_file.Initialize(base::FilePath(filename)));
+    size_t input_buffer_size =
+        media::VideoFrame::AllocationSize(kInputFormat, test_stream->size);
+    // Prepare aligned temporary file if frame size is not 64-bytes alignment.
+    if (input_buffer_size & 63) {

[Pawel Osciak, 2014/08/01 00:48:32]

Given the fact that I420 translates into V4L2_PIX_FMT_YUV420M, which is a
3-plane discontiguous/3-buffer format, you should position each plane separately
aligned.

I would just get rid of the branch and always do this.

[henryhsu, 2014/08/01 04:55:55]

Done.

+      PrepareAlignedTempFile(filename, input_buffer_size);
+      CHECK(test_stream->input_file.Initialize(
+            base::FilePath("temp" + filename)));

[Pawel Osciak, 2014/08/01 00:48:32]

The name should be generated by CreateTemporaryFile, but especially don't
hardcode it in more than one place please.

[henryhsu, 2014/08/01 04:55:55]

Done.

+    } else {
+      CHECK(test_stream->input_file.Initialize(base::FilePath(filename)));
+    }
     test_streams->push_back(test_stream);
   }
 }
@@ -554,6 +588,7 @@ VEAClient::VEAClient(const TestStream& test_stream,
 
   input_buffer_size_ =
       media::VideoFrame::AllocationSize(kInputFormat, test_stream.size);
+  input_buffer_size_ = ALIGN_64_BYTES(input_buffer_size_);

[Pawel Osciak, 2014/08/01 00:48:32]

It would be good if this was calculated in one place, less chance of error.

   CHECK_GT(input_buffer_size_, 0UL);
 
   // Calculate the number of frames in the input stream by dividing its length
@@ -637,7 +672,7 @@ void VEAClient::RequireBitstreamBuffers(unsigned int input_count,
   // If/when this is changed, the ARM-specific alignment check below should be
   // redone as well.
   input_coded_size_ = input_coded_size;
-  ASSERT_EQ(input_coded_size_, test_stream_.size);
+//  ASSERT_EQ(input_coded_size_, test_stream_.size);
 #if defined(ARCH_CPU_ARMEL)
   // ARM performs CPU cache management with CPU cache line granularity. We thus
   // need to ensure our buffers are CPU cache line-aligned (64 byte-aligned).