Delete VideoFrameFactory, CapturedFrame, and related code.

webrtc/media/base/videocapturer.cc

Index: webrtc/media/base/videocapturer.cc
diff --git a/webrtc/media/base/videocapturer.cc b/webrtc/media/base/videocapturer.cc
index 9d297bf9141e51941d606df52a3e9589754545b3..5436854bd42b493ee9c938702629f3fd47ba4a19 100644
--- a/webrtc/media/base/videocapturer.cc
+++ b/webrtc/media/base/videocapturer.cc
@@ -18,9 +18,7 @@
 #include "webrtc/base/common.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/systeminfo.h"
-#include "webrtc/media/base/videoframefactory.h"
 #include "webrtc/media/engine/webrtcvideoframe.h"
-#include "webrtc/media/engine/webrtcvideoframefactory.h"
 
 namespace cricket {
 
@@ -34,29 +32,6 @@ static const int kYU12Penalty = 16;  // Needs to be higher than MJPG index.
 }  // namespace
 
 /////////////////////////////////////////////////////////////////////
-// Implementation of struct CapturedFrame
-/////////////////////////////////////////////////////////////////////
-CapturedFrame::CapturedFrame()
-    : width(0),
-      height(0),
-      fourcc(0),
-      pixel_width(0),
-      pixel_height(0),
-      time_stamp(0),
-      data_size(0),
-      rotation(webrtc::kVideoRotation_0),
-      data(NULL) {}
-
-// TODO(fbarchard): Remove this function once lmimediaengine stops using it.
-bool CapturedFrame::GetDataSize(uint32_t* size) const {
-  if (!size || data_size == CapturedFrame::kUnknownDataSize) {
-    return false;
-  }
-  *size = data_size;
-  return true;
-}
-
-/////////////////////////////////////////////////////////////////////
 // Implementation of class VideoCapturer
 /////////////////////////////////////////////////////////////////////
 VideoCapturer::VideoCapturer() : apply_rotation_(false) {
@@ -67,16 +42,9 @@ VideoCapturer::VideoCapturer() : apply_rotation_(false) {
 void VideoCapturer::Construct() {
   enable_camera_list_ = false;
   capture_state_ = CS_STOPPED;
-  SignalFrameCaptured.connect(this, &VideoCapturer::OnFrameCaptured);
   scaled_width_ = 0;
   scaled_height_ = 0;
   enable_video_adapter_ = true;
-  // There are lots of video capturers out there that don't call
-  // set_frame_factory.  We can either go change all of them, or we
-  // can set this default.
-  // TODO(pthatcher): Remove this hack and require the frame factory
-  // to be passed in the constructor.
-  set_frame_factory(new WebRtcVideoFrameFactory());
 }
 
 const std::vector<VideoFormat>* VideoCapturer::GetSupportedFormats() const {
@@ -152,29 +120,6 @@ void VideoCapturer::ConstrainSupportedFormats(const VideoFormat& max_format) {
   UpdateFilteredSupportedFormats();
 }
 
-std::string VideoCapturer::ToString(const CapturedFrame* captured_frame) const {
-  std::string fourcc_name = GetFourccName(captured_frame->fourcc) + " ";
-  for (std::string::const_iterator i = fourcc_name.begin();
-       i < fourcc_name.end(); ++i) {
-    // Test character is printable; Avoid isprint() which asserts on negatives.
-    if (*i < 32 || *i >= 127) {
-      fourcc_name = "";
-      break;
-    }
-  }
-
-  std::ostringstream ss;
-  ss << fourcc_name << captured_frame->width << "x" << captured_frame->height;
-  return ss.str();
-}
-
-void VideoCapturer::set_frame_factory(VideoFrameFactory* frame_factory) {
-  frame_factory_.reset(frame_factory);
-  if (frame_factory) {
-    frame_factory->SetApplyRotation(apply_rotation_);
-  }
-}
-
 bool VideoCapturer::GetInputSize(int* width, int* height) {
   rtc::CritScope cs(&frame_stats_crit_);
   if (!input_size_valid_) {
@@ -204,9 +149,6 @@ void VideoCapturer::AddOrUpdateSink(
 void VideoCapturer::OnSinkWantsChanged(const rtc::VideoSinkWants& wants) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   apply_rotation_ = wants.rotation_applied;
-  if (frame_factory_) {
-    frame_factory_->SetApplyRotation(apply_rotation_);
-  }
 
   if (video_adapter()) {
     video_adapter()->OnResolutionRequest(wants.max_pixel_count,
@@ -259,54 +201,21 @@ bool VideoCapturer::AdaptFrame(int width,
   return true;
 }
 
-void VideoCapturer::OnFrameCaptured(VideoCapturer*,
-                                    const CapturedFrame* captured_frame) {
-  int out_width;
-  int out_height;
-  int crop_width;
-  int crop_height;
-  int crop_x;
-  int crop_y;
-
-  // TODO(nisse): We don't do timestamp translation on this input
-  // path. It seems straight-forward to enable translation, but that
-  // breaks the WebRtcVideoEngine2Test.PropagatesInputFrameTimestamp
-  // test. Probably not worth the effort to fix, instead, try to
-  // delete or refactor all code using VideoFrameFactory and
-  // SignalCapturedFrame.
-  if (!AdaptFrame(captured_frame->width, captured_frame->height,
-                  captured_frame->time_stamp / rtc::kNumNanosecsPerMicrosec,
-                  0,
-                  &out_width, &out_height,
-                  &crop_width, &crop_height, &crop_x, &crop_y, nullptr)) {
-    return;
-  }
-
-  if (!frame_factory_) {
-    LOG(LS_ERROR) << "No video frame factory.";
-    return;
-  }
-
-  // TODO(nisse): Reorganize frame factory methods. crop_x and crop_y
-  // are ignored for now.
-  std::unique_ptr<VideoFrame> adapted_frame(frame_factory_->CreateAliasedFrame(
-      captured_frame, crop_width, crop_height, out_width, out_height));
-
-  if (!adapted_frame) {
-    // TODO(fbarchard): LOG more information about captured frame attributes.
-    LOG(LS_ERROR) << "Couldn't convert to I420! "
-                  << "From " << ToString(captured_frame) << " To "
-                  << out_width << " x " << out_height;
-    return;
-  }
-
-  OnFrame(*adapted_frame, captured_frame->width, captured_frame->height);
-}
-
 void VideoCapturer::OnFrame(const VideoFrame& frame,
                             int orig_width,
                             int orig_height) {
-  broadcaster_.OnFrame(frame);
+  // For a child class which implements rotation itself, we should
+  // always have apply_rotation_ == false or frame.rotation() == 0.
+  // Except possibly during races where apply_rotation_ is changed
+  // mid-stream.
+  if (apply_rotation_) {
+    broadcaster_.OnFrame(WebRtcVideoFrame(

[perkj_webrtc, 2016/09/14 07:12:28]

this will not work for textures since this only rotates i420 buffers. Do you
remember the discussion with magjed on where Rotate should be implemented?  
apply_rotation_ is set in OnSinkWantsChanged. Are you saying that this class can
only be used in none texture case? 

[nisse-webrtc, 2016/09/14 07:58:15]

I'm not sure we really should implement rotation here at all; since we're moving
away from using cricket::VideoCapturer, it doesn't make so much sense to add new
features.

But to answer the question: With this cl, any capturer that produces texture
frames must handle apply_rotation_ itself, before passing frames on to this
method. 

Hmm, but for this to work, I420Buffer::Rotate would need to be changed to do the
check 

  if (rotation == webrtc::kVideoRotation_0) {
    return src;
  }

before accessing planar data. 

I'll have to think a bit more about this, but I'm leaning towards deleting this
code and possibly add a log or DCHECK for the invalid case apply_rotation_ true
and frame.rotation() != 0.

+        webrtc::I420Buffer::Rotate(frame.video_frame_buffer(),
+                                   frame.rotation()),
+        webrtc::kVideoRotation_0, frame.timestamp_us()));
+  } else {
+    broadcaster_.OnFrame(frame);
+  }
   UpdateInputSize(orig_width, orig_height);
 }