Add callback to copy texture backed frames in WebRtcVideoFrameAdapter

content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc

 // Copyright 2014 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/webrtc/webrtc_video_capturer_adapter.h"
 
 #include "base/bind.h"
 #include "base/memory/aligned_memory.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/trace_event/trace_event.h"
-#include "content/renderer/media/webrtc/webrtc_video_frame_adapter.h"
+#include "content/common/gpu/client/context_provider_command_buffer.h"
+#include "content/renderer/render_thread_impl.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/timestamp_constants.h"
 #include "media/base/video_util.h"
+#include "skia/ext/platform_canvas.h"
+#include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/convert_from.h"
 #include "third_party/libyuv/include/libyuv/scale.h"
+#include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/webrtc/common_video/include/video_frame_buffer.h"
 #include "third_party/webrtc/common_video/rotation.h"
 #include "third_party/webrtc/media/engine/webrtcvideoframe.h"
 
 namespace content {
 namespace {
 
 // Empty method used for keeping a reference to the original media::VideoFrame.
 // The reference to |frame| is kept in the closure that calls this method.
 void ReleaseOriginalFrame(const scoped_refptr<media::VideoFrame>& frame) {
 }
 
 }  // anonymous namespace
 
 WebRtcVideoCapturerAdapter::WebRtcVideoCapturerAdapter(bool is_screencast)
-    : is_screencast_(is_screencast),
-      running_(false) {
+    : main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      provider_(RenderThreadImpl::current()->SharedMainThreadContextProvider()),
+      is_screencast_(is_screencast),
+      running_(false),
+      weak_factory_(this) {
   thread_checker_.DetachFromThread();
+  copy_texture_callback_ =
+      base::Bind(&WebRtcVideoCapturerAdapter::CopyTextureFrame,
+                 weak_factory_.GetWeakPtr());
 }
 
 WebRtcVideoCapturerAdapter::~WebRtcVideoCapturerAdapter() {
   DVLOG(3) << " WebRtcVideoCapturerAdapter::dtor";

[mcasas, 2016/10/26 16:06:02]

unrelated nit: I'm trying to correct these statements to
DVLO(x) << __func__;

[emircan, 2016/10/27 00:37:56]

Done.

 }
 
-cricket::CaptureState WebRtcVideoCapturerAdapter::Start(
-    const cricket::VideoFormat& capture_format) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!running_);
-  DVLOG(3) << " WebRtcVideoCapturerAdapter::Start w = " << capture_format.width
-           << " h = " << capture_format.height;
-
-  running_ = true;
-  return cricket::CS_RUNNING;
-}
-
-void WebRtcVideoCapturerAdapter::Stop() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DVLOG(3) << " WebRtcVideoCapturerAdapter::Stop ";
-  DCHECK(running_);
-  running_ = false;
-  SetCaptureFormat(NULL);
-  SignalStateChange(this, cricket::CS_STOPPED);
-}
-
-bool WebRtcVideoCapturerAdapter::IsRunning() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return running_;
-}
-
-bool WebRtcVideoCapturerAdapter::GetPreferredFourccs(
-    std::vector<uint32_t>* fourccs) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!fourccs || fourccs->empty());
-  if (fourccs)
-    fourccs->push_back(cricket::FOURCC_I420);
-  return fourccs != NULL;
-}
-
-bool WebRtcVideoCapturerAdapter::IsScreencast() const {
-  return is_screencast_;
-}
-
-bool WebRtcVideoCapturerAdapter::GetBestCaptureFormat(
-    const cricket::VideoFormat& desired,
-    cricket::VideoFormat* best_format) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DVLOG(3) << " GetBestCaptureFormat:: "
-           << " w = " << desired.width
-           << " h = " << desired.height;
-
-  // Capability enumeration is done in MediaStreamVideoSource. The adapter can
-  // just use what is provided.
-  // Use the desired format as the best format.
-  best_format->width = desired.width;
-  best_format->height = desired.height;
-  best_format->fourcc = cricket::FOURCC_I420;
-  best_format->interval = desired.interval;
-  return true;
-}
-
 void WebRtcVideoCapturerAdapter::OnFrameCaptured(
     const scoped_refptr<media::VideoFrame>& input_frame) {
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("video", "WebRtcVideoCapturerAdapter::OnFrameCaptured");
   if (!(input_frame->IsMappable() &&
         (input_frame->format() == media::PIXEL_FORMAT_I420 ||
          input_frame->format() == media::PIXEL_FORMAT_YV12 ||
-         input_frame->format() == media::PIXEL_FORMAT_YV12A))) {
+         input_frame->format() == media::PIXEL_FORMAT_YV12A)) &&
+      !input_frame->HasTextures()) {
     // Since connecting sources and sinks do not check the format, we need to
     // just ignore formats that we can not handle.
+    LOG(ERROR) << "We cannot send frame with storage type: "
+               << input_frame->storage_type()
+               << " format: " << input_frame->format();

[mcasas, 2016/10/26 16:06:02]

These are going to print numbers, right?
Consider using media::VIdeoCaptureFormat::ToString().

[emircan, 2016/10/27 00:37:56]

Done.

     NOTREACHED();
     return;
   }
   scoped_refptr<media::VideoFrame> frame = input_frame;
   // Drop alpha channel since we do not support it yet.
   if (frame->format() == media::PIXEL_FORMAT_YV12A)
     frame = media::WrapAsI420VideoFrame(input_frame);
 
   const int orig_width = frame->natural_size().width();
   const int orig_height = frame->natural_size().height();
