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 50 matching lines @@
 
 struct TestStream {
   TestStream()
       : requested_bitrate(0),
         requested_framerate(0),
         requested_subsequent_bitrate(0),
         requested_subsequent_framerate(0) {}
   ~TestStream() {}
 
   gfx::Size size;
+  std::string in_filename;
   base::MemoryMappedFile input_file;
   media::VideoCodecProfile requested_profile;
+  base::FilePath temp_file;

[wuchengli, 2014/08/27 05:46:24]

Add comments for this variable. It's not obvious what's this used for.

[henryhsu, 2014/08/27 10:07:28]

Done.

   std::string out_filename;
   unsigned int requested_bitrate;
   unsigned int requested_framerate;
   unsigned int requested_subsequent_bitrate;
   unsigned int requested_subsequent_framerate;
 };
 
+// Use |coded_size| to copy YUV data into memory from |test_stream| input file.

[wuchengli, 2014/08/27 05:46:24]

Explain the result is saved back to test_stream->input_file.

[henryhsu, 2014/08/27 10:07:27]

Done.

+// Also calculate |input_buffer_size| frame size value.

[wuchengli, 2014/08/27 05:46:24]

Too many sizes here. It's confusing to see "frame size" here. I thought you were
talking about width,height. The following may be better.

Also calculate the byte size of an input frame and set it to
|input_buffer_size|.

[henryhsu, 2014/08/27 10:07:27]

Done.

+static bool PrepareAlignedTempFile(const gfx::Size& coded_size,

[wuchengli, 2014/08/27 05:46:24]

Return value not used. Remove.

Change the function name because this doesn't prepare a temp file anymore. This
saves the memory back to the test_stream->input_file.

[henryhsu, 2014/08/27 10:07:28]

Done.

+                                   TestStream* test_stream,
+                                   size_t* input_buffer_size) {
+  size_t input_num_planes = media::VideoFrame::NumPlanes(kInputFormat);
+  size_t* padding_size = new size_t[input_num_planes];

[wuchengli, 2014/08/27 05:46:23]

nit: I'd use a vector. Up to you.

s/padding_size/padding_sizes/

[henryhsu, 2014/08/27 10:07:28]

Done.

+  size_t visible_frame_size = 0;

[wuchengli, 2014/08/27 05:46:24]

Just use VideoFrame::AllocationSize.

[henryhsu, 2014/08/27 10:07:28]

Done.

+
+  // YUV plane starting address should be 64 bytes alignment.
+  // Calculate padding size for each plane, and frame size for visible size
+  // and coded size.
+  *input_buffer_size = 0;
+  for (off_t i = 0; i < input_num_planes; i++) {
+    size_t size = media::VideoFrame::PlaneAllocationSize(
+        kInputFormat, i, coded_size);
+    padding_size[i] = ALIGN_64_BYTES(size) - size;
+    visible_frame_size += media::VideoFrame::PlaneAllocationSize(
+        kInputFormat, i, test_stream->size);
+    *input_buffer_size += ALIGN_64_BYTES(size);
+  }
+
+  // Test case may have many encoders and memory should be prepared once.
+  if (!test_stream->input_file.IsValid()) {

[wuchengli, 2014/08/27 05:46:24]

nit: I'd return here to avoid a huge if clause.

[henryhsu, 2014/08/27 10:07:28]

Done.

+    base::MemoryMappedFile input_file;
+    CHECK(base::CreateTemporaryFile(&test_stream->temp_file));
+    CHECK(input_file.Initialize(
+          base::File(base::FilePath(test_stream->in_filename),
+              base::File::FLAG_OPEN |
+              base::File::FLAG_WRITE |
+              base::File::FLAG_READ).Pass()));
+
+    size_t num_frames = input_file.length() / visible_frame_size;
+    uint32 flags = base::File::FLAG_CREATE_ALWAYS |
+                   base::File::FLAG_WRITE |
+                   base::File::FLAG_READ;
+
+    // Create a temporary file with coded_size length

[wuchengli, 2014/08/27 05:46:24]

add period for consistency

+    base::File file(base::FilePath(test_stream->temp_file), flags);
+    file.Write(*input_buffer_size * num_frames - 1, ".", 1);

[wuchengli, 2014/08/27 05:46:24]

Why do we need to minus one here?

Just curious. What's the reason to choose "."?

[henryhsu, 2014/08/27 10:07:27]

Because first parameter is offset. We write one byte with minus one to obtain
input_buffer_size file length.
No special for ".". Just want to write anything at the end of file.
Do you have a suggestion for this one byte?

[wuchengli, 2014/08/28 08:33:00]

Can we use SetLength for this? Is SetLength fast? If SetLength cannot be used,
add some comments for this line. It's not obvious writing one byte can create a
huge file.

+    CHECK(test_stream->input_file.Initialize(file.Pass()));
+
+    off_t src_offset = 0, dest_offset = 0;
+    while (src_offset < static_cast<off_t>(input_file.length())) {
+      for (off_t i = 0; i < input_num_planes; i++) {
+        size_t coded_bpl =
+            media::VideoFrame::RowBytes(i, kInputFormat, coded_size.width());
+        size_t visible_bpl =
+            media::VideoFrame::RowBytes(i, kInputFormat,
+                                        test_stream->size.width());
+        off_t rows = media::VideoFrame::Rows(i, kInputFormat,

[wuchengli, 2014/08/27 05:46:25]

coded_bpl, visible_bpl, and rows are similar to padding_size. They can be all
pre-calculated outside while loop.

[henryhsu, 2014/08/27 10:07:28]

Done.

+                                             test_stream->size.height());

[wuchengli, 2014/08/27 05:46:24]

run git cl format

[henryhsu, 2014/08/27 10:07:28]

Done.

+        for (off_t j = 0; j < rows; j++) {
+          char *src = reinterpret_cast<char*>(

[wuchengli, 2014/08/27 05:46:24]

Use uint8* to avoid both castings?

s/char */char* / for the entire CL.

[henryhsu, 2014/08/27 10:07:28]

Done.

+              const_cast<uint8*>(input_file.data() + src_offset));
+          char *dest = reinterpret_cast<char*>(
+              const_cast<uint8*>(test_stream->input_file.data() + dest_offset));

[wuchengli, 2014/08/27 05:46:24]

Why we do need to cast twice? Can we just use reinterpret_cast?

[henryhsu, 2014/08/27 10:07:28]

input_file.data() will return const variable. Use const_cast to remove const
attribute.

+          memcpy(dest, src, visible_bpl);
+          src_offset += visible_bpl;
+          dest_offset += coded_bpl;
+        }
+        off_t padding_rows =
+            media::VideoFrame::Rows(i, kInputFormat, coded_size.height()) -
+            rows;
+        dest_offset += padding_rows * coded_bpl + padding_size[i];

[wuchengli, 2014/08/27 05:46:24]

Can we precalculate padding_rows * coded_bpl + padding_size[i]? padding_size can
be padding_rows * coded_bpl + original padding_size.

[henryhsu, 2014/08/27 10:07:28]

Done.

+      }
+    }
+  }
+  delete[] padding_size;

[wuchengli, 2014/08/27 05:46:24]

Are we done with the temp_file here? If yes, delete it. Why adding temp_file for
each stream if it is only used in this function?

[henryhsu, 2014/08/27 10:07:28]

Done.

+  return true;
+}
+
 // 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.
   for (size_t index = 0; index < test_streams_data.size(); ++index) {
     std::vector<base::FilePath::StringType> fields;
     base::SplitString(test_streams_data[index], ':', &fields);
     CHECK_GE(fields.size(), 4U) << data;
     CHECK_LE(fields.size(), 9U) << data;
     TestStream* test_stream = new TestStream();
 
-    base::FilePath::StringType filename = fields[0];
+    test_stream->in_filename = fields[0];
     int width, height;
     CHECK(base::StringToInt(fields[1], &width));
     CHECK(base::StringToInt(fields[2], &height));
     test_stream->size = gfx::Size(width, height);
     CHECK(!test_stream->size.IsEmpty());
     int profile;
     CHECK(base::StringToInt(fields[3], &profile));
     CHECK_GT(profile, media::VIDEO_CODEC_PROFILE_UNKNOWN);
     CHECK_LE(profile, media::VIDEO_CODEC_PROFILE_MAX);
     test_stream->requested_profile =
@@ skipping 11 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)));
     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 370 matching lines @@
   CHECK(validator_.get());
 
   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);
