Move passing of WebRTC rendering frames from main thread to compositor thread

content/renderer/media/webmediaplayer_ms.cc

 // Copyright 2013 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/webmediaplayer_ms.h"
 
 #include <stddef.h>
 #include <limits>
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "cc/blink/web_layer_impl.h"
 #include "cc/layers/video_frame_provider_client_impl.h"
 #include "cc/layers/video_layer.h"
 #include "content/common/gpu/client/context_provider_command_buffer.h"
 #include "content/public/renderer/media_stream_audio_renderer.h"
 #include "content/public/renderer/media_stream_renderer_factory.h"
 #include "content/public/renderer/media_stream_video_renderer.h"
 #include "content/renderer/media/web_media_element_source_utils.h"
 #include "content/renderer/media/webmediaplayer_ms_compositor.h"
 #include "content/renderer/render_frame_impl.h"
 #include "content/renderer/render_thread_impl.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/media_content_type.h"
 #include "media/base/media_log.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_rotation.h"
 #include "media/base/video_types.h"
 #include "media/blink/webmediaplayer_util.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayerClient.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayerSource.h"
 #include "third_party/WebKit/public/platform/WebRect.h"
 #include "third_party/WebKit/public/platform/WebSize.h"
 
 namespace content {
 
+// FrameDelivererOnCompositor is responsible for delivering frames received on
+// compositor thread by calling of EnqueueFrame() method of |compositor_|.
+//
+// It is created on main thread, but methods should be called and class should
+// be destructed on compositor thread.
+class WebMediaPlayerMS::FrameDelivererOnCompositor {
+ public:
+  FrameDelivererOnCompositor(
+      const base::WeakPtr<WebMediaPlayerMS>& player,
+      const base::WeakPtr<WebMediaPlayerMSCompositor>& compositor)
+      : last_frame_opaque_(true),
+        received_first_frame_(false),
+        main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+        player_(player),
+        compositor_(compositor),
+        weak_factory_for_compositor_(this) {
+    thread_checker_.DetachFromThread();
+  }
+  ~FrameDelivererOnCompositor() {
+    weak_factory_for_compositor_.InvalidateWeakPtrs();
+    DCHECK(thread_checker_.CalledOnValidThread());
+  }
+
+  void OnVideoFrame(const scoped_refptr<media::VideoFrame>& frame) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+#if defined(OS_ANDROID)
+    {
+      base::AutoLock auto_lock(render_frame_suspended_lock_);
+      if (render_frame_suspended_)
+        return;
+    }
+#endif  // defined(OS_ANDROID)
+
+    base::TimeTicks render_time;
+    if (frame->metadata()->GetTimeTicks(
+            media::VideoFrameMetadata::REFERENCE_TIME, &render_time)) {
+      TRACE_EVENT1("webrtc", "WebMediaPlayerMS::OnFrameAvailable",
+                   "Ideal Render Instant", render_time.ToInternalValue());
+    } else {
+      TRACE_EVENT0("webrtc", "WebMediaPlayerMS::OnFrameAvailable");
+    }
+    const bool is_opaque = media::IsOpaque(frame->format());
+
+    if (!received_first_frame_) {
+      received_first_frame_ = true;
+      last_frame_opaque_ = is_opaque;
+      media::VideoRotation video_rotation;
+      ignore_result(frame->metadata()->GetRotation(
+          media::VideoFrameMetadata::ROTATION, &video_rotation));
+      main_task_runner_->PostTask(

[qiangchen, 2016/11/09 19:44:39]

I am worrying that this might be an issue. Theoretically, it is possible that
other compositor thread tasks run before OnFirstFrameReceived got called. Could
there be an issue? Can you double checkthat?

[emircan, 2016/11/11 22:27:19]

Yes, compositor tasks may run on before OnFirstFrameReceived() is completed.
Unfortunately, we do not know rotation/opacity information until first frame is
received, so we do not have enough info for initialization. 

I can add an AsyncWaiter to make sure OnFirstFrameReceived() completes before
moving on, but again we would keep waiting when main thread is busy. 

WebMediaPlayerImpl does this based on metadata[0][1] but again posts a task on
main. I am not sure about the synchronization there. dalecurtis@ might know more
about it.

[0]
https://cs.chromium.org/chromium/src/media/base/pipeline_impl.cc?rcl=14788666...
[1]
https://cs.chromium.org/chromium/src/media/blink/webmediaplayer_impl.cc?rcl=1...

[qiangchen, 2016/11/15 00:04:54]

Never mind. I did some research, it is the "setWebLayer" call in the
|OnFirstFrameReceived| that starts calls to WebMediaPlayerMSCompositor on
compositor thread. So I think it is fine here.

+          FROM_HERE, base::Bind(&WebMediaPlayerMS::OnFirstFrameReceived,
+                                player_, video_rotation, is_opaque));
+    }
+
+    if (last_frame_opaque_ != is_opaque) {
+      last_frame_opaque_ = is_opaque;
+      main_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&WebMediaPlayerMS::OnOpacityChanged, player_, is_opaque));
+    }
+
+    if (compositor_.get())
+      compositor_->EnqueueFrame(frame);
+  }
+
+#if defined(OS_ANDROID)
+  void SetRenderFrameSuspended(bool render_frame_suspended) {
+    base::AutoLock auto_lock(render_frame_suspended_lock_);
+    render_frame_suspended_ = render_frame_suspended;
+  }
+#endif  // defined(OS_ANDROID)
+
+  base::WeakPtr<FrameDelivererOnCompositor> GetWeakPtr() {
+    return weak_factory_for_compositor_.GetWeakPtr();
+  }
+
+ private:
+  bool last_frame_opaque_;
+  bool received_first_frame_;
+
+#if defined(OS_ANDROID)
+  bool render_frame_suspended_ = false;
+  base::Lock render_frame_suspended_lock_;
+#endif  // defined(OS_ANDROID)
+
+  const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+  const base::WeakPtr<WebMediaPlayerMS> player_;
+  const base::WeakPtr<WebMediaPlayerMSCompositor> compositor_;
+
+  // Used for DCHECKs to ensure method calls executed in the correct thread,