@@ skipping 11 matching lines @@
                   frame->timestamp().InMicroseconds(),
                   rtc::TimeMicros(),
                   &adapted_width, &adapted_height,
                   &crop_width, &crop_height, &crop_x, &crop_y,
                   &translated_camera_time_us)) {
     return;
   }
 
   // Return |frame| directly if it is texture backed, because there is no
   // cropping support for texture yet. See http://crbug/503653.
-  // Return |frame| directly if it is GpuMemoryBuffer backed, as we want to
-  // keep the frame on native buffers.
   if (frame->HasTextures()) {
     OnFrame(cricket::WebRtcVideoFrame(
-                new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(frame),
+                new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(
+                    frame, copy_texture_callback_),
                 webrtc::kVideoRotation_0, translated_camera_time_us),
             orig_width, orig_height);
     return;
   }
 
   // Translate crop rectangle from natural size to visible size.
   gfx::Rect cropped_visible_rect(
       frame->visible_rect().x() +
           crop_x * frame->visible_rect().width() / orig_width,
       frame->visible_rect().y() +
           crop_y * frame->visible_rect().height() / orig_height,
       crop_width * frame->visible_rect().width() / orig_width,
       crop_height * frame->visible_rect().height() / orig_height);
 
   const gfx::Size adapted_size(adapted_width, adapted_height);
   scoped_refptr<media::VideoFrame> video_frame =
       media::VideoFrame::WrapVideoFrame(frame, frame->format(),
                                         cropped_visible_rect, adapted_size);
   if (!video_frame)
     return;
 
   video_frame->AddDestructionObserver(base::Bind(&ReleaseOriginalFrame, frame));
 
   // If no scaling is needed, return a wrapped version of |frame| directly.
   if (video_frame->natural_size() == video_frame->visible_rect().size()) {
     OnFrame(cricket::WebRtcVideoFrame(
-                new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(video_frame),
+                new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(
+                    video_frame, copy_texture_callback_),
                 webrtc::kVideoRotation_0, translated_camera_time_us),
             orig_width, orig_height);
     return;
   }
 
   // We need to scale the frame before we hand it over to webrtc.
   scoped_refptr<media::VideoFrame> scaled_frame =
       scaled_frame_pool_.CreateFrame(media::PIXEL_FORMAT_I420, adapted_size,
                                      gfx::Rect(adapted_size), adapted_size,
                                      frame->timestamp());
   libyuv::I420Scale(video_frame->visible_data(media::VideoFrame::kYPlane),
                     video_frame->stride(media::VideoFrame::kYPlane),
                     video_frame->visible_data(media::VideoFrame::kUPlane),
                     video_frame->stride(media::VideoFrame::kUPlane),
                     video_frame->visible_data(media::VideoFrame::kVPlane),
                     video_frame->stride(media::VideoFrame::kVPlane),
                     video_frame->visible_rect().width(),
                     video_frame->visible_rect().height(),
                     scaled_frame->data(media::VideoFrame::kYPlane),
                     scaled_frame->stride(media::VideoFrame::kYPlane),
                     scaled_frame->data(media::VideoFrame::kUPlane),
                     scaled_frame->stride(media::VideoFrame::kUPlane),
                     scaled_frame->data(media::VideoFrame::kVPlane),
                     scaled_frame->stride(media::VideoFrame::kVPlane),
                     adapted_width, adapted_height, libyuv::kFilterBilinear);
 
   OnFrame(cricket::WebRtcVideoFrame(
-              new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(scaled_frame),
+              new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(
+                  scaled_frame, copy_texture_callback_),
               webrtc::kVideoRotation_0, translated_camera_time_us),
           orig_width, orig_height);
 }
 
