Set overuse detector max frame interval based on target frame rate.

webrtc/video/vie_encoder.cc

Index: webrtc/video/vie_encoder.cc
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc
index 203d1c405a8813d05b627cd464803bb9df39f645..63fd7929723bfa6e63f9ee73a17bc4b640216ab3 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -49,19 +49,6 @@ const int kMinFramerateFps = 2;
 // to try and achieve desired bitrate.
 const int kMaxInitialFramedrop = 4;
 
-// TODO(pbos): Lower these thresholds (to closer to 100%) when we handle
-// pipelining encoders better (multiple input frames before something comes
-// out). This should effectively turn off CPU adaptations for systems that
-// remotely cope with the load right now.
-CpuOveruseOptions GetCpuOveruseOptions(bool full_overuse_time) {
-  CpuOveruseOptions options;
-  if (full_overuse_time) {
-    options.low_encode_usage_threshold_percent = 150;
-    options.high_encode_usage_threshold_percent = 200;
-  }
-  return options;
-}
-
 uint32_t MaximumFrameSizeForBitrate(uint32_t kbps) {
   if (kbps > 0) {
     if (kbps < 300 /* qvga */) {
@@ -249,10 +236,11 @@ class ViEEncoder::VideoSourceProxy {
     return true;
   }
 
-  bool RequestFramerateLowerThan(int fps) {
+  int RequestFramerateLowerThan(int fps) {
     // Called on the encoder task queue.
     // The input video frame rate will be scaled down to 2/3, rounding down.
-    return RestrictFramerate((fps * 2) / 3);
+    int wanted_framerate = (fps * 2) / 3;

[åsapersson, 2017/06/13 12:15:09]

nit: maybe framerate_wanted for consistency with RequestHigherFramerateThan

[sprang_webrtc, 2017/06/14 08:39:16]

Done.

+    return RestrictFramerate(wanted_framerate) ? wanted_framerate : -1;
   }
 
   bool RequestHigherResolutionThan(int pixel_count) {
@@ -289,14 +277,18 @@ class ViEEncoder::VideoSourceProxy {
     return true;
   }
 
-  bool RequestHigherFramerateThan(int fps) {
+  // Request upgrade in framerate. Returns the new requested frame, or -1 if
+  // no change requested. Note that maxint may be returned if limits due to
+  // adaptation requests are removed completely. In that case, consider
+  // |max_framerate_| to be the current limit (assuming the capturer complies).
+  int RequestHigherFramerateThan(int fps) {
     // Called on the encoder task queue.
     // The input frame rate will be scaled up to the last step, with rounding.
     int framerate_wanted = fps;
     if (fps != std::numeric_limits<int>::max())
       framerate_wanted = (fps * 3) / 2;
 
-    return IncreaseFramerate(framerate_wanted);
+    return IncreaseFramerate(framerate_wanted) ? framerate_wanted : -1;
   }
 
   bool RestrictFramerate(int fps) {
@@ -357,13 +349,15 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores,
       codec_type_(PayloadNameToCodecType(settings.payload_name)
                       .value_or(VideoCodecType::kVideoCodecUnknown)),
       video_sender_(Clock::GetRealTimeClock(), this, this),
-      overuse_detector_(GetCpuOveruseOptions(settings.full_overuse_time),
-                        this,
-                        encoder_timing,
-                        stats_proxy),
+      overuse_detector_(new OveruseFrameDetector(
+          GetCpuOveruseOptions(settings.full_overuse_time),
+          this,
+          encoder_timing,
+          stats_proxy)),
       stats_proxy_(stats_proxy),
       pre_encode_callback_(pre_encode_callback),
       module_process_thread_(nullptr),
+      max_framerate_(-1),
       pending_encoder_reconfiguration_(false),
       encoder_start_bitrate_bps_(0),
       max_data_payload_length_(0),
@@ -384,7 +378,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores,
   RTC_DCHECK(stats_proxy);
   encoder_queue_.PostTask([this] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
-    overuse_detector_.StartCheckForOveruse();
+    overuse_detector_->StartCheckForOveruse();
     video_sender_.RegisterExternalEncoder(
         settings_.encoder, settings_.payload_type, settings_.internal_source);
   });
@@ -396,12 +390,38 @@ ViEEncoder::~ViEEncoder() {
       << "Must call ::Stop() before destruction.";
 }
 
+// TODO(pbos): Lower these thresholds (to closer to 100%) when we handle
+// pipelining encoders better (multiple input frames before something comes
+// out). This should effectively turn off CPU adaptations for systems that
+// remotely cope with the load right now.
+CpuOveruseOptions ViEEncoder::GetCpuOveruseOptions(bool full_overuse_time) {
+  CpuOveruseOptions options;
+  if (full_overuse_time) {
+    options.low_encode_usage_threshold_percent = 150;
+    options.high_encode_usage_threshold_percent = 200;
+  }
+  return options;
+}
+
+void ViEEncoder::UpdateOveruseDetector(

[kthelgason, 2017/06/12 11:56:47]

Is this only used in the test? I'd prefer we not make all these changes just to
make that possible. Just make it injectable in the ctor instead.

[sprang_webrtc, 2017/06/14 08:39:17]

Done.

+    std::unique_ptr<OveruseFrameDetector> overuse_detector) {
+  rtc::Event event(false, false);
+  encoder_queue_.PostTask([this, &overuse_detector, &event] {
+    RTC_DCHECK_RUN_ON(&encoder_queue_);
+    overuse_detector_->StopCheckForOveruse();
+    overuse_detector_ = std::move(overuse_detector);
+    overuse_detector_->StartCheckForOveruse();
+    event.Set();
+  });
+  RTC_DCHECK(event.Wait(5000));
+}
+
 void ViEEncoder::Stop() {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   source_proxy_->SetSource(nullptr, VideoSendStream::DegradationPreference());
   encoder_queue_.PostTask([this] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
-    overuse_detector_.StopCheckForOveruse();
+    overuse_detector_->StopCheckForOveruse();
     rate_allocator_.reset();
     bitrate_observer_ = nullptr;
     video_sender_.RegisterExternalEncoder(nullptr, settings_.payload_type,
@@ -452,6 +472,12 @@ void ViEEncoder::SetSource(
     bool allow_scaling = IsResolutionScalingEnabled(degradation_preference_);
     initial_rampup_ = allow_scaling ? 0 : kMaxInitialFramedrop;
     ConfigureQualityScaler();
+    if (!IsFramerateScalingEnabled(degradation_preference) &&
+        max_framerate_ != -1) {
+      // If frame rate scaling is no longer allowed, remove any potential
+      // allowance for longer frame intervals.
+      overuse_detector_->OnTargetFramerateUpdated(max_framerate_);
+    }
   });
 }
 
@@ -521,6 +547,7 @@ void ViEEncoder::ReconfigureEncoder() {
       std::max(encoder_start_bitrate_bps_ / 1000, codec.minBitrate);
   codec.startBitrate = std::min(codec.startBitrate, codec.maxBitrate);
   codec.expect_encode_from_texture = last_frame_info_->is_texture;
+  max_framerate_ = codec.maxFramerate;
 
   bool success = video_sender_.RegisterSendCodec(
                      &codec, number_of_cores_,
@@ -533,19 +560,35 @@ void ViEEncoder::ReconfigureEncoder() {
   video_sender_.UpdateChannelParemeters(rate_allocator_.get(),
                                         bitrate_observer_);
 
-  int framerate = stats_proxy_->GetSendFrameRate();
-  if (framerate == 0)
-    framerate = codec.maxFramerate;
+  // Get the current actual framerate, as measured by the stats proxy. This is
+  // used to get the correct bitrate layer allocation.
+  int current_framerate = stats_proxy_->GetSendFrameRate();
+  if (current_framerate == 0)
+    current_framerate = codec.maxFramerate;
   stats_proxy_->OnEncoderReconfigured(
-      encoder_config_, rate_allocator_.get()
-                           ? rate_allocator_->GetPreferredBitrateBps(framerate)
-                           : codec.maxBitrate);
+      encoder_config_,
+      rate_allocator_.get()
+          ? rate_allocator_->GetPreferredBitrateBps(current_framerate)
+          : codec.maxBitrate);
 
   pending_encoder_reconfiguration_ = false;
 
   sink_->OnEncoderConfigurationChanged(
       std::move(streams), encoder_config_.min_transmit_bitrate_bps);
 
+  // Get the current target framerate, ie the maximum framerate as specified by
+  // the current codec configuration, or any limit imposed by cpu adaption in
+  // maintain-resolution or balanced mode. This is used to make sure overuse
+  // detection doesn't needlessly trigger in low and/or variable framerate
+  // scenarios.
+  int target_framerate = max_framerate_;
+  if (last_adaptation_request_ &&
+      last_adaptation_request_->framerate_fps_ > 0) {

[åsapersson, 2017/06/13 12:15:09]

last_adaptation_request_->framerate_fps_ will not hold the requested framrate,
right? Holds the input fps before the request.

[sprang_webrtc, 2017/06/14 08:39:16]

Hm, you're right. Guess I'll have to add another field to the adaptation request
struct :/

+    target_framerate =
+        std::min(target_framerate, last_adaptation_request_->framerate_fps_);
+  }
+  overuse_detector_->OnTargetFramerateUpdated(target_framerate);
+
   ConfigureQualityScaler();
 }
 
@@ -694,7 +737,7 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
   TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame.render_time_ms(),
                           "Encode");
 
-  overuse_detector_.FrameCaptured(video_frame, time_when_posted_us);
+  overuse_detector_->FrameCaptured(video_frame, time_when_posted_us);
 
   video_sender_.AddVideoFrame(video_frame, nullptr);
 }
@@ -725,7 +768,7 @@ EncodedImageCallback::Result ViEEncoder::OnEncodedImage(
   const int qp = encoded_image.qp_;
   encoder_queue_.PostTask([this, timestamp, time_sent_us, qp] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
-    overuse_detector_.FrameSent(timestamp, time_sent_us);
+    overuse_detector_->FrameSent(timestamp, time_sent_us);
     if (quality_scaler_ && qp >= 0)
       quality_scaler_->ReportQP(qp);
   });
@@ -851,14 +894,17 @@ void ViEEncoder::AdaptDown(AdaptReason reason) {
       }
       GetAdaptCounter().IncrementResolution(reason, 1);
       break;
-    case VideoSendStream::DegradationPreference::kMaintainResolution:
+    case VideoSendStream::DegradationPreference::kMaintainResolution: {
       // Scale down framerate.
-      if (!source_proxy_->RequestFramerateLowerThan(
-              adaptation_request.framerate_fps_)) {
+      int requested_framerate = source_proxy_->RequestFramerateLowerThan(

[kthelgason, 2017/06/12 11:56:48]

const

[sprang_webrtc, 2017/06/14 08:39:17]

Done.

+          adaptation_request.framerate_fps_);
+      if (requested_framerate == -1)
         return;
-      }
+      overuse_detector_->OnTargetFramerateUpdated(
+          std::min(max_framerate_, requested_framerate));
       GetAdaptCounter().IncrementFramerate(reason, 1);
       break;
+    }
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
   }
@@ -930,8 +976,14 @@ void ViEEncoder::AdaptUp(AdaptReason reason) {
         LOG(LS_INFO) << "Removing framerate down-scaling setting.";
         fps = std::numeric_limits<int>::max();
       }
-      if (!source_proxy_->RequestHigherFramerateThan(fps))
+
+      int requested_framerate = source_proxy_->RequestHigherFramerateThan(fps);

[kthelgason, 2017/06/12 11:56:47]

const

[sprang_webrtc, 2017/06/14 08:39:16]

Done.

+      if (requested_framerate == -1) {
+        overuse_detector_->OnTargetFramerateUpdated(max_framerate_);
         return;
+      }
+      overuse_detector_->OnTargetFramerateUpdated(
+          std::min(max_framerate_, requested_framerate));
       GetAdaptCounter().IncrementFramerate(reason, -1);
       break;
     }