Add switches: "target_fps" and "disable_rendering" to video_decode_accelerator_unittest.

content/common/gpu/media/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 70 matching lines @@
 // - |profile| is the media::VideoCodecProfile set during Initialization.
 // An empty value for a numeric field means "ignore".
 const base::FilePath::CharType* test_video_data =
     // FILE_PATH_LITERAL("test-25fps.vp8:320:240:250:250:50:175:11");
     FILE_PATH_LITERAL("test-25fps.h264:320:240:250:258:50:175:1");
 
 // The path of the frame delivery time log. We can enable the log and specify
 // the filename by the "--frame_delivery_log" switch.
 const base::FilePath::CharType* frame_delivery_log = NULL;
 
+// The value is set by the switch "--rendering_fps".
+double rendering_fps = 0;
+
+// Disable rendering, the value is set by the switch "--disable_rendering".
+bool disable_rendering = false;

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

This is not mentioned in the CL description.

[Owen Lin, 2013/07/31 10:01:36]

Done.

+
 // Magic constants for differentiating the reasons for NotifyResetDone being
 // called.
 enum ResetPoint {
   MID_STREAM_RESET = -2,
   END_OF_STREAM_RESET = -1
 };
 
 const int kMaxResetAfterFrameNum = 100;
 const int kMaxFramesToDelayReuse = 64;
 const int kReuseDelayMs = 1000;
