Initialize based on frame sizes in VideoTrackRecorder

content/renderer/media/video_track_recorder.cc

Index: content/renderer/media/video_track_recorder.cc
diff --git a/content/renderer/media/video_track_recorder.cc b/content/renderer/media/video_track_recorder.cc
index 29fcaedee7bd0e4ad8e1d799c5ee9064ce32e3a3..a2814a12d1ee3759963b6f889acbdce7516db23b 100644
--- a/content/renderer/media/video_track_recorder.cc
+++ b/content/renderer/media/video_track_recorder.cc
@@ -929,37 +929,23 @@ VideoTrackRecorder::VideoTrackRecorder(
     const blink::WebMediaStreamTrack& track,
     const OnEncodedVideoCB& on_encoded_video_callback,
     int32_t bits_per_second)
-    : track_(track) {
+    : main_task_runner_(base::MessageLoop::current()->task_runner()),
+      status_(NOT_INITIALIZED),
+      track_(track),
+      codec_(codec),
+      on_encoded_video_callback_(on_encoded_video_callback),
+      bits_per_second_(bits_per_second),
+      paused_(false),
+      weak_ptr_factory_(this) {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
   DCHECK(!track_.isNull());
   DCHECK(track_.getExtraData());
 
-  const auto& vea_supported_profile = CodecIdToVEAProfile(codec);
-  // TODO(emircan): Prioritize software based encoders in lower resolutions.
-  if (vea_supported_profile != media::VIDEO_CODEC_PROFILE_UNKNOWN) {
-    encoder_ = new VEAEncoder(on_encoded_video_callback, bits_per_second,
-                              vea_supported_profile);
-  } else {
-    switch (codec) {
-#if BUILDFLAG(RTC_USE_H264)
-      case CodecId::H264:
-        encoder_ = new H264Encoder(on_encoded_video_callback, bits_per_second);
-        break;
-#endif
-      case CodecId::VP8:
-      case CodecId::VP9:
-        encoder_ = new VpxEncoder(codec == CodecId::VP9,
-                                  on_encoded_video_callback, bits_per_second);
-        break;
-      default:
-        NOTREACHED() << "Unsupported codec";
-    }
-  }
-
-  // StartFrameEncode() will be called on Render IO thread.
+  // It is safe to use Unretained, because the class gets disconnected from
+  // track in dtor.
   MediaStreamVideoSink::ConnectToTrack(
       track_,
-      base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode, encoder_),
+      base::Bind(&VideoTrackRecorder::StartFrameEncode, base::Unretained(this)),

[mcasas, 2016/05/25 16:42:47]

Why not use weak_ptr_factory_.GetWeakPtr() here?

[emircan, 2016/05/27 04:27:20]

Done.

       false);
 }
 
@@ -969,22 +955,100 @@ VideoTrackRecorder::~VideoTrackRecorder() {
   track_.reset();
 }
 
+void VideoTrackRecorder::StartFrameEncode(
+    const scoped_refptr<VideoFrame>& frame,
+    base::TimeTicks capture_timestamp) {
+  if (!origin_task_runner_.get())
+    origin_task_runner_ = base::MessageLoop::current()->task_runner();
+  DCHECK(origin_task_runner_->BelongsToCurrentThread());
+
+  if (status_ == INITIALIZED) {
+    encoder_->StartFrameEncode(frame, capture_timestamp);
+    return;
+  }
+
+  if (status_ == NOT_INITIALIZED) {
+    main_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&VideoTrackRecorder::InitializeEncoder,
+                   weak_ptr_factory_.GetWeakPtr(), frame, capture_timestamp));

[mcasas, 2016/05/25 16:42:47]

Actually, it might be easier than all this thread
dancing.

|encoder_| lives on |origin_task_runner_|
(that we know to be IO thread). So what about
a) add a comment that |encoder_| member lives
on IO |origin_task_runner_|
b) not do this PostTask
c) Simplify InitializeEncoder (rename appropriately?)
d) in ~VideoTrackRecorder(), use sth like
 DeleteSoon(encoder_.release())
