Delete AndroidVideoCapturer::FrameFactory.

webrtc/api/androidvideocapturer.cc

 /*
  *  Copyright 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/api/androidvideocapturer.h"
 
 #include <memory>
 
 #include "webrtc/api/java/jni/native_handle_impl.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/media/engine/webrtcvideoframe.h"
 
 namespace webrtc {
 
-// A hack for avoiding deep frame copies in
-// cricket::VideoCapturer.SignalFrameCaptured() using a custom FrameFactory.
-// A frame is injected using UpdateCapturedFrame(), and converted into a
-// cricket::VideoFrame with CreateAliasedFrame(). UpdateCapturedFrame() should
-// be called before CreateAliasedFrame() for every frame.
-// TODO(magjed): Add an interface cricket::VideoCapturer::OnFrameCaptured()
-// for ref counted I420 frames instead of this hack.
-class AndroidVideoCapturer::FrameFactory : public cricket::VideoFrameFactory {
- public:
-  explicit FrameFactory(
-      const rtc::scoped_refptr<AndroidVideoCapturerDelegate>& delegate)
-      : delegate_(delegate) {
-    // Create a CapturedFrame that only contains header information, not the
-    // actual pixel data.
-    captured_frame_.pixel_height = 1;
-    captured_frame_.pixel_width = 1;
-    captured_frame_.data = nullptr;
-    captured_frame_.data_size = cricket::CapturedFrame::kUnknownDataSize;
-    captured_frame_.fourcc = static_cast<uint32_t>(cricket::FOURCC_ANY);
-  }
-
-  void UpdateCapturedFrame(
-      const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& buffer,
-      int rotation,
-      int64_t time_stamp_in_ns) {
-    RTC_DCHECK(rotation == 0 || rotation == 90 || rotation == 180 ||
-               rotation == 270);
-    buffer_ = buffer;
-    captured_frame_.width = buffer->width();
-    captured_frame_.height = buffer->height();
-    captured_frame_.time_stamp = time_stamp_in_ns;
-    captured_frame_.rotation = static_cast<webrtc::VideoRotation>(rotation);
-  }
-
-  void ClearCapturedFrame() {
-    buffer_ = nullptr;
-    captured_frame_.width = 0;
-    captured_frame_.height = 0;
-    captured_frame_.time_stamp = 0;
-  }
-
-  const cricket::CapturedFrame* GetCapturedFrame() const {
-    return &captured_frame_;
-  }
-
-  cricket::VideoFrame* CreateAliasedFrame(
-      const cricket::CapturedFrame* captured_frame,
-      int dst_width,
-      int dst_height) const override {
-    // Check that captured_frame is actually our frame.
-    RTC_CHECK(captured_frame == &captured_frame_);
-    RTC_CHECK(buffer_->native_handle() == nullptr);
-
-    std::unique_ptr<cricket::VideoFrame> frame(new cricket::WebRtcVideoFrame(
-        ShallowCenterCrop(buffer_, dst_width, dst_height),
-        captured_frame->time_stamp, captured_frame->rotation));
-    // Caller takes ownership.
-    // TODO(magjed): Change CreateAliasedFrame() to return a std::unique_ptr.
-    return apply_rotation_ ? frame->GetCopyWithRotationApplied()->Copy()
-                           : frame.release();
-  }
-
-  cricket::VideoFrame* CreateAliasedFrame(
-      const cricket::CapturedFrame* input_frame,
-      int cropped_input_width,
-      int cropped_input_height,
-      int output_width,
-      int output_height) const override {
-    if (buffer_->native_handle() != nullptr) {
-      rtc::scoped_refptr<webrtc::VideoFrameBuffer> scaled_buffer(
-          static_cast<webrtc_jni::AndroidTextureBuffer*>(buffer_.get())
-              ->CropScaleAndRotate(cropped_input_width, cropped_input_height,
-                                   output_width, output_height,
-                                   apply_rotation_ ? input_frame->rotation
-                                                   : webrtc::kVideoRotation_0));
-      return new cricket::WebRtcVideoFrame(
-          scaled_buffer, input_frame->time_stamp,
-          apply_rotation_ ? webrtc::kVideoRotation_0 : input_frame->rotation);
-    }
-    return VideoFrameFactory::CreateAliasedFrame(input_frame,
-                                                 cropped_input_width,
-                                                 cropped_input_height,
-                                                 output_width,
-                                                 output_height);
-  }
-
- private:
-  rtc::scoped_refptr<webrtc::VideoFrameBuffer> buffer_;
-  cricket::CapturedFrame captured_frame_;
-  rtc::scoped_refptr<AndroidVideoCapturerDelegate> delegate_;
-};
-
 AndroidVideoCapturer::AndroidVideoCapturer(
     const rtc::scoped_refptr<AndroidVideoCapturerDelegate>& delegate)
     : running_(false),
       delegate_(delegate),
-      frame_factory_(NULL),
       current_state_(cricket::CS_STOPPED) {
   thread_checker_.DetachFromThread();
   SetSupportedFormats(delegate_->GetSupportedFormats());
 }
 
 AndroidVideoCapturer::~AndroidVideoCapturer() {
   RTC_CHECK(!running_);
 }
 
 cricket::CaptureState AndroidVideoCapturer::Start(
     const cricket::VideoFormat& capture_format) {
   RTC_CHECK(thread_checker_.CalledOnValidThread());
   RTC_CHECK(!running_);
   const int fps = cricket::VideoFormat::IntervalToFps(capture_format.interval);
   LOG(LS_INFO) << " AndroidVideoCapturer::Start " << capture_format.width << "x"
                << capture_format.height << "@" << fps;
 
-  frame_factory_ = new AndroidVideoCapturer::FrameFactory(delegate_.get());
-  set_frame_factory(frame_factory_);
-
   running_ = true;
   delegate_->Start(capture_format.width, capture_format.height, fps, this);
   SetCaptureFormat(&capture_format);
   current_state_ = cricket::CS_STARTING;
   return current_state_;
 }
 
 void AndroidVideoCapturer::Stop() {
   LOG(LS_INFO) << " AndroidVideoCapturer::Stop ";
   RTC_CHECK(thread_checker_.CalledOnValidThread());
@@ skipping 20 matching lines @@
 void AndroidVideoCapturer::OnCapturerStarted(bool success) {
   RTC_CHECK(thread_checker_.CalledOnValidThread());
   cricket::CaptureState new_state =
       success ? cricket::CS_RUNNING : cricket::CS_FAILED;
   if (new_state == current_state_)
     return;
   current_state_ = new_state;
   SetCaptureState(new_state);
 }
 
 void AndroidVideoCapturer::OnIncomingFrame(

[magjed_webrtc, 2016/05/17 12:12:39]

Add two methods OnIncomingTexture and OnIncomingNV21 instead, and delegate from
androidvideocapturer_jni. You can use NV12ToI420Rotate directly instead of
rotating with GetCopyWithRotationApplied. Also, try to move the function
CropScaleAndRotate from AndroidTextureBuffer to NativeHandle instead.

[nisse-webrtc, 2016/05/18 11:16:33]

Done. (But named OnNV21Frame and OnTextureFrame).

Actually, I think we can choose whether to do this adaptation in
AndroidVideoCapturer or AndroidVideoCApturerJni, it uses only public
VideoCapturer methods, I think.

[magjed_webrtc, 2016/05/18 11:41:59]

If you can choose, I prefer to keep the conversion here in
AndroidVideoCapturerJni and only use public methods on VideoCapturer.

[nisse-webrtc, 2016/05/18 12:17:52]

I'll give it a try.

     const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& buffer,
-    int rotation,
+    int rotation_int,
     int64_t time_stamp) {
   RTC_CHECK(thread_checker_.CalledOnValidThread());
-  frame_factory_->UpdateCapturedFrame(buffer, rotation, time_stamp);
-  SignalFrameCaptured(this, frame_factory_->GetCapturedFrame());
-  frame_factory_->ClearCapturedFrame();
+  RTC_DCHECK(rotation_int == 0 || rotation_int == 90 || rotation_int == 180 ||
+             rotation_int == 270);
+  webrtc::VideoRotation rotation =
+      static_cast<webrtc::VideoRotation>(rotation_int);
+  int adapted_width;
+  int adapted_height;
+  int crop_width;
+  int crop_height;
+  int crop_x;
+  int crop_y;
+
+  if (!AdaptFrame(buffer->width(), buffer->height(),
+                  &adapted_width, &adapted_height,
+                  &crop_width, &crop_height,
+                  &crop_x, &crop_y)) {
+    return;
+  }
+
+  rtc::scoped_refptr<webrtc::VideoFrameBuffer> adapted;
+  if (buffer->native_handle() != nullptr) {
+    // Texture frame.
+    OnFrame(
+        cricket::WebRtcVideoFrame(
+            static_cast<webrtc_jni::AndroidTextureBuffer*>(buffer.get())
+                ->CropScaleAndRotate(
+                    crop_width, crop_height, crop_width, crop_height,
+                    adapted_width, adapted_height,
+                    apply_rotation() ? rotation : webrtc::kVideoRotation_0),
+            time_stamp, apply_rotation() ? webrtc::kVideoRotation_0 : rotation),
+        buffer->width(), buffer->height());
+  } else {
+    // Memory frame.
+    rtc::scoped_refptr<webrtc::VideoFrameBuffer> adapted;
+    if (adapted_width == buffer->width() &&
+        adapted_height == buffer->height() &&
+        crop_width == buffer->width() &&
+        crop_width == buffer->height()) {
+      adapted = buffer;
+    } else {
+      adapted = I420Buffer::CropAndScale(buffer, crop_x, crop_y,

[magjed_webrtc, 2016/05/17 12:19:12]

Use a frame pool instead of allocating new memory every time.

[nisse-webrtc, 2016/05/18 11:16:33]

Haven't done this yet. Would the pool be an additional argument? I'd like the
CropAndScale method to return an I420Buffer, but that doesn't quite work with
the current frame pool.

[magjed_webrtc, 2016/05/18 11:41:59]

I think it's best if the output I420Buffer is an additional argument instead of
the pool, because I don't think I420Buffer should depend on any specific pool
implementation.

[nisse-webrtc, 2016/05/18 12:17:52]

I really dislike destination arguments accessed via MutableDataX()...

I think we should strive to have code modifying pixel data limited to
I420Buffers which the modifying code owns, with the special case of newly
constructed frame buffers. Eventually, I'd like to remove MutableData from the
VideoFrameBuffer base class.

Have to think a bit about the options here.

BTW, I guess we should try to also reuse the buffer used for the unscaled data?

[nisse-webrtc, 2016/05/18 13:54:58]

I now use buffer_pool_ for the unscaled frame, similar to the old code. I guess
we'd need a separate pool for the scaled frame?

+                                         crop_width, crop_height,
+                                         adapted_width, adapted_height);
+    }
+    // TODO(nisse): Introduce a rotate method working with I420Buffer?
+    cricket::WebRtcVideoFrame frame(adapted, time_stamp, rotation);
+    OnFrame(apply_rotation() ? *frame.GetCopyWithRotationApplied() : frame,
+            buffer->width(), buffer->height());
+  }
 }
 
 void AndroidVideoCapturer::OnOutputFormatRequest(
     int width, int height, int fps) {
   RTC_CHECK(thread_checker_.CalledOnValidThread());
   cricket::VideoFormat format(width, height,
                               cricket::VideoFormat::FpsToInterval(fps), 0);
   video_adapter()->OnOutputFormatRequest(format);
 }
 
 bool AndroidVideoCapturer::GetBestCaptureFormat(
     const cricket::VideoFormat& desired,
     cricket::VideoFormat* best_format) {
   // Delegate this choice to VideoCapturer.startCapture().
   *best_format = desired;
   return true;
 }
 
 }  // namespace webrtc