Unit test for WebMediaPlayerMS

content/renderer/media/webmediaplayer_ms_compositor.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 CONTENT_RENDERER_MEDIA_WEBMEDIAPLAYER_MS_COMPOSITOR_H
 #define CONTENT_RENDERER_MEDIA_WEBMEDIAPLAYER_MS_COMPOSITOR_H
 
 #include <map>
 #include <vector>
 
@@ skipping 25 matching lines @@
 // WebMediaPlayerMS. It will be instantiated on the main thread, but destroyed
 // on the compositor thread.
 //
 // WebMediaPlayerMSCompositor utilizes VideoRendererAlgorithm to store the
 // incoming frames and select the best frame for rendering to maximize the
 // smoothness, if REFERENCE_TIMEs are populated for incoming VideoFrames.
 // Otherwise, WebMediaPlayerMSCompositor will simply store the most recent
 // frame, and submit it whenever asked by the compositor.
 class WebMediaPlayerMSCompositor : public cc::VideoFrameProvider {
  public:
-  // This |url| represents the media stream we are rendering.
+  // This |url| represents the media stream we are rendering. |url| is used to
+  // determine whether we enable algorithm or not by looking up
+  // MediaStreamDescriptor.

[mcasas, 2015/10/21 19:54:02]

Incorrect, |url| is used to find the MediaStream WMPMSC
is supposed to play. The usage of a sophisticated or more
conservative rendering algorithm is decided elsewhere.

[qiangchen, 2015/10/22 17:22:58]

It is decided here. And the main usage of |url| is to decide algorithm or not.

|url| --> remote? --> use algorithm? Added more comments.

   WebMediaPlayerMSCompositor(
       const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
       const blink::WebURL& url);
+
+  WebMediaPlayerMSCompositor(
+      const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
+      const bool algorithm_enabled);

[mcasas, 2015/10/21 19:54:02]

I think you could get rid of this ctor by:

a) registering your MS with a MockMediaStreamRegistry [1]
like is done in MediaRecorderHandlerTest [2] 
b) injecting the flag when appropriate using AppendSwitch() 
e.g. [3]

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/content/test/webrt...

[qiangchen, 2015/10/22 17:22:58]

Plan a) sounds better, but here is some difficulty to implement it, so I'll
follow plan b):
1. In load(), we used static function lookupMediaStreamDescriptor to find web
stream, where static function can't be overriden.
2. In order to avoid that we have to pass web stream to load() as a parameter.
However changing signature of load() is a big project itself, because it is an
overriden function, and the origin function is in Blink.

+
   ~WebMediaPlayerMSCompositor() override;
 
   void EnqueueFrame(const scoped_refptr<media::VideoFrame>& frame);
 
   // Statistical data
   gfx::Size GetCurrentSize();
   base::TimeDelta GetCurrentTime();
   size_t total_frame_count() const;
   size_t dropped_frame_count() const;
 
@@ skipping 66 matching lines @@
 
   std::map<base::TimeDelta, base::TimeTicks> timestamps_to_clock_times_;
 
   // |current_frame_lock_| protects |current_frame_used_by_compositor_|,
   // |current_frame_|, and |rendering_frame_buffer_|.
   base::Lock current_frame_lock_;
 };
 }
 
 #endif  // CONTENT_RENDERER_MEDIA_WEBMEDIAPLAYER_MS_COMPOSITOR_H