Use GpuMemoryBufferVideoFramePool for WebMediaPlayerMS and MediaStreamVideoRendererSink

content/renderer/media/media_stream_video_renderer_sink.cc

 // Copyright (c) 2012 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/media_stream_video_renderer_sink.h"
 
 #include "base/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
 
 const int kMinFrameSize = 2;
 
 namespace content {
 
 MediaStreamVideoRendererSink::MediaStreamVideoRendererSink(
     const blink::WebMediaStreamTrack& video_track,
     const base::Closure& error_cb,
-    const RepaintCB& repaint_cb)
+    const RepaintCB& repaint_cb,
+    const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
+    const scoped_refptr<base::TaskRunner>& worker_task_runner,
+    const scoped_refptr<media::GpuVideoAcceleratorFactories>& gpu_factories)
     : error_cb_(error_cb),
       repaint_cb_(repaint_cb),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       state_(STOPPED),
       frame_size_(kMinFrameSize, kMinFrameSize),
       video_track_(video_track),
+      media_task_runner_(media_task_runner),
       weak_factory_(this) {
+  DCHECK(gpu_factories);
+  if (gpu_factories &&

[mcasas, 2015/10/02 19:30:46]

For debugging is fine, but DCHECK(bla) + if(bla) is
discouraged [1]

[1]
https://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREAC...

[emircan, 2015/10/03 00:11:26]

Ok. Removing DCHECK.

+      gpu_factories->ShouldUseGpuMemoryBuffersForVideoFrames()) {
+    gpu_memory_buffer_pool_.reset(new media::GpuMemoryBufferVideoFramePool(
+        media_task_runner, worker_task_runner, gpu_factories));
+  }
 }
 
 MediaStreamVideoRendererSink::~MediaStreamVideoRendererSink() {
 }
 
 void MediaStreamVideoRendererSink::Start() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STOPPED);
 
   AddToVideoTrack(
@@ skipping 41 matching lines @@
     RenderSignalingFrame();
 }
 
 void MediaStreamVideoRendererSink::OnVideoFrame(
     const scoped_refptr<media::VideoFrame>& frame,
     base::TimeTicks estimated_capture_time) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   if (state_ != STARTED)
     return;
 
+  DCHECK(frame);

[mcasas, 2015/10/02 19:30:46]

nit: Preconditions in the beginning of the method.

[emircan, 2015/10/03 00:11:26]

Done.

+  if (gpu_memory_buffer_pool_) {

[mcasas, 2015/10/02 19:30:46]

  if (!gpu_memory_buffer_pool_) {
    FrameReady(frame);
    return;
  }
  // then bla...

[emircan, 2015/10/03 00:11:26]

Done.

+    media_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&MediaStreamVideoRendererSink::
+                                  MaybeCreateHardwareFrameOnMediaThread,
+                              this, frame));

[mcasas, 2015/10/02 19:30:46]

I think you can remove the two trampoline methods using some
PostTaskAndReply and BindToCurrentLoop bricolage :) :)

  media_task_runner_->PostTaskAndReply(
      FROM_HERE,
      base::Bind(&GpuMemoryBufferVideoFramePool::MaybeCreateHardwareFrame,
                 base::Unretained(gpu_memory_buffer_pool_), frame),
      media::BindToCurrentLoop(&MediaStreamVideoRendererSink::FrameReady, this,
                               frame));

WDYT?

[emircan, 2015/10/03 00:11:26]

Done.

[mcasas, 2015/10/05 17:49:45]

How so? I still see the two methods.

+  } else {
+    FrameReady(frame);
+  }
+}
+
+void MediaStreamVideoRendererSink::MaybeCreateHardwareFrameOnMediaThread(
+    const scoped_refptr<media::VideoFrame>& frame) {
+  DCHECK(media_task_runner_->BelongsToCurrentThread());
+  gpu_memory_buffer_pool_->MaybeCreateHardwareFrame(
+      frame, base::Bind(&MediaStreamVideoRendererSink::
+                            MaybeCreateHardwareFrameCompletedOnMediaThread,
+                        this));
+}
+
+void MediaStreamVideoRendererSink::
+    MaybeCreateHardwareFrameCompletedOnMediaThread(
+        const scoped_refptr<media::VideoFrame>& frame) {
+  DCHECK(media_task_runner_->BelongsToCurrentThread());
+  DCHECK(frame);
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&MediaStreamVideoRendererSink::FrameReady, this, frame));
+}
+
+void MediaStreamVideoRendererSink::FrameReady(
+    const scoped_refptr<media::VideoFrame>& frame) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
   frame_size_ = frame->natural_size();
-
-  TRACE_EVENT_INSTANT1("media_stream_video_renderer_sink",
-                       "OnVideoFrame",
-                       TRACE_EVENT_SCOPE_THREAD,
-                       "timestamp",
+  TRACE_EVENT_INSTANT1("media_stream_video_renderer_sink", "FrameReady",
+                       TRACE_EVENT_SCOPE_THREAD, "timestamp",
                        frame->timestamp().InMilliseconds());
   repaint_cb_.Run(frame);
 }
 
 void MediaStreamVideoRendererSink::RenderSignalingFrame() {
   // This is necessary to make sure audio can play if the video tag src is
   // a MediaStream video track that has been rejected or ended.
   // It also ensure that the renderer don't hold a reference to a real video
   // frame if no more frames are provided. This is since there might be a
   // finite number of available buffers. E.g, video that
   // originates from a video camera.
   scoped_refptr<media::VideoFrame> video_frame =
       media::VideoFrame::CreateBlackFrame(frame_size_);
   OnVideoFrame(video_frame, base::TimeTicks());
 }
 
 }  // namespace content