Refactor VideoTrackRecorder into smaller classes

content/renderer/media_recorder/video_track_recorder.h

Index: content/renderer/media_recorder/video_track_recorder.h
diff --git a/content/renderer/media_recorder/video_track_recorder.h b/content/renderer/media_recorder/video_track_recorder.h
index 1b623787919df9c2c38d8623962ff9684460aeea..4e6a6a7e79145b3d7e3427c3bf7cb8b64a074d5e 100644
--- a/content/renderer/media_recorder/video_track_recorder.h
+++ b/content/renderer/media_recorder/video_track_recorder.h
@@ -16,8 +16,18 @@
 #include "content/public/renderer/media_stream_video_sink.h"
 #include "media/muxers/webm_muxer.h"
 #include "third_party/WebKit/public/platform/WebMediaStreamTrack.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+namespace base {
+class Thread;
+}  // namespace base
+
+namespace cc {
+class PaintCanvas;
+}  // namespace cc
 
 namespace media {
+class SkCanvasVideoRenderer;
 class VideoFrame;
 }  // namespace media
 
@@ -46,7 +56,6 @@ class CONTENT_EXPORT VideoTrackRecorder
 #endif
     LAST
   };
-  class Encoder;
 
   using OnEncodedVideoCB =
       base::Callback<void(const media::WebmMuxer::VideoParameters& params,
@@ -55,6 +64,88 @@ class CONTENT_EXPORT VideoTrackRecorder
                           base::TimeTicks capture_timestamp,
                           bool is_key_frame)>;
 
+  // Base class to describe a generic Encoder, encapsulating all actual encoder
+  // (re)configurations, encoding and delivery of received frames. This class is
+  // ref-counted to allow the MediaStreamVideoTrack to hold a reference to it
+  // (via the callback that MediaStreamVideoSink passes along) and to jump back
+  // and forth to an internal encoder thread. Moreover, this class: - is created
+  // on its parent's thread (usually the main Render thread), that is,
+  // |main_task_runner_|. - receives VideoFrames on |origin_task_runner_| and
+  // runs OnEncodedVideoCB on that thread as well. This task runner is cached on
+  // first frame arrival, and is supposed to be the render IO thread (but this
+  // is not enforced); - uses an internal |encoding_task_runner_| for actual

[mcasas, 2017/04/05 01:23:59]

The dashes are supposed to go in each line: 
https://cs.chromium.org/chromium/src/content/renderer/media_recorder/video_tr...

[emircan, 2017/04/05 21:13:44]

Done.

+  // encoder interactions, namely configuration, encoding (which might take some
+  // time) and destruction. This task runner can be passed on the creation. If
+  // nothing is passed, a new encoding thread is created and used.
+  class Encoder : public base::RefCountedThreadSafe<Encoder> {
+   public:
+    Encoder(const OnEncodedVideoCB& on_encoded_video_callback,
+            int32_t bits_per_second,
+            scoped_refptr<base::SingleThreadTaskRunner> encoding_task_runner =
+                nullptr);
+
+    // Start encoding |frame|, returning via |on_encoded_video_callback_|. This
+    // call will also trigger an encode configuration upon first frame arrival
+    // or parameter change, and an EncodeOnEncodingTaskRunner() to actually
+    // encode the frame. If the |frame|'s data is not directly available (e.g.
+    // it's a texture) then RetrieveFrameOnMainThread() is called, and if even
+    // that fails, black frames are sent instead.
+    void StartFrameEncode(const scoped_refptr<media::VideoFrame>& frame,
+                          base::TimeTicks capture_timestamp);
+    void RetrieveFrameOnMainThread(
+        const scoped_refptr<media::VideoFrame>& video_frame,
+        base::TimeTicks capture_timestamp);
+
+    static void OnFrameEncodeCompleted(
+        const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_cb,
+        const media::WebmMuxer::VideoParameters& params,
+        std::unique_ptr<std::string> data,
+        std::unique_ptr<std::string> alpha_data,
+        base::TimeTicks capture_timestamp,
+        bool keyframe);
+
+    void SetPaused(bool paused);
+    virtual bool CanEncodeAlphaChannel();
+
+   protected:
+    friend class base::RefCountedThreadSafe<Encoder>;
+    virtual ~Encoder();
+
+    virtual void EncodeOnEncodingTaskRunner(
+        scoped_refptr<media::VideoFrame> frame,
+        base::TimeTicks capture_timestamp) = 0;
+
+    // Used to shutdown properly on the same thread we were created.
+    const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+
+    // Task runner where frames to encode and reply callbacks must happen.
+    scoped_refptr<base::SingleThreadTaskRunner> origin_task_runner_;
+
+    // Task runner where encoding interactions happen.
+    scoped_refptr<base::SingleThreadTaskRunner> encoding_task_runner_;
+
+    // Optional thread for encoding. Active for the lifetime of VpxEncoder.
+    std::unique_ptr<base::Thread> encoding_thread_;
+
+    // While |paused_|, frames are not encoded. Used only from
+    // |encoding_thread_|.
+    bool paused_;
+
+    // This callback should be exercised on IO thread.
+    const OnEncodedVideoCB on_encoded_video_callback_;
+
+    // Target bitrate for video encoding. If 0, a standard bitrate is used.
+    const int32_t bits_per_second_;
+
+    // Used to retrieve incoming opaque VideoFrames (i.e. VideoFrames backed by
+    // textures). Created on-demand on |main_task_runner_|.
+    std::unique_ptr<media::SkCanvasVideoRenderer> video_renderer_;
+    SkBitmap bitmap_;
+    std::unique_ptr<cc::PaintCanvas> canvas_;
+
+    DISALLOW_COPY_AND_ASSIGN(Encoder);
+  };
+
   static CodecId GetPreferredCodecId();
 
   VideoTrackRecorder(CodecId codec,
@@ -77,9 +168,6 @@ class CONTENT_EXPORT VideoTrackRecorder
                          const scoped_refptr<media::VideoFrame>& frame,
                          base::TimeTicks capture_time);
 
-  // TODO(emircan): Remove after refactor, see http://crbug.com/700433.
-  bool CanEncodeAlphaChannelForTesting();
-
   // Used to check that we are destroyed on the same thread we were created.
   base::ThreadChecker main_render_thread_checker_;