Move direct use of VideoCapturer::VideoAdapter to VideoSinkWants.

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 0dd8938dc3451758fb0441cb4df457baa6832abe..aebb7aee22c716003c18b96c8b4ab4d97db3affa 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -14,6 +14,7 @@
 #include <set>
 #include <string>
 
+#include "webrtc/base/asyncinvoker.h"
 #include "webrtc/base/buffer.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/stringutils.h"
@@ -314,6 +315,7 @@ static int GetMaxDefaultVideoBitrateKbps(int width, int height) {
     return 2500;
   }
 }
+
 }  // namespace
 
 // Constants defined in webrtc/media/engine/constants.h
@@ -994,13 +996,10 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
     send_ssrcs_.insert(used_ssrc);
 
   webrtc::VideoSendStream::Config config(this);
-  config.overuse_callback = this;
-
   WebRtcVideoSendStream* stream = new WebRtcVideoSendStream(
       call_, sp, config, external_encoder_factory_, options_,
-      bitrate_config_.max_bitrate_bps, send_codec_, send_rtp_extensions_,
-      send_params_);
-
+      signal_cpu_adaptation_, bitrate_config_.max_bitrate_bps, send_codec_,
+      send_rtp_extensions_, send_params_);
   uint32_t ssrc = sp.first_ssrc();
   RTC_DCHECK(ssrc != 0);
   send_streams_[ssrc] = stream;
@@ -1398,26 +1397,6 @@ void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) {
                           kVideoRtpBufferSize);
 }
 
-void WebRtcVideoChannel2::OnLoadUpdate(Load load) {
-  // OnLoadUpdate can not take any locks that are held while creating streams
-  // etc. Doing so establishes lock-order inversions between the webrtc process
-  // thread on stream creation and locks such as stream_crit_ while calling out.
-  rtc::CritScope stream_lock(&capturer_crit_);
-  if (!signal_cpu_adaptation_)
-    return;
-  // Do not adapt resolution for screen content as this will likely result in
-  // blurry and unreadable text.
-  for (auto& kv : capturers_) {
-    if (kv.second != nullptr
-        && !kv.second->IsScreencast()
-        && kv.second->video_adapter() != nullptr) {
-      kv.second->video_adapter()->OnCpuResolutionRequest(
-          load == kOveruse ? CoordinatedVideoAdapter::DOWNGRADE
-                           : CoordinatedVideoAdapter::UPGRADE);
-    }
-  }
-}
-
 bool WebRtcVideoChannel2::SendRtp(const uint8_t* data,
                                   size_t len,
                                   const webrtc::PacketOptions& options) {
@@ -1476,12 +1455,58 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::AllocatedEncoder::AllocatedEncoder(
   }
 }
 
+// Proxy class used for marshalling calls to webrtc::LoadObserver::OnLoadUpdate
+// from a media engine thread to the worker thread.
+class WebRtcVideoChannel2::WebRtcVideoSendStream::LoadObserverProxy {
+ public:
+  explicit LoadObserverProxy(webrtc::LoadObserver* observer) {
+    helper_ = new rtc::RefCountedObject<Helper>(observer);
+  }
+  ~LoadObserverProxy() { helper_->Detach(); }
+
+  webrtc::LoadObserver* proxy() { return helper_; }
+
+ private:
+  class Helper : public webrtc::LoadObserver, public rtc::RefCountInterface {
+   public:
+    explicit Helper(webrtc::LoadObserver* observer)
+        : thread_(rtc::Thread::Current()), observer_(observer) {}
+    void Detach() {
+      RTC_DCHECK(thread_checker_.CalledOnValidThread());
+      observer_ = nullptr;
+    }
+    void OnLoadUpdate(webrtc::LoadObserver::Load load) override {
+      if (rtc::Thread::Current() == thread_) {
+        observer_->OnLoadUpdate(load);
+        return;
+      }
+      invoker_.AsyncInvoke<void>(
+          thread_, rtc::Bind(&Helper::OnLoadUpdateOnCorrectThread, this, load));
+    }
+    void OnLoadUpdateOnCorrectThread(webrtc::LoadObserver::Load load) {
+      RTC_DCHECK(thread_checker_.CalledOnValidThread());
+      if (observer_) {
+        observer_->OnLoadUpdate(load);
+      }
+    }
+
+   private:
+    rtc::ThreadChecker thread_checker_;
+    rtc::AsyncInvoker invoker_;
+    rtc::Thread* thread_;
+    webrtc::LoadObserver* observer_;
+  };
+
+  rtc::scoped_refptr<Helper> helper_;
+};
+
 WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
     webrtc::Call* call,
     const StreamParams& sp,
     const webrtc::VideoSendStream::Config& config,
     WebRtcVideoEncoderFactory* external_encoder_factory,
     const VideoOptions& options,
+    bool enable_cpu_overuse_detection,
     int max_bitrate_bps,
     const rtc::Optional<VideoCodecSettings>& codec_settings,
     const std::vector<webrtc::RtpExtension>& rtp_extensions,
@@ -1491,6 +1516,7 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
     : ssrcs_(sp.ssrcs),
       ssrc_groups_(sp.ssrc_groups),
       call_(call),
+      load_proxy_(new LoadObserverProxy(this)),
       external_encoder_factory_(external_encoder_factory),
       stream_(NULL),
       parameters_(config, options, max_bitrate_bps, codec_settings),
