Narrow VDA Decode() API and remove multi-frame tests.

content/common/gpu/media/video_decode_accelerator_unittest.cc

Index: content/common/gpu/media/video_decode_accelerator_unittest.cc
diff --git a/content/common/gpu/media/video_decode_accelerator_unittest.cc b/content/common/gpu/media/video_decode_accelerator_unittest.cc
index 4a671627839eeba8bb3197814d0599890ff47fa6..4c2ce9509f722c8a98cf9ea3bf3004347cf97c20 100644
--- a/content/common/gpu/media/video_decode_accelerator_unittest.cc
+++ b/content/common/gpu/media/video_decode_accelerator_unittest.cc
@@ -15,7 +15,6 @@
 //   infrastructure.
 
 #include <fcntl.h>
-#include <math.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <deque>
@@ -400,7 +399,6 @@ class GLRenderingVDAClient
  public:
   // Doesn't take ownership of |rendering_helper| or |note|, which must outlive
   // |*this|.
-  // |num_fragments_per_decode| counts NALUs for h264 and frames for VP8.
   // |num_play_throughs| indicates how many times to play through the video.
   // |reset_after_frame_num| can be a frame number >=0 indicating a mid-stream
   // Reset() should be done after that frame number is delivered, or
@@ -419,7 +417,6 @@ class GLRenderingVDAClient
                        int rendering_window_id,
                        ClientStateNotification<ClientState>* note,
                        const std::string& encoded_data,
-                       int num_fragments_per_decode,
                        int num_in_flight_decodes,
                        int num_play_throughs,
                        int reset_after_frame_num,
@@ -468,24 +465,23 @@ class GLRenderingVDAClient
   void DeleteDecoder();
 
   // Compute & return the first encoded bytes (including a start frame) to send
-  // to the decoder, starting at |start_pos| and returning
-  // |num_fragments_per_decode| units. Skips to the first decodable position.
-  std::string GetBytesForFirstFragments(size_t start_pos, size_t* end_pos);
-  // Compute & return the next encoded bytes to send to the decoder (based on
-  // |start_pos| & |num_fragments_per_decode_|).
-  std::string GetBytesForNextFragments(size_t start_pos, size_t* end_pos);
-  // Helpers for GetRangeForNextFragments above.
+  // to the decoder, starting at |start_pos| and returning one fragment. Skips
+  // to the first decodable position.
+  std::string GetBytesForFirstFragment(size_t start_pos, size_t* end_pos);
+  // Compute & return the encoded bytes of next fragment to send to the decoder
+  // (based on |start_pos|).
+  std::string GetBytesForNextFragment(size_t start_pos, size_t* end_pos);
+  // Helpers for GetBytesForNextFragment above.
   void GetBytesForNextNALU(size_t start_pos, size_t* end_pos);  // For h.264.