[perkj_chrome, 2016/11/09 15:27:13]

/s on the

[emircan, 2016/11/11 22:27:19]

Done.

+  // i.e. compositor thread.
+  base::ThreadChecker thread_checker_;

[qiangchen, 2016/11/09 19:44:38]

nit: Can you rename it to compositor_thread_checker_? As it would make the code
readability better.

[emircan, 2016/11/11 22:27:19]

Done.

[qiangchen, 2016/11/15 00:04:54]

Acknowledged.

+
+  base::WeakPtrFactory<FrameDelivererOnCompositor> weak_factory_for_compositor_;
+
+  DISALLOW_COPY_AND_ASSIGN(FrameDelivererOnCompositor);
+};
+
 WebMediaPlayerMS::WebMediaPlayerMS(
     blink::WebFrame* frame,
     blink::WebMediaPlayerClient* client,
     base::WeakPtr<media::WebMediaPlayerDelegate> delegate,
     media::MediaLog* media_log,
     std::unique_ptr<MediaStreamRendererFactory> factory,
     const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
     const scoped_refptr<base::TaskRunner>& worker_task_runner,
     media::GpuVideoAcceleratorFactories* gpu_factories,
     const blink::WebString& sink_id,
     const blink::WebSecurityOrigin& security_origin)
     : frame_(frame),
       network_state_(WebMediaPlayer::NetworkStateEmpty),
       ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       buffered_(static_cast<size_t>(0)),
       client_(client),
       delegate_(delegate),
       delegate_id_(0),
-      last_frame_opaque_(true),
       paused_(true),
-      render_frame_suspended_(false),
-      received_first_frame_(false),
       video_rotation_(media::VIDEO_ROTATION_0),
       media_log_(media_log),
       renderer_factory_(std::move(factory)),
       media_task_runner_(media_task_runner),
       worker_task_runner_(worker_task_runner),
       gpu_factories_(gpu_factories),
       compositor_task_runner_(compositor_task_runner),
       initial_audio_output_device_id_(sink_id.utf8()),
       initial_security_origin_(security_origin.isNull()
                                    ? url::Origin()
@@ skipping 11 matching lines @@
 }
 
 WebMediaPlayerMS::~WebMediaPlayerMS() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Destruct compositor resources in the proper order.
   get_client()->setWebLayer(nullptr);
   if (video_weblayer_)
     static_cast<cc::VideoLayer*>(video_weblayer_->layer())->StopUsingProvider();
