vda_unittest: Add a new test case to verify the visible size.

media/gpu/video_decode_accelerator_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.
 //
 // The bulk of this file is support code; sorry about that.  Here's an overview
 // to hopefully help readers of this code:
 // - RenderingHelper is charged with interacting with X11/{EGL/GLES2,GLX/GL} or
 //   Win/EGL.
 // - ClientState is an enum for the state of the decode client used by the test.
 // - ClientStateNotification is a barrier abstraction that allows the test code
@@ skipping 164 matching lines @@
   int height;
   int num_frames;
   int num_fragments;
   int min_fps_render;
   int min_fps_no_render;
   VideoCodecProfile profile;
   int reset_after_frame_num;
   std::string data_str;
 };
 
+struct VisibleRectEntry {

[Pawel Osciak, 2016/11/17 07:50:01]

Please add a short sentence describing what this is, and also what count means.

+  gfx::Rect rect;
+  int count;
+  VisibleRectEntry(const gfx::Rect rect, int count)

[Pawel Osciak, 2016/11/17 07:50:01]

const&

+      : rect(rect), count(count) {}
+};
+
 const gfx::Size kThumbnailsPageSize(1600, 1200);
 const gfx::Size kThumbnailSize(160, 120);
 const int kMD5StringLength = 32;
 
 // Read in golden MD5s for the thumbnailed rendering of this video
 void ReadGoldenThumbnailMD5s(const TestVideoFile* video_file,
                              std::vector<std::string>* md5_strings) {
   base::FilePath filepath(video_file->file_name);
   filepath = filepath.AddExtension(FILE_PATH_LITERAL(".md5"));
   std::string all_md5s;
@@ skipping 249 matching lines @@
   void OutputFrameDeliveryTimes(base::File* output);
 
   // Simple getters for inspecting the state of the Client.
   int num_done_bitstream_buffers() { return num_done_bitstream_buffers_; }
   int num_skipped_fragments() { return num_skipped_fragments_; }
   int num_queued_fragments() { return num_queued_fragments_; }
   int num_decoded_frames() { return num_decoded_frames_; }
   double frames_per_second();
   // Return the median of the decode time of all decoded frames.
   base::TimeDelta decode_time_median();
+  const std::vector<VisibleRectEntry>& visible_rects() {
+    return visible_rects_;
+  }
   bool decoder_deleted() { return !decoder_.get(); }
 
  private:
   typedef std::map<int32_t, scoped_refptr<TextureRef>> TextureRefMap;
 
   void SetState(ClientState new_state);
   void FinishInitialization();
   void ReturnPicture(int32_t picture_buffer_id);
 
   // Delete the associated decoder helper.
@@ skipping 41 matching lines @@
   int fake_decoder_;
   GLenum texture_target_;
   VideoPixelFormat pixel_format_;
   bool suppress_rendering_;
   std::vector<base::TimeTicks> frame_delivery_times_;
   int delay_reuse_after_frame_num_;
   // A map from bitstream buffer id to the decode start time of the buffer.
   std::map<int, base::TimeTicks> decode_start_time_;
   // The decode time of all decoded frames.
   std::vector<base::TimeDelta> decode_time_;
+  // The visible rectangles of the picture returned from PictureReady().
+  std::vector<VisibleRectEntry> visible_rects_;
   // The number of VDA::Decode calls per second. This is to simulate webrtc.
   int decode_calls_per_second_;
   bool render_as_thumbnails_;
 
   // A map of the textures that are currently active for the decoder, i.e.,
   // have been created via AssignPictureBuffers() and not dismissed via
   // DismissPictureBuffer(). The keys in the map are the IDs of the
   // corresponding picture buffers, and the values are TextureRefs to the
   // textures.
   TextureRefMap active_textures_;
@@ skipping 198 matching lines @@
   // We shouldn't be getting pictures delivered after Reset has completed.
   LOG_ASSERT(state_ < CS_RESET);
 
   if (decoder_deleted())
     return;
 
   base::TimeTicks now = base::TimeTicks::Now();
 
   frame_delivery_times_.push_back(now);
 
+  if (visible_rects_.empty() ||
+      visible_rects_.back().rect != picture.visible_rect()) {
+    visible_rects_.push_back(VisibleRectEntry(picture.visible_rect(), 1));

[Pawel Osciak, 2016/11/17 07:50:01]

Perhaps just have count=1 as default when constructing? Up to you.

+  } else {
+    ++visible_rects_.back().count;
+  }
+
   // Save the decode time of this picture.
   std::map<int, base::TimeTicks>::iterator it =
       decode_start_time_.find(picture.bitstream_buffer_id());
   ASSERT_NE(decode_start_time_.end(), it);
   decode_time_.push_back(now - it->second);
   decode_start_time_.erase(it);
 
   LOG_ASSERT(picture.bitstream_buffer_id() <= next_bitstream_buffer_id_);
   ++num_decoded_frames_;
 
@@ skipping 958 matching lines @@
   LOG(INFO) << output_string;
 
   if (g_output_log != NULL)
     OutputLogFile(g_output_log, output_string);
 
   g_env->GetRenderingTaskRunner()->DeleteSoon(FROM_HERE, client);
   g_env->GetRenderingTaskRunner()->DeleteSoon(FROM_HERE, note);
   WaitUntilIdle();
 };
 
