Fix VideoTrackAdapter issues with zero-size frames.

This patch makes VideoTrackAdapter ignore aspect-ratio limits when
the input or output frame has zero area.
Also, output dimensions are clamped to media::limits::kMaxDimension.

Before this CL, an input source with zero area (e.g., from element capture)
could result in undefined behavior when trying to correct the frame's
aspect ratio due to casting of infinite or NaN floating-point values to int.

Bug: 763676
Change-Id: I23794e7aba965ee8443bde5c36ef2adfe8cc2ba3
Reviewed-on: https://chromium-review.googlesource.com/663177
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501506}

content/renderer/media/video_track_adapter.cc

@@ -5,6 +5,7 @@
 #include "content/renderer/media/video_track_adapter.h"
 
 #include <algorithm>
+#include <cmath>
 #include <limits>
 #include <memory>
 #include <string>
@@ -21,6 +22,7 @@
 #include "build/build_config.h"
 #include "content/public/common/content_switches.h"
 #include "media/base/bind_to_current_loop.h"
+#include "media/base/limits.h"
 #include "media/base/video_util.h"
 
 namespace content {
@@ -51,6 +53,11 @@ void ResetCallbackOnMainRenderThread(
   // |callback| will be deleted when this exits.
 }
 
+int ClampToValidDimension(int dimension) {
+  return std::min(static_cast<int>(media::limits::kMaxDimension),
+                  std::max(0, dimension));
+}
+
 }  // anonymous namespace
 
 // VideoFrameResolutionAdapter is created on and lives on the IO-thread. It does