@@ skipping 79 matching lines @@
 }
 
 // State of the GLRenderingVDAClient below.  Order matters here as the test
 // makes assumptions about it.
 enum ClientState {
   CS_CREATED = 0,
   CS_DECODER_SET = 1,
   CS_INITIALIZED = 2,
   CS_FLUSHING = 3,
   CS_FLUSHED = 4,
-  CS_DONE = 5,
-  CS_RESETTING = 6,
-  CS_RESET = 7,
-  CS_ERROR = 8,
-  CS_DESTROYED = 9,
+  CS_RESETTING = 5,
+  CS_RESET = 6,
+  CS_ERROR = 7,
+  CS_DESTROYED = 8,
   CS_MAX,  // Must be last entry.
 };
 
 // Helper class allowing one thread to wait on a notification from another.
 // If notifications come in faster than they are Wait()'d for, they are
 // accumulated (so exactly as many Wait() calls will unblock as Notify() calls
 // were made, regardless of order).
 class ClientStateNotification {
  public:
   ClientStateNotification();
@@ skipping 21 matching lines @@
 
 ClientState ClientStateNotification::Wait() {
   base::AutoLock auto_lock(lock_);
   while (pending_states_for_notification_.empty())
     cv_.Wait();
   ClientState ret = pending_states_for_notification_.front();
   pending_states_for_notification_.pop();
   return ret;
 }
 
+class ThrottlingVDAClient : public VideoDecodeAccelerator::Client {

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

doco the point of this class.

[Owen Lin, 2013/07/31 10:01:36]

Done.

+ public:
+  // Callback invoked whan the picture is dropped and should be reused for
+  // the decoder again.
+  typedef base::Callback<void(int32 picture_buffer_id)> ReusePictureCB;
+
+  ThrottlingVDAClient(VideoDecodeAccelerator::Client*,

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

missing param name

[Owen Lin, 2013/07/31 10:01:36]

Done.

+                      double fps,
+                      ReusePictureCB reuse_pic_cb);

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

s/pic/picture/

[Owen Lin, 2013/07/31 10:01:36]

Done.

+  virtual ~ThrottlingVDAClient();
+
+  virtual void ProvidePictureBuffers(uint32 requested_num_of_buffers,
+                                     const gfx::Size& dimensions,
+                                     uint32 texture_target) OVERRIDE;
+  virtual void DismissPictureBuffer(int32 picture_buffer_id) OVERRIDE;
+  virtual void PictureReady(const media::Picture& picture) OVERRIDE;
+
+  // Simple state changes.

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

What's so "simple" about them? :)
Generally chrome code docos overridden virtuals by grouping them all together
(in the order of declaration in the base class) and commenting 
// FooInterface implementation.
like 
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/codec/vid...

[Owen Lin, 2013/07/31 10:01:36]

I just copy the description from GLRenderingVDAClient.
Thanks.

+  virtual void NotifyInitializeDone() OVERRIDE;
+  virtual void NotifyEndOfBitstreamBuffer(int32 bitstream_buffer_id) OVERRIDE;
+  virtual void NotifyFlushDone() OVERRIDE;
+  virtual void NotifyResetDone() OVERRIDE;
+  virtual void NotifyError(VideoDecodeAccelerator::Error error) OVERRIDE;
+
+  int num_decoded_frames() { return num_decoded_frames_; }
+
+ private:
+  VideoDecodeAccelerator::Client* client_;
+  ReusePictureCB reuse_pic_cb_;
+  base::TimeTicks last_frame_delivered_time_;
+  base::TimeDelta frame_duration_;
+
+  int num_decoded_frames_;
+  int pending_pictures_;
+
+  void CallClientPictureReady(const media::Picture& picture);

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

style: methods go above members

[Owen Lin, 2013/07/31 10:01:36]

Done.

+};

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

DISALLOW_IMPLICIT_CONSTRUCTORS

[Owen Lin, 2013/07/31 10:01:36]

Done.

+
+ThrottlingVDAClient::ThrottlingVDAClient(
+    VideoDecodeAccelerator::Client* client,
+    double fps,
+    ReusePictureCB reuse_pic_cb)
+    : client_(client),
+      reuse_pic_cb_(reuse_pic_cb),
+      num_decoded_frames_(0),
+      pending_pictures_(0) {
+  CHECK(client_);
+  CHECK_GT(fps, 0);
+  frame_duration_ = base::TimeDelta::FromSeconds(1) / fps;
+}
+
+ThrottlingVDAClient::~ThrottlingVDAClient() {}
+
+void ThrottlingVDAClient::ProvidePictureBuffers(
+    uint32 requested_num_of_buffers,
+    const gfx::Size& dimensions,
+    uint32 texture_target) {
+  client_->ProvidePictureBuffers(requested_num_of_buffers,
+                                 dimensions,
+                                 texture_target);
+}
+
+void ThrottlingVDAClient::DismissPictureBuffer(int32 picture_buffer_id) {
+  client_->DismissPictureBuffer(picture_buffer_id);
+}
+
+void ThrottlingVDAClient::PictureReady(const media::Picture& picture) {
+    base::TimeTicks now = base::TimeTicks::Now();

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

indent is off here and elsewhere.

[Owen Lin, 2013/07/31 10:01:36]

Done.

+    if (num_decoded_frames_ == 0)
+      last_frame_delivered_time_ = now;
+    else
+      last_frame_delivered_time_ += frame_duration_;

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

This var name doesn't make sense.
Is it supposed to embody "time of beginning of duration of the latest frame
scheduled for rendering"?  Because that's what I _think_ it's computing.  But
then the test at l.316 below is wrong, because if "now" falls in the middle of
the frame's duration you wouldn't drop it, you'd still want to show it (for less
than its scheduled duration if the next frame is delivered on time).

Basically I think you want to rename some things and probably rejigger some
other things in this method :)

[Owen Lin, 2013/07/31 10:01:36]

The code has been changed to address the NotifyResetDone() issue you mentioned.

+
+    ++num_decoded_frames_;
+
+    if (last_frame_delivered_time_ < now) {
+      // Frame dropped, skip rendering.
+      LOG(INFO) << "Frame dropped";
+      last_frame_delivered_time_ += frame_duration_;
+      reuse_pic_cb_.Run(picture.picture_buffer_id());
+      return;
+    }
+    ++pending_pictures_;
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ThrottlingVDAClient::CallClientPictureReady,
+                   base::Unretained(this),
+                   picture),
+        last_frame_delivered_time_ - now);
+}
+
+void ThrottlingVDAClient::CallClientPictureReady(
+    const media::Picture& picture) {
+  --pending_pictures_;
+  client_->PictureReady(picture);
+}
+
+void ThrottlingVDAClient::NotifyInitializeDone() {
+  client_->NotifyInitializeDone();
+}
+
+void ThrottlingVDAClient::NotifyEndOfBitstreamBuffer(
+    int32 bitstream_buffer_id) {
+  client_->NotifyEndOfBitstreamBuffer(bitstream_buffer_id);
+}
+
+void ThrottlingVDAClient::NotifyFlushDone() {
+  if (pending_pictures_ > 0) {
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ThrottlingVDAClient::NotifyFlushDone,
+                   base::Unretained(this)),
+        last_frame_delivered_time_ - base::TimeTicks::Now());
+    return;
+  }
+  client_->NotifyFlushDone();
+}
+
+void ThrottlingVDAClient::NotifyResetDone() {
+  if (pending_pictures_ > 0) {

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

This delays reset for rendering, which is unlikely to mirror what a real
VDA::Client does; real clients drop queued frames when they request a Reset(). 
I think you want to do the same here.

[Owen Lin, 2013/07/31 10:01:36]

Done.

+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ThrottlingVDAClient::NotifyResetDone,
+                   base::Unretained(this)),
+        last_frame_delivered_time_ - base::TimeTicks::Now());
+    return;
+  }
+  client_->NotifyResetDone();
+}
+
+void ThrottlingVDAClient::NotifyError(VideoDecodeAccelerator::Error error) {
+  client_->NotifyError(error);
+}
+
 // Client that can accept callbacks from a VideoDecodeAccelerator and is used by
 // the TESTs below.
 class GLRenderingVDAClient : public VideoDecodeAccelerator::Client {
  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
   // END_OF_STREAM_RESET to indicate no mid-stream Reset().
   // |delete_decoder_state| indicates when the underlying decoder should be
   // Destroy()'d and deleted and can take values: N<0: delete after -N Decode()
   // calls have been made, N>=0 means interpret as ClientState.
   // Both |reset_after_frame_num| & |delete_decoder_state| apply only to the
   // last play-through (governed by |num_play_throughs|).
+  // |rendering_fps| indicates the target rendering fps. 0 means no target fps
+  // and it would render as fast as possible.
+  // Use |suppress_rendering| to skip the rendering part, so it only decodes

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

Rewrite this line to match the rest of the paragraph

[Owen Lin, 2013/07/31 10:01:36]

Done.

+  // the frames and won't shown on display.
   // After |delay_reuse_after_frame_num| frame has been delivered, the client
   // will start delaying the call to ReusePictureBuffer() for kReuseDelayMs.
   GLRenderingVDAClient(RenderingHelper* rendering_helper,
                        int rendering_window_id,
                        ClientStateNotification* 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,
                        int delete_decoder_state,
                        int frame_width,
                        int frame_height,
                        int profile,
+                       double rendering_fps,
                        bool suppress_rendering,
                        int delay_reuse_after_frame_num);
   virtual ~GLRenderingVDAClient();
   void CreateDecoder();
 
   // VideoDecodeAccelerator::Client implementation.
   // The heart of the Client.
   virtual void ProvidePictureBuffers(uint32 requested_num_of_buffers,
                                      const gfx::Size& dimensions,
                                      uint32 texture_target) OVERRIDE;
   virtual void DismissPictureBuffer(int32 picture_buffer_id) OVERRIDE;
   virtual void PictureReady(const media::Picture& picture) OVERRIDE;
   // Simple state changes.
   virtual void NotifyInitializeDone() OVERRIDE;
   virtual void NotifyEndOfBitstreamBuffer(int32 bitstream_buffer_id) OVERRIDE;
   virtual void NotifyFlushDone() OVERRIDE;
   virtual void NotifyResetDone() OVERRIDE;
   virtual void NotifyError(VideoDecodeAccelerator::Error error) OVERRIDE;
 
   void OutputFrameDeliveryTimes(base::PlatformFile output);
 
+  void NotifyFrameDropped(int32 picture_buffer_id);
+
   // 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();
   bool decoder_deleted() { return !decoder_.get(); }
 
  private:
   typedef std::map<int, media::PictureBuffer*> PictureBufferById;
@@ skipping 36 matching lines @@
   int num_skipped_fragments_;
   int num_queued_fragments_;
   int num_decoded_frames_;
   int num_done_bitstream_buffers_;
   PictureBufferById picture_buffers_by_id_;
   base::TimeTicks initialize_done_ticks_;
   int profile_;
   bool suppress_rendering_;
   std::vector<base::TimeTicks> frame_delivery_times_;
   int delay_reuse_after_frame_num_;
+  scoped_ptr<ThrottlingVDAClient> throttling_client_;
 };
 
 GLRenderingVDAClient::GLRenderingVDAClient(
     RenderingHelper* rendering_helper,
     int rendering_window_id,
     ClientStateNotification* 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,
     int delete_decoder_state,
     int frame_width,
     int frame_height,
     int profile,
+    double rendering_fps,
     bool suppress_rendering,
     int delay_reuse_after_frame_num)
     : 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), next_bitstream_buffer_id_(0),
       note_(note),
       remaining_play_throughs_(num_play_throughs),
       reset_after_frame_num_(reset_after_frame_num),
       delete_decoder_state_(delete_decoder_state),
       state_(CS_CREATED),
       num_skipped_fragments_(0), num_queued_fragments_(0),
       num_decoded_frames_(0), num_done_bitstream_buffers_(0),
       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);
+  if (rendering_fps > 0)
+    throttling_client_.reset(new ThrottlingVDAClient(
+        this,
+        rendering_fps,
+        base::Bind(&GLRenderingVDAClient::NotifyFrameDropped,
+                   base::Unretained(this))));
 }
 
 GLRenderingVDAClient::~GLRenderingVDAClient() {
   DeleteDecoder();  // Clean up in case of expected error.
   CHECK(decoder_deleted());
   STLDeleteValues(&picture_buffers_by_id_);
   SetState(CS_DESTROYED);
 }
 
 static bool DoNothingReturnTrue() { return true; }
 
 void GLRenderingVDAClient::CreateDecoder() {
   CHECK(decoder_deleted());
   CHECK(!decoder_.get());
+
+  VideoDecodeAccelerator::Client* client = this;
+  if (throttling_client_)
+      client = throttling_client_.get();
 #if defined(OS_WIN)
   decoder_.reset(new DXVAVideoDecodeAccelerator(
-      this, base::Bind(&DoNothingReturnTrue)));
+      client, base::Bind(&DoNothingReturnTrue)));
 #elif defined(OS_CHROMEOS)
 #if defined(ARCH_CPU_ARMEL)
   decoder_.reset(
       new ExynosVideoDecodeAccelerator(
           static_cast<EGLDisplay>(rendering_helper_->GetGLDisplay()),
           static_cast<EGLContext>(rendering_helper_->GetGLContext()),
-          this, base::Bind(&DoNothingReturnTrue)));
+          client, base::Bind(&DoNothingReturnTrue)));
 #elif defined(ARCH_CPU_X86_FAMILY)
   decoder_.reset(new VaapiVideoDecodeAccelerator(
       static_cast<Display*>(rendering_helper_->GetGLDisplay()),
       static_cast<GLXContext>(rendering_helper_->GetGLContext()),
-      this, base::Bind(&DoNothingReturnTrue)));
+      client, base::Bind(&DoNothingReturnTrue)));
 #endif  // ARCH_CPU_ARMEL
 #endif  // OS_WIN
   CHECK(decoder_.get());
   SetState(CS_DECODER_SET);
   if (decoder_deleted())
     return;
 
   // Configure the decoder.
   media::VideoCodecProfile profile = media::H264PROFILE_BASELINE;
   if (profile_ != -1)
@@ skipping 34 matching lines @@
   delete it->second;
   picture_buffers_by_id_.erase(it);
 }
 
 void GLRenderingVDAClient::PictureReady(const media::Picture& picture) {
   // We shouldn't be getting pictures delivered after Reset has completed.
   CHECK_LT(state_, CS_RESET);
 
   if (decoder_deleted())
     return;
+
   frame_delivery_times_.push_back(base::TimeTicks::Now());
 
-  CHECK_LE(picture.bitstream_buffer_id(), next_bitstream_buffer_id_);
-  ++num_decoded_frames_;
-
   // Mid-stream reset applies only to the last play-through per constructor
   // comment.
   if (remaining_play_throughs_ == 1 &&
       reset_after_frame_num_ == num_decoded_frames_) {
     reset_after_frame_num_ = MID_STREAM_RESET;
     decoder_->Reset();
     // Re-start decoding from the beginning of the stream to avoid needing to
     // know how to find I-frames and so on in this test.
     encoded_data_next_pos_to_decode_ = 0;
   }
 
+  CHECK_LE(picture.bitstream_buffer_id(), next_bitstream_buffer_id_);
+  ++num_decoded_frames_;

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

Why did you move these two lines?

[Owen Lin, 2013/07/31 10:01:36]

Move them back.

+
   media::PictureBuffer* picture_buffer =
       picture_buffers_by_id_[picture.picture_buffer_id()];
   CHECK(picture_buffer);
   if (!suppress_rendering_) {
     rendering_helper_->RenderTexture(picture_buffer->texture_id());
   }
-
   if (num_decoded_frames_ > delay_reuse_after_frame_num_) {
     base::MessageLoop::current()->PostDelayedTask(FROM_HERE, base::Bind(
         &VideoDecodeAccelerator::ReusePictureBuffer,
-        base::Unretained(decoder_.get()), picture.picture_buffer_id()),
+        decoder_->AsWeakPtr(), picture.picture_buffer_id()),

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

Should be gone after rebase?

[Owen Lin, 2013/07/31 10:01:36]

Yes. It should be gone.

         base::TimeDelta::FromMilliseconds(kReuseDelayMs));
   } else {
     decoder_->ReusePictureBuffer(picture.picture_buffer_id());
   }
 }
 
 void GLRenderingVDAClient::NotifyInitializeDone() {
   SetState(CS_INITIALIZED);
   initialize_done_ticks_ = base::TimeTicks::Now();
   for (int i = 0; i < num_in_flight_decodes_; ++i)
     DecodeNextFragments();
   DCHECK_EQ(outstanding_decodes_, num_in_flight_decodes_);
 }
 
 void GLRenderingVDAClient::NotifyEndOfBitstreamBuffer(
     int32 bitstream_buffer_id) {
   // TODO(fischman): this test currently relies on this notification to make
   // forward progress during a Reset().  But the VDA::Reset() API doesn't
   // guarantee this, so stop relying on it (and remove the notifications from
   // VaapiVideoDecodeAccelerator::FinishReset()).
   ++num_done_bitstream_buffers_;
   --outstanding_decodes_;
-  DecodeNextFragments();
+  // Don't decode if it is in mid-stream resetting
+  if (reset_after_frame_num_ != MID_STREAM_RESET)

[Owen Lin, 2013/07/24 06:30:45]

This is for the MidStreamReset test case.

In the new code, we delay the "NotifyResetDone" until all PictureReady. While we
are waiting for it, we still keep feeding data to the decoder. Eventually, it
will call flush() and change the state to FLUSHING. This is not what we want. I
think we should stop feeding data here.

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

See comment above; I think you don't want this and instead want to change how
TVC-based Resetting works.

[Owen Lin, 2013/07/31 10:01:36]

Done.

+    DecodeNextFragments();

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

AFAICT the point of this CL is to slow down decode to match a target rendering
FPS, to evaluate the CPU power needed for a realistic decode (i.e. not a
full-speed, non-rendering decode) and to ensure that the decoder can keep up
with the desired frame-rate (i.e. that there are no dropped frames).

IWBN if the CL description was updated to reflect the result of these goals on
at least one platform.  Something like: "using test video XXX with target FPS
YYY CPU rate goes from xx to yy and dropped frames are at zzz".

[Owen Lin, 2013/07/31 10:01:36]

Done.

 }
 
 void GLRenderingVDAClient::NotifyFlushDone() {
   if (decoder_deleted())
     return;
   SetState(CS_FLUSHED);
   --remaining_play_throughs_;
   DCHECK_GE(remaining_play_throughs_, 0);
-  if (decoder_deleted())
-    return;
   decoder_->Reset();

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

Why do you no longer need the decoder_deleted() guard?

[Owen Lin, 2013/07/31 10:01:36]

Oops, reverted. I didn't notice that decoder would be deleted in SetState().

   SetState(CS_RESETTING);
 }
 
 void GLRenderingVDAClient::NotifyResetDone() {
   if (decoder_deleted())
     return;
 
   if (reset_after_frame_num_ == MID_STREAM_RESET) {
     reset_after_frame_num_ = END_OF_STREAM_RESET;
     DecodeNextFragments();
@@ skipping 22 matching lines @@
   base::TimeTicks t0 = initialize_done_ticks_;
   for (size_t i = 0; i < frame_delivery_times_.size(); ++i) {
     s = base::StringPrintf("frame %04" PRIuS ": %" PRId64 " us\n",
                            i,
                            (frame_delivery_times_[i] - t0).InMicroseconds());
     t0 = frame_delivery_times_[i];
     base::WritePlatformFileAtCurrentPos(output, s.data(), s.length());
   }
 }
 
+void GLRenderingVDAClient::NotifyFrameDropped(int32 picture_buffer_id) {
+  decoder_->ReusePictureBuffer(picture_buffer_id);

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

Shouldn't this be counting dropped frames or something?

(more useful would probably be to track max _dropped frames per second_, but if
the total count of dropped frames is zero then that matters less)

[Owen Lin, 2013/07/31 10:01:36]

I plan to derive the number of dropped frame from frame's delivery times. 
So, I don't need to test or log it here.

+}
+
 static bool LookingAtNAL(const std::string& encoded, size_t pos) {
   return encoded[pos] == 0 && encoded[pos + 1] == 0 &&
       encoded[pos + 2] == 0 && encoded[pos + 3] == 1;
 }
 
 void GLRenderingVDAClient::SetState(ClientState new_state) {
   note_->Notify(new_state);
   state_ = new_state;
   if (!remaining_play_throughs_ && new_state == delete_decoder_state_) {
     CHECK(!decoder_deleted());
@@ skipping 143 matching lines @@
 // - whether to test slow rendering by delaying ReusePictureBuffer().
 class VideoDecodeAcceleratorTest
     : public ::testing::TestWithParam<
   Tuple7<int, int, int, int, ResetPoint, ClientState, 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 Tuple6<int, int, int, int, ResetPoint, ClientState>& t) {
+    const Tuple7<int, int, int, int, ResetPoint, ClientState, bool>& t) {
   return os << t.a << ", " << t.b << ", " << t.c << ", " << t.d << ", " << t.e
-            << ", " << t.f;
+            << ", " << t.f << ", " << t.g;
 }
 
 // Wait for |note| to report a state and if it's not |expected_state| then
 // assert |client| has deleted its decoder.
 static void AssertWaitForStateOrDeleted(ClientStateNotification* note,
                                         GLRenderingVDAClient* client,
                                         ClientState expected_state) {
   ClientState state = note->Wait();
   if (state == expected_state) return;
   ASSERT_TRUE(client->decoder_deleted())
       << "Decoder not deleted but Wait() returned " << state
       << ", instead of " << expected_state;
 }
 
 // We assert a minimal number of concurrent decoders we expect to succeed.
 // Different platforms can support more concurrent decoders, so we don't assert
 // failure above this.
 enum { kMinSupportedNumConcurrentDecoders = 3 };
 
 // Test the most straightforward case possible: data is decoded from a single
 // chunk and rendered to the screen.
 TEST_P(VideoDecodeAcceleratorTest, TestSimpleDecode) {
-  // Can be useful for debugging VLOGs from OVDA.
-  // logging::SetMinLogLevel(-1);
-
   // 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;
 
   std::vector<TestVideoFile*> test_video_files;
   ParseAndReadTestVideoData(test_video_data, num_concurrent_decoders,
                             reset_point, &test_video_files);
 
   // Suppress GL rendering when we are logging the frame delivery time and a
   // few other tests, to cut down overall test runtime.
   const bool suppress_rendering = num_fragments_per_decode > 1 ||
-      frame_delivery_log != NULL;
+      content::disable_rendering;

[Ami GONE FROM CHROMIUM, 2013/07/24 18:23:16]

You dropped
frame_delivery_log != NULL
from the condition, but didn't update the comment.

I'm not sure whether the right answer is to update the comment or to restore the
condition.  Can you choose one and explain your choice?

[Owen Lin, 2013/07/31 10:01:36]

A new switch "--disable_rendering" was add in this change. 
So that we can control it when we have different type of test cases. 

 
   std::vector<ClientStateNotification*> notes(num_concurrent_decoders, NULL);
   std::vector<GLRenderingVDAClient*> clients(num_concurrent_decoders, NULL);
 
   // Initialize the rendering helper.
   base::Thread rendering_thread("GLRenderingVDAClientThread");
   base::Thread::Options options;
   options.message_loop_type = base::MessageLoop::TYPE_DEFAULT;
 #if defined(OS_WIN)
   // For windows the decoding thread initializes the media foundation decoder
@@ skipping 29 matching lines @@
     if (test_reuse_delay &&
         kMaxFramesToDelayReuse * 2 < video_file->num_frames) {
       delay_after_frame_num = video_file->num_frames - kMaxFramesToDelayReuse;
     }
 
     GLRenderingVDAClient* client = new GLRenderingVDAClient(
         rendering_helper.get(), index, note, video_file->data_str,
         num_fragments_per_decode, num_in_flight_decodes, num_play_throughs,
         video_file->reset_after_frame_num, delete_decoder_state,
         video_file->width, video_file->height, video_file->profile,
-        suppress_rendering, delay_after_frame_num);
+        rendering_fps, suppress_rendering, delay_after_frame_num);
     clients[index] = client;
 
     rendering_thread.message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&GLRenderingVDAClient::CreateDecoder,
                    base::Unretained(client)));
 
     ASSERT_EQ(note->Wait(), CS_DECODER_SET);
   }
   // Then wait for all the decodes to finish.
@@ skipping 18 matching lines @@
       if (n > 0) {
         ASSERT_NO_FATAL_FAILURE(
             AssertWaitForStateOrDeleted(note, clients[i], CS_INITIALIZED));
       }
       // InitializeDone kicks off decoding inside the client, so we just need to
       // wait for Flush.
       ASSERT_NO_FATAL_FAILURE(
           AssertWaitForStateOrDeleted(note, clients[i], CS_FLUSHING));
       ASSERT_NO_FATAL_FAILURE(
           AssertWaitForStateOrDeleted(note, clients[i], CS_FLUSHED));
-      // FlushDone requests Reset().
       ASSERT_NO_FATAL_FAILURE(
           AssertWaitForStateOrDeleted(note, clients[i], CS_RESETTING));
     }
     ASSERT_NO_FATAL_FAILURE(
         AssertWaitForStateOrDeleted(note, clients[i], CS_RESET));
+
     // ResetDone requests Destroy().
     ASSERT_NO_FATAL_FAILURE(
         AssertWaitForStateOrDeleted(note, clients[i], CS_DESTROYED));
   }
   // Finally assert that decoding went as expected.
   for (size_t i = 0; i < num_concurrent_decoders &&
            !skip_performance_and_correctness_checks; ++i) {
     // We can only make performance/correctness assertions if the decoder was
     // allowed to finish.
     if (delete_decoder_state < CS_FLUSHED)
@@ skipping 153 matching lines @@
   for (CommandLine::SwitchMap::const_iterator it = switches.begin();
        it != switches.end(); ++it) {
     if (it->first == "test_video_data") {
       content::test_video_data = it->second.c_str();
       continue;
     }
     if (it->first == "frame_delivery_log") {
       content::frame_delivery_log = it->second.c_str();
       continue;
     }
+    if (it->first == "rendering_fps") {
+      CHECK(base::StringToDouble(it->second, &content::rendering_fps));
+      continue;
+    }
+    if (it->first == "disable_rendering") {
+      content::disable_rendering = true;
+      continue;
+    }
     if (it->first == "v" || it->first == "vmodule")
       continue;
     LOG(FATAL) << "Unexpected switch: " << it->first << ":" << it->second;
   }
 
   base::ShadowingAtExitManager at_exit_manager;
 
 #if defined(OS_WIN)
   content::DXVAVideoDecodeAccelerator::PreSandboxInitialization();
 #elif defined(OS_CHROMEOS)
 #if defined(ARCH_CPU_ARMEL)
   content::ExynosVideoDecodeAccelerator::PreSandboxInitialization();
 #elif defined(ARCH_CPU_X86_FAMILY)
   content::VaapiWrapper::PreSandboxInitialization();
 #endif  // ARCH_CPU_ARMEL
 #endif  // OS_CHROMEOS
 
   return RUN_ALL_TESTS();
 }