-  std::string GetBytesForNextFrames(
+  std::string GetBytesForNextFrame(
       size_t start_pos, size_t* end_pos);  // For VP8.
 
-  // Request decode of the next batch of fragments in the encoded data.
-  void DecodeNextFragments();
+  // Request decode of the next fragment in the encoded data.
+  void DecodeNextFragment();
 
   RenderingHelper* rendering_helper_;
   int rendering_window_id_;
   std::string encoded_data_;
-  const int num_fragments_per_decode_;
   const int num_in_flight_decodes_;
   int outstanding_decodes_;
   size_t encoded_data_next_pos_to_decode_;
@@ -517,7 +513,6 @@ GLRenderingVDAClient::GLRenderingVDAClient(
     int rendering_window_id,
     ClientStateNotification<ClientState>* note,
     const std::string& encoded_data,
-    int num_fragments_per_decode,
     int num_in_flight_decodes,
     int num_play_throughs,
     int reset_after_frame_num,
@@ -531,7 +526,6 @@ GLRenderingVDAClient::GLRenderingVDAClient(
     : rendering_helper_(rendering_helper),
       rendering_window_id_(rendering_window_id),
       encoded_data_(encoded_data),
-      num_fragments_per_decode_(num_fragments_per_decode),
       num_in_flight_decodes_(num_in_flight_decodes),
       outstanding_decodes_(0),
       encoded_data_next_pos_to_decode_(0),
@@ -548,7 +542,6 @@ GLRenderingVDAClient::GLRenderingVDAClient(
       profile_(profile),
       suppress_rendering_(suppress_rendering),
       delay_reuse_after_frame_num_(delay_reuse_after_frame_num) {
-  CHECK_GT(num_fragments_per_decode, 0);
   CHECK_GT(num_in_flight_decodes, 0);
   CHECK_GT(num_play_throughs, 0);
   CHECK_GE(rendering_fps, 0);
@@ -697,7 +690,7 @@ void GLRenderingVDAClient::NotifyInitializeDone() {
   }
 
   for (int i = 0; i < num_in_flight_decodes_; ++i)
-    DecodeNextFragments();
+    DecodeNextFragment();
   DCHECK_EQ(outstanding_decodes_, num_in_flight_decodes_);
 }
 
@@ -709,7 +702,7 @@ void GLRenderingVDAClient::NotifyEndOfBitstreamBuffer(
   // VaapiVideoDecodeAccelerator::FinishReset()).
   ++num_done_bitstream_buffers_;
   --outstanding_decodes_;
-  DecodeNextFragments();
+  DecodeNextFragment();
 }
 
 void GLRenderingVDAClient::NotifyFlushDone() {
@@ -730,12 +723,12 @@ void GLRenderingVDAClient::NotifyResetDone() {
 
   if (reset_after_frame_num_ == MID_STREAM_RESET) {
     reset_after_frame_num_ = END_OF_STREAM_RESET;
-    DecodeNextFragments();
+    DecodeNextFragment();
     return;
   } else if (reset_after_frame_num_ == START_OF_STREAM_RESET) {
     reset_after_frame_num_ = END_OF_STREAM_RESET;
     for (int i = 0; i < num_in_flight_decodes_; ++i)
-      DecodeNextFragments();
+      DecodeNextFragment();
     return;
   }
 
@@ -801,13 +794,13 @@ void GLRenderingVDAClient::DeleteDecoder() {
     SetState(static_cast<ClientState>(i));
 }
 
-std::string GLRenderingVDAClient::GetBytesForFirstFragments(
+std::string GLRenderingVDAClient::GetBytesForFirstFragment(
     size_t start_pos, size_t* end_pos) {
   if (profile_ < media::H264PROFILE_MAX) {
     *end_pos = start_pos;
     while (*end_pos + 4 < encoded_data_.size()) {
       if ((encoded_data_[*end_pos + 4] & 0x1f) == 0x7) // SPS start frame
-        return GetBytesForNextFragments(*end_pos, end_pos);
+        return GetBytesForNextFragment(*end_pos, end_pos);
       GetBytesForNextNALU(*end_pos, end_pos);
       num_skipped_fragments_++;
     }
@@ -815,25 +808,21 @@ std::string GLRenderingVDAClient::GetBytesForFirstFragments(
     return std::string();
   }
   DCHECK_LE(profile_, media::VP8PROFILE_MAX);
-  return GetBytesForNextFragments(start_pos, end_pos);
+  return GetBytesForNextFragment(start_pos, end_pos);
 }
 
-std::string GLRenderingVDAClient::GetBytesForNextFragments(
+std::string GLRenderingVDAClient::GetBytesForNextFragment(
     size_t start_pos, size_t* end_pos) {
   if (profile_ < media::H264PROFILE_MAX) {
-    size_t new_end_pos = start_pos;
     *end_pos = start_pos;
-    for (int i = 0; i < num_fragments_per_decode_; ++i) {
-      GetBytesForNextNALU(*end_pos, &new_end_pos);
-      if (*end_pos == new_end_pos)
-        break;
-      *end_pos = new_end_pos;
+    GetBytesForNextNALU(*end_pos, end_pos);
+    if (start_pos != *end_pos) {
       num_queued_fragments_++;
     }
     return encoded_data_.substr(start_pos, *end_pos - start_pos);
   }
   DCHECK_LE(profile_, media::VP8PROFILE_MAX);
-  return GetBytesForNextFrames(start_pos, end_pos);
+  return GetBytesForNextFrame(start_pos, end_pos);
 }
 
 void GLRenderingVDAClient::GetBytesForNextNALU(
@@ -851,26 +840,22 @@ void GLRenderingVDAClient::GetBytesForNextNALU(
     *end_pos = encoded_data_.size();
 }
 
-std::string GLRenderingVDAClient::GetBytesForNextFrames(
+std::string GLRenderingVDAClient::GetBytesForNextFrame(
     size_t start_pos, size_t* end_pos) {
   // Helpful description: http://wiki.multimedia.cx/index.php?title=IVF
   std::string bytes;
   if (start_pos == 0)
     start_pos = 32;  // Skip IVF header.
   *end_pos = start_pos;
-  for (int i = 0; i < num_fragments_per_decode_; ++i) {
-    uint32 frame_size = *reinterpret_cast<uint32*>(&encoded_data_[*end_pos]);
-    *end_pos += 12;  // Skip frame header.
-    bytes.append(encoded_data_.substr(*end_pos, frame_size));
-    *end_pos += frame_size;
-    num_queued_fragments_++;
-    if (*end_pos + 12 >= encoded_data_.size())
-      return bytes;
-  }
+  uint32 frame_size = *reinterpret_cast<uint32*>(&encoded_data_[*end_pos]);
+  *end_pos += 12;  // Skip frame header.
+  bytes.append(encoded_data_.substr(*end_pos, frame_size));
+  *end_pos += frame_size;
+  num_queued_fragments_++;
   return bytes;
 }
 
-void GLRenderingVDAClient::DecodeNextFragments() {
+void GLRenderingVDAClient::DecodeNextFragment() {
   if (decoder_deleted())
     return;
   if (encoded_data_next_pos_to_decode_ == encoded_data_.size()) {
@@ -883,14 +868,14 @@ void GLRenderingVDAClient::DecodeNextFragments() {
   size_t end_pos;
   std::string next_fragment_bytes;
   if (encoded_data_next_pos_to_decode_ == 0) {
-    next_fragment_bytes = GetBytesForFirstFragments(0, &end_pos);
+    next_fragment_bytes = GetBytesForFirstFragment(0, &end_pos);
   } else {
     next_fragment_bytes =
-        GetBytesForNextFragments(encoded_data_next_pos_to_decode_, &end_pos);
+        GetBytesForNextFragment(encoded_data_next_pos_to_decode_, &end_pos);
   }
   size_t next_fragment_size = next_fragment_bytes.size();
 
-  // Populate the shared memory buffer w/ the fragments, duplicate its handle,
+  // Populate the shared memory buffer w/ the fragment, duplicate its handle,
   // and hand it off to the decoder.
   base::SharedMemory shm;
   CHECK(shm.CreateAndMapAnonymous(next_fragment_size));
@@ -924,7 +909,6 @@ double GLRenderingVDAClient::frames_per_second() {
 }
 
 // Test parameters:
-// - Number of fragments per Decode() call.
 // - Number of concurrent decoders.
 // - Number of concurrent in-flight Decode() calls per decoder.
 // - Number of play-throughs.
@@ -934,16 +918,16 @@ double GLRenderingVDAClient::frames_per_second() {
 // - whether the video frames are rendered as thumbnails.
 class VideoDecodeAcceleratorTest
     : public ::testing::TestWithParam<
-  Tuple8<int, int, int, int, ResetPoint, ClientState, bool, bool> > {
+  Tuple7<int, int, int, ResetPoint, ClientState, bool, bool> > {
 };
 
 // Helper so that gtest failures emit a more readable version of the tuple than
 // its byte representation.
 ::std::ostream& operator<<(
     ::std::ostream& os,
-    const Tuple8<int, int, int, int, ResetPoint, ClientState, bool, bool>& t) {
+    const Tuple7<int, int, int, ResetPoint, ClientState, bool, bool>& t) {
   return os << t.a << ", " << t.b << ", " << t.c << ", " << t.d << ", " << t.e
-            << ", " << t.f << ", " << t.g << ", " << t.h;
+            << ", " << t.f << ", " << t.g;
 }
 
 // Wait for |note| to report a state and if it's not |expected_state| then
@@ -970,14 +954,13 @@ TEST_P(VideoDecodeAcceleratorTest, TestSimpleDecode) {
   // Required for Thread to work.  Not used otherwise.
   base::ShadowingAtExitManager at_exit_manager;
 
-  const int num_fragments_per_decode = GetParam().a;
-  const size_t num_concurrent_decoders = GetParam().b;
-  const size_t num_in_flight_decodes = GetParam().c;
-  const int num_play_throughs = GetParam().d;
-  const int reset_point = GetParam().e;
-  const int delete_decoder_state = GetParam().f;
-  bool test_reuse_delay = GetParam().g;
-  const bool render_as_thumbnails = GetParam().h;
+  const size_t num_concurrent_decoders = GetParam().a;
+  const size_t num_in_flight_decodes = GetParam().b;
+  const int num_play_throughs = GetParam().c;
+  const int reset_point = GetParam().d;
+  const int delete_decoder_state = GetParam().e;
+  bool test_reuse_delay = GetParam().f;
+  const bool render_as_thumbnails = GetParam().g;
 
   std::vector<TestVideoFile*> test_video_files;
   ParseAndReadTestVideoData(g_test_video_data,
@@ -986,10 +969,7 @@ TEST_P(VideoDecodeAcceleratorTest, TestSimpleDecode) {
                             &test_video_files);
 
   // Suppress GL rendering for all tests when the "--disable_rendering" is set.
-  // Otherwise, suppress rendering in all but a few tests, to cut down overall
-  // test runtime.
-  const bool suppress_rendering =
-      num_fragments_per_decode > 1 || g_disable_rendering;
+  const bool suppress_rendering = g_disable_rendering;
 
   std::vector<ClientStateNotification<ClientState>*>
       notes(num_concurrent_decoders, NULL);
@@ -1056,7 +1036,6 @@ TEST_P(VideoDecodeAcceleratorTest, TestSimpleDecode) {
                                  index,
                                  note,
                                  video_file->data_str,
-                                 num_fragments_per_decode,
                                  num_in_flight_decodes,
                                  num_play_throughs,
                                  video_file->reset_after_frame_num,
@@ -1136,8 +1115,7 @@ TEST_P(VideoDecodeAcceleratorTest, TestSimpleDecode) {
       EXPECT_EQ(video_file->num_fragments, client->num_skipped_fragments() +
                 client->num_queued_fragments());
       EXPECT_EQ(client->num_done_bitstream_buffers(),
-                ceil(static_cast<double>(client->num_queued_fragments()) /
-                     num_fragments_per_decode));
+                client->num_queued_fragments());
     }
     LOG(INFO) << "Decoder " << i << " fps: " << client->frames_per_second();
     if (!render_as_thumbnails) {
@@ -1230,7 +1208,7 @@ TEST_P(VideoDecodeAcceleratorTest, TestSimpleDecode) {
 INSTANTIATE_TEST_CASE_P(
     ReplayAfterEOS, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 4, END_OF_STREAM_RESET, CS_RESET, false, false)));
+        MakeTuple(1, 1, 4, END_OF_STREAM_RESET, CS_RESET, false, false)));
 
 // This hangs on Exynos, preventing further testing and wasting test machine
 // time.
@@ -1240,7 +1218,7 @@ INSTANTIATE_TEST_CASE_P(
 INSTANTIATE_TEST_CASE_P(
     ResetBeforeDecode, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 1, START_OF_STREAM_RESET, CS_RESET, false, false)));
+        MakeTuple(1, 1, 1, START_OF_STREAM_RESET, CS_RESET, false, false)));
 #endif  // ARCH_CPU_X86_FAMILY
 
 // Test that Reset() mid-stream works fine and doesn't affect decoding even when
@@ -1248,53 +1226,39 @@ INSTANTIATE_TEST_CASE_P(
 INSTANTIATE_TEST_CASE_P(
     MidStreamReset, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 1, MID_STREAM_RESET, CS_RESET, false, false)));
+        MakeTuple(1, 1, 1, MID_STREAM_RESET, CS_RESET, false, false)));
 
 INSTANTIATE_TEST_CASE_P(
     SlowRendering, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, true, false)));
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_RESET, true, false)));
 
 // Test that Destroy() mid-stream works fine (primarily this is testing that no
 // crashes occur).
 INSTANTIATE_TEST_CASE_P(
     TearDownTiming, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_DECODER_SET, false,
-                  false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_INITIALIZED, false,
-                  false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_FLUSHING, false, false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_FLUSHED, false, false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_RESETTING, false, false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET,
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_DECODER_SET, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_INITIALIZED, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_FLUSHING, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_FLUSHED, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_RESETTING, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET,
                   static_cast<ClientState>(-1), false, false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET,
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET,
                   static_cast<ClientState>(-10), false, false),
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET,
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET,
                   static_cast<ClientState>(-100), false, false)));
 
-// Test that decoding various variation works: multiple fragments per Decode()
-// call and multiple in-flight decodes.
+// Test that decoding various variation works with multiple in-flight decodes.
 INSTANTIATE_TEST_CASE_P(
     DecodeVariations, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(1, 1, 10, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
+        MakeTuple(1, 10, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
         // Tests queuing.
-        MakeTuple(1, 1, 15, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(2, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(3, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(5, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(8, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false),
-        // TODO(fischman): decoding more than 15 NALUs at once breaks decode -
-        // visual artifacts are introduced as well as spurious frames are
-        // delivered (more pictures are returned than NALUs are fed to the
-        // decoder).  Increase the "15" below when
-        // http://code.google.com/p/chrome-os-partner/issues/detail?id=4378 is
-        // fixed.
-        MakeTuple(15, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, false)));
+        MakeTuple(1, 15, 1, END_OF_STREAM_RESET, CS_RESET, false, false)));
 
 // Find out how many concurrent decoders can go before we exhaust system
 // resources.
@@ -1302,16 +1266,16 @@ INSTANTIATE_TEST_CASE_P(
     ResourceExhaustion, VideoDecodeAcceleratorTest,
     ::testing::Values(
         // +0 hack below to promote enum to int.
-        MakeTuple(1, kMinSupportedNumConcurrentDecoders + 0, 1, 1,
+        MakeTuple(kMinSupportedNumConcurrentDecoders + 0, 1, 1,
                   END_OF_STREAM_RESET, CS_RESET, false, false),
-        MakeTuple(1, kMinSupportedNumConcurrentDecoders + 1, 1, 1,
+        MakeTuple(kMinSupportedNumConcurrentDecoders + 1, 1, 1,
                   END_OF_STREAM_RESET, CS_RESET, false, false)));
 
 // Thumbnailing test
 INSTANTIATE_TEST_CASE_P(
     Thumbnail, VideoDecodeAcceleratorTest,
     ::testing::Values(
-        MakeTuple(1, 1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, true)));
+        MakeTuple(1, 1, 1, END_OF_STREAM_RESET, CS_RESET, false, true)));
 
 // TODO(fischman, vrk): add more tests!  In particular:
 // - Test life-cycle: Seek/Stop/Pause/Play for a single decoder.