Passing Texture backed VideoFrame through WebRTC wrappers

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/trace_event/trace_event.h"
 #include "content/renderer/media/webrtc/webrtc_video_frame_adapter.h"
@@ skipping 53 matching lines @@
   cricket::VideoFrame* CreateAliasedFrame(
       const cricket::CapturedFrame* input_frame,
       int cropped_input_width,
       int cropped_input_height,
       int output_width,
       int output_height) const override {
     // Check that captured_frame is actually our frame.
     DCHECK(input_frame == &captured_frame_);
     DCHECK(frame_.get());
 
+    const int64_t timestamp_ns = frame_->timestamp().InMicroseconds() *
+                                 base::Time::kNanosecondsPerMicrosecond;
+
+    // Return |frame_| directly if it is texture backed, because there is no
+    // cropping support for texture yet. See http://crbug/503653.
+    if (frame_->HasTextures()) {
+      return new cricket::WebRtcVideoFrame(
+          new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(frame_),
+          captured_frame_.elapsed_time, timestamp_ns);
+    }
+
     // Create a centered cropped visible rect that preservers aspect ratio for
     // cropped natural size.
     gfx::Rect visible_rect = frame_->visible_rect();
     visible_rect.ClampToCenteredSize(gfx::Size(
         visible_rect.width() * cropped_input_width / input_frame->width,
         visible_rect.height() * cropped_input_height / input_frame->height));
 
     const gfx::Size output_size(output_width, output_height);
     scoped_refptr<media::VideoFrame> video_frame =
         media::VideoFrame::WrapVideoFrame(frame_, visible_rect, output_size);
     video_frame->AddDestructionObserver(
         base::Bind(&ReleaseOriginalFrame, frame_));
 
-    const int64_t timestamp_ns = frame_->timestamp().InMicroseconds() *
-                                 base::Time::kNanosecondsPerMicrosecond;
-
     // If no scaling is needed, return a wrapped version of |frame_| directly.
     if (video_frame->natural_size() == video_frame->visible_rect().size()) {
       return new cricket::WebRtcVideoFrame(
           new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(video_frame),
           captured_frame_.elapsed_time, timestamp_ns);
     }
 
     // We need to scale the frame before we hand it over to cricket.
     scoped_refptr<media::VideoFrame> scaled_frame =
         scaled_frame_pool_.CreateFrame(media::VideoFrame::I420, output_size,
@@ skipping 101 matching lines @@
   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>& frame) {
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("video", "WebRtcVideoCapturerAdapter::OnFrameCaptured");
-  if (!(media::VideoFrame::I420 == frame->format() ||
-        media::VideoFrame::YV12 == frame->format())) {
-    // Some types of sources support textures as output. Since connecting
-    // sources and sinks do not check the format, we need to just ignore
-    // formats that we can not handle.
+  if (!((frame->IsMappable() && (frame->format() == media::VideoFrame::I420 ||
+                                 frame->format() == media::VideoFrame::YV12)) ||
+        frame->HasTextures())) {

[mcasas, 2015/06/26 00:18:32]

Is l.221 correct? I thought with this CL we can accept VideoFrames
that HasTextures().

[emircan, 2015/06/26 19:47:09]

Correct. The initial ! applies to both statements.

+    // Since connecting sources and sinks do not check the format, we need to
+    // just ignore formats that we can not handle.
     NOTREACHED();
     return;
   }
 
   if (first_frame_timestamp_ == media::kNoTimestamp())
     first_frame_timestamp_ = frame->timestamp();
 
   const int64 elapsed_time =
       (frame->timestamp() - first_frame_timestamp_).InMicroseconds() *
       base::Time::kNanosecondsPerMicrosecond;
 
   // Inject the frame via the VideoFrameFractory.
   DCHECK(frame_factory_ == frame_factory());

[mcasas, 2015/06/26 00:18:32]

DCHECK_EQ?
what exactly is this line testing for? :?

[emircan, 2015/06/26 19:47:09]

l.152 passes the ownership of the frame_factory_ to the base class via
VideoCapturer::set_frame_factory(). AFAIU, this makes sure that they are still
pointing to the same thing.

[mcasas, 2015/06/26 23:52:56]

Ugh, there is a 
MediaVideoFrameFactory* frame_factory_;  // [1] 
and a
rtc::scoped_ptr<VideoFrameFactory> frame_factory_;  // [2]
in its base class :? :?

Why not 
set_frame_factory(new MediaVideoFrameFactory)
in [3] and remove the derived naked pointer?


[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

[emircan, 2015/06/27 00:37:16]

The method used from |MediaVideoFrameFactory* frame_factory_| in this class are
not a part of VideoFrameFactory interface, such as SetFrame(),
GetCapturedFrame(), ReleaseFrame(). I guess, it was kept as member so that we
don't need to cast it, considering OnFrameCaptured() is called a lot. What do
you think? 

Still, I went ahead and implemented it though.

   frame_factory_->SetFrame(frame, elapsed_time);
 
   // This signals to libJingle that a new VideoFrame is available.
   SignalFrameCaptured(this, frame_factory_->GetCapturedFrame());
 
   frame_factory_->ReleaseFrame();  // Release the frame ASAP.
 }
 
 }  // namespace content