Revert of Delete the class WebRtcVideoCapturerAdapter::MediaVideoFrameFactory...

Revert of Delete the class WebRtcVideoCapturerAdapter::MediaVideoFrameFactory (patchset #5 id:80001 of https://codereview.chromium.org/2244213002/ )

Reason for revert:
Somehow breaks timestamping. Reverting for investigation. To reproduce, run

./browser_tests --gtest_filter='WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest*' --run-manual --ui-test-action-max-timeout=200000

This produces lots of warnings like

[1:14:0822/153830:WARNING:video_capture_input.cc(79)] Same/old NTP timestamp (3680861906110 <= 3680861906110) for incoming frame. Dropping.
[1:14:0822/153830:WARNING:video_capture_input.cc(79)] Same/old NTP timestamp (3680861906042 <= 3680861906042) for incoming frame. Dropping.

Original issue's description:
> Delete the class WebRtcVideoCapturerAdapter::MediaVideoFrameFactory
>
> Instead, use the AdaptFrame and OnFrame methods of the VideoCapturer
> base class.
>
> This cl also applies the new timestamp aligner logic, to translate
> Chrome's camera timestamps into the timescale of rtc::TimeMicros(). No
> other change in behavior is intended.
>
> BUG=webrtc:5682, webrtc:5740, 516700
>
> Committed: https://crrev.com/5b9a4b339447b033ca52244a02c0bef17067385f
> Cr-Commit-Position: refs/heads/master@{#413093}

TBR=perkj@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:5682, webrtc:5740, 516700

Review-Url: https://codereview.chromium.org/2263183002
Cr-Commit-Position: refs/heads/master@{#413444}

content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc

@@ -9,11 +9,14 @@
 #include "base/trace_event/trace_event.h"
 #include "content/renderer/media/webrtc/webrtc_video_frame_adapter.h"
 #include "media/base/timestamp_constants.h"
+#include "media/base/video_frame.h"
+#include "media/base/video_frame_pool.h"
 #include "media/base/video_util.h"
 #include "third_party/libyuv/include/libyuv/convert_from.h"
 #include "third_party/libyuv/include/libyuv/scale.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/base/videoframefactory.h"
 #include "third_party/webrtc/media/engine/webrtcvideoframe.h"
 
 namespace content {
@@ -26,10 +29,132 @@ void ReleaseOriginalFrame(const scoped_refptr<media::VideoFrame>& frame) {
 
 }  // anonymous namespace
 
+// A cricket::VideoFrameFactory for media::VideoFrame. The purpose of this
+// class is to avoid a premature frame copy. A media::VideoFrame is injected
+// with SetFrame, and converted into a cricket::VideoFrame with
+// CreateAliasedFrame. SetFrame should be called before CreateAliasedFrame
+// for every frame.
+class WebRtcVideoCapturerAdapter::MediaVideoFrameFactory
+    : public cricket::VideoFrameFactory {
+ public:
+  void SetFrame(const scoped_refptr<media::VideoFrame>& frame) {
+    DCHECK(frame.get());
+    // Create a CapturedFrame that only contains header information, not the
+    // actual pixel data.
+    captured_frame_.width = frame->natural_size().width();
+    captured_frame_.height = frame->natural_size().height();
+    captured_frame_.time_stamp = frame->timestamp().InMicroseconds() *
+                                 base::Time::kNanosecondsPerMicrosecond;
+    captured_frame_.pixel_height = 1;
+    captured_frame_.pixel_width = 1;
+    captured_frame_.rotation = webrtc::kVideoRotation_0;
+    captured_frame_.data = NULL;
+    captured_frame_.data_size = cricket::CapturedFrame::kUnknownDataSize;
+    captured_frame_.fourcc = static_cast<uint32_t>(cricket::FOURCC_ANY);
+
+    frame_ = frame;
+  }
+
+  void ReleaseFrame() { frame_ = NULL; }
+
+  const cricket::CapturedFrame* GetCapturedFrame() const {
+    return &captured_frame_;
+  }
+
+  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.
+    // Return |frame_| directly if it is GpuMemoryBuffer backed, as we want to
+    // keep the frame on native buffers.
+    if (frame_->HasTextures() ||
+        frame_->storage_type() ==
+            media::VideoFrame::STORAGE_GPU_MEMORY_BUFFERS) {
+      return new cricket::WebRtcVideoFrame(
+          new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(frame_),
+          timestamp_ns, webrtc::kVideoRotation_0);
+    }
+
+    // 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_, frame_->format(),
+                                          visible_rect, output_size);
+    if (!video_frame)
+      return nullptr;
+    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()) {
+      return new cricket::WebRtcVideoFrame(
+          new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(video_frame),
+          timestamp_ns, webrtc::kVideoRotation_0);
+    }
+
+    // We need to scale the frame before we hand it over to cricket.
+    scoped_refptr<media::VideoFrame> scaled_frame =
+        scaled_frame_pool_.CreateFrame(media::PIXEL_FORMAT_I420, output_size,
+                                       gfx::Rect(output_size), output_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),
+                      output_width, output_height, libyuv::kFilterBilinear);
+    return new cricket::WebRtcVideoFrame(
+        new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(scaled_frame),
+        timestamp_ns, webrtc::kVideoRotation_0);
+  }
+
+  cricket::VideoFrame* CreateAliasedFrame(
+      const cricket::CapturedFrame* input_frame,
+      int output_width,
+      int output_height) const override {
+    return CreateAliasedFrame(input_frame, input_frame->width,
+                              input_frame->height, output_width, output_height);
+  }
+
+ private:
+  scoped_refptr<media::VideoFrame> frame_;
+  cricket::CapturedFrame captured_frame_;
+  // This is used only if scaling is needed.
+  mutable media::VideoFramePool scaled_frame_pool_;
+};
+
 WebRtcVideoCapturerAdapter::WebRtcVideoCapturerAdapter(bool is_screencast)
     : is_screencast_(is_screencast),
       running_(false) {
   thread_checker_.DetachFromThread();
+  // The base class takes ownership of the frame factory.
+  set_frame_factory(new MediaVideoFrameFactory);
 }
 
 WebRtcVideoCapturerAdapter::~WebRtcVideoCapturerAdapter() {
@@ -93,108 +218,32 @@ bool WebRtcVideoCapturerAdapter::GetBestCaptureFormat(
 }
 
 void WebRtcVideoCapturerAdapter::OnFrameCaptured(
-    const scoped_refptr<media::VideoFrame>& input_frame) {
+    const scoped_refptr<media::VideoFrame>& video_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))) {
+  if (!(video_frame->IsMappable() &&
+        (video_frame->format() == media::PIXEL_FORMAT_I420 ||
+         video_frame->format() == media::PIXEL_FORMAT_YV12 ||
+         video_frame->format() == media::PIXEL_FORMAT_YV12A))) {
     // Since connecting sources and sinks do not check the format, we need to
     // just ignore formats that we can not handle.
     NOTREACHED();
     return;
   }
-  scoped_refptr<media::VideoFrame> frame = input_frame;
+  scoped_refptr<media::VideoFrame> frame = video_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();
-  int adapted_width;
-  int adapted_height;
-  // The VideoAdapter is only used for cpu-adaptation downscaling, no
-  // aspect changes. So we ignore these crop-related outputs.
-  int crop_width;
-  int crop_height;
-  int crop_x;
-  int crop_y;
-  int64_t translated_camera_time_us;
-
-  if (!AdaptFrame(orig_width, orig_height,
-                  frame->timestamp().InMicroseconds(),
-                  rtc::TimeMicros(),
-                  &adapted_width, &adapted_height,
-                  &crop_width, &crop_height, &crop_x, &crop_y,
-                  &translated_camera_time_us)) {
-    return;
-  }
+    frame = media::WrapAsI420VideoFrame(video_frame);
 
-  // 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() ||
-      frame->storage_type() ==
-      media::VideoFrame::STORAGE_GPU_MEMORY_BUFFERS) {
-    OnFrame(cricket::WebRtcVideoFrame(
-                new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(frame),
-                translated_camera_time_us, webrtc::kVideoRotation_0),
-            orig_width, orig_height);
-    return;
-  }
+  // Inject the frame via the VideoFrameFactory of base class.
+  MediaVideoFrameFactory* media_video_frame_factory =
+      reinterpret_cast<MediaVideoFrameFactory*>(frame_factory());
+  media_video_frame_factory->SetFrame(frame);
 
