Move direct use of VideoCapturer::VideoAdapter to VideoSinkWants.

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 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/media/engine/webrtcvideoengine2.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 
+#include "webrtc/base/asyncinvoker.h"
 #include "webrtc/base/buffer.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/stringutils.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/base/trace_event.h"
 #include "webrtc/call.h"
 #include "webrtc/media/base/videocapturer.h"
 #include "webrtc/media/base/videorenderer.h"
 #include "webrtc/media/engine/constants.h"
 #include "webrtc/media/engine/simulcast.h"
@@ skipping 280 matching lines @@
   if (width * height <= 320 * 240) {
     return 600;
   } else if (width * height <= 640 * 480) {
     return 1700;
   } else if (width * height <= 960 * 540) {
     return 2000;
   } else {
     return 2500;
   }
 }
+
 }  // namespace
 
 // Constants defined in webrtc/media/engine/constants.h
 // TODO(pbos): Move these to a separate constants.cc file.
 const int kMinVideoBitrate = 30;
 const int kStartVideoBitrate = 300;
 
 const int kVideoMtu = 1200;
 const int kVideoRtpBufferSize = 65536;
 
 // This constant is really an on/off, lower-level configurable NACK history
 // duration hasn't been implemented.
 static const int kNackHistoryMs = 1000;
 
 static const int kDefaultQpMax = 56;
 
 static const int kDefaultRtcpReceiverReportSsrc = 1;
 
