Fix SurfaceTexture DecomposeTransformMatrix

This CL handles adds handling of few more cases:
* Small videos have bad matrix. We get small sizes likely because
  decoding errors
* Shrink amount other than 1.0. This is best guess approach as there is
  no way knowing what shrink amount was used.
* Empty matrix case (when either of scales is zero). Shouldn't happen
  outside of small size case.

This CL also add fallback to visible_size if coded_size/visible_rect
seem wrong or matrix was broken and adds DumpWithoutCrashing for that
case.

Bug: 1081695, 1086624, 1076564
Change-Id: Ife0b70bc85b9249fa2805df1db59733ac8cdbd53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216731
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772493}

gpu/command_buffer/service/surface_texture_gl_owner.cc

@@ -9,6 +9,8 @@
 #include "base/android/scoped_hardware_buffer_fence_sync.h"
 #include "base/bind.h"
 #include "base/check_op.h"
+#include "base/debug/alias.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/notreached.h"
@@ -123,17 +125,55 @@ void SurfaceTextureGLOwner::GetCodedSizeAndVisibleRect(
   float mtx[16];
   surface_texture_->GetTransformMatrix(mtx);
 
-  DecomposeTransform(mtx, rotated_visible_size, coded_size, visible_rect);
+  bool result =
+      DecomposeTransform(mtx, rotated_visible_size, coded_size, visible_rect);
+
+  constexpr gfx::Rect kMaxRect(16536, 16536);
+  gfx::Rect coded_rect(*coded_size);
+
+  if (!result || !coded_rect.Contains(*visible_rect) ||
+      !kMaxRect.Contains(coded_rect)) {
+    // Save old values for minidump.
+    gfx::Size coded_size_for_debug = *coded_size;
+    gfx::Rect visible_rect_for_debug = *visible_rect;
+
+    // Sanitize returning values to avoid crashes.
+    *coded_size = rotated_visible_size;
+    *visible_rect = gfx::Rect(rotated_visible_size);
+
+    // Alias values to prevent optimization out and do logging/dump.
+    base::debug::Alias(mtx);
+    base::debug::Alias(&coded_size_for_debug);
+    base::debug::Alias(&visible_rect_for_debug);
+
+    LOG(ERROR) << "Wrong matrix decomposition: coded: "
+               << coded_size_for_debug.ToString()
+               << "visible: " << visible_rect_for_debug.ToString()
+               << "matrix: " << mtx[0] << ", " << mtx[1] << ", " << mtx[4]
+               << ", " << mtx[5] << ", " << mtx[12] << ", " << mtx[13];
+
+    base::debug::DumpWithoutCrashing();
+  }
 }
 
 // static
-void SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
+bool SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
                                                gfx::Size rotated_visible_size,
                                                gfx::Size* coded_size,
                                                gfx::Rect* visible_rect) {
   DCHECK(coded_size);
   DCHECK(visible_rect);
 
+  // Due to shrinking of visible rect by 1 pixels from each side matrix can be
+  // zero for visible sizes less then 4x4.
+  if (rotated_visible_size.width() < 4 || rotated_visible_size.height() < 4) {
+    *coded_size = rotated_visible_size;
+    *visible_rect = gfx::Rect(rotated_visible_size);
+
+    // Note as this is expected case we return true, to avoid crash dump above.
+    return true;
+  }
+
   float sx, sy;
   *visible_rect = gfx::Rect();
   // The matrix is in column order and contains transform of (0, 0)x(1, 1)
@@ -176,6 +216,11 @@ void SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
   sx = std::abs(sx);
   sy = std::abs(sy);
 
+  // We got zero matrix, so we can't decompose anything.
+  if (!sx || !sy) {
+    return false;
+  }
+
   *coded_size = visible_rect->size();
 
   // Note: Below calculation is reverse operation of computing matrix in
@@ -185,27 +230,33 @@ void SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
   // sampling the texture.
 
   // In order to prevent bilinear sampling beyond the edge of the
-  // crop rectangle was shrunk by 2 texels in each dimension.  Normally this
-  // would just need to take 1/2 a texel off each end, but because the chroma
-  // channels of YUV420 images are subsampled we may need to shrink the crop
-  // region by a whole texel on each side. As format is not known here we assume
-  // the worst case.
-  const float shrink_amount = 1.0f;
-
-  // TODO(crbug.com/1076564): Revisit if we need to handle it gracefully.
-  CHECK(sx);
-  CHECK(sy);
-
-  if (sx < 1.0f) {
-    coded_size->set_width(
-        std::round((visible_rect->width() - 2.0f * shrink_amount) / sx));
-    visible_rect->set_x(std::round(tx * coded_size->width() - shrink_amount));
-  }
-  if (sy < 1.0f) {
-    coded_size->set_height(
-        std::round((visible_rect->height() - 2.0f * shrink_amount) / sy));
-    visible_rect->set_y(std::round(ty * coded_size->height() - shrink_amount));
+  // crop rectangle might have been shrunk by up to 2 texels in each dimension
+  // depending on format and if the filtering was enabled. We will try with
+  // worst case of YUV as most common and work down.
+
+  const float possible_shrinks_amounts[] = {1.0f, 0.5f, 0.0f};
+
+  for (float shrink_amount : possible_shrinks_amounts) {
+    if (sx < 1.0f) {
+      coded_size->set_width(
+          std::round((visible_rect->width() - 2.0f * shrink_amount) / sx));
+      visible_rect->set_x(std::round(tx * coded_size->width() - shrink_amount));
+    }
+    if (sy < 1.0f) {
+      coded_size->set_height(
+          std::round((visible_rect->height() - 2.0f * shrink_amount) / sy));
+      visible_rect->set_y(
+          std::round(ty * coded_size->height() - shrink_amount));
+    }
+
+    // If origin of visible_rect is negative we likely trying too big
+    // |shrink_amount| so we need to check for next value. Otherwise break the
+    // loop.
+    if (visible_rect->x() >= 0 && visible_rect->y() >= 0) {
+      break;
+    }
   }
+  return true;
 }
 
 }  // namespace gpu

