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 e1f5ad02018ca9bfc2653fb97d06d812d41a7d0f..2a1bd6706f2e4e2e175490eff85531526c380808 100644
--- a/content/renderer/media/video_track_recorder.cc
+++ b/content/renderer/media/video_track_recorder.cc
@@ -935,37 +935,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()),
+      track_(track),
+      codec_(codec),
+      on_encoded_video_callback_(on_encoded_video_callback),
+      bits_per_second_(bits_per_second),
+      paused_before_init_(false),
+      weak_ptr_factory_(this) {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
   DCHECK(!track_.isNull());
   DCHECK(track_.getTrackData());
 
-  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.
+  // InitializeEncoder() 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_),
+      track_, base::Bind(&VideoTrackRecorder::InitializeEncoder,
+                         base::Unretained(this)),

[mcasas, 2016/06/08 18:51:24]

weak_ptr_factory_.GetWeakPtr()

[emircan, 2016/06/09 03:03:43]

Weak_ptr would be attached to the IO thread then?

[mcasas, 2016/06/12 08:56:34]

Yes, the WeakPtrFactory (and associated pointers)
get attached to the appropriate thread on first use [1],
however in this case that'd pose a problem for 
destruction. However, using a base::Unretained 
makes me feel uneasy. What about:

  MediaStreamVideoSink::ConnectToTrack(
      track_, media::BindToCurrentLoop(
                  base::Bind(&VideoTrackRecorder::InitializeEncoder,
                             weak_ptr_factory_.GetWeakPtr())));

Which will bounce us back to main thread, 
and allow for using |weak_ptr_factory_| ?


[1]
https://cs.chromium.org/chromium/src/base/memory/weak_ptr.h?sq=package:chromi...

[emircan, 2016/06/13 19:55:40]

As I said in my previous comment, "I initially had a one-shot callback where I
add media::BindToCurrentLoop in
l.953. However, I still need to set |origin_task_runner_| so I can send the
first frame for encode on the right thread, see l.1044."

       false);
 }
 
@@ -977,20 +963,94 @@ VideoTrackRecorder::~VideoTrackRecorder() {
 
 void VideoTrackRecorder::Pause() {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
-  DCHECK(encoder_);
-  encoder_->SetPaused(true);
+  if (encoder_)
+    encoder_->SetPaused(true);
+  else
+    paused_before_init_ = true;
 }
 
 void VideoTrackRecorder::Resume() {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
-  DCHECK(encoder_);
-  encoder_->SetPaused(false);
+  if (encoder_)
+    encoder_->SetPaused(false);
+  else
+    paused_before_init_ = false;
 }
 
 void VideoTrackRecorder::OnVideoFrameForTesting(
     const scoped_refptr<media::VideoFrame>& frame,
     base::TimeTicks timestamp) {
-  encoder_->StartFrameEncode(frame, timestamp);
+  // We run on a single thread for tests. However, we still need to initialize
+  // these variables for checks.
+  if (!origin_task_runner_.get())
+    origin_task_runner_ = base::MessageLoop::current()->task_runner();
+
+  if (encoder_)
+    encoder_->StartFrameEncode(frame, timestamp);
+  else
+    InitializeEncoderTask(frame, timestamp);
+}
+
+void VideoTrackRecorder::InitializeEncoder(
+    const scoped_refptr<media::VideoFrame>& frame,
+    base::TimeTicks capture_time) {
+  if (!origin_task_runner_.get())
+    origin_task_runner_ = base::MessageLoop::current()->task_runner();
+  DCHECK(origin_task_runner_->BelongsToCurrentThread());
+
+  main_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoTrackRecorder::InitializeEncoderTask,
+                 weak_ptr_factory_.GetWeakPtr(), frame, capture_time));

[mcasas, 2016/06/08 18:51:24]

I like the new model of PostTask() here,
then DisconnectFromTrack() - reconnect to
VTR::Encoder::StartFrameEncode at the end 
of the other method.

What if we receive >1 frame here before 
InitializeEncodeTask() is finished? I think we need
a one-shot Callback with InitializeEncoderTask
inside. 

[emircan, 2016/06/09 03:03:43]

I initially had a one-shot callback where I add media::BindToCurrentLoop in
l.953. However, I still need to set |origin_task_runner_| so I can send the
first frame for encode on the right thread, see l.1044.

You are right about this being called >1 times. I will add a async waiter so
that if it is called for the second time, i can directly send it to |encoder_|.

+}
+
+void VideoTrackRecorder::InitializeEncoderTask(
+    const scoped_refptr<media::VideoFrame>& frame,
+    base::TimeTicks capture_time) {
+  DVLOG(3) << __FUNCTION__;
+  DCHECK(main_render_thread_checker_.CalledOnValidThread());
+
+  MediaStreamVideoSink::DisconnectFromTrack();
+
+  const gfx::Size& input_size = frame->visible_rect().size();
+  const auto& vea_supported_profile = CodecIdToVEAProfile(codec_);
+  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";
+    }
+  }
+
+  if (paused_before_init_) {
+    encoder_->SetPaused(paused_before_init_);
+  } else {
+    // Send the first frame for encode on Render IO thread.
+    origin_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode,
+                              encoder_, frame, capture_time));
+  }
+
+  // StartFrameEncode() will be called on Render IO thread.
+  MediaStreamVideoSink::ConnectToTrack(
+      track_,
+      base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode, encoder_),
+      false);
 }
 
 }  // namespace content