+cricket::CaptureState WebRtcVideoCapturerAdapter::Start(
+    const cricket::VideoFormat& capture_format) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!running_);
+  DVLOG(3) << " WebRtcVideoCapturerAdapter::Start w = " << capture_format.width
+           << " h = " << capture_format.height;
+
+  running_ = true;
+  return cricket::CS_RUNNING;
+}
+
+void WebRtcVideoCapturerAdapter::Stop() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(3) << " WebRtcVideoCapturerAdapter::Stop ";
+  DCHECK(running_);
+  running_ = false;
+  SetCaptureFormat(NULL);
+  SignalStateChange(this, cricket::CS_STOPPED);
+}
+
+bool WebRtcVideoCapturerAdapter::IsRunning() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return running_;
+}
+
+bool WebRtcVideoCapturerAdapter::GetPreferredFourccs(
+    std::vector<uint32_t>* fourccs) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!fourccs || fourccs->empty());

[mcasas, 2016/10/26 16:06:02]

nit: not your code, but here we should not DCHECK() a 
case we're handling later.  Suggestion for this method:

if (!fourccs)
  return false;
DCHECK(fourccs->empty());
fourcss->push_back(cricket::FOURCC_I420);
return true;

[emircan, 2016/10/27 00:37:56]

Done.

+  if (fourccs)
+    fourccs->push_back(cricket::FOURCC_I420);
+  return fourccs != NULL;
+}
+
+bool WebRtcVideoCapturerAdapter::IsScreencast() const {
+  return is_screencast_;
+}
+
+bool WebRtcVideoCapturerAdapter::GetBestCaptureFormat(
+    const cricket::VideoFormat& desired,
+    cricket::VideoFormat* best_format) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(3) << " GetBestCaptureFormat:: "
+           << " w = " << desired.width
+           << " h = " << desired.height;

[mcasas, 2016/10/26 16:06:02]

nit:
DVLOG(3) << __func__ << " desired: " << desired.ToString();

here and in l.165;

[emircan, 2016/10/27 00:37:56]

Done.

+
+  // Capability enumeration is done in MediaStreamVideoSource. The adapter can
+  // just use what is provided.
+  // Use the desired format as the best format.
+  best_format->width = desired.width;
+  best_format->height = desired.height;
+  best_format->fourcc = cricket::FOURCC_I420;
+  best_format->interval = desired.interval;
+  return true;
+}
+
+void WebRtcVideoCapturerAdapter::CopyTextureFrame(
+    const scoped_refptr<media::VideoFrame>& frame,
+    scoped_refptr<media::VideoFrame>* new_frame) {
+  base::WaitableEvent waiter(base::WaitableEvent::ResetPolicy::MANUAL,
+                             base::WaitableEvent::InitialState::NOT_SIGNALED);
+  main_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&WebRtcVideoCapturerAdapter::CopyTextureFrameOnMainThread,
+                 base::Unretained(this), frame, new_frame, &waiter));

[mcasas, 2016/10/26 16:06:02]

weak_factory_.GetWeakPtr() ?

[emircan, 2016/10/27 00:37:56]

I can add a WeakPtr here to be used on main thread and invalidate on it. But
then, I cannot use the weakptr for the |copy_texture_callback_| that runs on
some rtc platform thread or invalidate.

