Delete VideoFrameFactory, CapturedFrame, and related code.

webrtc/media/base/videocapturer.cc

 /*
  *  Copyright (c) 2010 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.
  */
 
 // Implementation file of class VideoCapturer.
 
 #include "webrtc/media/base/videocapturer.h"
 
 #include <algorithm>
 
 #include "libyuv/scale_argb.h"
 #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 {
 
 namespace {
 
 static const int64_t kMaxDistance = ~(static_cast<int64_t>(1) << 63);
 #ifdef WEBRTC_LINUX
 static const int kYU12Penalty = 16;  // Needs to be higher than MJPG index.
 #endif
 
 }  // 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) {
   thread_checker_.DetachFromThread();
   Construct();
 }
 
 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 {
   return &filtered_supported_formats_;
 }
 
 bool VideoCapturer::StartCapturing(const VideoFormat& capture_format) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   CaptureState result = Start(capture_format);
   const bool success = (result == CS_RUNNING) || (result == CS_STARTING);
@@ skipping 55 matching lines @@
   return true;
 }
 
 void VideoCapturer::ConstrainSupportedFormats(const VideoFormat& max_format) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   max_format_.reset(new VideoFormat(max_format));
   LOG(LS_VERBOSE) << " ConstrainSupportedFormats " << max_format.ToString();
   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_) {
     return false;
   }
   *width = input_width_;
   *height = input_height_;
 
   return true;
 }
 
 void VideoCapturer::RemoveSink(
     rtc::VideoSinkInterface<cricket::VideoFrame>* sink) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   broadcaster_.RemoveSink(sink);
   OnSinkWantsChanged(broadcaster_.wants());
 }
 
 void VideoCapturer::AddOrUpdateSink(
     rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
     const rtc::VideoSinkWants& wants) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   broadcaster_.AddOrUpdateSink(sink, wants);
   OnSinkWantsChanged(broadcaster_.wants());
 }
 
 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,
                                          wants.max_pixel_count_step_up);
   }
 }
 
 bool VideoCapturer::AdaptFrame(int width,
                                int height,
                                int64_t camera_time_us,
@@ skipping 32 matching lines @@
     *crop_y = 0;
   }
 
   if (translated_camera_time_us) {
     *translated_camera_time_us = timestamp_aligner_.ClipTimestamp(
         camera_time_us + offset_us, system_time_us);
   }
   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);
 }
 
 void VideoCapturer::SetCaptureState(CaptureState state) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   if (state == capture_state_) {
     // Don't trigger a state changed callback if the state hasn't changed.
     return;
   }
   capture_state_ = state;
@@ skipping 131 matching lines @@
 void VideoCapturer::UpdateInputSize(int width, int height) {
   // Update stats protected from fetches from different thread.
   rtc::CritScope cs(&frame_stats_crit_);
 
   input_size_valid_ = true;
   input_width_ = width;
   input_height_ = height;
 }
 
 }  // namespace cricket