@@ -444,46 +451,49 @@ void VideoTrackAdapter::CalculateTargetSize(
     double min_aspect_ratio,
     double max_aspect_ratio,
     gfx::Size* desired_size) {
-  const gfx::Size& input_size =
-      is_rotated
-          ? gfx::Size(original_input_size.height(), original_input_size.width())
-          : original_input_size;
-
-  // If |frame| has larger width or height than requested, or the aspect ratio
-  // does not match the requested, we want to create a wrapped version of this
-  // frame with a size that fulfills the constraints.
-  const double input_ratio =
-      static_cast<double>(input_size.width()) / input_size.height();
-
-  if (input_size.width() > max_frame_size.width() ||
-      input_size.height() > max_frame_size.height() ||
-      input_ratio > max_aspect_ratio || input_ratio < min_aspect_ratio) {
-    int desired_width = std::min(max_frame_size.width(), input_size.width());
-    int desired_height = std::min(max_frame_size.height(), input_size.height());
-
-    const double resulting_ratio =
-        static_cast<double>(desired_width) / desired_height;
-    // Make sure |min_aspect_ratio| < |requested_ratio| < |max_aspect_ratio|.
-    const double requested_ratio =
-        std::max(std::min(resulting_ratio, max_aspect_ratio), min_aspect_ratio);
-
-    if (resulting_ratio < requested_ratio) {
-      desired_height = static_cast<int>((desired_height * resulting_ratio) /
-                                        requested_ratio);
-      // Make sure we scale to an even height to avoid rounding errors
-      desired_height = (desired_height + 1) & ~1;
-    } else if (resulting_ratio > requested_ratio) {
-      desired_width =
-          static_cast<int>((desired_width * requested_ratio) / resulting_ratio);
-      // Make sure we scale to an even width to avoid rounding errors.
-      desired_width = (desired_width + 1) & ~1;
+  DCHECK(!std::isnan(min_aspect_ratio));
+  DCHECK(!std::isnan(max_aspect_ratio));
+
+  // Perform all the cropping computations as if the device was never rotated.
+  int width =
+      is_rotated ? original_input_size.height() : original_input_size.width();
+  int height =
+      is_rotated ? original_input_size.width() : original_input_size.height();
+
+  // Adjust the size of the frame to the maximum allowed size.
+  width = ClampToValidDimension(std::min(width, max_frame_size.width()));
+  height = ClampToValidDimension(std::min(height, max_frame_size.height()));
+
+  // If the area of the frame is zero, ignore aspect-ratio correction.
+  if (width * height > 0) {
+    double ratio = static_cast<double>(width) / height;
+    DCHECK(std::isfinite(ratio));
+    if (ratio > max_aspect_ratio || ratio < min_aspect_ratio) {
+      // Make sure |min_aspect_ratio| <= |desired_ratio| <= |max_aspect_ratio|.
+      double desired_ratio =
+          std::max(std::min(ratio, max_aspect_ratio), min_aspect_ratio);
+      DCHECK(std::isfinite(desired_ratio));
+      DCHECK_NE(desired_ratio, 0.0);
+
+      if (ratio < desired_ratio) {
+        double desired_height_fp = (height * ratio) / desired_ratio;
+        DCHECK(std::isfinite(desired_height_fp));
+        height = static_cast<int>(desired_height_fp);
+        // Make sure we scale to an even height to avoid rounding errors
+        height = (height + 1) & ~1;
+      } else if (ratio > desired_ratio) {
+        double desired_width_fp = (width * desired_ratio) / ratio;
+        DCHECK(std::isfinite(desired_width_fp));
+        width = static_cast<int>(desired_width_fp);
+        // Make sure we scale to an even width to avoid rounding errors.
+        width = (width + 1) & ~1;
+      }
     }
-
-    *desired_size = is_rotated ? gfx::Size(desired_height, desired_width)
-                               : gfx::Size(desired_width, desired_height);
-  } else {
-    *desired_size = original_input_size;
   }
+
+  // Output back taking device rotation into account.
+  *desired_size =
+      is_rotated ? gfx::Size(height, width) : gfx::Size(width, height);
 }
 
 void VideoTrackAdapter::StartFrameMonitoringOnIO(

content/renderer/media/video_track_adapter.h

@@ -13,6 +13,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
+#include "content/common/content_export.h"
 #include "content/renderer/media/media_stream_video_track.h"
 #include "media/base/video_frame.h"
 #include "ui/gfx/geometry/size.h"
@@ -85,12 +86,14 @@ class VideoTrackAdapter
 
   void SetSourceFrameSize(const gfx::Size& source_frame_size);
 
-  static void CalculateTargetSize(bool is_rotated,
-                                  const gfx::Size& input_size,
-                                  const gfx::Size& max_frame_size,
-                                  double min_aspect_ratio,
-                                  double max_aspect_ratio,
-                                  gfx::Size* desired_size);
+  // Exported for testing.
+  CONTENT_EXPORT static void CalculateTargetSize(
+      bool is_rotated,
+      const gfx::Size& input_size,
+      const gfx::Size& max_frame_size,
+      double min_aspect_ratio,
+      double max_aspect_ratio,
+      gfx::Size* desired_size);
 
  private:
   virtual ~VideoTrackAdapter();

content/renderer/media/video_track_adapter_unittest.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/media/video_track_adapter.h"
+
+#include <limits>
+
+#include "media/base/limits.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+// Most VideoTrackAdapter functionality is tested in MediaStreamVideoSourceTest.
+// These tests focus on the computation of cropped frame sizes in edge cases
+// that cannot be easily reproduced by a mocked video source, such as tests
+// involving frames of zero size.
+// Such frames can be produced by sources in the wild (e.g., element capture).
+
+// Test that cropped sizes with zero-area input frames are correctly computed.
+// Aspect ratio limits should be ignored.
+TEST(VideoTrackAdapterTest, ZeroInputArea) {
+  const double kMinAspectRatio = 0.1;
+  const double kMaxAspectRatio = 2.0;
+  const int kMaxWidth = 640;
+  const int kMaxHeight = 480;
+  const bool kIsRotatedValues[] = {true, false};
+
+  for (bool is_rotated : kIsRotatedValues) {
+    gfx::Size desired_size;
+
+    VideoTrackAdapter::CalculateTargetSize(
+        is_rotated, gfx::Size(0, 0), gfx::Size(kMaxWidth, kMaxHeight),
+        kMinAspectRatio, kMaxAspectRatio, &desired_size);
+    EXPECT_EQ(desired_size.width(), 0);
+    EXPECT_EQ(desired_size.height(), 0);
+
+    // Zero width.
+    VideoTrackAdapter::CalculateTargetSize(
+        is_rotated, gfx::Size(0, 300), gfx::Size(kMaxWidth, kMaxHeight),
+        kMinAspectRatio, kMaxAspectRatio, &desired_size);
+    EXPECT_EQ(desired_size.width(), 0);
+    EXPECT_EQ(desired_size.height(), 300);
+
+    // Zero height.
+    VideoTrackAdapter::CalculateTargetSize(
+        is_rotated, gfx::Size(300, 0), gfx::Size(kMaxWidth, kMaxHeight),
+        kMinAspectRatio, kMaxAspectRatio, &desired_size);
+    EXPECT_EQ(desired_size.width(), 300);
+    EXPECT_EQ(desired_size.height(), 0);
+
+    // Requires "cropping" of height.
+    VideoTrackAdapter::CalculateTargetSize(
+        is_rotated, gfx::Size(0, 1000), gfx::Size(kMaxWidth, kMaxHeight),
+        kMinAspectRatio, kMaxAspectRatio, &desired_size);
+    EXPECT_EQ(desired_size.width(), 0);
+    EXPECT_EQ(desired_size.height(), is_rotated ? kMaxWidth : kMaxHeight);
+
+    // Requires "cropping" of width.
+    VideoTrackAdapter::CalculateTargetSize(
+        is_rotated, gfx::Size(1000, 0), gfx::Size(kMaxWidth, kMaxHeight),
+        kMinAspectRatio, kMaxAspectRatio, &desired_size);
+    EXPECT_EQ(desired_size.width(), is_rotated ? kMaxHeight : kMaxWidth);
+    EXPECT_EQ(desired_size.height(), 0);
+  }
+}
+
+// Test that zero-size cropped areas are correctly computed. Aspect ratio
+// limits should be ignored.
+TEST(VideoTrackAdapterTest, ZeroOutputArea) {
+  const double kMinAspectRatio = 0.1;
+  const double kMaxAspectRatio = 2.0;
+  const int kInputWidth = 640;
+  const int kInputHeight = 480;
+
+  gfx::Size desired_size;
+
+  VideoTrackAdapter::CalculateTargetSize(
+      false /* is_rotated */, gfx::Size(kInputWidth, kInputHeight),
+      gfx::Size(0, 0), kMinAspectRatio, kMaxAspectRatio, &desired_size);
+  EXPECT_EQ(desired_size.width(), 0);
+  EXPECT_EQ(desired_size.height(), 0);
+
+  // Width is cropped to zero.
+  VideoTrackAdapter::CalculateTargetSize(
+      false /* is_rotated */, gfx::Size(kInputWidth, kInputHeight),
+      gfx::Size(0, 1000), kMinAspectRatio, kMaxAspectRatio, &desired_size);
+  EXPECT_EQ(desired_size.width(), 0);
+  EXPECT_EQ(desired_size.height(), kInputHeight);
+
+  // Requires "cropping" of width and height.
+  VideoTrackAdapter::CalculateTargetSize(
+      false /* is_rotated */, gfx::Size(kInputWidth, kInputHeight),
+      gfx::Size(0, 300), kMinAspectRatio, kMaxAspectRatio, &desired_size);
+  EXPECT_EQ(desired_size.width(), 0);
+  EXPECT_EQ(desired_size.height(), 300);
+
+  // Height is cropped to zero.
+  VideoTrackAdapter::CalculateTargetSize(
+      false /* is_rotated */, gfx::Size(kInputWidth, kInputHeight),
+      gfx::Size(1000, 0), kMinAspectRatio, kMaxAspectRatio, &desired_size);
+  EXPECT_EQ(desired_size.width(), kInputWidth);
+  EXPECT_EQ(desired_size.height(), 0);
+
+  // Requires "cropping" of width and height.
+  VideoTrackAdapter::CalculateTargetSize(
+      false /* is_rotated */, gfx::Size(kInputWidth, kInputHeight),
+      gfx::Size(300, 0), kMinAspectRatio, kMaxAspectRatio, &desired_size);
+  EXPECT_EQ(desired_size.width(), 300);
+  EXPECT_EQ(desired_size.height(), 0);
+}
+
+// Test that large frame sizes are clamped to the maximum supported dimension.
+TEST(VideoTrackAdapterTest, ClampToMaxDimension) {
+  const double kMinAspectRatio = 0.0;
+  const double kMaxAspectRatio = HUGE_VAL;
+  const int kInputWidth = std::numeric_limits<int>::max();
+  const int kInputHeight = std::numeric_limits<int>::max();
+  const int kMaxWidth = std::numeric_limits<int>::max();
+  const int kMaxHeight = std::numeric_limits<int>::max();
+
+  gfx::Size desired_size;
+
+  VideoTrackAdapter::CalculateTargetSize(
+      false /* is_rotated */, gfx::Size(kInputWidth, kInputHeight),
+      gfx::Size(kMaxWidth, kMaxHeight), kMinAspectRatio, kMaxAspectRatio,
+      &desired_size);
+  EXPECT_EQ(desired_size.width(), media::limits::kMaxDimension);
+  EXPECT_EQ(desired_size.height(), media::limits::kMaxDimension);
+}
+
+}  // namespace content

content/test/BUILD.gn

@@ -1726,6 +1726,7 @@ test("content_unittests") {
       "../renderer/media/rtc_peer_connection_handler_unittest.cc",
       "../renderer/media/speech_recognition_audio_sink_unittest.cc",
       "../renderer/media/user_media_client_impl_unittest.cc",
+      "../renderer/media/video_track_adapter_unittest.cc",
       "../renderer/media/webmediaplayer_ms_unittest.cc",
       "../renderer/media/webrtc/media_stream_remote_video_source_unittest.cc",
       "../renderer/media/webrtc/media_stream_track_metrics_unittest.cc",