From 916aac82c54d1cc78eb95ad3d916eaaf0fa2c910 Mon Sep 17 00:00:00 2001 From: Pascal Massimino Date: Tue, 10 Jul 2018 17:27:21 -0700 Subject: [PATCH] fix rescaling rounding inaccuracy We should be using 'floor' when doing the final divide. -> new MACRO is MULT_FIX_FLOOR() XXX*** Mips code is DISABLED for now ***XXX I'll update and re-enable it in a later patch, since this code needs some refactoring first. BUG=oss-fuzz:9179 Change-Id: Ic0693cdca4e71f5beab1029475e35c4d06b12d13 (cherry picked from commit 2563db475946d0ae3eb0defdb9b3805f75e6ef1e) --- src/dsp/rescaler.c | 4 +++- src/dsp/rescaler_mips32.c | 4 +++- src/dsp/rescaler_mips_dsp_r2.c | 10 +++++++--- src/dsp/rescaler_msa.c | 7 +++++-- src/dsp/rescaler_neon.c | 18 +++++++++++++---- src/dsp/rescaler_sse2.c | 35 ++++++++++++++++++++++++++++++++-- 6 files changed, 65 insertions(+), 13 deletions(-) diff --git a/src/dsp/rescaler.c b/src/dsp/rescaler.c index f307d350..753f84fc 100644 --- a/src/dsp/rescaler.c +++ b/src/dsp/rescaler.c @@ -21,6 +21,7 @@ #define ROUNDER (WEBP_RESCALER_ONE >> 1) #define MULT_FIX(x, y) (((uint64_t)(x) * (y) + ROUNDER) >> WEBP_RESCALER_RFIX) +#define MULT_FIX_FLOOR(x, y) (((uint64_t)(x) * (y)) >> WEBP_RESCALER_RFIX) //------------------------------------------------------------------------------ // Row import @@ -138,7 +139,7 @@ void WebPRescalerExportRowShrink_C(WebPRescaler* const wrk) { if (yscale) { for (x_out = 0; x_out < x_out_max; ++x_out) { const uint32_t frac = (uint32_t)MULT_FIX(frow[x_out], yscale); - const int v = (int)MULT_FIX(irow[x_out] - frac, wrk->fxy_scale); + const int v = (int)MULT_FIX_FLOOR(irow[x_out] - frac, wrk->fxy_scale); assert(v >= 0 && v <= 255); dst[x_out] = v; irow[x_out] = frac; // new fractional start @@ -153,6 +154,7 @@ void WebPRescalerExportRowShrink_C(WebPRescaler* const wrk) { } } +#undef MULT_FIX_FLOOR #undef MULT_FIX #undef ROUNDER diff --git a/src/dsp/rescaler_mips32.c b/src/dsp/rescaler_mips32.c index 542f7e59..61f63c61 100644 --- a/src/dsp/rescaler_mips32.c +++ b/src/dsp/rescaler_mips32.c @@ -209,6 +209,7 @@ static void ExportRowExpand_MIPS32(WebPRescaler* const wrk) { } } +#if 0 // disabled for now. TODO(skal): make match the C-code static void ExportRowShrink_MIPS32(WebPRescaler* const wrk) { const int x_out_max = wrk->dst_width * wrk->num_channels; uint8_t* dst = wrk->dst; @@ -273,6 +274,7 @@ static void ExportRowShrink_MIPS32(WebPRescaler* const wrk) { ); } } +#endif // 0 //------------------------------------------------------------------------------ // Entry point @@ -283,7 +285,7 @@ WEBP_TSAN_IGNORE_FUNCTION void WebPRescalerDspInitMIPS32(void) { WebPRescalerImportRowExpand = ImportRowExpand_MIPS32; WebPRescalerImportRowShrink = ImportRowShrink_MIPS32; WebPRescalerExportRowExpand = ExportRowExpand_MIPS32; - WebPRescalerExportRowShrink = ExportRowShrink_MIPS32; +// WebPRescalerExportRowShrink = ExportRowShrink_MIPS32; } #else // !WEBP_USE_MIPS32 diff --git a/src/dsp/rescaler_mips_dsp_r2.c b/src/dsp/rescaler_mips_dsp_r2.c index b78aac15..ce9e6486 100644 --- a/src/dsp/rescaler_mips_dsp_r2.c +++ b/src/dsp/rescaler_mips_dsp_r2.c @@ -20,10 +20,12 @@ #define ROUNDER (WEBP_RESCALER_ONE >> 1) #define MULT_FIX(x, y) (((uint64_t)(x) * (y) + ROUNDER) >> WEBP_RESCALER_RFIX) +#define MULT_FIX_FLOOR(x, y) (((uint64_t)(x) * (y)) >> WEBP_RESCALER_RFIX) //------------------------------------------------------------------------------ // Row export +#if 0 // disabled for now. TODO(skal): make match the C-code static void ExportRowShrink_MIPSdspR2(WebPRescaler* const wrk) { int i; const int x_out_max = wrk->dst_width * wrk->num_channels; @@ -106,7 +108,7 @@ static void ExportRowShrink_MIPSdspR2(WebPRescaler* const wrk) { } for (i = 0; i < (x_out_max & 0x3); ++i) { const uint32_t frac = (uint32_t)MULT_FIX(*frow++, yscale); - const int v = (int)MULT_FIX(*irow - frac, wrk->fxy_scale); + const int v = (int)MULT_FIX_FLOOR(*irow - frac, wrk->fxy_scale); assert(v >= 0 && v <= 255); *dst++ = v; *irow++ = frac; // new fractional start @@ -154,13 +156,14 @@ static void ExportRowShrink_MIPSdspR2(WebPRescaler* const wrk) { ); } for (i = 0; i < (x_out_max & 0x3); ++i) { - const int v = (int)MULT_FIX(*irow, wrk->fxy_scale); + const int v = (int)MULT_FIX_FLOOR(*irow, wrk->fxy_scale); assert(v >= 0 && v <= 255); *dst++ = v; *irow++ = 0; } } } +#endif // 0 static void ExportRowExpand_MIPSdspR2(WebPRescaler* const wrk) { int i; @@ -294,6 +297,7 @@ static void ExportRowExpand_MIPSdspR2(WebPRescaler* const wrk) { } } +#undef MULT_FIX_FLOOR #undef MULT_FIX #undef ROUNDER @@ -304,7 +308,7 @@ extern void WebPRescalerDspInitMIPSdspR2(void); WEBP_TSAN_IGNORE_FUNCTION void WebPRescalerDspInitMIPSdspR2(void) { WebPRescalerExportRowExpand = ExportRowExpand_MIPSdspR2; - WebPRescalerExportRowShrink = ExportRowShrink_MIPSdspR2; +// WebPRescalerExportRowShrink = ExportRowShrink_MIPSdspR2; } #else // !WEBP_USE_MIPS_DSP_R2 diff --git a/src/dsp/rescaler_msa.c b/src/dsp/rescaler_msa.c index f3bc99f1..c5592548 100644 --- a/src/dsp/rescaler_msa.c +++ b/src/dsp/rescaler_msa.c @@ -22,6 +22,7 @@ #define ROUNDER (WEBP_RESCALER_ONE >> 1) #define MULT_FIX(x, y) (((uint64_t)(x) * (y) + ROUNDER) >> WEBP_RESCALER_RFIX) +#define MULT_FIX_FLOOR(x, y) (((uint64_t)(x) * (y)) >> WEBP_RESCALER_RFIX) #define CALC_MULT_FIX_16(in0, in1, in2, in3, scale, shift, dst) do { \ v4u32 tmp0, tmp1, tmp2, tmp3; \ @@ -262,6 +263,7 @@ static void RescalerExportRowExpand_MIPSdspR2(WebPRescaler* const wrk) { } } +#if 0 // disabled for now. TODO(skal): make match the C-code static WEBP_INLINE void ExportRowShrink_0(const uint32_t* frow, uint32_t* irow, uint8_t* dst, int length, const uint32_t yscale, @@ -341,7 +343,7 @@ static WEBP_INLINE void ExportRowShrink_0(const uint32_t* frow, uint32_t* irow, } for (x_out = 0; x_out < length; ++x_out) { const uint32_t frac = (uint32_t)MULT_FIX(frow[x_out], yscale); - const int v = (int)MULT_FIX(irow[x_out] - frac, wrk->fxy_scale); + const int v = (int)MULT_FIX_FLOOR(irow[x_out] - frac, wrk->fxy_scale); assert(v >= 0 && v <= 255); dst[x_out] = v; irow[x_out] = frac; @@ -426,6 +428,7 @@ static void RescalerExportRowShrink_MIPSdspR2(WebPRescaler* const wrk) { ExportRowShrink_1(irow, dst, x_out_max, wrk); } } +#endif // 0 //------------------------------------------------------------------------------ // Entry point @@ -434,7 +437,7 @@ extern void WebPRescalerDspInitMSA(void); WEBP_TSAN_IGNORE_FUNCTION void WebPRescalerDspInitMSA(void) { WebPRescalerExportRowExpand = RescalerExportRowExpand_MIPSdspR2; - WebPRescalerExportRowShrink = RescalerExportRowShrink_MIPSdspR2; +// WebPRescalerExportRowShrink = RescalerExportRowShrink_MIPSdspR2; } #else // !WEBP_USE_MSA diff --git a/src/dsp/rescaler_neon.c b/src/dsp/rescaler_neon.c index 3eff9fba..a553f06f 100644 --- a/src/dsp/rescaler_neon.c +++ b/src/dsp/rescaler_neon.c @@ -22,6 +22,7 @@ #define ROUNDER (WEBP_RESCALER_ONE >> 1) #define MULT_FIX_C(x, y) (((uint64_t)(x) * (y) + ROUNDER) >> WEBP_RESCALER_RFIX) +#define MULT_FIX_FLOOR_C(x, y) (((uint64_t)(x) * (y)) >> WEBP_RESCALER_RFIX) #define LOAD_32x4(SRC, DST) const uint32x4_t DST = vld1q_u32((SRC)) #define LOAD_32x8(SRC, DST0, DST1) \ @@ -35,8 +36,11 @@ #if (WEBP_RESCALER_RFIX == 32) #define MAKE_HALF_CST(C) vdupq_n_s32((int32_t)((C) >> 1)) -#define MULT_FIX(A, B) /* note: B is actualy scale>>1. See MAKE_HALF_CST */ \ +// note: B is actualy scale>>1. See MAKE_HALF_CST +#define MULT_FIX(A, B) \ vreinterpretq_u32_s32(vqrdmulhq_s32(vreinterpretq_s32_u32((A)), (B))) +#define MULT_FIX_FLOOR(A, B) \ + vreinterpretq_u32_s32(vqdmulhq_s32(vreinterpretq_s32_u32((A)), (B))) #else #error "MULT_FIX/WEBP_RESCALER_RFIX need some more work" #endif @@ -135,8 +139,8 @@ static void RescalerExportRowShrink_NEON(WebPRescaler* const wrk) { const uint32x4_t A1 = MULT_FIX(in1, yscale_half); const uint32x4_t B0 = vqsubq_u32(in2, A0); const uint32x4_t B1 = vqsubq_u32(in3, A1); - const uint32x4_t C0 = MULT_FIX(B0, fxy_scale_half); - const uint32x4_t C1 = MULT_FIX(B1, fxy_scale_half); + const uint32x4_t C0 = MULT_FIX_FLOOR(B0, fxy_scale_half); + const uint32x4_t C1 = MULT_FIX_FLOOR(B1, fxy_scale_half); const uint16x4_t D0 = vmovn_u32(C0); const uint16x4_t D1 = vmovn_u32(C1); const uint8x8_t E = vmovn_u16(vcombine_u16(D0, D1)); @@ -145,7 +149,7 @@ static void RescalerExportRowShrink_NEON(WebPRescaler* const wrk) { } for (; x_out < x_out_max; ++x_out) { const uint32_t frac = (uint32_t)MULT_FIX_C(frow[x_out], yscale); - const int v = (int)MULT_FIX_C(irow[x_out] - frac, wrk->fxy_scale); + const int v = (int)MULT_FIX_FLOOR_C(irow[x_out] - frac, fxy_scale); assert(v >= 0 && v <= 255); dst[x_out] = v; irow[x_out] = frac; // new fractional start @@ -170,6 +174,12 @@ static void RescalerExportRowShrink_NEON(WebPRescaler* const wrk) { } } +#undef MULT_FIX_FLOOR_C +#undef MULT_FIX_C +#undef MULT_FIX_FLOOR +#undef MULT_FIX +#undef ROUNDER + //------------------------------------------------------------------------------ extern void WebPRescalerDspInitNEON(void); diff --git a/src/dsp/rescaler_sse2.c b/src/dsp/rescaler_sse2.c index 64c50dea..f7461a45 100644 --- a/src/dsp/rescaler_sse2.c +++ b/src/dsp/rescaler_sse2.c @@ -25,6 +25,7 @@ #define ROUNDER (WEBP_RESCALER_ONE >> 1) #define MULT_FIX(x, y) (((uint64_t)(x) * (y) + ROUNDER) >> WEBP_RESCALER_RFIX) +#define MULT_FIX_FLOOR(x, y) (((uint64_t)(x) * (y)) >> WEBP_RESCALER_RFIX) // input: 8 bytes ABCDEFGH -> output: A0E0B0F0C0G0D0H0 static void LoadTwoPixels_SSE2(const uint8_t* const src, __m128i* out) { @@ -224,6 +225,35 @@ static WEBP_INLINE void ProcessRow_SSE2(const __m128i* const A0, _mm_storel_epi64((__m128i*)dst, G); } +static WEBP_INLINE void ProcessRow_Floor_SSE2(const __m128i* const A0, + const __m128i* const A1, + const __m128i* const A2, + const __m128i* const A3, + const __m128i* const mult, + uint8_t* const dst) { + const __m128i mask = _mm_set_epi32(0xffffffffu, 0, 0xffffffffu, 0); + const __m128i B0 = _mm_mul_epu32(*A0, *mult); + const __m128i B1 = _mm_mul_epu32(*A1, *mult); + const __m128i B2 = _mm_mul_epu32(*A2, *mult); + const __m128i B3 = _mm_mul_epu32(*A3, *mult); + const __m128i D0 = _mm_srli_epi64(B0, WEBP_RESCALER_RFIX); + const __m128i D1 = _mm_srli_epi64(B1, WEBP_RESCALER_RFIX); +#if (WEBP_RESCALER_RFIX < 32) + const __m128i D2 = + _mm_and_si128(_mm_slli_epi64(B2, 32 - WEBP_RESCALER_RFIX), mask); + const __m128i D3 = + _mm_and_si128(_mm_slli_epi64(B3, 32 - WEBP_RESCALER_RFIX), mask); +#else + const __m128i D2 = _mm_and_si128(B2, mask); + const __m128i D3 = _mm_and_si128(B3, mask); +#endif + const __m128i E0 = _mm_or_si128(D0, D2); + const __m128i E1 = _mm_or_si128(D1, D3); + const __m128i F = _mm_packs_epi32(E0, E1); + const __m128i G = _mm_packus_epi16(F, F); + _mm_storel_epi64((__m128i*)dst, G); +} + static void RescalerExportRowExpand_SSE2(WebPRescaler* const wrk) { int x_out; uint8_t* const dst = wrk->dst; @@ -322,12 +352,12 @@ static void RescalerExportRowShrink_SSE2(WebPRescaler* const wrk) { const __m128i G1 = _mm_or_si128(D1, F3); _mm_storeu_si128((__m128i*)(irow + x_out + 0), G0); _mm_storeu_si128((__m128i*)(irow + x_out + 4), G1); - ProcessRow_SSE2(&E0, &E1, &E2, &E3, &mult_xy, dst + x_out); + ProcessRow_Floor_SSE2(&E0, &E1, &E2, &E3, &mult_xy, dst + x_out); } } for (; x_out < x_out_max; ++x_out) { const uint32_t frac = (int)MULT_FIX(frow[x_out], yscale); - const int v = (int)MULT_FIX(irow[x_out] - frac, wrk->fxy_scale); + const int v = (int)MULT_FIX_FLOOR(irow[x_out] - frac, wrk->fxy_scale); assert(v >= 0 && v <= 255); dst[x_out] = v; irow[x_out] = frac; // new fractional start @@ -352,6 +382,7 @@ static void RescalerExportRowShrink_SSE2(WebPRescaler* const wrk) { } } +#undef MULT_FIX_FLOOR #undef MULT_FIX #undef ROUNDER