I thought again about the design and decided to extract these copy methods to
another class TextureCopier which will be ref counted. That way we can pass the
class to callbacks safely. Let me know if you would prefer to move it to a
seperate file and/or directory.

+  waiter.Wait();
+}
+
+void WebRtcVideoCapturerAdapter::CopyTextureFrameOnMainThread(
+    const scoped_refptr<media::VideoFrame>& frame,
+    scoped_refptr<media::VideoFrame>* new_frame,
+    base::WaitableEvent* waiter) {
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
+  DCHECK(frame->format() == media::PIXEL_FORMAT_ARGB ||
+         frame->format() == media::PIXEL_FORMAT_XRGB ||
+         frame->format() == media::PIXEL_FORMAT_I420 ||
+         frame->format() == media::PIXEL_FORMAT_UYVY ||
+         frame->format() == media::PIXEL_FORMAT_NV12);
+  *new_frame = media::VideoFrame::CreateFrame(
+      media::PIXEL_FORMAT_I420, frame->coded_size(), frame->visible_rect(),
+      frame->natural_size(), frame->timestamp());
+
+  sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(
+      frame->visible_rect().width(), frame->visible_rect().height());
+
+  if (surface && provider_) {
+    DCHECK(provider_->ContextGL());
+    canvas_video_renderer_.Copy(
+        frame.get(), surface->getCanvas(),
+        media::Context3D(provider_->ContextGL(), provider_->GrContext()));
+  } else {
+    // Return a black frame (yuv = {0, 0x80, 0x80}).
+    *new_frame = media::VideoFrame::CreateColorFrame(
+        frame->visible_rect().size(), 0u, 0x80, 0x80, frame->timestamp());

[mcasas, 2016/10/26 16:06:02]

Shouldn't you return here (and in l.259)? Ah but then |waiter| 
would not be signalled. I looked around but there is no 
ScopedWaitableEvent that would Signal on destruction.
Consider adding a class

class ScopedWaitableEvent {
 public:
  explicit ScopedWaitableEvent(base::WaitableEvent* event) : event_(event) {}
  ~ScopedWaitableEvent() {
    if (event_)
      event_->Wait();
  }

 private:
  base::WaitableEvent* const event_;
};

using it to wrap |waiter| in l.239 after a
DCHECK(waiter), and then just return as usual in every
error path, removing l.277-278, WDYT?

[emircan, 2016/10/27 00:37:56]

Thanks. I added it to this file and restructured the conditionals accordingly.
It would be better to move it to base, but I came across to a lot of opposition
when trying to add to base earlier :(

+  }
+
+  SkPixmap pixmap;
+  const bool result = surface->getCanvas()->peekPixels(&pixmap);
+  DCHECK(result) << "Error trying to access SkSurface's pixels";
+
+  const uint32 source_pixel_format =
+      (kN32_SkColorType == kRGBA_8888_SkColorType) ? cricket::FOURCC_ABGR
+                                                   : cricket::FOURCC_ARGB;
+  libyuv::ConvertToI420(static_cast<const uint8*>(pixmap.addr(0, 0)),
+                        pixmap.getSafeSize64(),
+                        (*new_frame)->visible_data(media::VideoFrame::kYPlane),
+                        (*new_frame)->stride(media::VideoFrame::kYPlane),
+                        (*new_frame)->visible_data(media::VideoFrame::kUPlane),
+                        (*new_frame)->stride(media::VideoFrame::kUPlane),
+                        (*new_frame)->visible_data(media::VideoFrame::kVPlane),
+                        (*new_frame)->stride(media::VideoFrame::kVPlane),
+                        0 /* crop_x */, 0 /* crop_y */, pixmap.width(),
+                        pixmap.height(), (*new_frame)->visible_rect().width(),
+                        (*new_frame)->visible_rect().height(), libyuv::kRotate0,
+                        source_pixel_format);
+
+  if (waiter)
+    waiter->Signal();

[mcasas, 2016/10/26 16:06:02]

Is there a chance that |waiter| might be nullptr?
if not, just DCHECK() it (otherwise we would not
crash, but playback would hold up).
Edit: forget this in light of my comment in l.254.

[emircan, 2016/10/27 00:37:56]

Done.

+}
+
 }  // namespace content