+static const int kMaxCpuDowngrades = 2;  // Downgrade at most 2 times for CPU.
+
 std::vector<VideoCodec> DefaultVideoCodecList() {
   std::vector<VideoCodec> codecs;
   codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp8PlType,
                                                            kVp8CodecName));
   codecs.push_back(
       VideoCodec::CreateRtxCodec(kDefaultRtxVp8PlType, kDefaultVp8PlType));
   if (CodecIsInternallySupported(kVp9CodecName)) {
     codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp9PlType,
                                                              kVp9CodecName));
     codecs.push_back(
@@ skipping 642 matching lines @@
 
   rtc::CritScope stream_lock(&stream_crit_);
 
   if (!ValidateSendSsrcAvailability(sp))
     return false;
 
   for (uint32_t used_ssrc : sp.ssrcs)
     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;
 
   if (rtcp_receiver_report_ssrc_ == kDefaultRtcpReceiverReportSsrc) {
     rtcp_receiver_report_ssrc_ = ssrc;
     LOG(LS_INFO) << "SetLocalSsrc on all the receive streams because we added "
                     "a send stream.";
     for (auto& kv : receive_streams_)
       kv.second->SetLocalSsrc(ssrc);
@@ skipping 377 matching lines @@
 
   // Speculative change to increase the outbound socket buffer size.
   // In b/15152257, we are seeing a significant number of packets discarded
   // due to lack of socket buffer space, although it's not yet clear what the
   // ideal value should be.
   MediaChannel::SetOption(NetworkInterface::ST_RTP,
                           rtc::Socket::OPT_SNDBUF,
                           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) {
   rtc::Buffer packet(data, len, kMaxRtpPacketLen);
   rtc::PacketOptions rtc_options;
   rtc_options.packet_id = options.packet_id;
   return MediaChannel::SendPacket(&packet, rtc_options);
 }
 
 bool WebRtcVideoChannel2::SendRtcp(const uint8_t* data, size_t len) {
@@ skipping 38 matching lines @@
       external_encoder(nullptr),
       type(type),
       external(external) {
   if (external) {
     external_encoder = encoder;
     this->encoder =
         new webrtc::VideoEncoderSoftwareFallbackWrapper(type, encoder);
   }
 }
 
+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_;
+};
+

[pthatcher1, 2016/02/17 23:05:56]

Can you explain why this proxy is necessary?

[perkj_webrtc, 2016/02/18 13:22:46]

OnLoad is called on a media engine thread. Not the worker thread.
So it is to not have to make the videobroadcaster have to take care of
AddOrUpdateSink calls from multiple threads + probably in the near future -
accept frames to be delivered on a third thread.

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

It seems like a lot of complexity to add.

And I think we need to allow AddOrUpdateSink to be callable from any thread
anyway.   Otherwise, the caller needs to know what thread is has to call it on,
which I think would be a mess.  Shouldn't that be pretty simple with a
CriticalSection?

Similarly,  I think we should allow OnFrame to be called from any thread, and
have a sink do a thread-hop or lock if it needs to.  Several places we already
do that, because frames can come from random threads on different mobile OSes.  

So, can we just make both OnFrame and AddOrUpdateSink thread-safe, and not have
to deal with this proxy complexity?

[perkj_webrtc, 2016/02/19 14:57:52]

I agreee with OnFrame as said before. 


I don't so much like having AddOrUpdateSink and RemoveSink be called on
arbitrary threads..

If AddOrUpdateSink is called on two threads for the same sink - it feels like
you can end up in a weird state. Ex- CPU is low so the CPU monitor allows the
sink to increase the bitrate. But the encoder thread say that the produced
quality is awesome - so it thinks it can produce frames with a higher resolution
and calls AddOrUpdateSink on another tread... Somewhere these two requests
should be coordinated. I agree that is does not necessarily has to be here. 

AddOrUpdateSink for two separate sinks on two separate threads might make more
sence. But it means that the source implementation must be thread safe too so
that it can handle the changed sinkwants.

Also- pbos pointed out a possible problem of holding |lock_| in
webrtcvideosendstream::onload from a tread the webrtc engine. That might cause a
possible deadlock when the webrtcvideosendstream is reconstructed. 

 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,
     // TODO(deadbeef): Don't duplicate information between send_params,
     // rtp_extensions, options, etc.
     const VideoSendParameters& send_params)
     : 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),
       pending_encoder_reconfiguration_(false),
       allocated_encoder_(NULL, webrtc::kVideoCodecUnknown, false),
       capturer_(NULL),
       sending_(false),
       muted_(false),
       old_adapt_changes_(0),
       first_frame_timestamp_ms_(0),
       last_frame_timestamp_ms_(0) {
   parameters_.config.rtp.max_packet_size = kVideoMtu;
 
   sp.GetPrimarySsrcs(&parameters_.config.rtp.ssrcs);
   sp.GetFidSsrcs(parameters_.config.rtp.ssrcs,
                  &parameters_.config.rtp.rtx.ssrcs);
   parameters_.config.rtp.c_name = sp.cname;
   parameters_.config.rtp.extensions = rtp_extensions;
   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);
   }
 }
 
 WebRtcVideoChannel2::WebRtcVideoSendStream::~WebRtcVideoSendStream() {
   DisconnectCapturer();
   if (stream_ != NULL) {
     call_->DestroyVideoSendStream(stream_);
@@ skipping 103 matching lines @@
   muted_ = mute;
 }
 
 bool WebRtcVideoChannel2::WebRtcVideoSendStream::DisconnectCapturer() {
   cricket::VideoCapturer* capturer;
   {
     rtc::CritScope cs(&lock_);
     if (capturer_ == NULL)
       return false;
 
-    if (capturer_->video_adapter() != nullptr)
-      old_adapt_changes_ += capturer_->video_adapter()->adaptation_changes();
-
     capturer = capturer_;
     capturer_ = NULL;
   }
   capturer->RemoveSink(this);
 
   return true;
 }
 
 const std::vector<uint32_t>&
 WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const {
@@ skipping 271 matching lines @@
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::Stop() {
   rtc::CritScope cs(&lock_);
   if (stream_ != NULL) {
     stream_->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;
+  }
+
+  if (load == kOveruse) {
+    if (cpu_downgrade_count_ < kMaxCpuDowngrades) {
+      cpu_downgrade_count_++;
+      sink_wants_.max_number_of_pixels =
+          (last_dimensions_.height * last_dimensions_.width) / 2;
+    }
+  } else {
+    if (cpu_downgrade_count_ > 0) {
+      cpu_downgrade_count_--;
+      sink_wants_.max_number_of_pixels =
+          (last_dimensions_.height * last_dimensions_.width) * 2;
+    }

[pthatcher1, 2016/02/17 23:05:56]

This doesn't seem to work right for situations where the frame size changes on
its own, like with a screencast being resized.

For example:

start at 1080p
handle overuse, set max to 1080/2 = 720p
screencast changes on its own to 360p
handle non-overuse, set max to 360p*2 = 720p.

Wouldn't it make more sense to do:

sink_wants_.max_number_of_pixels *= 2;
or
sink_wants_.max_number_of_pixels /= 2;

?


Also, instead of recording the count of the number of times we've gone down, I
think it would make sense to record the amount we've scaled it down (1, 2, 4),
and have a max for that (4).

In fact, what I think we need to do is:

1.  Have the adapter here, near the encoder.
2.  Have the adapter act like a source and a sink, and the encoder attaches to
it as a source, and it attaches to the main source as a sink.  Then, adaptation
happens even if local preview is attached to the original video track at the
beginning of the pipeline.

[perkj_webrtc, 2016/02/18 13:22:46]

Currently - CPU adaptation is not done for screen cast - I added the check in
VideoCapturer instead of as it was (See old
WebRtcVideoChannel2::OnLoadUpdate(Load load). I can move the logic back here if
you want. 

But from your example- isn't that exactly what we want to express here: The load
for a certain resolution is either too high or too low. We would like to get the
max_number_of_pixels as an argument to OnLoad so that we know what resolution
the decision was based on but this is as good guess as we can currently do.
Alternatively we can have a scale factor or just forwarw Up /Down in the wants
(See patchset 1). But that gets klunky since that express a request for a change
not a want. Nisse did not like it and I agree.

An adapter does not necessarily know how to scale a native handle currently.
That can be solved by adding a scale factor to the video frames as we discussed
and actually let the encoder do the scaling. 
But that is a bit larger change that I think we should do down the line. This
currently work since the adapter causes the frame factory in the capture
implementation to create the correct sized frames from the beginning.

Regardless - I think it make sense to notify the source of what resolution the
sinks wants. In the future we can decide to reconfigure the camera to the
resolution that the sinks are actually using.

Currently adaptation happens even if local preview is attached, that is no
change from how it is now. The source produce the minimum resolution requested
by all sinks. I also think that make sence for CPU adaptation - doing scaling
early saves CPU for all sinks instead of them doing scaling individually.

[perkj_webrtc, 2016/02/18 14:30:54]

So back to your example- pbos convinced me of your visdom in not expressing up /
down in terms of max_number_of_pixels like this.
How about changing the wants again to express Up / Down from a resolution?  

+  }
+  capturer_->AddOrUpdateSink(this, sink_wants_);
+}
+
 VideoSenderInfo
 WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo() {
   VideoSenderInfo info;
   webrtc::VideoSendStream::Stats stats;
   {
     rtc::CritScope cs(&lock_);
     for (uint32_t ssrc : parameters_.config.rtp.ssrcs)
       info.add_ssrc(ssrc);
 
     if (parameters_.codec_settings)
@@ skipping 18 matching lines @@
 
     if (capturer_ != NULL) {
       if (!capturer_->IsMuted()) {
         VideoFormat last_captured_frame_format;
         capturer_->GetStats(&info.adapt_frame_drops, &info.effects_frame_drops,
                             &info.capturer_frame_time,
                             &last_captured_frame_format);
         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();
-      }
     }
   }
 
   // Get bandwidth limitation info from stream_->GetStats().
   // Input resolution (output from video_adapter) can be further scaled down or
   // higher video layer(s) can be dropped due to bitrate constraints.
   // Note, adapt_changes only include changes from the video_adapter.
   if (stats.bw_limited_resolution)
     info.adapt_reason |= CoordinatedVideoAdapter::ADAPTREASON_BANDWIDTH;
 
@@ skipping 499 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             fec_settings.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket