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 d65e238e8795e5d07b6ffcba163d6c135fb684c0..d8bf3bf3e21490434106eabc05fb51fd23371ebc 100644
--- a/content/renderer/media/video_track_recorder.cc
+++ b/content/renderer/media/video_track_recorder.cc
@@ -71,13 +71,15 @@ media::VideoCodecProfile CodecIdToVEAProfile(
 #if defined(OS_CHROMEOS)
   return media::VIDEO_CODEC_PROFILE_UNKNOWN;
 #endif  // defined(OS_CHROMEOS)
-  content::RenderThreadImpl* render_thread_impl =
+  content::RenderThreadImpl* const render_thread_impl =
       content::RenderThreadImpl::current();
-  if (!render_thread_impl)
+  if (!render_thread_impl) {
+    DVLOG(3) << "Couldn't access the render thread";
     return media::VIDEO_CODEC_PROFILE_UNKNOWN;
+  }
 
-  media::GpuVideoAcceleratorFactories* gpu_factories =
-      content::RenderThreadImpl::current()->GetGpuFactories();
+  media::GpuVideoAcceleratorFactories* const gpu_factories =
+      render_thread_impl->GetGpuFactories();
   if (!gpu_factories || !gpu_factories->IsGpuVideoAcceleratorEnabled()) {
     DVLOG(3) << "Couldn't initialize GpuVideoAcceleratorFactories";
     return media::VIDEO_CODEC_PROFILE_UNKNOWN;
@@ -936,37 +938,24 @@ 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),
+      is_first_frame_received_for_initialization_(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.
+  // ReceiveFirstFrameOnIOThread() 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::ReceiveFirstFrameOnIOThread,
+                         base::Unretained(this)),
       false);
 }
 
@@ -978,20 +967,100 @@ 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);
+  DVLOG(3) << __FUNCTION__;
+  // 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
+    InitializeEncoderOnMainThread(frame, timestamp);
+}
+
+void VideoTrackRecorder::ReceiveFirstFrameOnIOThread(
+    const scoped_refptr<media::VideoFrame>& frame,
+    base::TimeTicks capture_time) {
+  DVLOG(3) << __FUNCTION__;
+
+  // We use the first frame for initialization and drop the frames coming after
+  // until the new sink is connected to th track.

[mcasas, 2016/06/30 22:55:12]

s/th/the/

Also you could say "...and drop eventual subsequent frames 
until the new sink..."

+  if (is_first_frame_received_for_initialization_)
+    return;
+
+  is_first_frame_received_for_initialization_ = true;
+  origin_task_runner_ = base::MessageLoop::current()->task_runner();
+  main_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoTrackRecorder::InitializeEncoderOnMainThread,
+                 weak_ptr_factory_.GetWeakPtr(), frame, capture_time));
+}
+
+void VideoTrackRecorder::InitializeEncoderOnMainThread(
+    const scoped_refptr<media::VideoFrame>& frame,
+    base::TimeTicks capture_time) {
+  DVLOG(3) << __FUNCTION__ << frame->visible_rect().size().ToString();
+  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 |origin_task_runner_|.
+    origin_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode,
+                              encoder_, frame, capture_time));

[mcasas, 2016/06/30 22:55:12]

I'm not sure we should do this. Rationale:
If this method takes a while, we'd ignore a few
incoming frames in ReceiveFirstFrameOnIOThread()
before |encoder_| is initialized; in that case we could 
end up in a scenario where the first frame is encoded
(because of this line) and the next one being 
encoded comes way later in time --> not good
for encoders since that'd kill temporal redundancy.

I think we should simply drop the first frame, and
let |encoder_| the first available frame after it
does the MSVSink::ConnectToTrack().

And if we follow that rationale, we only need 
frame->visible_rect() in this method. WDYT?

+  }
+
+  // StartFrameEncode() will be called on Render IO thread.
+  MediaStreamVideoSink::ConnectToTrack(
+      track_,
+      base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode, encoder_),
+      false);
 }
 
 }  // namespace content