+
+  if (frame_deliverer_)
+    compositor_task_runner_->DeleteSoon(FROM_HERE, frame_deliverer_.release());
+
   if (compositor_)
     compositor_task_runner_->DeleteSoon(FROM_HERE, compositor_.release());
 
   if (video_frame_provider_)
     video_frame_provider_->Stop();
 
   if (audio_renderer_)
     audio_renderer_->Stop();
 
   media_log_->AddEvent(
@@ skipping 19 matching lines @@
 
   compositor_.reset(new WebMediaPlayerMSCompositor(compositor_task_runner_,
                                                    web_stream, AsWeakPtr()));
 
   SetNetworkState(WebMediaPlayer::NetworkStateLoading);
   SetReadyState(WebMediaPlayer::ReadyStateHaveNothing);
   std::string stream_id =
       web_stream.isNull() ? std::string() : web_stream.id().utf8();
   media_log_->AddEvent(media_log_->CreateLoadEvent(stream_id));
 
+  frame_deliverer_.reset(new WebMediaPlayerMS::FrameDelivererOnCompositor(
+      AsWeakPtr(), compositor_->GetWeakPtr()));
   video_frame_provider_ = renderer_factory_->GetVideoRenderer(
-      web_stream, base::Bind(&WebMediaPlayerMS::OnSourceError, AsWeakPtr()),
-      base::Bind(&WebMediaPlayerMS::OnFrameAvailable, AsWeakPtr()),
-      media_task_runner_, worker_task_runner_, gpu_factories_);
+      web_stream, media::BindToCurrentLoop(base::Bind(
+                      &WebMediaPlayerMS::OnSourceError, AsWeakPtr())),
+      base::Bind(&FrameDelivererOnCompositor::OnVideoFrame,
+                 frame_deliverer_->GetWeakPtr()),
+      compositor_task_runner_, media_task_runner_, worker_task_runner_,
+      gpu_factories_);
 
   RenderFrame* const frame = RenderFrame::FromWebFrame(frame_);
 
   if (frame) {
     // Report UMA and RAPPOR metrics.
     GURL url = source.isURL() ? GURL(source.getAsURL()) : GURL();
     media::ReportMetrics(load_type, url, frame_->getSecurityOrigin());
 
     audio_renderer_ = renderer_factory_->GetAudioRenderer(
         web_stream, frame->GetRoutingID(), initial_audio_output_device_id_,
@@ skipping 249 matching lines @@
 void WebMediaPlayerMS::OnHidden() {
 #if defined(OS_ANDROID)
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Method called when the RenderFrame is sent to background and suspended
   // (android). Substitute the displayed VideoFrame with a copy to avoid
   // holding on to it unnecessarily.
   //
   // During undoable tab closures OnHidden() may be called back to back, so we
   // can't rely on |render_frame_suspended_| being false here.
+  if (frame_deliverer_)
+    frame_deliverer_->SetRenderFrameSuspended(true);
 
-  render_frame_suspended_ = true;
   if (!paused_)
     compositor_->ReplaceCurrentFrameWithACopy();
 #endif  // defined(OS_ANDROID)
 }
 
 void WebMediaPlayerMS::OnShown() {
 #if defined(OS_ANDROID)
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  render_frame_suspended_ = false;
+  frame_deliverer_->SetRenderFrameSuspended(false);
 
   // Resume playback on visibility. play() clears |should_play_upon_shown_|.
   if (should_play_upon_shown_)
     play();
 #endif  // defined(OS_ANDROID)
 }
 
 bool WebMediaPlayerMS::OnSuspendRequested(bool must_suspend) {
 #if defined(OS_ANDROID)
   if (!must_suspend)
     return false;
 
   if (!paused_) {
     pause();
     should_play_upon_shown_ = true;
   }
 
   if (delegate_)
     delegate_->PlayerGone(delegate_id_);
 
-  render_frame_suspended_ = true;
-#endif
+  frame_deliverer_->SetRenderFrameSuspended(true);
+#endif  // defined(OS_ANDROID)
   return true;
 }
 
 void WebMediaPlayerMS::OnPlay() {
   // TODO(perkj, magjed): It's not clear how WebRTC should work with an
   // MediaSession, until these issues are resolved, disable session controls.
   // http://crbug.com/595297.
 }
 
 void WebMediaPlayerMS::OnPause() {
@@ skipping 26 matching lines @@
   // GPU Process crashed.
   if (!provider)
     return false;
   context_3d = media::Context3D(provider->ContextGL(), provider->GrContext());
   DCHECK(context_3d.gl);
   return video_renderer_.CopyVideoFrameTexturesToGLTexture(
       context_3d, gl, video_frame.get(), texture, internal_format, type,
       premultiply_alpha, flip_y);
 }
 
-void WebMediaPlayerMS::OnFrameAvailable(
-    const scoped_refptr<media::VideoFrame>& frame) {
-  DVLOG(3) << __func__;
+void WebMediaPlayerMS::OnFirstFrameReceived(media::VideoRotation video_rotation,
+                                            bool is_opaque) {
+  DVLOG(1) << __func__;
+  DCHECK(thread_checker_.CalledOnValidThread());
+  video_rotation_ = video_rotation;

[qiangchen, 2016/11/09 23:23:48]

Is it possible to eliminate video_rotation_? I found it is just used once below
within this function.

[emircan, 2016/11/11 22:27:19]

It is also used in WebMediaPlayerMS::naturalSize() and
WebMediaPlayerMS::paint().

[qiangchen, 2016/11/15 00:04:54]

Acknowledged.

+  SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
+  SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+
+  video_weblayer_.reset(new cc_blink::WebLayerImpl(
+      cc::VideoLayer::Create(compositor_.get(), video_rotation_)));
+  video_weblayer_->layer()->SetContentsOpaque(is_opaque);
+  video_weblayer_->SetContentsOpaqueIsFixed(true);
+  get_client()->setWebLayer(video_weblayer_.get());
+}
+
+void WebMediaPlayerMS::OnOpacityChanged(bool is_opaque) {
+  DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (render_frame_suspended_)
-    return;
-
-  base::TimeTicks render_time;
-  if (frame->metadata()->GetTimeTicks(media::VideoFrameMetadata::REFERENCE_TIME,
-                                      &render_time)) {
-    TRACE_EVENT1("webrtc", "WebMediaPlayerMS::OnFrameAvailable",
-                 "Ideal Render Instant", render_time.ToInternalValue());
-  } else {
-    TRACE_EVENT0("webrtc", "WebMediaPlayerMS::OnFrameAvailable");
-  }
-  const bool is_opaque = media::IsOpaque(frame->format());
-
-  if (!received_first_frame_) {
-    received_first_frame_ = true;
-    last_frame_opaque_ = is_opaque;
-    SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
-    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
-
-    if (video_frame_provider_.get()) {
-      ignore_result(frame->metadata()->GetRotation(
-          media::VideoFrameMetadata::ROTATION, &video_rotation_));
-
-      video_weblayer_.reset(new cc_blink::WebLayerImpl(
-          cc::VideoLayer::Create(compositor_.get(), video_rotation_)));
-      video_weblayer_->layer()->SetContentsOpaque(is_opaque);
-      video_weblayer_->SetContentsOpaqueIsFixed(true);
-      get_client()->setWebLayer(video_weblayer_.get());
-    }
-  }
-
-  // Only configure opacity on changes, since marking it as transparent is
-  // expensive, see https://crbug.com/647886.
-  if (video_weblayer_ && last_frame_opaque_ != is_opaque) {
-    last_frame_opaque_ = is_opaque;
-    video_weblayer_->layer()->SetContentsOpaque(is_opaque);
-  }
-
-  compositor_->EnqueueFrame(frame);
+  video_weblayer_->layer()->SetContentsOpaque(is_opaque);
 }
 
 void WebMediaPlayerMS::RepaintInternal() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
   get_client()->repaint();
 }
 
 void WebMediaPlayerMS::OnSourceError() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 22 matching lines @@
 void WebMediaPlayerMS::ResetCanvasCache() {
   DCHECK(thread_checker_.CalledOnValidThread());
   video_renderer_.ResetCache();
 }
 
 void WebMediaPlayerMS::TriggerResize() {
   get_client()->sizeChanged();
 }
 
 }  // namespace content