Add ability to scale to arbitrary factors

webrtc/media/base/videoadapter.cc

 /*
  *  Copyright (c) 2010 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.
  */
 
 #include "webrtc/media/base/videoadapter.h"
 
 #include <algorithm>
+#include <cmath>
 #include <cstdlib>
 #include <limits>
 
+#include "webrtc/base/arraysize.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/base/optional.h"
 #include "webrtc/media/base/mediaconstants.h"
 #include "webrtc/media/base/videocommon.h"
 
 namespace {
-
 struct Fraction {
   int numerator;
   int denominator;
 };
 
 // Scale factors optimized for in libYUV that we accept.
 // Must be sorted in decreasing scale factors for FindScaleLargerThan to work.
 const Fraction kScaleFractions[] = {
   {1, 1},
   {3, 4},
   {1, 2},
   {3, 8},
   {1, 4},
   {3, 16},
 };
 
 // Round |valueToRound| to a multiple of |multiple|. Prefer rounding upwards,
 // but never more than |maxValue|.
 int roundUp(int valueToRound, int multiple, int maxValue) {

[magjed_webrtc, 2016/12/07 13:44:51]

nit: I know it's unrelated but can you please clean up my naming mistake, i.e.
rename |valueToRound| to |value_to_round| and same for |maxValue|.

[kthelgason, 2016/12/08 08:42:03]

Done.

   const int roundedValue = (valueToRound + multiple - 1) / multiple * multiple;
   return roundedValue <= maxValue ? roundedValue
                                   : (maxValue / multiple * multiple);
 }
 
 Fraction FindScaleLessThanOrEqual(int input_num_pixels, int target_num_pixels) {
+  // Start searching from the last of the optimal fractions;

[magjed_webrtc, 2016/12/07 13:44:51]

It would be nice with a high level comment explaining what this functions does.

[kthelgason, 2016/12/08 08:42:03]

Done.

+  Fraction best_scale = kScaleFractions[arraysize(kScaleFractions) - 1];
+  while (true) {
+    const float scale =
+        best_scale.numerator / static_cast<float>(best_scale.denominator);
+    float test_num_pixels = input_num_pixels * scale * scale;
+    if (target_num_pixels >= test_num_pixels) {

[magjed_webrtc, 2016/12/07 13:44:51]

Can we do like this instead:
const float target_scale =
    sqrt(target_num_pixels / static_cast<float>(input_num_pixels));
Fraction best_scale = kScaleFractions[arraysize(kScaleFractions) - 1];
while (best_scale.numerator > target_scale * best_scale.denominator) {
  ...
}
?

[kthelgason, 2016/12/08 08:42:03]

Done.

+      if (best_scale.numerator == 1) {

[magjed_webrtc, 2016/12/07 13:44:51]

nit: I think this makes the intent clearer:
if (best_scale.numerator % 3 == 0 && best_scale.denominator % 2 == 0) {
  // Multiply with 2/3.
  best_scale.numerator /= 3;
  best_scale.denominator /= 2;
} else {
  // Multiply with 3/4.
  best_scale.numerator *= 3;
  best_scale.denominator *= 4;
} 

[kthelgason, 2016/12/08 08:42:03]

Done.

+        best_scale.numerator = 3;
+        best_scale.denominator *= 4;
+      } else {
+        best_scale.numerator = 1;
+        best_scale.denominator /= 2;
+      }
+    } else {
+      return best_scale;
+    }
+  }
+}
+
+rtc::Optional<Fraction> FindOptimizedScaleLessThanOrEqual(
+    int input_num_pixels,
+    int target_num_pixels) {
   float best_distance = std::numeric_limits<float>::max();
-  Fraction best_scale = {0, 1};  // Default to 0 if nothing matches.
+  rtc::Optional<Fraction> best_scale;
   for (const auto& fraction : kScaleFractions) {
     const float scale =
         fraction.numerator / static_cast<float>(fraction.denominator);
     float test_num_pixels = input_num_pixels * scale * scale;
     float diff = target_num_pixels - test_num_pixels;
     if (diff < 0) {
       continue;
     }
     if (diff < best_distance) {
       best_distance = diff;
-      best_scale = fraction;
+      best_scale = rtc::Optional<Fraction>(fraction);
       if (best_distance == 0) {  // Found exact match.
         break;
       }
     }
   }
   return best_scale;
 }
 
-Fraction FindScaleLargerThan(int input_num_pixels,
-                             int target_num_pixels,
-                             int* resulting_number_of_pixels) {
+Fraction FindOptimizedScaleLargerThan(int input_num_pixels,
+                                      int target_num_pixels,
+                                      int* resulting_number_of_pixels) {
   float best_distance = std::numeric_limits<float>::max();
   Fraction best_scale = {1, 1};  // Default to unscaled if nothing matches.
   // Default to input number of pixels.
   float best_number_of_pixels = input_num_pixels;
   for (const auto& fraction : kScaleFractions) {
     const float scale =
         fraction.numerator / static_cast<float>(fraction.denominator);
     float test_num_pixels = input_num_pixels * scale * scale;
     float diff = test_num_pixels - target_num_pixels;
     if (diff <= 0) {
       break;
     }
     if (diff < best_distance) {
       best_distance = diff;
       best_scale = fraction;
       best_number_of_pixels = test_num_pixels;
     }
   }
 
   *resulting_number_of_pixels = static_cast<int>(best_number_of_pixels + .5f);
   return best_scale;
 }
 
+rtc::Optional<Fraction> FindOptimizedScale(int input_num_pixels,
+                                           int max_pixel_count_step_up,
+                                           int max_pixel_count) {
+  // Try scale just above |max_pixel_count_step_up_|.

[magjed_webrtc, 2016/12/07 13:44:51]

I find the code in the existing FindOptimizedScaleLessThanOrEqual and
FindOptimizedScaleLargerThan really ugly. I think it would be cleaner to remove
them and do std::upper_bound on kScaleFractions directly here + logic for
handling the max_pixel_count_step_up.

But this is totally unrelated so do it only if you want and have time.

[kthelgason, 2016/12/08 08:42:03]

I agree that this code is more complex than it needs to be, but in the interest
of keeping this CL small and landing it sooner as is does fix an actual crash
I'd like to do it in a follow-up CL. I filed 6848 and assigned to me to track
it.

+  if (max_pixel_count_step_up > 0) {
+    int resulting_pixel_count;
+    const Fraction scale = FindOptimizedScaleLargerThan(
+        input_num_pixels, max_pixel_count_step_up, &resulting_pixel_count);
+    if (resulting_pixel_count <= max_pixel_count)
+      return rtc::Optional<Fraction>(scale);
+  }
+  // Return largest scale below |max_pixel_count|.
+  return FindOptimizedScaleLessThanOrEqual(input_num_pixels, max_pixel_count);
+}
+
 Fraction FindScale(int input_num_pixels,
                    int max_pixel_count_step_up,
                    int max_pixel_count) {
-  // Try scale just above |max_pixel_count_step_up_|.
-  if (max_pixel_count_step_up > 0) {
-    int resulting_pixel_count;
-    const Fraction scale = FindScaleLargerThan(
-        input_num_pixels, max_pixel_count_step_up, &resulting_pixel_count);
-    if (resulting_pixel_count <= max_pixel_count)
-      return scale;
-  }
-  // Return largest scale below |max_pixel_count|.
+  const rtc::Optional<Fraction> optimized_scale = FindOptimizedScale(
+      input_num_pixels, max_pixel_count_step_up, max_pixel_count);
+  if (optimized_scale)
+    return *optimized_scale;
   return FindScaleLessThanOrEqual(input_num_pixels, max_pixel_count);
 }
-
 }  // namespace
 
 namespace cricket {
 
-VideoAdapter::VideoAdapter()
+VideoAdapter::VideoAdapter(int required_resolution_divisor)
     : frames_in_(0),
       frames_out_(0),
       frames_scaled_(0),
       adaption_changes_(0),
       previous_width_(0),
       previous_height_(0),
+      required_resolution_divisor_(required_resolution_divisor),
       resolution_request_max_pixel_count_(std::numeric_limits<int>::max()),
       resolution_request_max_pixel_count_step_up_(0) {}
 
+VideoAdapter::VideoAdapter() : VideoAdapter(1) {}
+
 VideoAdapter::~VideoAdapter() {}
 
 bool VideoAdapter::KeepFrame(int64_t in_timestamp_ns) {
   rtc::CritScope cs(&critical_section_);
   if (!requested_format_ || requested_format_->interval == 0)
     return true;
 
   if (next_frame_timestamp_ns_) {
     // Time until next frame should be outputted.
     const int64_t time_until_next_frame_ns =
@@ skipping 69 matching lines @@
       std::swap(requested_format_->width, requested_format_->height);
     }
     const float requested_aspect =
         requested_format_->width /
         static_cast<float>(requested_format_->height);
     *cropped_width =
         std::min(in_width, static_cast<int>(in_height * requested_aspect));
     *cropped_height =
         std::min(in_height, static_cast<int>(in_width / requested_aspect));
   }
-
-  // Find best scale factor.
   const Fraction scale =
       FindScale(*cropped_width * *cropped_height,
                 resolution_request_max_pixel_count_step_up_, max_pixel_count);
-
   // Adjust cropping slightly to get even integer output size and a perfect
-  // scale factor.
-  *cropped_width = roundUp(*cropped_width, scale.denominator, in_width);
-  *cropped_height = roundUp(*cropped_height, scale.denominator, in_height);
-  RTC_DCHECK_EQ(0, *cropped_width % scale.denominator);
-  RTC_DCHECK_EQ(0, *cropped_height % scale.denominator);
+  // scale factor. Make sure the resulting dimensions are a multiple of 4

[magjed_webrtc, 2016/12/07 13:44:51]

Update the comment.

[kthelgason, 2016/12/08 08:42:03]

Done.

+  // to be nice to hardware encoders.
+  *cropped_width =
+      roundUp(*cropped_width, scale.denominator * required_resolution_divisor_,
+              in_width);
+  *cropped_height =
+      roundUp(*cropped_height, scale.denominator * required_resolution_divisor_,
+              in_height);
+  RTC_DCHECK_EQ(

[magjed_webrtc, 2016/12/07 13:44:51]

I think it's more clear to do:
RTC_DCHECK_EQ(0, *cropped_width % scale.denominator);
and
RTC_DCHECK_EQ(0, *out_width %  required_resolution_divisor_);

[kthelgason, 2016/12/08 08:42:03]

Done.

+      0, *cropped_width % scale.denominator * required_resolution_divisor_);
+  RTC_DCHECK_EQ(
+      0, *cropped_height % scale.denominator * required_resolution_divisor_);
 
   // Calculate final output size.
   *out_width = *cropped_width / scale.denominator * scale.numerator;
   *out_height = *cropped_height / scale.denominator * scale.numerator;
-
   ++frames_out_;
   if (scale.numerator != scale.denominator)
     ++frames_scaled_;
 
   if (previous_width_ && (previous_width_ != *out_width ||
                           previous_height_ != *out_height)) {
     ++adaption_changes_;
     LOG(LS_INFO) << "Frame size changed: scaled " << frames_scaled_ << " / out "
                  << frames_out_ << " / in " << frames_in_
                  << " Changes: " << adaption_changes_ << " Input: " << in_width
@@ skipping 19 matching lines @@
     rtc::Optional<int> max_pixel_count,
     rtc::Optional<int> max_pixel_count_step_up) {
   rtc::CritScope cs(&critical_section_);
   resolution_request_max_pixel_count_ =
       max_pixel_count.value_or(std::numeric_limits<int>::max());
   resolution_request_max_pixel_count_step_up_ =
       max_pixel_count_step_up.value_or(0);
 }
 
 }  // namespace cricket