Change rtc::VideoSinkWants to have target and a max pixel count

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 171 matching lines @@
       // task queue.
       return;
     }
     // 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;
     sink_wants_.max_pixel_count = rtc::Optional<int>(pixels_wanted);
-    sink_wants_.max_pixel_count_step_up = rtc::Optional<int>();
+    sink_wants_.target_pixel_count = rtc::Optional<int>();

[perkj_webrtc, 2017/02/13 08:48:03]

nit: isn the word target_pixel_count  here equally confusing?  Naming is hard
and I dont have a strong prefens either or. 

[sprang_webrtc, 2017/02/13 09:29:20]

I think it makes sense, since we want the source to return frames at a
resolution as close as possible to target_pixel_count, rather than "a step up
from X, regardless of the step size". Since we control the step size down, it
makes sense to also try to control the step size up, imo.

     if (source_)
       source_->AddOrUpdateSink(vie_encoder_, sink_wants_);
   }
 
   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;
     }
     // The input video frame size will have a resolution with "one step up"
     // pixels than |max_pixel_count_step_up| where "one step up" depends on
-    // how the source can scale the input frame size.
-    sink_wants_.max_pixel_count = rtc::Optional<int>();
-    sink_wants_.max_pixel_count_step_up = rtc::Optional<int>(pixel_count);
+    // how the source can scale the input frame size. We still cap the step up
+    // to be at most twice the number of pixels.

[perkj_webrtc, 2017/02/13 08:48:03]

nope, you have limited to be 4* times the number  of pixels.

[sprang_webrtc, 2017/02/13 09:29:20]

This whole comment is misleading. I'll put up CL to clarify it.

+    sink_wants_.target_pixel_count = rtc::Optional<int>((pixel_count * 5) / 3);
+    sink_wants_.max_pixel_count = rtc::Optional<int>(pixel_count * 4);
     if (source_)
       source_->AddOrUpdateSink(vie_encoder_, sink_wants_);
   }
 
  private:
   bool IsResolutionScalingEnabledLocked() const
       EXCLUSIVE_LOCKS_REQUIRED(&crit_) {
     return degradation_preference_ !=
            DegradationPreference::kMaintainResolution;
   }
@@ skipping 389 matching lines @@
 
   EncodedImageCallback::Result result =
       sink_->OnEncodedImage(encoded_image, codec_specific_info, fragmentation);
 
   int64_t time_sent_us = rtc::TimeMicros();
   uint32_t timestamp = encoded_image._timeStamp;
   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);
-    if (quality_scaler_)
+    if (quality_scaler_ && qp >= 0)

[perkj_webrtc, 2017/02/13 08:48:03]

correct cl? Does this fix a bug?

[sprang_webrtc, 2017/02/13 09:29:20]

