]> granicus.if.org Git - libvpx/commitdiff
Enable ssse3 bilinear tests
authorJohann <johannkoenig@google.com>
Fri, 9 Sep 2016 01:46:14 +0000 (18:46 -0700)
committerJohann <johannkoenig@google.com>
Fri, 16 Sep 2016 06:16:26 +0000 (23:16 -0700)
The code only has issues when xoffset == 0 and yoffset == 0 which
represents a simple copy. Presumably this case does not need to be
handled because the issue has existed since 2010.

BUG=webm:1287

Change-Id: Ic47e2653f3b729e99b40e53d8d2d8d1501edaaa9

test/predict_test.cc
vp8/common/filter.c
vp8/common/x86/subpixel_ssse3.asm

index dd797c61ad99cfb421b449edbdbc3f520caebe0e..5672ca937ad1fcdfe1bbc1ecce06aecb37045739 100644 (file)
@@ -146,9 +146,15 @@ class PredictTestBase : public ::testing::TestWithParam<PredictParam> {
   void TestWithRandomData(PredictFunc reference) {
     ACMRandom rnd(ACMRandom::DeterministicSeed());
 
-    // Run tests for all possible offsets.
+    // Run tests for almost all possible offsets.
     for (int xoffset = 0; xoffset < 8; ++xoffset) {
       for (int yoffset = 0; yoffset < 8; ++yoffset) {
+        if (xoffset == 0 && yoffset == 0) {
+          // This represents a copy which is not required to be handled by this
+          // module.
+          continue;
+        }
+
         for (int i = 0; i < kSrcSize; ++i) {
           src_[i] = rnd.Rand8();
         }
@@ -172,6 +178,9 @@ class PredictTestBase : public ::testing::TestWithParam<PredictParam> {
     if (width_ == 4 && height_ == 4) {
       for (int xoffset = 0; xoffset < 8; ++xoffset) {
         for (int yoffset = 0; yoffset < 8; ++yoffset) {
+          if (xoffset == 0 && yoffset == 0) {
+            continue;
+          }
           for (int i = 0; i < kSrcSize; ++i) {
             src_[i] = rnd.Rand8();
           }
@@ -354,7 +363,7 @@ INSTANTIATE_TEST_CASE_P(
 #endif
 #if HAVE_SSSE3
 INSTANTIATE_TEST_CASE_P(
-    DISABLED_SSSE3, BilinearPredictTest,
+    SSSE3, BilinearPredictTest,
     ::testing::Values(make_tuple(16, 16, &vp8_bilinear_predict16x16_ssse3),
                       make_tuple(8, 8, &vp8_bilinear_predict8x8_ssse3)));
 #endif
index a312efb6c680465a4b62738d5151427ac8b443f6..267498335ca9df40f2c89b97cf863e15da7359fa 100644 (file)
@@ -8,8 +8,9 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "filter.h"
+#include <assert.h>
 #include "./vp8_rtcd.h"
+#include "vp8/common/filter.h"
 
 DECLARE_ALIGNED(16, const short, vp8_bilinear_filters[8][2]) = {
   { 128, 0 }, { 112, 16 }, { 96, 32 }, { 80, 48 },
@@ -324,27 +325,11 @@ void vp8_bilinear_predict4x4_c(unsigned char *src_ptr, int src_pixels_per_line,
   const short *HFilter;
   const short *VFilter;
 
+  // This represents a copy and is not required to be handled by optimizations.
+  assert((xoffset | yoffset) != 0);
+
   HFilter = vp8_bilinear_filters[xoffset];
   VFilter = vp8_bilinear_filters[yoffset];
-#if 0
-    {
-        int i;
-        unsigned char temp1[16];
-        unsigned char temp2[16];
-
-        bilinear_predict4x4_mmx(src_ptr, src_pixels_per_line, xoffset, yoffset, temp1, 4);
-        filter_block2d_bil(src_ptr, temp2, src_pixels_per_line, 4, HFilter, VFilter, 4, 4);
-
-        for (i = 0; i < 16; ++i)
-        {
-            if (temp1[i] != temp2[i])
-            {
-                bilinear_predict4x4_mmx(src_ptr, src_pixels_per_line, xoffset, yoffset, temp1, 4);
-                filter_block2d_bil(src_ptr, temp2, src_pixels_per_line, 4, HFilter, VFilter, 4, 4);
-            }
-        }
-    }
-#endif
   filter_block2d_bil(src_ptr, dst_ptr, src_pixels_per_line, dst_pitch, HFilter,
                      VFilter, 4, 4);
 }
@@ -355,6 +340,8 @@ void vp8_bilinear_predict8x8_c(unsigned char *src_ptr, int src_pixels_per_line,
   const short *HFilter;
   const short *VFilter;
 
+  assert((xoffset | yoffset) != 0);
+
   HFilter = vp8_bilinear_filters[xoffset];
   VFilter = vp8_bilinear_filters[yoffset];
 
@@ -368,6 +355,8 @@ void vp8_bilinear_predict8x4_c(unsigned char *src_ptr, int src_pixels_per_line,
   const short *HFilter;
   const short *VFilter;
 
+  assert((xoffset | yoffset) != 0);
+
   HFilter = vp8_bilinear_filters[xoffset];
   VFilter = vp8_bilinear_filters[yoffset];
 
@@ -382,6 +371,8 @@ void vp8_bilinear_predict16x16_c(unsigned char *src_ptr,
   const short *HFilter;
   const short *VFilter;
 
+  assert((xoffset | yoffset) != 0);
+
   HFilter = vp8_bilinear_filters[xoffset];
   VFilter = vp8_bilinear_filters[yoffset];
 
index c06f24556e687c2f6653bdbb0dfb2840a4e377f5..1f6cbd1d1e593ab627e166f79b159a311f6ca626 100644 (file)
@@ -1291,6 +1291,8 @@ sym(vp8_bilinear_predict8x8_ssse3):
         movq        xmm7,       XMMWORD PTR [rsp+96]
         punpcklbw   xmm5,       xmm6
 
+        ; Because the source register (xmm0) is always treated as signed by
+        ; pmaddubsw, the constant '128' is treated as '-128'.
         pmaddubsw   xmm1,       xmm0
         pmaddubsw   xmm2,       xmm0
 
@@ -1319,6 +1321,10 @@ sym(vp8_bilinear_predict8x8_ssse3):
         psraw       xmm5,       VP8_FILTER_SHIFT
 
         psraw       xmm6,       VP8_FILTER_SHIFT
+
+        ; Having multiplied everything by '-128' and obtained negative
+        ; numbers, the unsigned saturation truncates those values to 0,
+        ; resulting in incorrect handling of xoffset == 0 && yoffset == 0
         packuswb    xmm1,       xmm1
 
         packuswb    xmm2,       xmm2