Commit a7da6622 authored by Haiyang Pan's avatar Haiyang Pan Committed by Commit Bot

Revert "Fix SurfaceTexture DecomposeTransformMatrix"

This reverts commit fcd03993.

Reason for revert: Tests fail on quite a few android builders.
See https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=media_unittests&tests=SurfaceTextureTransformTest

Original change's description:
> 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}

TBR=backer@chromium.org,vasilyt@chromium.org

Change-Id: I1fe1e970dcd4db63c5ec1a7f7e39452a0ca37202
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1081695, 1086624, 1076564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220268Reviewed-by: default avatarHaiyang Pan <hypan@google.com>
Commit-Queue: Haiyang Pan <hypan@google.com>
Cr-Commit-Position: refs/heads/master@{#772731}
parent cf260f14
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
#include "base/android/scoped_hardware_buffer_fence_sync.h" #include "base/android/scoped_hardware_buffer_fence_sync.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/check_op.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/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/notreached.h" #include "base/notreached.h"
...@@ -125,55 +123,17 @@ void SurfaceTextureGLOwner::GetCodedSizeAndVisibleRect( ...@@ -125,55 +123,17 @@ void SurfaceTextureGLOwner::GetCodedSizeAndVisibleRect(
float mtx[16]; float mtx[16];
surface_texture_->GetTransformMatrix(mtx); surface_texture_->GetTransformMatrix(mtx);
bool result = DecomposeTransform(mtx, rotated_visible_size, coded_size, visible_rect);
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 // static
bool SurfaceTextureGLOwner::DecomposeTransform(float mtx[16], void SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
gfx::Size rotated_visible_size, gfx::Size rotated_visible_size,
gfx::Size* coded_size, gfx::Size* coded_size,
gfx::Rect* visible_rect) { gfx::Rect* visible_rect) {
DCHECK(coded_size); DCHECK(coded_size);
DCHECK(visible_rect); 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; float sx, sy;
*visible_rect = gfx::Rect(); *visible_rect = gfx::Rect();
// The matrix is in column order and contains transform of (0, 0)x(1, 1) // The matrix is in column order and contains transform of (0, 0)x(1, 1)
...@@ -216,11 +176,6 @@ bool SurfaceTextureGLOwner::DecomposeTransform(float mtx[16], ...@@ -216,11 +176,6 @@ bool SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
sx = std::abs(sx); sx = std::abs(sx);
sy = std::abs(sy); sy = std::abs(sy);
// We got zero matrix, so we can't decompose anything.
if (!sx || !sy) {
return false;
}
*coded_size = visible_rect->size(); *coded_size = visible_rect->size();
// Note: Below calculation is reverse operation of computing matrix in // Note: Below calculation is reverse operation of computing matrix in
...@@ -230,33 +185,27 @@ bool SurfaceTextureGLOwner::DecomposeTransform(float mtx[16], ...@@ -230,33 +185,27 @@ bool SurfaceTextureGLOwner::DecomposeTransform(float mtx[16],
// sampling the texture. // sampling the texture.
// In order to prevent bilinear sampling beyond the edge of the // In order to prevent bilinear sampling beyond the edge of the
// crop rectangle might have been shrunk by up to 2 texels in each dimension // crop rectangle was shrunk by 2 texels in each dimension. Normally this
// depending on format and if the filtering was enabled. We will try with // would just need to take 1/2 a texel off each end, but because the chroma
// worst case of YUV as most common and work down. // 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
const float possible_shrinks_amounts[] = {1.0f, 0.5f, 0.0f}; // the worst case.
const float shrink_amount = 1.0f;
for (float shrink_amount : possible_shrinks_amounts) {
if (sx < 1.0f) { // TODO(crbug.com/1076564): Revisit if we need to handle it gracefully.
coded_size->set_width( CHECK(sx);
std::round((visible_rect->width() - 2.0f * shrink_amount) / sx)); CHECK(sy);
visible_rect->set_x(std::round(tx * coded_size->width() - shrink_amount));
} if (sx < 1.0f) {
if (sy < 1.0f) { coded_size->set_width(
coded_size->set_height( std::round((visible_rect->width() - 2.0f * shrink_amount) / sx));
std::round((visible_rect->height() - 2.0f * shrink_amount) / sy)); visible_rect->set_x(std::round(tx * coded_size->width() - shrink_amount));
visible_rect->set_y( }
std::round(ty * coded_size->height() - shrink_amount)); if (sy < 1.0f) {
} coded_size->set_height(
std::round((visible_rect->height() - 2.0f * shrink_amount) / sy));
// If origin of visible_rect is negative we likely trying too big visible_rect->set_y(std::round(ty * coded_size->height() - shrink_amount));
// |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 } // namespace gpu
...@@ -52,7 +52,7 @@ class GPU_GLES2_EXPORT SurfaceTextureGLOwner : public TextureOwner { ...@@ -52,7 +52,7 @@ class GPU_GLES2_EXPORT SurfaceTextureGLOwner : public TextureOwner {
SurfaceTextureGLOwner(std::unique_ptr<gles2::AbstractTexture> texture); SurfaceTextureGLOwner(std::unique_ptr<gles2::AbstractTexture> texture);
~SurfaceTextureGLOwner() override; ~SurfaceTextureGLOwner() override;
static bool DecomposeTransform(float matrix[16], static void DecomposeTransform(float matrix[16],
gfx::Size rotated_visible_size, gfx::Size rotated_visible_size,
gfx::Size* coded_size, gfx::Size* coded_size,
gfx::Rect* visible_rect); gfx::Rect* visible_rect);
......
...@@ -26,8 +26,8 @@ class SurfaceTextureTransformTest : public testing::Test { ...@@ -26,8 +26,8 @@ class SurfaceTextureTransformTest : public testing::Test {
bool rotated) { bool rotated) {
gfx::Size coded_size; gfx::Size coded_size;
gfx::Rect visible_rect; gfx::Rect visible_rect;
DCHECK(SurfaceTextureGLOwner::DecomposeTransform( SurfaceTextureGLOwner::DecomposeTransform(matrix, rotated_visible_size,
matrix, rotated_visible_size, &coded_size, &visible_rect)); &coded_size, &visible_rect);
EXPECT_EQ(coded_size, expected_coded_size); EXPECT_EQ(coded_size, expected_coded_size);
EXPECT_EQ(visible_rect.origin(), gfx::Point(0, 0)); EXPECT_EQ(visible_rect.origin(), gfx::Point(0, 0));
...@@ -80,45 +80,4 @@ TEST_F(SurfaceTextureTransformTest, Rotation270) { ...@@ -80,45 +80,4 @@ TEST_F(SurfaceTextureTransformTest, Rotation270) {
DoTest(matrix, rotated_visible_size, coded_size, true); 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 } // namespace gpu
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment