Update TestVideoRenderer with video decoding capabilities

remoting/test/test_video_renderer.h

 // Copyright 2015 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.
 
 #ifndef REMOTING_TEST_TEST_VIDEO_RENDERER_H_
 #define REMOTING_TEST_TEST_VIDEO_RENDERER_H_
 
-#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "media/base/video_frame.h"
 #include "remoting/client/video_renderer.h"
+#include "remoting/protocol/session_config.h"
 #include "remoting/protocol/video_stub.h"
 
+namespace base {
+class Thread;
+class SingleThreadTaskRunner;
+}
+
+namespace webrtc {
+class DesktopFrame;
+}
+
 namespace remoting {
 namespace test {
 
 // Processes video packets as they are received from the remote host.
 class TestVideoRenderer : public VideoRenderer, public protocol::VideoStub {
  public:
   TestVideoRenderer();
   ~TestVideoRenderer() override;
 
   // VideoRenderer interface.
   void OnSessionConfig(const protocol::SessionConfig& config) override;
   ChromotingStats* GetStats() override;
   protocol::VideoStub* GetVideoStub() override;
 
- private:
   // protocol::VideoStub interface.
   void ProcessVideoPacket(scoped_ptr<VideoPacket> video_packet,
                           const base::Closure& done) override;
 
-  // Track the number of populated video frames which have been received.
-  int video_frames_processed_;
+  // Initialize a decoder to decode video packets.
+  void SetDecoder(const protocol::ChannelConfig::Codec codec);

[joedow, 2015/06/30 17:52:27]

Should this be renamed "SetCodecForDecoding"?

[liaoyuke, 2015/06/30 19:34:59]

Done.

+
+  // Runs a RunLoop until the video packet is processed.

[joedow, 2015/06/30 17:52:27]

Interface comments shouldn't document internal implementations.  Can you update
this comment such that it describes what the method does (instead of how)?

Actually, now that I think about it, this method may not be necessary.  A better
mechanism would be for ProcessVideoPacket to pass the done closure to the
decoding thread and then have the decoding thread (once it has done its work)
post the done callback back to the main thread.  (Check out how this is done in
SoftwareVideoRenderer.cc (lines 349, 165)

[liaoyuke, 2015/06/30 19:35:00]

Done.

+  void WaitForPacketDone(const base::Closure& done);
+
+  // Returns a copy of the current buffer.
+  scoped_ptr<webrtc::DesktopFrame> GetBufferForTest() const;
+
+ private:
+  // The actual implementation resides in Core class.
+  class Core;
+  scoped_ptr<Core> core_;
+
+  // Signals when is packet is processed.
+  void OnPacketDone();
+
+  // Used to quit the RunLoop of main thread.
+  base::Closure main_closure_;

[joedow, 2015/06/30 17:52:27]

I think we should go with the approach I mention above, then you can get rid of
special case members like this which are only used for testing.

[liaoyuke, 2015/06/30 19:34:59]

Done.

+
+  // Used to ensure TestVideoRenderer methods are called on the same thread.
+  base::ThreadChecker thread_checker_;
+
+  // Used to decode and process video packets.
+  scoped_ptr<base::Thread>  video_decode_thread_;
+
+  // Used to post tasks to video decode thread.
+  scoped_refptr<base::SingleThreadTaskRunner> video_decode_task_runner_;
+
+  // Used to weakly bind |this| to |OnPacketDone| method.
+  base::WeakPtrFactory<TestVideoRenderer> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(TestVideoRenderer);
 };
 
 }  // namespace test
 }  // namespace remoting
 
 #endif  // REMOTING_TEST_TEST_VIDEO_RENDERER_H_