gpu/command_buffer/service/surface_texture_gl_owner.h

@@ -52,7 +52,7 @@ class GPU_GLES2_EXPORT SurfaceTextureGLOwner : public TextureOwner {
   SurfaceTextureGLOwner(std::unique_ptr<gles2::AbstractTexture> texture);
   ~SurfaceTextureGLOwner() override;
 
-  static void DecomposeTransform(float matrix[16],
+  static bool DecomposeTransform(float matrix[16],
                                  gfx::Size rotated_visible_size,
                                  gfx::Size* coded_size,
                                  gfx::Rect* visible_rect);

gpu/command_buffer/service/surface_texture_transform_unittest.cc

@@ -26,8 +26,8 @@ class SurfaceTextureTransformTest : public testing::Test {
               bool rotated) {
     gfx::Size coded_size;
     gfx::Rect visible_rect;
-    SurfaceTextureGLOwner::DecomposeTransform(matrix, rotated_visible_size,
-                                              &coded_size, &visible_rect);
+    DCHECK(SurfaceTextureGLOwner::DecomposeTransform(
+        matrix, rotated_visible_size, &coded_size, &visible_rect));
 
     EXPECT_EQ(coded_size, expected_coded_size);
     EXPECT_EQ(visible_rect.origin(), gfx::Point(0, 0));
@@ -80,4 +80,45 @@ TEST_F(SurfaceTextureTransformTest, Rotation270) {
   DoTest(matrix, rotated_visible_size, coded_size, true);
 }
 
+TEST_F(SurfaceTextureTransformTest, SmallSize) {
+  float matrix[16] = {0.000000, 0.000000, 0.000000, 0.000000,
+                      0.000000, 0.000000, 0.000000, 0.000000,
+                      0.000000, 0.000000, 0.000000, 0.000000,
+                      0.000000, 0.000000, 0.000000, 0.000000};
+  // With video size less then 4x4 matrix might not make sense because of
+  // shrinking crop rect by pixel from each side. It's important to not crash in
+  // that case.
+  gfx::Size landscape_size(4, 2);
+  gfx::Size portrait_size(2, 4);
+
+  // We assume coded size is the same as video size in this case.
+  DoTest(matrix, landscape_size, landscape_size, false);
+  DoTest(matrix, portrait_size, portrait_size, false);
+}
+
+// This tests case where matrix was created with |shrink_amount|=0, e.g no
+// linear filtering.
+TEST_F(SurfaceTextureTransformTest, NoShrink) {
+  float matrix[16] = {0.986111, 0.000000,  0.000000, 0.000000,
+                      0.000000, -1.000000, 0.000000, 0.000000,
+                      0.000000, 0.000000,  1.000000, 0.000000,
+                      0.000000, 1.000000,  0.000000, 1.000000};
+  gfx::Size coded_size(432, 240);
+  gfx::Size rotated_visible_size(426, 240);
+
+  DoTest(matrix, rotated_visible_size, coded_size, false);
+}
+
+// This tests case where |shrink_amount|=0.5, e.g rgb codec size.
+TEST_F(SurfaceTextureTransformTest, Shrink_05) {
+  float matrix[16] = {0.986111, 0.000000,  0.000000, 0.000000,
+                      0.000000, -1.000000, 0.000000, 0.000000,
+                      0.000000, 0.000000,  1.000000, 0.000000,
+                      0.001157, 1.000000,  0.000000, 1.000000};
+  gfx::Size coded_size(432, 240);
+  gfx::Size rotated_visible_size(427, 240);
+
+  DoTest(matrix, rotated_visible_size, coded_size, false);
+}
+
 }  // namespace gpu