Fix crash and off-by-one error in MMX YUVA->ARGB conversion.

BUG=370520
TEST=YUVConvertTest.YUVAtoARGB_MMX_MatchReference

Review URL: https://codereview.chromium.org/271443006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269428 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 77 insertions, 2 deletions

diff --git a/media/base/simd/convert_yuva_to_argb_mmx.inc b/media/base/simd/convert_yuva_to_argb_mmx.inc
index 5faa6a5..d493383 100644
--- a/media/base/simd/convert_yuva_to_argb_mmx.inc
+++ b/media/base/simd/convert_yuva_to_argb_mmx.inc
@@ -9,9 +9,10 @@
 
 mangle(SYMBOL):
   %assign   stack_offset 0
-  PROLOGUE  7, 7, 3, Y, U, V, A, ARGB, WIDTH, TABLE, TEMP
+  PROLOGUE  7, 7, 3, Y, U, V, A, ARGB, WIDTH, TABLE
   PUSH      WIDTHq
   DEFINE_ARGS Y, U, V, A, ARGB, TABLE, TEMP
+  mov       TABLEq, TEMPq
   jmp       .convertend
 
 .convertloop:
@@ -39,11 +40,25 @@ mangle(SYMBOL):
   psraw     mm2, 6
   packuswb  mm1, mm2
 
-  ; Unpack and multiply by alpha value, then repack high bytes of words.
+  ; Unpack
   movq      mm0, mm1
   pxor      mm2, mm2
   punpcklbw mm0, mm2
   punpckhbw mm1, mm2
+
+  ; Add one to our alpha values, this is a somewhat unfortunate hack; while
+  ; the pack/unpack above handle saturating any negative numbers to 0, they also
+  ; truncate the alpha value to 255. The math ahead wants to produce the same
+  ; ARGB alpha value as the source pixel in YUVA, but this depends on the alpha
+  ; value in |mm0| and |mm1| being 256, (let A be the source image alpha,
+  ; 256 * A >> 8 == A, whereas 255 * A >> 8 is off by one except at 0).
+  mov       TEMPq, 0x00010000
+  movd      mm2, TEMPd
+  psllq     mm2, 32
+  paddsw    mm0, mm2
+  paddsw    mm1, mm2
+
+  ; Multiply by alpha value, then repack high bytes of words.
   movzx     TEMPd, BYTE [Aq]
   movq      mm2, [TABLEq + 6144 + 8 * TEMPq]
   pmullw    mm0, mm2
@@ -79,6 +94,13 @@ mangle(SYMBOL):
   ; Multiply ARGB by alpha value.
   pxor      mm0, mm0
   punpcklbw mm1, mm0
+
+  ; See above note about this hack.
+  mov       TEMPq, 0x00010000
+  movd      mm0, TEMPd
+  psllq     mm0, 32
+  paddsw    mm1, mm0
+
   movzx     TEMPd, BYTE [Aq]
   movq      mm0, [TABLEq + 6144 + 8 * TEMPq]
   pmullw    mm1, mm0
diff --git a/media/base/yuv_convert_unittest.cc b/media/base/yuv_convert_unittest.cc
index 73deb1f..85d3693 100644
--- a/media/base/yuv_convert_unittest.cc
+++ b/media/base/yuv_convert_unittest.cc
@@ -37,6 +37,11 @@ static const int kRGBSizeScaled = kScaledWidth * kScaledHeight * kBpp;
 static const int kRGB24Size = kSourceYSize * 3;
 static const int kRGBSizeConverted = kSourceYSize * kBpp;
 
+#if !defined(ARCH_CPU_ARM_FAMILY) && !defined(ARCH_CPU_MIPS_FAMILY)
+static const int kSourceAOffset = kSourceYSize * 12 / 8;
+static const int kYUVA12Size = kSourceYSize * 20 / 8;
+#endif
+
 // Helper for reading test data into a scoped_ptr<uint8[]>.
 static void ReadData(const base::FilePath::CharType* filename,
                      int expected_size,
@@ -69,6 +74,12 @@ static void ReadYV16Data(scoped_ptr<uint8[]>* data) {
   ReadData(FILE_PATH_LITERAL("bali_640x360_P422.yuv"), kYUV16Size, data);
 }
 
+#if !defined(ARCH_CPU_ARM_FAMILY) && !defined(ARCH_CPU_MIPS_FAMILY)
+static void ReadYV12AData(scoped_ptr<uint8[]>* data) {
+  ReadData(FILE_PATH_LITERAL("bali_640x360_P420_alpha.yuv"), kYUVA12Size, data);
+}
+#endif
+
 static void ReadRGB24Data(scoped_ptr<uint8[]>* data) {
   ReadData(FILE_PATH_LITERAL("bali_640x360_RGB24.rgb"), kRGB24Size, data);
 }
@@ -517,6 +528,48 @@ TEST(YUVConvertTest, DownScaleYUVToRGB32WithRect) {
 }
 
 #if !defined(ARCH_CPU_ARM_FAMILY) && !defined(ARCH_CPU_MIPS_FAMILY)
+TEST(YUVConvertTest, YUVAtoARGB_MMX_MatchReference) {
+  // Allocate all surfaces.
+  scoped_ptr<uint8[]> yuv_bytes;
+  scoped_ptr<uint8[]> rgb_bytes(new uint8[kRGBSize]);
+  scoped_ptr<uint8[]> rgb_converted_bytes(new uint8[kRGBSizeConverted]);
+  scoped_ptr<uint8[]> rgb_converted_bytes_ref(new uint8[kRGBSizeConverted]);
+
+  // Read YUV reference data from file.
+  ReadYV12AData(&yuv_bytes);
+
+  // Convert a frame of YUV to 32 bit ARGB using both C and MMX versions.
+  media::ConvertYUVAToARGB_C(yuv_bytes.get(),
+                             yuv_bytes.get() + kSourceUOffset,
+                             yuv_bytes.get() + kSourceVOffset,
+                             yuv_bytes.get() + kSourceAOffset,
+                             rgb_converted_bytes_ref.get(),
+                             kSourceWidth,
+                             kSourceHeight,
+                             kSourceWidth,
+                             kSourceWidth / 2,
+                             kSourceWidth,
+                             kSourceWidth * kBpp,
+                             media::YV12);
+  media::ConvertYUVAToARGB_MMX(yuv_bytes.get(),
+                               yuv_bytes.get() + kSourceUOffset,
+                               yuv_bytes.get() + kSourceVOffset,
+                               yuv_bytes.get() + kSourceAOffset,
+                               rgb_converted_bytes.get(),
+                               kSourceWidth,
+                               kSourceHeight,
+                               kSourceWidth,
+                               kSourceWidth / 2,
+                               kSourceWidth,
+                               kSourceWidth * kBpp,
+                               media::YV12);
+
+  EXPECT_EQ(0,
+            memcmp(rgb_converted_bytes.get(),
+                   rgb_converted_bytes_ref.get(),
+                   kRGBSizeConverted));
+}
+
 TEST(YUVConvertTest, RGB32ToYUV_SSE2_MatchReference) {
   base::CPU cpu;
   if (!cpu.has_sse2()) {