media: Support default Renderer in PipelineImpl

Currently PipelineImpl always uses the CreateRendererCB to create
the Renderer asynchronously, which involves a post to the render main
thread and another post back to the media thread. This has caused
regressions in start-to-play-time especially on lower end devices.

This CL partially restores the previous behavior by creating a default
Renderer on the main thread before pipeline start. The default Renderer
will be used in normal cases so we avoid the round trip to the main
thread to create the Renderer. In special cases where we need a
different Renderer, the CreateRendererCB will be used to create one.

Bug: 1018372,1023579
Test: Tests updated
Change-Id: I71627a959cdfe12cd0f5ef0ee358f461323f5a7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1915226
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715550}

media/base/pipeline_impl.cc

@@ -44,22 +44,26 @@ gfx::Size GetRotatedVideoSize(VideoRotation rotation, gfx::Size natural_size) {
 
 }  // namespace
 
+// A wrapper of Renderer that runs on the |media_task_runner|.
+// |default_renderer| in Start() and Resume() helps avoid a round trip to the
+// render main task runner for Renderer creation in most cases which could add
+// latency to start-to-play time.
 class PipelineImpl::RendererWrapper : public DemuxerHost,
                                       public RendererClient {
  public:
   RendererWrapper(scoped_refptr<base::SingleThreadTaskRunner> media_task_runner,
                   scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-                  CreateRendererCB create_renderer_cb,
                   MediaLog* media_log);
   ~RendererWrapper() final;
 
   void Start(StartType start_type,
              Demuxer* demuxer,
+             std::unique_ptr<Renderer> default_renderer,
              base::WeakPtr<PipelineImpl> weak_pipeline);
   void Stop();
   void Seek(base::TimeDelta time);
   void Suspend();
-  void Resume(base::TimeDelta time);
+  void Resume(std::unique_ptr<Renderer> default_renderer, base::TimeDelta time);
   void SetPlaybackRate(double playback_rate);
   void SetVolume(float volume);
   base::TimeDelta GetMediaTime() const;
@@ -163,11 +167,17 @@ class PipelineImpl::RendererWrapper : public DemuxerHost,
 
   const scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
   const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
-  CreateRendererCB create_renderer_cb_;
   MediaLog* const media_log_;
 
+  // A weak pointer to PipelineImpl. Must only use on the main task runner.
   base::WeakPtr<PipelineImpl> weak_pipeline_;
+
   Demuxer* demuxer_;
+
+  // Default renderer to be used. If a different Renderer is needed,
+  // PipelineImpl::AsyncCreateRenderer() will be called to create a new one.
+  std::unique_ptr<Renderer> default_renderer_;
+
   double playback_rate_;
   float volume_;
   CdmContext* cdm_context_;
@@ -209,11 +219,9 @@ class PipelineImpl::RendererWrapper : public DemuxerHost,
 PipelineImpl::RendererWrapper::RendererWrapper(
     scoped_refptr<base::SingleThreadTaskRunner> media_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-    CreateRendererCB create_renderer_cb,
     MediaLog* media_log)
     : media_task_runner_(std::move(media_task_runner)),
       main_task_runner_(std::move(main_task_runner)),
-      create_renderer_cb_(create_renderer_cb),
       media_log_(media_log),
       demuxer_(nullptr),
       playback_rate_(kDefaultPlaybackRate),
@@ -238,6 +246,7 @@ PipelineImpl::RendererWrapper::~RendererWrapper() {
 void PipelineImpl::RendererWrapper::Start(
     StartType start_type,
     Demuxer* demuxer,
+    std::unique_ptr<Renderer> default_renderer,
     base::WeakPtr<PipelineImpl> weak_pipeline) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   DCHECK(state_ == kCreated || state_ == kStopped)
@@ -248,6 +257,7 @@ void PipelineImpl::RendererWrapper::Start(
 
   SetState(kStarting);
   demuxer_ = demuxer;
+  default_renderer_ = std::move(default_renderer);
   weak_pipeline_ = weak_pipeline;
 
   // Setup |error_cb_| on the media thread.
@@ -388,7 +398,9 @@ void PipelineImpl::RendererWrapper::Suspend() {
                                           weak_factory_.GetWeakPtr()));
 }
 
-void PipelineImpl::RendererWrapper::Resume(base::TimeDelta timestamp) {
+void PipelineImpl::RendererWrapper::Resume(
+    std::unique_ptr<Renderer> default_renderer,
+    base::TimeDelta timestamp) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   // Suppress resuming if we're not suspended.
@@ -407,6 +419,7 @@ void PipelineImpl::RendererWrapper::Resume(base::TimeDelta timestamp) {
     DCHECK(!shared_state_.renderer);
   }
 
+  default_renderer_ = std::move(default_renderer);
   renderer_ended_ = false;
   text_renderer_ended_ = false;
   base::TimeDelta start_timestamp =
@@ -491,11 +504,11 @@ void PipelineImpl::RendererWrapper::SetCdm(CdmContext* cdm_context,
     if (create_renderer_done_cb_) {
       DCHECK(state_ == kStarting || state_ == kResuming);
       DVLOG(1) << __func__ << ": CDM set; continue pipeline start/resume.";
-      // Use BindToCurrentLoop to make sure OnRendererCreated() is called on the
-      // media task runner.
-      create_renderer_cb_.Run(BindToCurrentLoop(base::BindOnce(
-          &RendererWrapper::OnRendererCreated, weak_factory_.GetWeakPtr(),
-          std::move(create_renderer_done_cb_))));
+      // TODO(crbug.com/1018372): Use PipelineImpl::AsyncCreateRenderer() to
+      // create a new Renderer when needed, e.g. the CDM requires a different
+      // Renderer.
+      OnRendererCreated(std::move(create_renderer_done_cb_),
+                        std::move(default_renderer_));
     }
 
     return;
@@ -916,11 +929,9 @@ void PipelineImpl::RendererWrapper::CreateRenderer(
     return;
   }
 
-  // Use BindToCurrentLoop to make sure OnRendererCreated() is called on the
-  // media task runner.
-  create_renderer_cb_.Run(BindToCurrentLoop(
-      base::BindOnce(&RendererWrapper::OnRendererCreated,
-                     weak_factory_.GetWeakPtr(), std::move(done_cb))));
+  // TODO(crbug.com/1018372): Use PipelineImpl::AsyncCreateRenderer() to create
+  // a new Renderer when needed, e.g. the CDM requires a different Renderer.
+  OnRendererCreated(std::move(done_cb), std::move(default_renderer_));
 }
 
 void PipelineImpl::RendererWrapper::OnRendererCreated(
@@ -1063,15 +1074,17 @@ PipelineImpl::PipelineImpl(
     CreateRendererCB create_renderer_cb,
     MediaLog* media_log)
     : media_task_runner_(media_task_runner),
+      create_renderer_cb_(create_renderer_cb),
       media_log_(media_log),
       client_(nullptr),
       playback_rate_(kDefaultPlaybackRate),
       volume_(kDefaultVolume),
       is_suspended_(false) {
   DVLOG(2) << __func__;
-  renderer_wrapper_.reset(
-      new RendererWrapper(media_task_runner_, std::move(main_task_runner),
-                          std::move(create_renderer_cb), media_log_));
+  DCHECK(create_renderer_cb_);
+
+  renderer_wrapper_.reset(new RendererWrapper(
+      media_task_runner_, std::move(main_task_runner), media_log_));
 }
 
 PipelineImpl::~PipelineImpl() {
@@ -1080,7 +1093,8 @@ PipelineImpl::~PipelineImpl() {
   DCHECK(!client_) << "Stop() must complete before destroying object";
   DCHECK(!seek_cb_);
   DCHECK(!suspend_cb_);
-  DCHECK(!weak_factory_.HasWeakPtrs());
+  DCHECK(!weak_factory_.HasWeakPtrs())
+      << "Stop() should have invalidated all weak pointers";
 
   // RendererWrapper is deleted on the media thread.
   media_task_runner_->DeleteSoon(FROM_HERE, renderer_wrapper_.release());
@@ -1104,10 +1118,10 @@ void PipelineImpl::Start(StartType start_type,
   seek_time_ = kNoTimestamp;
 
   media_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&RendererWrapper::Start,
-                     base::Unretained(renderer_wrapper_.get()), start_type,
-                     demuxer, weak_factory_.GetWeakPtr()));
+      FROM_HERE, base::BindOnce(&RendererWrapper::Start,
+                                base::Unretained(renderer_wrapper_.get()),
+                                start_type, demuxer, create_renderer_cb_.Run(),
+                                weak_factory_.GetWeakPtr()));
 }
 
 void PipelineImpl::Stop() {
@@ -1188,9 +1202,9 @@ void PipelineImpl::Resume(base::TimeDelta time,
   last_media_time_ = base::TimeDelta();
 
   media_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&RendererWrapper::Resume,
-                     base::Unretained(renderer_wrapper_.get()), time));
+      FROM_HERE, base::BindOnce(&RendererWrapper::Resume,
+                                base::Unretained(renderer_wrapper_.get()),
+                                create_renderer_cb_.Run(), time));
 }
 
 bool PipelineImpl::IsRunning() const {
@@ -1339,6 +1353,13 @@ const char* PipelineImpl::GetStateString(State state) {
 
 #undef RETURN_STRING
 
+void PipelineImpl::AsyncCreateRenderer(RendererCreatedCB renderer_created_cb) {
+  DVLOG(2) << __func__;
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  std::move(renderer_created_cb).Run(create_renderer_cb_.Run());
+}
+
 void PipelineImpl::OnError(PipelineStatus error) {
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());

media/base/pipeline_impl.h

@@ -24,8 +24,9 @@ namespace media {
 class MediaLog;
 
 // Callbacks used for Renderer creation.
+using CreateRendererCB = base::RepeatingCallback<std::unique_ptr<Renderer>()>;
 using RendererCreatedCB = base::OnceCallback<void(std::unique_ptr<Renderer>)>;
-using CreateRendererCB = base::RepeatingCallback<void(RendererCreatedCB)>;
+using AsyncCreateRendererCB = base::RepeatingCallback<void(RendererCreatedCB)>;
 
 // Pipeline runs the media pipeline.  Filters are created and called on the
 // task runner injected into this object. Pipeline works like a state
@@ -73,8 +74,7 @@ using CreateRendererCB = base::RepeatingCallback<void(RendererCreatedCB)>;
 // Some annoying differences between the two paths need to be removed first.
 class MEDIA_EXPORT PipelineImpl : public Pipeline {
  public:
-  // Constructs a media pipeline that will execute media tasks on
-  // |media_task_runner|.
+  // Constructs a pipeline that will execute media tasks on |media_task_runner|.
   // |create_renderer_cb|: to create renderers when starting and resuming.
   PipelineImpl(scoped_refptr<base::SingleThreadTaskRunner> media_task_runner,
                scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
@@ -135,6 +135,10 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline {
   };
   static const char* GetStateString(State state);
 
+  // Create a Renderer asynchronously. Must be called on the main task runner
+  // and the callback will be called on the main task runner as well.
+  void AsyncCreateRenderer(RendererCreatedCB renderer_created_cb);
+
   // Notifications from RendererWrapper.
   void OnError(PipelineStatus error);
   void OnEnded();
@@ -158,6 +162,7 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline {
 
   // Parameters passed in the constructor.
   const scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
+  CreateRendererCB create_renderer_cb_;
   MediaLog* const media_log_;
 
   // Pipeline client. Valid only while the pipeline is running.

media/base/pipeline_impl_unittest.cc

@@ -292,8 +292,8 @@ class PipelineImplTest : public ::testing::Test {
     ResetRenderer();
   }
 
-  void CreateRenderer(RendererCreatedCB renderer_created_cb) {
-    std::move(renderer_created_cb).Run(std::move(scoped_renderer_));
+  std::unique_ptr<Renderer> CreateRenderer() {
+    return std::move(scoped_renderer_);
   }
 
   void ResetRenderer() {

media/blink/webmediaplayer_impl.cc

@@ -334,12 +334,17 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
 
   weak_this_ = weak_factory_.GetWeakPtr();
 
+  // Using base::Unretained(this) is safe because the |pipeline| is owned by
+  // |this| and the callback will always be made on the main task runner.
+  // Not using BindToCurrentLoop() because CreateRenderer() is a sync call.
+  auto pipeline = std::make_unique<PipelineImpl>(
+      media_task_runner_, main_task_runner_,
+      base::BindRepeating(&WebMediaPlayerImpl::CreateRenderer,
+                          base::Unretained(this)),
+      media_log_.get());
+
   pipeline_controller_ = std::make_unique<PipelineController>(
-      std::make_unique<PipelineImpl>(
-          media_task_runner_, main_task_runner_,
-          BindToCurrentLoop(base::BindRepeating(
-              &WebMediaPlayerImpl::CreateRenderer, weak_this_)),
-          media_log_.get()),
+      std::move(pipeline),
       base::BindRepeating(&WebMediaPlayerImpl::OnPipelineSeeked, weak_this_),
       base::BindRepeating(&WebMediaPlayerImpl::OnPipelineSuspended, weak_this_),
       base::BindRepeating(&WebMediaPlayerImpl::OnBeforePipelineResume,
@@ -2600,7 +2605,7 @@ void WebMediaPlayerImpl::MaybeSendOverlayInfoToDecoder() {
   }
 }
 
-void WebMediaPlayerImpl::CreateRenderer(RendererCreatedCB renderer_created_cb) {
+std::unique_ptr<Renderer> WebMediaPlayerImpl::CreateRenderer() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   // Make sure that overlays are enabled if they're always allowed.
@@ -2614,13 +2619,9 @@ void WebMediaPlayerImpl::CreateRenderer(RendererCreatedCB renderer_created_cb) {
 #endif
   reported_renderer_type_ = renderer_factory_selector_->GetCurrentFactoryType();
 
-  auto renderer =
-      renderer_factory_selector_->GetCurrentFactory()->CreateRenderer(
-          media_task_runner_, worker_task_runner_, audio_source_provider_.get(),
-          compositor_.get(), request_overlay_info_cb,
-          client_->TargetColorSpace());
-
-  std::move(renderer_created_cb).Run(std::move(renderer));
+  return renderer_factory_selector_->GetCurrentFactory()->CreateRenderer(
+      media_task_runner_, worker_task_runner_, audio_source_provider_.get(),
+      compositor_.get(), request_overlay_info_cb, client_->TargetColorSpace());
 }
 
 void WebMediaPlayerImpl::StartPipeline() {

media/blink/webmediaplayer_impl.h

@@ -377,7 +377,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
       const ProvideOverlayInfoCB& provide_overlay_info_cb);
 
   // Creates a Renderer via the |renderer_factory_selector_|.
-  void CreateRenderer(RendererCreatedCB renderer_created_cb);
+  std::unique_ptr<Renderer> CreateRenderer();
 
   // Finishes starting the pipeline due to a call to load().
   void StartPipeline();

media/test/pipeline_integration_test_base.cc

@@ -146,7 +146,7 @@ PipelineIntegrationTestBase::PipelineIntegrationTestBase()
   pipeline_ = std::make_unique<PipelineImpl>(
       task_environment_.GetMainThreadTaskRunner(),
       task_environment_.GetMainThreadTaskRunner(),
-      base::BindRepeating(&PipelineIntegrationTestBase::CreateRendererAsync,
+      base::BindRepeating(&PipelineIntegrationTestBase::CreateRenderer,
                           base::Unretained(this)),
       &media_log_);
 
@@ -535,11 +535,6 @@ std::unique_ptr<Renderer> PipelineIntegrationTestBase::CreateRenderer() {
   return std::move(renderer_impl);
 }
 
-void PipelineIntegrationTestBase::CreateRendererAsync(
-    RendererCreatedCB renderer_created_cb) {
-  std::move(renderer_created_cb).Run(CreateRenderer());
-}
-
 void PipelineIntegrationTestBase::OnVideoFramePaint(
     scoped_refptr<VideoFrame> frame) {
   last_video_frame_format_ = frame->format();