Compare adapt up/down request with sink_wants_ in VideoSourceProxy methods to make sure it is higher

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 212 matching lines @@
         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;
   }
 
   bool RequestResolutionLowerThan(int pixel_count) {
     // Called on the encoder task queue.
     rtc::CritScope lock(&crit_);
-    if (!IsResolutionScalingEnabledLocked()) {
+    if (!source_ || !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 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.
+    // 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)
+    if (pixels_wanted < kMinPixelsPerFrame ||
+        pixels_wanted >= sink_wants_.max_pixel_count) {
       return false;
-
+    }
+    LOG(LS_INFO) << "Scaling down resolution.";

[sprang_webrtc, 2017/05/29 14:45:47]

Might be nice with the actual pixel counts in the log message too? Here and
below.

[åsapersson, 2017/05/30 13:57:31]

Done.

     sink_wants_.max_pixel_count = pixels_wanted;
     sink_wants_.target_pixel_count = rtc::Optional<int>();
-    if (source_)
-      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
     return true;
   }
 
-  void RequestFramerateLowerThan(int framerate_fps) {
+  bool RequestFramerateLowerThan(int fps) {
+    // Called on the encoder task queue.

[sprang_webrtc, 2017/05/29 14:45:47]

Can we add thread/sequence checkers for this maybe? here and elsewhere

[åsapersson, 2017/05/30 13:57:30]

I can have a look but maybe in a separate cl?

[sprang_webrtc, 2017/05/31 08:44:30]

Sure, maybe you can add it to one of the upcoming ones?

+    // The input video frame rate will be scaled down to 2/3, rounding down.
+    return RestrictFramerate((fps * 2) / 3);
+  }
+
+  bool RequestHigherResolutionThan(int pixel_count) {
     // Called on the encoder task queue.
     rtc::CritScope lock(&crit_);
-    if (!IsFramerateScalingEnabledLocked()) {
+    if (!source_ || !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 rate will be scaled down to 2/3 of input fps,
-    // rounding down.
-    const int framerate_wanted =
-        std::max(kMinFramerateFps, (framerate_fps * 2) / 3);
-    sink_wants_.max_framerate_fps = framerate_wanted;
-    if (source_)
-      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
-  }
+    int max_pixels_wanted = pixel_count;
+    if (max_pixels_wanted != std::numeric_limits<int>::max())
+      max_pixels_wanted = pixel_count * 4;
 
-  void RequestHigherResolutionThan(int pixel_count) {
-    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;
-    }
+    if (max_pixels_wanted <= sink_wants_.max_pixel_count)
+      return false;

[sprang_webrtc, 2017/05/29 14:45:47]

When would this happen? Should we reset sink_wants_.max_pixel_count in that
case?

[åsapersson, 2017/05/30 13:57:31]

In balanced mode (https://codereview.webrtc.org/2887303003/),
last_adaptation_request_ is not used. Equal pixel count can happen if
AdaptUp/AdaptDown is called frequently. The intention is for it to work similar
to last_adaptation_request_ (and maybe later remove last_adaptation_request_ if
possible).

[sprang_webrtc, 2017/05/31 08:44:31]

Ack. Maybe add comment about that?

 
-    if (pixel_count == std::numeric_limits<int>::max()) {
+    sink_wants_.max_pixel_count = max_pixels_wanted;
+    if (max_pixels_wanted == std::numeric_limits<int>::max()) {
       // Remove any constraints.
       sink_wants_.target_pixel_count.reset();
-      sink_wants_.max_pixel_count = std::numeric_limits<int>::max();
     } else {
       // On step down we request at most 3/5 the pixel count of the previous
       // resolution, so in order to take "one step up" we request a resolution
       // as close as possible to 5/3 of the current resolution. The actual pixel
       // count selected depends on the capabilities of the source. In order to
       // not take a too large step up, we cap the requested pixel count to be at
       // most four time the current number of pixels.
       sink_wants_.target_pixel_count =
           rtc::Optional<int>((pixel_count * 5) / 3);
-      sink_wants_.max_pixel_count = pixel_count * 4;
     }
-    if (source_)
-      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    LOG(LS_INFO) << "Scaling up resolution.";
+    source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
   }
 
-  void RequestHigherFramerateThan(int framerate_fps) {
+  bool 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);
+  }
+
+  bool RestrictFramerate(int 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;
-    }
-    if (framerate_fps == std::numeric_limits<int>::max()) {
-      // Remove any restrains.
-      sink_wants_.max_framerate_fps = std::numeric_limits<int>::max();
-    } else {
-      // The input video frame rate will be scaled up to the last step, with
-      // rounding.
-      const int framerate_wanted = (framerate_fps * 3) / 2;
-      sink_wants_.max_framerate_fps = framerate_wanted;
-    }
-    if (source_)
-      source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    if (!source_ || !IsFramerateScalingEnabledLocked())
+      return false;
+
+    const int fps_wanted = std::max(kMinFramerateFps, fps);
+    if (fps_wanted >= sink_wants_.max_framerate_fps)
+      return false;
+
+    LOG(LS_INFO) << "Scaling down framerate: " << fps_wanted;
+    sink_wants_.max_framerate_fps = fps_wanted;
+    source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
+  }
+
+  bool IncreaseFramerate(int fps) {
+    // Called on the encoder task queue.
+    rtc::CritScope lock(&crit_);
+    if (!source_ || !IsFramerateScalingEnabledLocked())
+      return false;
+
+    const int fps_wanted = std::max(kMinFramerateFps, fps);
+    if (fps_wanted <= sink_wants_.max_framerate_fps)
+      return false;
+
+    LOG(LS_INFO) << "Scaling up framerate: " << fps_wanted;
+    sink_wants_.max_framerate_fps = fps_wanted;
+    source_->AddOrUpdateSink(vie_encoder_, GetActiveSinkWants());
+    return true;
   }
 
  private:
   bool IsResolutionScalingEnabledLocked() const
       EXCLUSIVE_LOCKS_REQUIRED(&crit_) {
     return degradation_preference_ ==
                VideoSendStream::DegradationPreference::kMaintainFramerate ||
            degradation_preference_ ==
                VideoSendStream::DegradationPreference::kBalanced;
   }
@@ skipping 523 matching lines @@
 
   if (reason == kCpu) {
     if (GetConstAdaptCounter().TotalCount(kCpu) >= max_downgrades)
       return;
   }
 
   switch (degradation_preference_) {
     case VideoSendStream::DegradationPreference::kBalanced:
       FALLTHROUGH();
     case VideoSendStream::DegradationPreference::kMaintainFramerate:
+      // Scale down resolution.
       if (!source_proxy_->RequestResolutionLowerThan(
               adaptation_request.input_pixel_count_)) {
         return;
       }
-      LOG(LS_INFO) << "Scaling down resolution.";
       GetAdaptCounter().IncrementResolution(reason, 1);
       break;
     case VideoSendStream::DegradationPreference::kMaintainResolution:
-      source_proxy_->RequestFramerateLowerThan(
-          adaptation_request.framerate_fps_);
-      LOG(LS_INFO) << "Scaling down framerate.";
+      // Scale down framerate.
+      if (!source_proxy_->RequestFramerateLowerThan(
+              adaptation_request.framerate_fps_)) {
+        return;
+      }
       GetAdaptCounter().IncrementFramerate(reason, 1);
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
   }
 
   last_adaptation_request_.emplace(adaptation_request);
 
   UpdateAdaptationStats(reason);
 
@@ skipping 34 matching lines @@
       // TODO(sprang): Don't request higher framerate if we are already at
       // max requested fps?
       break;
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       return;
   }
 
   switch (degradation_preference_) {
     case VideoSendStream::DegradationPreference::kBalanced:
       FALLTHROUGH();
-    case VideoSendStream::DegradationPreference::kMaintainFramerate:
-      if (adapt_counter.TotalCount() == 1) {
+    case VideoSendStream::DegradationPreference::kMaintainFramerate: {
+      // Scale up resolution.
+      int pixel_count = adaptation_request.input_pixel_count_;
+      if (adapt_counter.ResolutionCount() == 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.";
+        pixel_count = std::numeric_limits<int>::max();
       }
+      if (!source_proxy_->RequestHigherResolutionThan(pixel_count))
+        return;
       GetAdaptCounter().IncrementResolution(reason, -1);
       break;
-    case VideoSendStream::DegradationPreference::kMaintainResolution:
-      if (adapt_counter.TotalCount() == 1) {
+    }
+    case VideoSendStream::DegradationPreference::kMaintainResolution: {
+      // Scale up framerate.
+      int fps = adaptation_request.framerate_fps_;
+      if (adapt_counter.FramerateCount() == 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.";
+        fps = std::numeric_limits<int>::max();
       }
+      if (!source_proxy_->RequestHigherFramerateThan(fps))
+        return;
       GetAdaptCounter().IncrementFramerate(reason, -1);
       break;
+    }
     case VideoSendStream::DegradationPreference::kDegradationDisabled:
       RTC_NOTREACHED();
   }
 
   last_adaptation_request_.emplace(adaptation_request);
 
   UpdateAdaptationStats(reason);
 
   LOG(LS_INFO) << adapt_counter.ToString();
 }
@@ skipping 103 matching lines @@
 std::string ViEEncoder::AdaptCounter::ToString(
     const std::vector<int>& counters) const {
   std::stringstream ss;
   for (size_t reason = 0; reason < kScaleReasonSize; ++reason) {
     ss << (reason ? " cpu" : "quality") << ":" << counters[reason];
   }
   return ss.str();
 }
 
 }  // namespace webrtc