@@ -1512,6 +1538,8 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
   parameters_.config.rtp.rtcp_mode = send_params.rtcp.reduced_size
                                          ? webrtc::RtcpMode::kReducedSize
                                          : webrtc::RtcpMode::kCompound;
+  parameters_.config.overuse_callback =
+      enable_cpu_overuse_detection ? load_proxy_->proxy() : nullptr;
 
   if (codec_settings) {
     SetCodecAndOptions(*codec_settings, parameters_.options);
@@ -1635,9 +1663,6 @@ bool WebRtcVideoChannel2::WebRtcVideoSendStream::DisconnectCapturer() {
     if (capturer_ == NULL)
       return false;
 
-    if (capturer_->video_adapter() != nullptr)
-      old_adapt_changes_ += capturer_->video_adapter()->adaptation_changes();
-
     capturer = capturer_;
     capturer_ = NULL;
   }
@@ -1929,6 +1954,26 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::Stop() {
   sending_ = false;
 }
 
+void WebRtcVideoChannel2::WebRtcVideoSendStream::OnLoadUpdate(Load load) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  LOG(LS_INFO) << "OnLoadUpdate " << load;
+  rtc::CritScope cs(&lock_);
+  if (!capturer_) {
+    return;
+  }
+
+  rtc::VideoSinkWants::ResolutionRequest request;
+  request.request = load == kOveruse
+                        ? rtc::VideoSinkWants::ResolutionRequest::kSmallerThan
+                        : rtc::VideoSinkWants::ResolutionRequest::kLargerThan;
+  request.seen_number_of_pixels =
+      last_dimensions_.height * last_dimensions_.width;
+
+  sink_wants_.resolution =
+      rtc::Optional<rtc::VideoSinkWants::ResolutionRequest>(request);

[pthatcher1, 2016/02/19 06:08:40]

I think I liked what we had better before :(.  This just seems too complex.

But let's step back for a moment an think of what we're trying to accomplish. 
We want the following:

1.  When we detect CPU overuse at resolution X, start scaling down to resolution
X/A.  This can happen multiple times down to some limit of X/Z.

2.  When we detect CPU underuse at resolution X/B, scale down less, to only X/A.
 This can happen multiple times all the way up to X/1 == X.

3.  If the video source changes resolution (and perhaps aspect ratio) on its
own, from X to Y, adjust the scale factor from A to B so that we have X/A =~
Y/B.  If Y > X, then B > A.  If Y < X, then B < A.

Once we know the scale factor A, then the scaler has a really easy job: just
scale with that factor.


The simplest thing would be for the encoder sink to tell the source "I want
frames scaled down by factor A".  When overuse is detected, we scale down
further (to B > A).  When underuse is detected, we scale down less (to B < A). 
The video broadcaster simply chooses the sink with the largest factor.  That is
simple and easy.  

However, it has the downside that when the video source changes resolution from
X to Y, the frame the encoder sink sees will change from X/A to Y/A.  Then we'll
have to wait until another overuse/underuse cycle to change Y/A to Y/B, which
would be undesirable.

A slight tweak to this would be for the encoder sink to tell the source "I want
frames scaled down to X/A".  When overuse is detected, we scale down further (to
X/B < X/A).  When underuse is detected, we scale down less (to X/B > X/A).  The
video broadcaster simply chooses the sink with the smallest scaled resolution. 
That is also simple and easy.

However, it has the upside that when the video source changes resolutions from X
to Y, the source can say to itself "I should scale down to Y/B == X/A".  Then
the encoder sink will see the resolution Y/B, which should be about the same as
it wanted with X/A.  And if it sees underuse or overuse again, it will tell the
source "I want to see Z/C".


And thus the big question is: how do we represent X/A?  And here are the
options:

A.  As scale factor A or B or C.  Really easy, but doesn't react well to
changing resolutions.  However, because we don't adapt for screencast and we
also change the source object when we switch cameras, I think we don't have a
current use case for this.  So, it would meet our current needs and be very
simple.

B.  As pixel_count(X)/A, or scaled pixel count, or max_pixel_count.   This is
what you had before, and now that I think about it more, I think your code you
had before is correct, and pretty simple.   I'd like to hear pbos's objections
to it.  Like I said, at first glance, it seems better (much more simple) than
this updated patch.

C.  As pixel_count + direction request, which you have here. This model seems
more complex because it puts the source in charge of how much to go up or down
and it makes the aggregation of wants so much more complicated.  Can someone
explain to me the advantage of option C over option B?

By the way, I think this is equivalent to having two values in SinkWants:
less_than_pixel_count and more_than_pixel_count, which I'd greatly prefer over
the pixel_count + direction if we go with C.   

D.  As resolution + scale.  Basically "if the resolution is X, scale down by A".
 Then it's clear what the source should do (scale down by A), and if it does
happen to change resolution (if we ever adapt for screencast, for example), then
the source can figure out what to do.  The aggregation might still be a bit
complex, though.


I think I prefer B > A > D > C.

+  capturer_->AddOrUpdateSink(this, sink_wants_);
+}
+
 VideoSenderInfo
 WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo() {
   VideoSenderInfo info;
@@ -1967,10 +2012,6 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo() {
         info.input_frame_width = last_captured_frame_format.width;
         info.input_frame_height = last_captured_frame_format.height;
       }
-      if (capturer_->video_adapter() != nullptr) {
-        info.adapt_changes += capturer_->video_adapter()->adaptation_changes();
-        info.adapt_reason = capturer_->video_adapter()->adapt_reason();
-      }
     }
   }