Yes, if fixes a bug but it only seems to be triggered by FakeEncoder, which
emits frames with qp = -1. It caused the test I added to fail because of
incorrect qp scaling. See earlier comment.

       quality_scaler_->ReportQP(qp);
   });
 
   return result;
 }
 
 void ViEEncoder::OnDroppedFrame() {
   encoder_queue_.PostTask([this] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
     if (quality_scaler_)
@@ skipping 68 matching lines @@
 
   if (stats_proxy_ && video_suspension_changed) {
     LOG(LS_INFO) << "Video suspend state changed to: "
                  << (video_is_suspended ? "suspended" : "not suspended");
     stats_proxy_->OnSuspendChange(video_is_suspended);
   }
 }
 
 void ViEEncoder::AdaptDown(AdaptReason reason) {
   RTC_DCHECK_RUN_ON(&encoder_queue_);
-  if (degradation_preference_ != DegradationPreference::kBalanced)
+  if (degradation_preference_ != DegradationPreference::kBalanced ||
+      !last_frame_info_) {

[perkj_webrtc, 2017/02/13 08:48:03]

Looks like a bug if last_frame_info_ = nullptr. How can we adapt unless a frame
have been received. 

[sprang_webrtc, 2017/02/13 09:29:20]

I think some test triggered that. I may want to figure out why and fix the test
+ DCHECK here instead.

     return;
-  // Request lower resolution if the current resolution is lower than last time
-  // we asked for the resolution to be lowered.
-  int current_pixel_count =
-      last_frame_info_ ? last_frame_info_->pixel_count() : 0;
-  if (max_pixel_count_ && current_pixel_count >= *max_pixel_count_)
+  }
+  int current_pixel_count = last_frame_info_->pixel_count();
+  if (last_adaptation_request_ &&
+      last_adaptation_request_->mode_ == AdaptationRequest::Mode::kAdaptDown &&
+      current_pixel_count >= last_adaptation_request_->input_pixel_count_) {
+    // Don't request lower resolution if the current resolution is not lower
+    // than the last time we asked for the resolution to be lowered.
     return;
+  }
+  last_adaptation_request_.emplace(AdaptationRequest{
+      current_pixel_count, AdaptationRequest::Mode::kAdaptDown});
+
   switch (reason) {
     case kQuality:
       stats_proxy_->OnQualityRestrictedResolutionChanged(
           scale_counter_[reason] + 1);
       break;
     case kCpu:
       if (scale_counter_[reason] >= kMaxCpuDowngrades)
         return;
       // Update stats accordingly.
       stats_proxy_->OnCpuRestrictedResolutionChanged(true);
       break;
   }
-  max_pixel_count_ = rtc::Optional<int>(current_pixel_count);
-  max_pixel_count_step_up_ = rtc::Optional<int>();
   ++scale_counter_[reason];
   source_proxy_->RequestResolutionLowerThan(current_pixel_count);
   LOG(LS_INFO) << "Scaling down resolution.";
   for (size_t i = 0; i < kScaleReasonSize; ++i) {
     LOG(LS_INFO) << "Scaled " << scale_counter_[i]
                  << " times for reason: " << (i ? "cpu" : "quality");
   }
 }
 
 void ViEEncoder::AdaptUp(AdaptReason reason) {
   RTC_DCHECK_RUN_ON(&encoder_queue_);
   if (scale_counter_[reason] == 0 ||
-      degradation_preference_ != DegradationPreference::kBalanced) {
+      degradation_preference_ != DegradationPreference::kBalanced ||
+      !last_frame_info_) {
     return;
   }
-  // Only scale if resolution is higher than last time
-  // we requested higher resolution.
-  int current_pixel_count =
-      last_frame_info_ ? last_frame_info_->pixel_count() : 0;
-  if (current_pixel_count <= max_pixel_count_step_up_.value_or(0))
+  // Only scale if resolution is higher than last time we requested higher
+  // resolution.
+  int current_pixel_count = last_frame_info_->pixel_count();
+  if (last_adaptation_request_ &&
+      last_adaptation_request_->mode_ == AdaptationRequest::Mode::kAdaptUp &&
+      current_pixel_count <= last_adaptation_request_->input_pixel_count_) {
+    // Don't request higher resolution if the current resolution is not higher
+    // than the last time we asked for the resolution to be higher.
     return;
+  }
+  last_adaptation_request_.emplace(AdaptationRequest{
+      current_pixel_count, AdaptationRequest::Mode::kAdaptUp});
+
   switch (reason) {
     case kQuality:
       stats_proxy_->OnQualityRestrictedResolutionChanged(
           scale_counter_[reason] - 1);
       break;
     case kCpu:
       // Update stats accordingly.
       stats_proxy_->OnCpuRestrictedResolutionChanged(scale_counter_[reason] >
                                                      1);
       break;
   }
-  max_pixel_count_ = rtc::Optional<int>();
-  max_pixel_count_step_up_ = rtc::Optional<int>(current_pixel_count);
   --scale_counter_[reason];
   source_proxy_->RequestHigherResolutionThan(current_pixel_count);
   LOG(LS_INFO) << "Scaling up resolution.";
   for (size_t i = 0; i < kScaleReasonSize; ++i) {
     LOG(LS_INFO) << "Scaled " << scale_counter_[i]
                  << " times for reason: " << (i ? "cpu" : "quality");
   }
 }
 
 }  // namespace webrtc