-  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);
-
-  // We may need to loop over the stream more than once if more frames than
-  // provided is required for bitrate tests.
-  if (force_bitrate_ && num_frames_in_stream_ < kMinFramesForBitrateTests) {
-    DVLOG(1) << "Stream too short for bitrate test (" << num_frames_in_stream_
-             << " frames), will loop it to reach " << kMinFramesForBitrateTests
-             << " frames";
-    num_frames_to_encode_ = kMinFramesForBitrateTests;
-  } else {
-    num_frames_to_encode_ = num_frames_in_stream_;
-  }
-
   thread_checker_.DetachFromThread();
 }
 
 VEAClient::~VEAClient() { CHECK(!has_encoder()); }
 
 void VEAClient::CreateEncoder() {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(!has_encoder());
 
 #if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
@@ skipping 34 matching lines @@
   return num_encoded_frames_ / duration.InSecondsF();
 }
 
 void VEAClient::RequireBitstreamBuffers(unsigned int input_count,
                                         const gfx::Size& input_coded_size,
                                         size_t output_size) {
   DCHECK(thread_checker_.CalledOnValidThread());
   ASSERT_EQ(state_, CS_INITIALIZED);
   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.
+  PrepareAlignedTempFile(input_coded_size,
+                         const_cast<TestStream*>(&test_stream_),
+                         &input_buffer_size_);
+  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);
+
+  // We may need to loop over the stream more than once if more frames than
+  // provided is required for bitrate tests.
+  if (force_bitrate_ && num_frames_in_stream_ < kMinFramesForBitrateTests) {
+    DVLOG(1) << "Stream too short for bitrate test (" << num_frames_in_stream_
+             << " frames), will loop it to reach " << kMinFramesForBitrateTests
+             << " frames";
+    num_frames_to_encode_ = kMinFramesForBitrateTests;
+  } else {
+    num_frames_to_encode_ = num_frames_in_stream_;
+  }
+
   input_coded_size_ = input_coded_size;
-  ASSERT_EQ(input_coded_size_, test_stream_.size);
 #if defined(ARCH_CPU_ARMEL)

[wuchengli, 2014/08/27 05:46:24]

I think this paragraph should be moved to the function comments of
PrepareAlignedTempFile to explain. Also, asserting input_buffer_size_ is not
enough because we require every plane to 64-byte aligned. Maybe just remove the
assertions. Or you should assert every plane.

[henryhsu, 2014/08/27 10:07:28]

Done.

   // 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
-  // that result in proper alignment, and it would have also interfered with
-  // performance testing. So just assert that the frame size is a multiple of
-  // 64 bytes. This ensures all frames start at 64-byte boundary, because
+  // alignment, if the starting virtual addresses of YUV planes 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.
+  // Now we test resolutions differ from coded size and prepare chunks before

[wuchengli, 2014/08/27 05:46:24]

s/differ/different/. Differ is a verb.

[henryhsu, 2014/08/27 10:07:28]

Done.

+  // testing to avoid performance impact.
+  // So just assert that the frame size is a multiple of 64 bytes.

[wuchengli, 2014/08/27 05:46:24]

Do you intend to make this a paragraph? If not, this should go after the
previous line?

[henryhsu, 2014/08/27 10:07:27]

Done.

+  // This ensures all frames start at 64-byte boundary, because

[wuchengli, 2014/08/27 05:46:24]

80 chars alignment. This should be in the previous line.

[henryhsu, 2014/08/27 10:07:28]

Done.

   // MemoryMappedFile should be mmapp()ed at virtual page start as well.
   ASSERT_EQ(input_buffer_size_ & 63, 0u)
       << "Frame size has to be a multiple of 64 bytes";
   ASSERT_EQ(reinterpret_cast<off_t>(test_stream_.input_file.data()) & 63, 0)
       << "Mapped file should be mapped at a 64 byte boundary";
 #endif
 
   num_required_input_buffers_ = input_count;
   ASSERT_GT(num_required_input_buffers_, 0UL);
 
@@ skipping 72 matching lines @@
 void VEAClient::InputNoLongerNeededCallback(int32 input_id) {
   std::set<int32>::iterator it = inputs_at_client_.find(input_id);
   ASSERT_NE(it, inputs_at_client_.end());
   inputs_at_client_.erase(it);
   FeedEncoderWithInputs();
 }
 
 scoped_refptr<media::VideoFrame> VEAClient::PrepareInputFrame(off_t position) {
   CHECK_LE(position + input_buffer_size_, test_stream_.input_file.length());
 
-  uint8* frame_data =
+  uint8* frame_data_y =
       const_cast<uint8*>(test_stream_.input_file.data() + position);
+  uint8* frame_data_u = frame_data_y +
+      ALIGN_64_BYTES(media::VideoFrame::PlaneAllocationSize(
+          kInputFormat, 0, input_coded_size_));
+  uint8* frame_data_v = frame_data_u +
+      ALIGN_64_BYTES(media::VideoFrame::PlaneAllocationSize(
+          kInputFormat, 1, input_coded_size_));
 
   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(),
           input_coded_size_.width() / 2,
           input_coded_size_.width() / 2,
-          frame_data,
-          frame_data + input_coded_size_.GetArea(),
-          frame_data + (input_coded_size_.GetArea() * 5 / 4),
+          frame_data_y,
+          frame_data_u,
+          frame_data_v,
           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);
   ++next_input_id_;
@@ skipping 194 matching lines @@
   for (size_t state_no = 0; state_no < arraysize(state_transitions); ++state_no)
     for (size_t i = 0; i < num_concurrent_encoders; i++)
       ASSERT_EQ(notes[i]->Wait(), state_transitions[state_no]);
 
   for (size_t i = 0; i < num_concurrent_encoders; ++i) {
     encoder_thread.message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&VEAClient::DestroyEncoder, base::Unretained(clients[i])));
   }
 
+  // Delete temporary files when test finished.
+  for (size_t i = 0; i < test_streams.size(); i++)
+    base::DeleteFile(test_streams[i]->temp_file, false);
+
   // This ensures all tasks have finished.
   encoder_thread.Stop();
 }
 
 INSTANTIATE_TEST_CASE_P(
     SimpleEncode,
     VideoEncodeAcceleratorTest,
     ::testing::Values(MakeTuple(1, true, 0, false, false, false, false)));
 
 INSTANTIATE_TEST_CASE_P(
@@ skipping 68 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();
 }