-  // 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() * adapted_width / orig_width,
-      visible_rect.height() * adapted_height / orig_height));
-
-  const gfx::Size adapted_size(adapted_width, adapted_height);
-  scoped_refptr<media::VideoFrame> video_frame =
-      media::VideoFrame::WrapVideoFrame(frame, frame->format(),
-                                        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),
-                translated_camera_time_us, webrtc::kVideoRotation_0),
-            orig_width, orig_height);
-    return;
-  }
+  // This signals to libJingle that a new VideoFrame is available.
+  SignalFrameCaptured(this, media_video_frame_factory->GetCapturedFrame());
 
-  // 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),
-              translated_camera_time_us, webrtc::kVideoRotation_0),
-          orig_width, orig_height);
+  media_video_frame_factory->ReleaseFrame();  // Release the frame ASAP.
 }
 
 }  // namespace content

content/renderer/media/webrtc/webrtc_video_capturer_adapter.h

@@ -15,7 +15,6 @@
 #include "content/common/content_export.h"
 #include "media/base/video_capture_types.h"
 #include "media/base/video_frame.h"
-#include "media/base/video_frame_pool.h"
 #include "third_party/webrtc/media/base/videocapturer.h"
 
 namespace content {
@@ -57,7 +56,7 @@ class CONTENT_EXPORT WebRtcVideoCapturerAdapter
   const bool is_screencast_;
   bool running_;
 
-  media::VideoFramePool scaled_frame_pool_;
+  class MediaVideoFrameFactory;
 
   DISALLOW_COPY_AND_ASSIGN(WebRtcVideoCapturerAdapter);
 };

