Implement kBalanced degradation preference.

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 a3b071ac00fb058cad52b3290d00d163550744df..62ef1a84cb478419c82a3757bd02c8baebb27459 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -73,6 +73,49 @@ uint32_t MaximumFrameSizeForBitrate(uint32_t kbps) {
   return std::numeric_limits<uint32_t>::max();
 }
 
+// Limits for kBalanced degradation preference.
+//  >640x480:  |                   max                       |
+//  <=640x480: |         15fps                   |    max    |
+//  <=480x270: |     10fps     |        15fps                |
+//  <=320x240: |  7fps |             10fps                   |
+//  ----------------------------------------------------------
+//       kbps: 0       60     130               500          |

[kthelgason, 2017/05/19 08:34:14]

Just out of curiosity, how are these determined?

[åsapersson, 2017/06/08 13:55:32]

The bitrate limits were set roughly to a value when bits per pixel were below
some threshold (around 0.07). 

+
+int MinFps(int pixels) {
+  if (pixels <= 320 * 240)
+    return 7;
+  else if (pixels <= 480 * 270)
+    return 10;
+  else if (pixels <= 640 * 480)
+    return 15;
+  else
+    return std::numeric_limits<int>::max();
+}
+
+int MaxFps(int pixels) {
+  if (pixels <= 320 * 240)
+    return 10;
+  else if (pixels <= 480 * 270)
+    return 15;
+  else
+    return std::numeric_limits<int>::max();
+}
+
+uint32_t BitrateLimit(int pixels) {
+  if (pixels <= 320 * 240)
+    return 60000;
+  else if (pixels <= 480 * 270)
+    return 130000;
+  else if (pixels <= 640 * 480)
+    return 500000;
+  else
+    return std::numeric_limits<uint32_t>::max();
+}
+
+int MaxFpsForBitrateAndResolution(uint32_t bps, int pixels) {
+  return (bps <= BitrateLimit(pixels)) ? MinFps(pixels) : MaxFps(pixels);
+}
+
 bool IsResolutionScalingEnabled(
     VideoSendStream::DegradationPreference degradation_preference) {
   return degradation_preference ==
@@ -211,7 +254,7 @@ class ViEEncoder::VideoSourceProxy {
     // the used degradation_preference.
     switch (degradation_preference_) {
       case VideoSendStream::DegradationPreference::kBalanced:
-        FALLTHROUGH();
+        break;
       case VideoSendStream::DegradationPreference::kMaintainFramerate:
         wants.max_framerate_fps = std::numeric_limits<int>::max();
         break;
@@ -314,7 +357,85 @@ class ViEEncoder::VideoSourceProxy {
       source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
   }
 
+  bool RestrictFpsOrRequestResolutionLowerThan(int max_fps,
+                                               int pixel_count,
+                                               bool* fps_changed) {
+    // Called on the encoder task queue.

[kthelgason, 2017/05/19 08:34:15]

Do you want to add a thread_checker here to make this assumption explicit?

[åsapersson, 2017/06/08 13:55:32]

Can maybe do it in a separate cl.

+    rtc::CritScope lock(&crit_);
+    // Restrict framerate if it is lower.
+    *fps_changed = false;
+    const int fps_wanted = std::max(kMinFramerateFps, max_fps);
+    if (fps_wanted < sink_wants_.max_framerate_fps &&
+        RequestFramerateLocked(fps_wanted)) {
+      *fps_changed = true;
+      return true;

[kthelgason, 2017/05/19 08:34:14]

I don't really like this weird way of signalling what changed. Can we just
return a bit-vector that's 1 if fps changed, 2 if resolution changed and 0
otherwise? Or perhaps we can do something else like return a struct or enum or
something.

[åsapersson, 2017/06/08 13:55:32]

This part has been changed in https://codereview.webrtc.org/2903563002/. 

+    }
+    // Request lower resolution.
+    if (!IsResolutionScalingEnabledLocked())
+      return false;
+
+    // The input video frame size will have a resolution less than or equal to
+    // |max_pixel_count| depending on how the source can scale the frame size.
+    const int pixels_wanted = (pixel_count * 3) / 5;
+    if (pixels_wanted < kMinPixelsPerFrame ||
+        pixels_wanted >= sink_wants_.max_pixel_count) {
+      return false;
+    }
+    sink_wants_.max_pixel_count = pixels_wanted;
+    sink_wants_.target_pixel_count = rtc::Optional<int>();
+    if (source_)

[kthelgason, 2017/05/19 08:34:14]

Maybe we should just bail out right at the start if there is no source. Isn't
all of this pointless if there is no source that can be adapted?

[åsapersson, 2017/06/08 13:55:32]

Done in https://codereview.webrtc.org/2903563002/.

+      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
+  }
+
+  bool IncreaseFpsOrRequestHigherResolutionThan(int max_fps,
+                                                int pixel_count,
+                                                bool* fps_changed) {
+    // Called on the encoder task queue.
+    rtc::CritScope lock(&crit_);
+    // Increase framerate if it is higher.
+    *fps_changed = false;
+    const int fps_wanted = std::max(kMinFramerateFps, max_fps);
+    if (fps_wanted > sink_wants_.max_framerate_fps &&
+        RequestFramerateLocked(fps_wanted)) {
+      *fps_changed = true;
+      return true;
+    }
+    // Request higher resolution.
+    if (!IsResolutionScalingEnabledLocked())
+      return false;
+
+    int max_pixels_wanted = pixel_count;
+    if (max_pixels_wanted != std::numeric_limits<int>::max())
+      max_pixels_wanted = pixel_count * 4;
+
+    if (max_pixels_wanted <= sink_wants_.max_pixel_count)
+      return false;
+
+    sink_wants_.max_pixel_count = max_pixels_wanted;
+    if (max_pixels_wanted == std::numeric_limits<int>::max()) {
+      sink_wants_.target_pixel_count.reset();
+    } else {
+      sink_wants_.target_pixel_count =
+          rtc::Optional<int>((pixel_count * 5) / 3);
+    }
+    if (source_)
+      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
+  }
+
  private:
+  bool RequestFramerateLocked(int framerate_fps)
+      EXCLUSIVE_LOCKS_REQUIRED(&crit_) {
+    if (!IsFramerateScalingEnabledLocked())
+      return false;
+
+    sink_wants_.max_framerate_fps = framerate_fps;
+    if (source_)
+      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
+  }
+
   bool IsResolutionScalingEnabledLocked() const
       EXCLUSIVE_LOCKS_REQUIRED(&crit_) {
     return degradation_preference_ ==
@@ -325,9 +446,10 @@ class ViEEncoder::VideoSourceProxy {
 
   bool IsFramerateScalingEnabledLocked() const
       EXCLUSIVE_LOCKS_REQUIRED(&crit_) {
-    // TODO(sprang): Also accept kBalanced here?
     return degradation_preference_ ==
-           VideoSendStream::DegradationPreference::kMaintainResolution;
+               VideoSendStream::DegradationPreference::kMaintainResolution ||
+           degradation_preference_ ==
+               VideoSendStream::DegradationPreference::kBalanced;
   }
 
   rtc::CriticalSection crit_;
@@ -817,7 +939,8 @@ void ViEEncoder::AdaptDown(AdaptReason reason) {
   int max_downgrades = 0;
   switch (degradation_preference_) {
     case VideoSendStream::DegradationPreference::kBalanced:
-      FALLTHROUGH();
+      max_downgrades = kMaxCpuResolutionDowngrades;
+      break;
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
       max_downgrades = kMaxCpuResolutionDowngrades;
       if (downgrade_requested &&
@@ -852,21 +975,37 @@ void ViEEncoder::AdaptDown(AdaptReason reason) {
   }
 
   switch (degradation_preference_) {
-    case VideoSendStream::DegradationPreference::kBalanced:
-      FALLTHROUGH();
+    case VideoSendStream::DegradationPreference::kBalanced: {
+      int max_fps = MaxFpsForBitrateAndResolution(
+          last_observed_bitrate_bps_, last_frame_info_->pixel_count());
+      bool fps_restricted;
+      if (!source_proxy_->RestrictFpsOrRequestResolutionLowerThan(
+              max_fps, adaptation_request.input_pixel_count_,
+              &fps_restricted)) {
+        return;
+      }
+      if (fps_restricted) {
+        LOG(LS_INFO) << "Scaling down framerate: " << max_fps;
+        GetAdaptCounter().IncrementFramerate(reason);
+      } else {
+        LOG(LS_INFO) << "Scaling down resolution.";
+        GetAdaptCounter().IncrementResolution(reason);
+      }
+      break;
+    }
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
       if (!source_proxy_->RequestResolutionLowerThan(
               adaptation_request.input_pixel_count_)) {
         return;
       }
       LOG(LS_INFO) << "Scaling down resolution.";
-      GetAdaptCounter().IncrementResolution(reason, 1);
+      GetAdaptCounter().IncrementResolution(reason);
       break;
     case VideoSendStream::DegradationPreference::kMaintainResolution:
       source_proxy_->RequestFramerateLowerThan(
           adaptation_request.framerate_fps_);
       LOG(LS_INFO) << "Scaling down framerate.";
-      GetAdaptCounter().IncrementFramerate(reason, 1);
+      GetAdaptCounter().IncrementFramerate(reason);
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
@@ -899,7 +1038,7 @@ void ViEEncoder::AdaptUp(AdaptReason reason) {
 
   switch (degradation_preference_) {
     case VideoSendStream::DegradationPreference::kBalanced:
-      FALLTHROUGH();
+      break;
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
       if (adapt_up_requested &&
           adaptation_request.input_pixel_count_ <=
@@ -918,8 +1057,27 @@ void ViEEncoder::AdaptUp(AdaptReason reason) {
   }
 
   switch (degradation_preference_) {

[kthelgason, 2017/05/19 08:34:14]

Why do we have these two identical switch statements right after each other?

[åsapersson, 2017/06/08 13:55:32]

Removed the one above.

-    case VideoSendStream::DegradationPreference::kBalanced:
-      FALLTHROUGH();
+    case VideoSendStream::DegradationPreference::kBalanced: {
+      int pixel_count = adaptation_request.input_pixel_count_;
+      if (adapt_counter.ResolutionCount() == 1) {
+        LOG(LS_INFO) << "Removing down-scaling setting.";
+        pixel_count = std::numeric_limits<int>::max();
+      }
+      int fps = MaxFps(last_frame_info_->pixel_count());
+      bool fps_changed;
+      if (!source_proxy_->IncreaseFpsOrRequestHigherResolutionThan(
+              fps, pixel_count, &fps_changed)) {
+        return;
+      }
+      if (fps_changed) {
+        LOG(LS_INFO) << "Scaling up framerate: " << fps;
+        GetAdaptCounter().DecrementFramerate(reason);
+      } else {
+        LOG(LS_INFO) << "Scaling up resolution.";
+        GetAdaptCounter().DecrementResolution(reason);
+      }
+      break;
+    }
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
       if (adapt_counter.TotalCount() == 1) {
         LOG(LS_INFO) << "Removing resolution down-scaling setting.";
@@ -930,7 +1088,7 @@ void ViEEncoder::AdaptUp(AdaptReason reason) {
             adaptation_request.input_pixel_count_);
         LOG(LS_INFO) << "Scaling up resolution.";
       }
-      GetAdaptCounter().IncrementResolution(reason, -1);
+      GetAdaptCounter().DecrementResolution(reason);
       break;
     case VideoSendStream::DegradationPreference::kMaintainResolution:
       if (adapt_counter.TotalCount() == 1) {
@@ -942,7 +1100,7 @@ void ViEEncoder::AdaptUp(AdaptReason reason) {
             adaptation_request.framerate_fps_);
         LOG(LS_INFO) << "Scaling up framerate.";
       }
-      GetAdaptCounter().IncrementFramerate(reason, -1);
+      GetAdaptCounter().DecrementFramerate(reason);
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
@@ -1003,6 +1161,8 @@ const ViEEncoder::AdaptCounter& ViEEncoder::GetConstAdaptCounter() {
 ViEEncoder::AdaptCounter::AdaptCounter() {
   fps_counters_.resize(kScaleReasonSize);
   resolution_counters_.resize(kScaleReasonSize);
+  RTC_DCHECK_EQ(fps_counters_.size(), 2) << "Update MoveCount()";
+  RTC_DCHECK_EQ(resolution_counters_.size(), 2) << "Update MoveCount()";
 }
 
 ViEEncoder::AdaptCounter::~AdaptCounter() {}
@@ -1021,12 +1181,34 @@ ViEEncoder::AdaptCounts ViEEncoder::AdaptCounter::Counts(int reason) const {
   return counts;
 }
 
-void ViEEncoder::AdaptCounter::IncrementFramerate(int reason, int delta) {
-  fps_counters_[reason] += delta;
+void ViEEncoder::AdaptCounter::IncrementFramerate(int reason) {
+  ++(fps_counters_[reason]);
 }
 
-void ViEEncoder::AdaptCounter::IncrementResolution(int reason, int delta) {
-  resolution_counters_[reason] += delta;
+void ViEEncoder::AdaptCounter::IncrementResolution(int reason) {
+  ++(resolution_counters_[reason]);
+}
+
+void ViEEncoder::AdaptCounter::DecrementFramerate(int reason) {
+  if (fps_counters_[reason] == 0) {
+    // Adapt up is in a different order, switch reason.
+    // E.g. framerate adapt down: quality, framerate adapt up: cpu.

[kthelgason, 2017/05/19 08:34:14]

Why is this the case? This seems very confusing.

[åsapersson, 2017/06/08 13:55:32]

Updated comment.

+    MoveCount(&resolution_counters_, reason);
+    MoveCount(&fps_counters_, (reason + 1) % kScaleReasonSize);
+  }
+  --(fps_counters_[reason]);
+  RTC_DCHECK_GE(fps_counters_[reason], 0);
+}
+
+void ViEEncoder::AdaptCounter::DecrementResolution(int reason) {
+  if (resolution_counters_[reason] == 0) {
+    // Adapt up is in a different order, switch reason.
+    // E.g. resolution adapt down: quality, resolution adapt up: cpu.
+    MoveCount(&fps_counters_, reason);
+    MoveCount(&resolution_counters_, (reason + 1) % kScaleReasonSize);
+  }
+  --(resolution_counters_[reason]);
+  RTC_DCHECK_GE(resolution_counters_[reason], 0);
 }
 
 int ViEEncoder::AdaptCounter::FramerateCount() const {
@@ -1057,6 +1239,14 @@ int ViEEncoder::AdaptCounter::Count(const std::vector<int>& counters) const {
   return std::accumulate(counters.begin(), counters.end(), 0);
 }
 
+void ViEEncoder::AdaptCounter::MoveCount(std::vector<int>* counters,
+                                         int from_reason) {
+  int to_reason = (from_reason + 1) % kScaleReasonSize;
+  ++((*counters)[to_reason]);
+  --((*counters)[from_reason]);
+  RTC_DCHECK_GE((*counters)[from_reason], 0);
+}
+
 std::string ViEEncoder::AdaptCounter::ToString(
     const std::vector<int>& counters) const {
   std::stringstream ss;