+// This test Logs the visible rectangles of the picture returned from

[Pawel Osciak, 2016/11/17 07:50:01]

s/Logs/verifies/

+// PictureReady().
+TEST_F(VideoDecodeAcceleratorTest, TestVisibleRect) {
+  RenderingHelperParams helper_params;
+
+  // Disable rendering by setting the rendering_fps = 0.
+  helper_params.rendering_fps = 0;
+  helper_params.warm_up_iterations = 0;
+  helper_params.render_as_thumbnails = false;
+
+  ClientStateNotification<ClientState>* note =
+      new ClientStateNotification<ClientState>();
+  GLRenderingVDAClient* client = new GLRenderingVDAClient(
+      0, &rendering_helper_, note, test_video_files_[0]->data_str, 1, 1,
+      test_video_files_[0]->reset_after_frame_num, CS_RESET,
+      test_video_files_[0]->width, test_video_files_[0]->height,
+      test_video_files_[0]->profile, g_fake_decoder, true,
+      std::numeric_limits<int>::max(), 0, false /* render_as_thumbnail */);
+  helper_params.window_sizes.push_back(
+      gfx::Size(test_video_files_[0]->width, test_video_files_[0]->height));
+  InitializeRenderingHelper(helper_params);
+  CreateAndStartDecoder(client, note);

[Pawel Osciak, 2016/11/17 07:50:01]

I wonder if we need an additional test case just for this, we could maybe just
verify this in SimpleDecode, otherwise we are making the automated tests run
longer...

[wuchengli, 2016/11/17 08:23:41]

Adding a test case has a benefits that it's clear what is broken.
TestDecodeTimeMedian is a performance test and runs very long. We can remove
TestDecodeTimeMedian from the default test suite to save some time.

[Pawel Osciak, 2016/11/17 08:40:31]

Yes, but I believe making the test run longer outweights this advantage, and
proper logging via EXPECT() should be enough to see what went wrong.
That's an orthogonal issue. The test stream is 10 seconds long. By adding this
test, we are making each run of this test 10 seconds longer in the test lab for
each run, but we could achieve this by adding this verification to one of the
existing test cases, just like we are already verifying multiple things in them,
like the number of frames decoded.

+  WaitUntilDecodeFinish(note);
+
+  for (const auto& entry : client->visible_rects()) {
+    const auto& r = entry.rect;
+    std::string output_string =
+        base::StringPrintf("(%d, %d, %d, %d) %d", r.x(), r.y(), r.width(),

[kcwu, 2016/11/17 07:54:51]

How about 
LOG(INFO) << entry.rect << " " << entry.count;

+                           r.height(), entry.count);
+    LOG(INFO) << output_string;

[Pawel Osciak, 2016/11/17 07:50:01]

Perhaps just DVLOG?

+  }
+
+  // Resolution change video is not supported yet. Simply verify there is
+  // only one resolution and it matches to the expected visible rect.
+  ASSERT_EQ(1U, client->visible_rects().size());
+  gfx::Rect expected_rect(test_video_files_[0]->width,
+                          test_video_files_[0]->height);
+  const auto& entry = client->visible_rects().front();
+  ASSERT_TRUE(entry.rect.IsEmpty() || (entry.rect == expected_rect));

[Pawel Osciak, 2016/11/17 07:50:01]

I think we could just EXPECT?

[kcwu, 2016/11/17 07:54:51]

EXPECT_TRUE

+  ASSERT_EQ(test_video_files_[0]->num_frames, entry.count);

[kcwu, 2016/11/17 07:54:51]

EXPECT_EQ

+
+  g_env->GetRenderingTaskRunner()->DeleteSoon(FROM_HERE, client);
+  g_env->GetRenderingTaskRunner()->DeleteSoon(FROM_HERE, note);
+  WaitUntilIdle();
+};
+
 // TODO(fischman, vrk): add more tests!  In particular:
 // - Test life-cycle: Seek/Stop/Pause/Play for a single decoder.
 // - Test alternate configurations
 // - Test failure conditions.
 // - Test frame size changes mid-stream
 
 }  // namespace
 }  // namespace media
 
 int main(int argc, char** argv) {
@@ skipping 83 matching lines @@
   media::DXVAVideoDecodeAccelerator::PreSandboxInitialization();
 #endif
 
   media::g_env =
       reinterpret_cast<media::VideoDecodeAcceleratorTestEnvironment*>(
           testing::AddGlobalTestEnvironment(
               new media::VideoDecodeAcceleratorTestEnvironment()));
 
   return RUN_ALL_TESTS();
 }