Scale counter and resolution downgrade stats may be incorrect.

webrtc/video/vie_encoder.cc

 /*
  *  Copyright (c) 2012 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.
  */
 
@@ skipping 192 matching lines @@
         wants.target_pixel_count.reset();
         break;
       case VideoSendStream::DegradationPreference::kDegradationDisabled:
         wants.max_pixel_count = std::numeric_limits<int>::max();
         wants.target_pixel_count.reset();
         wants.max_framerate_fps = std::numeric_limits<int>::max();
     }
     return wants;
   }
 
-  void RequestResolutionLowerThan(int pixel_count) {
+  bool RequestResolutionLowerThan(int pixel_count) {
     // Called on the encoder task queue.
     rtc::CritScope lock(&crit_);
     if (!IsResolutionScalingEnabledLocked()) {
       // This can happen since |degradation_preference_| is set on libjingle's
       // worker thread but the adaptation is done on the encoder task queue.
-      return;
+      return false;
     }
     // The input video frame size will have a resolution with less than or
     // equal to |max_pixel_count| depending on how the source can scale the
     // input frame size.
     const int pixels_wanted = (pixel_count * 3) / 5;
     if (pixels_wanted < kMinPixelsPerFrame)
-      return;
+      return false;
+
     sink_wants_.max_pixel_count = pixels_wanted;
     sink_wants_.target_pixel_count = rtc::Optional<int>();
     if (source_)
       source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
   }
 
   void RequestFramerateLowerThan(int framerate_fps) {
     // Called on the encoder task queue.
     rtc::CritScope lock(&crit_);
     if (!IsFramerateScalingEnabledLocked()) {
       // This can happen since |degradation_preference_| is set on libjingle's
       // worker thread but the adaptation is done on the encoder task queue.
       return;
     }
@@ skipping 585 matching lines @@
         // we have to estimate, and can fluctuate naturally over time, don't
         // make the same kind of limitations as for resolution, but trust the
         // overuse detector to not trigger too often.
         return;
       }
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       return;
   }
 
-  last_adaptation_request_.emplace(adaptation_request);
-  const std::vector<int>& scale_counter = GetScaleCounters();
-
-  switch (reason) {
-    case kQuality:
-      stats_proxy_->OnQualityRestrictedResolutionChanged(scale_counter[reason] +
-                                                         1);
-      break;
-    case kCpu:
-      if (scale_counter[reason] >= max_downgrades)
-        return;
-      // Update stats accordingly.
-      stats_proxy_->OnCpuRestrictedResolutionChanged(true);
-      break;
+  if (reason == kCpu) {
+    int cpu_scale_counter = GetScaleCounters()[reason];

[kthelgason, 2017/04/19 08:23:04]

this can be const right?

[åsapersson, 2017/04/20 13:44:47]

Done.

+    if (cpu_scale_counter >= max_downgrades)
+      return;
   }
 
-  IncrementScaleCounter(reason, 1);
-
   switch (degradation_preference_) {
     case VideoSendStream::DegradationPreference::kBalanced:
       FALLTHROUGH();
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
-      source_proxy_->RequestResolutionLowerThan(
-          adaptation_request.input_pixel_count_);
+      if (!source_proxy_->RequestResolutionLowerThan(
+              adaptation_request.input_pixel_count_)) {
+        return;
+      }
       LOG(LS_INFO) << "Scaling down resolution.";
       break;
     case VideoSendStream::DegradationPreference::kMaintainResolution:
       source_proxy_->RequestFramerateLowerThan(
           adaptation_request.framerate_fps_);
       LOG(LS_INFO) << "Scaling down framerate.";
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
   }
 
+  last_adaptation_request_.emplace(adaptation_request);
+
+  IncrementScaleCounter(reason, 1);
+
+  // Update stats.
+  const std::vector<int>& scale_counters = GetScaleCounters();
+  switch (reason) {
+    case kQuality:
+      stats_proxy_->OnQualityRestrictedResolutionChanged(
+          scale_counters[reason]);
+      break;
+    case kCpu:
+      stats_proxy_->OnCpuRestrictedResolutionChanged(true);
+      break;
+  }
+
   for (size_t i = 0; i < kScaleReasonSize; ++i) {
-    LOG(LS_INFO) << "Scaled " << GetScaleCounters()[i]
+    LOG(LS_INFO) << "Scaled " << scale_counters[i]
                  << " times for reason: " << (i ? "cpu" : "quality");
   }
 }
 
 void ViEEncoder::AdaptUp(AdaptReason reason) {
   RTC_DCHECK_RUN_ON(&encoder_queue_);
   int scale_counter = GetScaleCounters()[reason];
   if (scale_counter == 0)
     return;
   RTC_DCHECK_GT(scale_counter, 0);
@@ skipping 18 matching lines @@
       }
       break;
     case VideoSendStream::DegradationPreference::kMaintainResolution:
       // TODO(sprang): Don't request higher framerate if we are already at
       // max requested fps?
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       return;
   }
 
-  last_adaptation_request_.emplace(adaptation_request);
-
-  switch (reason) {
-    case kQuality:
-      stats_proxy_->OnQualityRestrictedResolutionChanged(scale_counter - 1);
-      break;
-    case kCpu:
-      // Update stats accordingly.
-      stats_proxy_->OnCpuRestrictedResolutionChanged(scale_counter > 1);
-      break;
-  }
-
-  // Decrease counter of how many times we have scaled down, for this
-  // degradation preference mode and reason.
-  IncrementScaleCounter(reason, -1);
-
   // Get a sum of how many times have scaled down, in total, for this
-  // degradation preference mode. If it is 0, remove any restraints.
+  // degradation preference mode. If it is 1, remove any restraints.
   const std::vector<int>& current_scale_counters = GetScaleCounters();
   const int scale_sum = std::accumulate(current_scale_counters.begin(),
                                         current_scale_counters.end(), 0);
   switch (degradation_preference_) {
     case VideoSendStream::DegradationPreference::kBalanced:
       FALLTHROUGH();
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
-      if (scale_sum == 0) {
+      if (scale_sum == 1) {
         LOG(LS_INFO) << "Removing resolution down-scaling setting.";
         source_proxy_->RequestHigherResolutionThan(
             std::numeric_limits<int>::max());
       } else {
         source_proxy_->RequestHigherResolutionThan(
             adaptation_request.input_pixel_count_);
         LOG(LS_INFO) << "Scaling up resolution.";
       }
       break;
     case VideoSendStream::DegradationPreference::kMaintainResolution:
-      if (scale_sum == 0) {
+      if (scale_sum == 1) {
         LOG(LS_INFO) << "Removing framerate down-scaling setting.";
         source_proxy_->RequestHigherFramerateThan(
             std::numeric_limits<int>::max());
       } else {
         source_proxy_->RequestHigherFramerateThan(
             adaptation_request.framerate_fps_);
         LOG(LS_INFO) << "Scaling up framerate.";
       }
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
   }
 
+  last_adaptation_request_.emplace(adaptation_request);
+
+  // Decrease counter of how many times we have scaled down, for this
+  // degradation preference mode and reason.
+  IncrementScaleCounter(reason, -1);

[kthelgason, 2017/04/19 08:23:04]

Why move this below the switch-case? IMO it's less intuitive to check if the sum
is 1, but no big deal. The stats update is better after the scale counter has
been updated though.

[åsapersson, 2017/04/20 13:44:47]

Moved this up.

+
+  // Update stats.
+  const std::vector<int>& scale_counters = GetScaleCounters();
+  switch (reason) {
+    case kQuality:
+      stats_proxy_->OnQualityRestrictedResolutionChanged(
+          scale_counters[reason]);
+      break;
+    case kCpu:
+      stats_proxy_->OnCpuRestrictedResolutionChanged(scale_counters[reason] >
+                                                     0);
+      break;
+  }
+
   for (size_t i = 0; i < kScaleReasonSize; ++i) {
-    LOG(LS_INFO) << "Scaled " << current_scale_counters[i]
+    LOG(LS_INFO) << "Scaled " << scale_counters[i]
                  << " times for reason: " << (i ? "cpu" : "quality");
   }
 }
 
 const std::vector<int>& ViEEncoder::GetScaleCounters() {
   auto it = scale_counters_.find(degradation_preference_);
   if (it == scale_counters_.end()) {
     scale_counters_[degradation_preference_].resize(kScaleReasonSize);
     return scale_counters_[degradation_preference_];
   }
   return it->second;
 }
 
 void ViEEncoder::IncrementScaleCounter(int reason, int delta) {
   // Get the counters and validate. This may also lazily initialize the state.
   const std::vector<int>& counter = GetScaleCounters();
   if (delta < 0) {
     RTC_DCHECK_GE(counter[reason], delta);
   }
   scale_counters_[degradation_preference_][reason] += delta;
 }
 
 }  // namespace webrtc