Initialize based on frame sizes in VideoTrackRecorder

content/renderer/media/video_track_recorder.cc

 // 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.
 
 #include "content/renderer/media/video_track_recorder.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/sys_info.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "content/renderer/media/renderer_gpu_video_accelerator_factories.h"
 #include "content/renderer/render_thread_impl.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
@@ skipping 906 matching lines @@
 }
 #endif //#if BUILDFLAG(RTC_USE_H264)
 
 }  // anonymous namespace
 
 VideoTrackRecorder::VideoTrackRecorder(
     CodecId codec,
     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)),
       false);
 }
 
 VideoTrackRecorder::~VideoTrackRecorder() {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
   MediaStreamVideoSink::DisconnectFromTrack();
   track_.reset();
 }
 
 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, nullptr);
+}
+
+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());
+
+  if (!encoder_) {
+    base::WaitableEvent initialization_waiter(
+        base::WaitableEvent::ResetPolicy::MANUAL,
+        base::WaitableEvent::InitialState::NOT_SIGNALED);
+    main_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&VideoTrackRecorder::InitializeEncoderTask,
+                              weak_ptr_factory_.GetWeakPtr(), frame,
+                              capture_time, &initialization_waiter));
+    initialization_waiter.Wait();
+  } else {
+    encoder_->StartFrameEncode(frame, capture_time);
+  }

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

Assuming you take in my suggestion before 
using media::BindToCurrentLoop (which I strongly
encourage you to), we wouldn't need the PostTask 
in l.1006-1009, but then we'd need a PostTask here 
instead. However, IIUC, by the time we're done 
creating |encoder_| via InitializeEncoderTask() l.1060, 
the Track will already be delivering to 
VideoTrackEncoder::Encoder::StartFrameEncode(),
correct?

Moreover, we should not use |encoder_| from
two threads without a lock either.

So, what I propose is the following pseudo code, 
based on BindToCurrentLoop():
- when the first frame arrives (on main thread), 
 then simply do the InitializeEncoderTask() steps.
- at the end of those steps, we are reconnecting 
 the Track to deliver to VTR::Encoder, which is fine.
- since MessageLoops are not reentrant, if there 
 were still frames in flight they would be queued up 
 in the BindToCurrentLoop posting of tasks, and 
 they'd come up _after_ these initialization steps. 
 In that case, just PostTask those to |encoder_| 
 StartFrameEncode(). 

SG?

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

I agree that it is the right model if we are using BindToCurrentLoop. But still
my only concern is losing that precious first frame for encode. We need to hold
onto |origin_task_runner| so that we can send it for encode on that thread.
Therefore, we still need to post tasks in between. WDYT?

[mcasas, 2016/06/23 00:27:36]

Reluctantly, because is ugly, I admit that this
thread-hopper-monster is the only solution.
However, I think we can remove the else{ ...}
since, efffectively, l.1010 is going to lock the
IO thread frame delivery, so as long as there is
no Signal on the WaitableEvent, PC would not
move from l.1010, and when it does, we would 
have executed already l.1060, so frames would
be going to |encoder_|. We can turn instead this
if() else into a 
DCHECK(!encoder);
// WaitableEvent, PostTask etc
DCHECK(encoder);

[emircan, 2016/06/28 22:10:41]

Done.

+}
+
+void VideoTrackRecorder::InitializeEncoderTask(
+    const scoped_refptr<media::VideoFrame>& frame,
+    base::TimeTicks capture_time,
+    base::WaitableEvent* async_waiter) {
+  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.

[mcasas, 2016/06/23 00:27:36]

nit: s/on Render IO/on |origin_task_runner_|/

+  MediaStreamVideoSink::ConnectToTrack(
+      track_,
+      base::Bind(&VideoTrackRecorder::Encoder::StartFrameEncode, encoder_),
+      false);
+
+  if (async_waiter)
+    async_waiter->Signal();
 }
 
 }  // namespace content