e) and update Pause() and Resume() to trivial 
PostTasks ?
f) Update comments in Encoder about its
threading model (now it'll be fully living on the 
construction thread).

Essentially, moving the whole |encoder_| management
and interaction into the IO thread.

Then, probably, you can simplify |status_| into a
boolean |is_initialized_| or, even better, remove it
in favour of a method 
boolean IsIinitialized() const { return !!encoder_; }

[emircan, 2016/05/27 04:27:20]

VEAEncoder needs to access RenderThreadImpl methods and those are only
accessible via Main Render thread. See
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r....
I understand that these thread hops look dangerous, but we need Main thread for
creation of objects and IO thread for interaction. WDYT?

+    status_ = INITIALIZING;
+  }
+}
+
 void VideoTrackRecorder::Pause() {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
-  DCHECK(encoder_);
-  encoder_->SetPaused(true);
+  if (status_ == INITIALIZED)
+    encoder_->SetPaused(true);
+  else
+    paused_ = true;
 }
 
 void VideoTrackRecorder::Resume() {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
-  DCHECK(encoder_);
-  encoder_->SetPaused(false);
+  if (status_ == INITIALIZED)
+    encoder_->SetPaused(false);
+  else
+    paused_ = false;
 }
 
 void VideoTrackRecorder::OnVideoFrameForTesting(
     const scoped_refptr<media::VideoFrame>& frame,
     base::TimeTicks timestamp) {
-  encoder_->StartFrameEncode(frame, timestamp);
+  if (!origin_task_runner_.get())
+    origin_task_runner_ = base::MessageLoop::current()->task_runner();
+  DCHECK(origin_task_runner_->BelongsToCurrentThread());
+
+  if (status_ == INITIALIZED) {
+    encoder_->StartFrameEncode(frame, timestamp);
+  }
+
+  if (status_ == NOT_INITIALIZED) {
+    main_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&VideoTrackRecorder::InitializeEncoder,
+                   weak_ptr_factory_.GetWeakPtr(), frame, timestamp));
+    status_ = INITIALIZING;
+  }
+}
+
+void VideoTrackRecorder::InitializeEncoder(
+    const scoped_refptr<VideoFrame>& frame,
+    base::TimeTicks capture_timestamp) {
+  DVLOG(3) << __FUNCTION__;
+  DCHECK(main_render_thread_checker_.CalledOnValidThread());
+
+  const gfx::Size& input_size = frame->visible_rect().size();
+  const auto& vea_supported_profile = CodecIdToVEAProfile(codec_);
+  // Prioritizing software based encoders in lower resolutions.
+  if (vea_supported_profile != media::VIDEO_CODEC_PROFILE_UNKNOWN &&
+      input_size.width() >= kVEAEncoderMinResolutionWidth &&
+      input_size.height() >= kVEAEncoderMinResolutionHeight) {
+      encoder_ = new VEAEncoder(on_encoded_video_callback_, bits_per_second_,
+                                vea_supported_profile);
+    }
+  else {
+    switch (codec_) {
+#if BUILDFLAG(RTC_USE_H264)
+      case CodecId::H264:
+        encoder_ =
+            new H264Encoder(on_encoded_video_callback_, bits_per_second_);
+        break;
+#endif
+      case CodecId::VP8:
+      case CodecId::VP9:
+        encoder_ = new VpxEncoder(codec_ == CodecId::VP9,
+                                  on_encoded_video_callback_, bits_per_second_);
+        break;
+      default:
+        NOTREACHED() << "Unsupported codec";
+    }
+  }
+  encoder_->SetPaused(paused_);
+  origin_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode,
+                            encoder_, frame, capture_timestamp));
+  status_ = INITIALIZED;
 }
 
 }  // namespace content