content/renderer/media/webrtc/webrtc_video_capturer_adapter_unittest.cc

@@ -11,18 +11,17 @@
 namespace content {
 
 class WebRtcVideoCapturerAdapterTest
-    : public rtc::VideoSinkInterface<cricket::VideoFrame>,
+    : public sigslot::has_slots<>,
       public ::testing::Test {
  public:
   WebRtcVideoCapturerAdapterTest()
       : adapter_(false),
         output_frame_width_(0),
         output_frame_height_(0) {
-    adapter_.AddOrUpdateSink(this, rtc::VideoSinkWants());
-  }
-  ~WebRtcVideoCapturerAdapterTest() override {
-    adapter_.RemoveSink(this);
+    adapter_.SignalFrameCaptured.connect(
+        this, &WebRtcVideoCapturerAdapterTest::OnFrameCaptured);
   }
+  ~WebRtcVideoCapturerAdapterTest() override {}
 
   void TestSourceCropFrame(int capture_width,
                            int capture_height,
@@ -43,11 +42,11 @@ class WebRtcVideoCapturerAdapterTest
     EXPECT_EQ(natural_width, output_frame_width_);
     EXPECT_EQ(natural_height, output_frame_height_);
   }
-
-  // rtc::VideoSinkInterface
-  void OnFrame(const cricket::VideoFrame& frame) override {
-    output_frame_width_ = frame.width();
-    output_frame_height_ = frame.height();
+ protected:
+  void OnFrameCaptured(cricket::VideoCapturer* capturer,
+                       const cricket::CapturedFrame* frame) {
+    output_frame_width_ = frame->width;
+    output_frame_height_ = frame->height;
   }
 
  private: