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

content/common/gpu/media/video_encode_accelerator_unittest.cc

 // Copyright 2013 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 "base/at_exit.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/memory/scoped_vector.h"
@@ skipping 15 matching lines @@
 #endif
 
 #if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
 #include "content/common/gpu/media/v4l2_video_encode_accelerator.h"
 #elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY) && defined(USE_X11)
 #include "content/common/gpu/media/vaapi_video_encode_accelerator.h"
 #else
 #error The VideoEncodeAcceleratorUnittest is not supported on this platform.
 #endif
 
+#define ALIGN_64_BYTES(x) (((x) + 63) & ~63)
+
 using media::VideoEncodeAccelerator;
 
 namespace content {
 namespace {
 
 const media::VideoFrame::Format kInputFormat = media::VideoFrame::I420;
 
 // Arbitrarily chosen to add some depth to the pipeline.
 const unsigned int kNumOutputBuffers = 4;
 const unsigned int kNumExtraInputFrames = 4;
@@ skipping 59 matching lines @@
   gfx::Size size;
   base::MemoryMappedFile input_file;
   media::VideoCodecProfile requested_profile;
   std::string out_filename;
   unsigned int requested_bitrate;
   unsigned int requested_framerate;
   unsigned int requested_subsequent_bitrate;
   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,
                                        ScopedVector<TestStream>* test_streams) {
   // Split the string to individual test stream data.
   std::vector<base::FilePath::StringType> test_streams_data;
   base::SplitString(data, ';', &test_streams_data);
   CHECK_GE(test_streams_data.size(), 1U) << data;
 
   // Parse each test stream data and read the input file.
@@ skipping 29 matching lines @@
     if (fields.size() >= 8 && !fields[7].empty()) {
       CHECK(base::StringToUint(fields[7],
                                &test_stream->requested_subsequent_bitrate));
     }
 
     if (fields.size() >= 9 && !fields[8].empty()) {
       CHECK(base::StringToUint(fields[8],
                                &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);
   }
 }
 
 // Set default parameters of |test_streams| and update the parameters according
 // to |mid_stream_bitrate_switch| and |mid_stream_framerate_switch|.
 static void UpdateTestStreamData(bool mid_stream_bitrate_switch,
                                  bool mid_stream_framerate_switch,
                                  ScopedVector<TestStream>* test_streams) {
   for (size_t i = 0; i < test_streams->size(); i++) {
@@ skipping 372 matching lines @@
   if (save_to_file_) {
     CHECK(!test_stream_.out_filename.empty());
     base::FilePath out_filename(test_stream_.out_filename);
     // This creates or truncates out_filename.
     // Without it, AppendToFile() will not work.
     EXPECT_EQ(0, base::WriteFile(out_filename, NULL, 0));
   }
 
   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
   // in bytes by frame size in bytes.
   CHECK_EQ(test_stream_.input_file.length() % input_buffer_size_, 0U)
       << "Stream byte size is not a product of calculated frame byte size";
   num_frames_in_stream_ = test_stream_.input_file.length() / input_buffer_size_;
   CHECK_GT(num_frames_in_stream_, 0UL);
   CHECK_LE(num_frames_in_stream_, kMaxFrameNum);
 
@@ skipping 63 matching lines @@
   SetState(CS_ENCODING);
 
   // TODO(posciak): For now we only support input streams that meet encoder
   // size requirements exactly (i.e. coded size == visible size), so that we
   // can simply mmap the stream file and feed the encoder directly with chunks
   // of that, instead of memcpying from mmapped file into a separate set of
   // input buffers that would meet the coded size and alignment requirements.
   // 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).
   // Otherwise newer kernels will refuse to accept them, and on older kernels
   // we'll be treating ourselves to random corruption.
   // Since we are just mmapping and passing chunks of the input file, to ensure
   // alignment, if the starting virtual addresses of the frames in it were not
   // 64 byte-aligned, we'd have to use a separate set of input buffers and copy
   // the frames into them before sending to the encoder. It would have been an
   // overkill here though, because, for now at least, we only test resolutions
@@ skipping 95 matching lines @@
   uint8* frame_data =
       const_cast<uint8*>(test_stream_.input_file.data() + position);
 
   CHECK_GT(current_framerate_, 0U);
   scoped_refptr<media::VideoFrame> frame =
       media::VideoFrame::WrapExternalYuvData(
           kInputFormat,
           input_coded_size_,
           gfx::Rect(test_stream_.size),
           test_stream_.size,
           input_coded_size_.width(),

[wuchengli, 2014/07/30 09:32:50]

All strides should be test_stream_.size. Right?

           input_coded_size_.width() / 2,
           input_coded_size_.width() / 2,
           frame_data,
           frame_data + input_coded_size_.GetArea(),

[wuchengli, 2014/07/30 09:32:50]

The offsets of u and v planes should also be calculated by test_stream_.size.
Right?

           frame_data + (input_coded_size_.GetArea() * 5 / 4),
           base::TimeDelta().FromMilliseconds(
               next_input_id_ * base::Time::kMillisecondsPerSecond /
               current_framerate_),
           media::BindToCurrentLoop(
               base::Bind(&VEAClient::InputNoLongerNeededCallback,
                          base::Unretained(this),
                          next_input_id_)));
 
   CHECK(inputs_at_client_.insert(next_input_id_).second);
@@ skipping 283 matching lines @@
       test_stream_data->assign(it->second.c_str());
       continue;
     }
     if (it->first == "v" || it->first == "vmodule")
       continue;
     LOG(FATAL) << "Unexpected switch: " << it->first << ":" << it->second;
   }
 
   return RUN_ALL_TESTS();
 }