Add ability to scale to arbitrary factors

webrtc/media/base/videoadapter.cc

Index: webrtc/media/base/videoadapter.cc
diff --git a/webrtc/media/base/videoadapter.cc b/webrtc/media/base/videoadapter.cc
index acb0e2c1a19888d758cd2d46893fca9e2b93a477..748843a3e27b08851ae310d9ed2a6c5ed5470abf 100644
--- a/webrtc/media/base/videoadapter.cc
+++ b/webrtc/media/base/videoadapter.cc
@@ -11,16 +11,18 @@
 #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;
@@ -46,8 +48,31 @@ int roundUp(int valueToRound, int multiple, int maxValue) {
 }
 
 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);
@@ -58,7 +83,7 @@ Fraction FindScaleLessThanOrEqual(int input_num_pixels, int target_num_pixels) {
     }
     if (diff < best_distance) {
       best_distance = diff;
-      best_scale = fraction;
+      best_scale = rtc::Optional<Fraction>(fraction);
       if (best_distance == 0) {  // Found exact match.
         break;
       }
@@ -67,9 +92,9 @@ Fraction FindScaleLessThanOrEqual(int input_num_pixels, int target_num_pixels) {
   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.
@@ -93,35 +118,47 @@ Fraction FindScaleLargerThan(int input_num_pixels,
   return best_scale;
 }
 
-Fraction FindScale(int input_num_pixels,
-                   int max_pixel_count_step_up,
-                   int max_pixel_count) {
+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 = FindScaleLargerThan(
+    const Fraction scale = FindOptimizedScaleLargerThan(
         input_num_pixels, max_pixel_count_step_up, &resulting_pixel_count);
     if (resulting_pixel_count <= max_pixel_count)
-      return scale;
+      return rtc::Optional<Fraction>(scale);
   }
   // Return largest scale below |max_pixel_count|.
-  return FindScaleLessThanOrEqual(input_num_pixels, 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) {
+  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) {
@@ -211,23 +248,26 @@ bool VideoAdapter::AdaptFrameResolution(int in_width,
     *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_;