From ba6837848836a3635412a67f790c9d4d26d27d3e Mon Sep 17 00:00:00 2001 From: Frank Barchard Date: Mon, 24 Aug 2026 11:18:53 -0700 Subject: [PATCH] Fix off by 32 rounding for I420ToAR30 - x86 SSSE3 / AVX2 / AVX-512: subtract 24 in YUVTORGB_SETUP_AR30 - ARM NEON64: adjust BB (+24), BG (-24), BR (+24) in YUVTORGB_SETUP_AR30 - Windows MSVC AVX2 / AVX-512BW: subtract 24 from kYBiasToRgb - C reference: adjust Yb in LOAD_YUV_CONSTANTS_AR30 - Unit tests: add +0.5 rounding to expected 10-bit Y reference Bug: 551985141 Change-Id: I2bc418edb2d6efb76b4fb1c6bc01205525f4a49c Reviewed-on: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/8280769 Reviewed-by: Wan-Teh Chang Commit-Queue: Frank Barchard --- README.chromium | 2 +- docs/getting_started.md | 1 + include/libyuv/row.h | 6 ++++- include/libyuv/row_sve.h | 12 +++++++++- include/libyuv/version.h | 2 +- source/row_common.cc | 29 ++++++++++++++++++++++- source/row_gcc.cc | 42 +++++++++++++++++++++++++++++++--- source/row_neon64.cc | 13 ++++++++++- source/row_win.cc | 12 ++++++++++ unit_test/convert_argb_test.cc | 8 +++---- 10 files changed, 114 insertions(+), 13 deletions(-) diff --git a/README.chromium b/README.chromium index 6501e94e7..64c0968d4 100644 --- a/README.chromium +++ b/README.chromium @@ -1,6 +1,6 @@ Name: libyuv URL: https://chromium.googlesource.com/libyuv/libyuv/ -Version: 1962 +Version: 1963 Revision: DEPS License: BSD-3-Clause License File: LICENSE diff --git a/docs/getting_started.md b/docs/getting_started.md index 06160bb20..287e35718 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -72,6 +72,7 @@ Additional commonly used compiler options can be passed to Bazel via `--copt`: bazel build -c opt --config=android_arm64 \ --copt=-DLIBYUV_UNLIMITED_DATA \ + --copt=-DLIBYUV_UNBIASED_DATA \ --copt=-DENABLE_ROW_TESTS \ //:libyuv_test diff --git a/include/libyuv/row.h b/include/libyuv/row.h index 32f3f71bd..c8ed007e3 100644 --- a/include/libyuv/row.h +++ b/include/libyuv/row.h @@ -214,7 +214,9 @@ extern "C" { #define HAS_I400TOARGBROW_SSE2 #define HAS_I410TOAR30ROW_SSSE3 #define HAS_I410TOARGBROW_SSSE3 +#if defined(__x86_64__) #define HAS_I422TOAR30ROW_SSSE3 +#endif #define HAS_MERGEARGBROW_SSE2 #define HAS_MERGERGBROW_SSSE3 #define HAS_MERGEXRGBROW_SSE2 @@ -298,7 +300,9 @@ extern "C" { #define HAS_I400TOARGBROW_AVX2 #define HAS_I410TOAR30ROW_AVX2 #define HAS_I410TOARGBROW_AVX2 +#if defined(__x86_64__) #define HAS_I422TOAR30ROW_AVX2 +#endif #define HAS_I422TOUYVYROW_AVX2 #define HAS_I422TOYUY2ROW_AVX2 #define HAS_MERGEAR64ROW_AVX2 @@ -339,13 +343,13 @@ extern "C" { defined(LIBYUV_ENABLE_ROWWIN)) #define HAS_RAWTOARGBROW_AVX2 #define HAS_I422TORGB24ROW_AVX2 -#define HAS_I422TOAR30ROW_AVX2 #define HAS_RGB24TOARGBROW_AVX2 #define HAS_RGB565TOARGBROW_AVX2 #define HAS_ARGB1555TOARGBROW_AVX2 #define HAS_ARGB4444TOARGBROW_AVX2 #define HAS_ARGBSHUFFLEROW_AVX2 #if defined(__x86_64__) || defined(_M_X64) +#define HAS_I422TOAR30ROW_AVX2 #define HAS_RAWTOARGBROW_AVX512BW #define HAS_RGB24TOARGBROW_AVX512BW #define HAS_ARGBSHUFFLEROW_AVX512BW diff --git a/include/libyuv/row_sve.h b/include/libyuv/row_sve.h index 9c5aee8a1..328a6de6b 100644 --- a/include/libyuv/row_sve.h +++ b/include/libyuv/row_sve.h @@ -39,6 +39,16 @@ extern "C" { "ld1rh {z26.h}, p0/z, [%[kRGBCoeffBias], #4] \n" \ "ld1rh {z27.h}, p0/z, [%[kRGBCoeffBias], #6] \n" +#if defined(LIBYUV_UNBIASED_DATA) +#define YUVTORGB_SVE_SETUP_AR30 \ + YUVTORGB_SVE_SETUP \ + "add z25.h, z25.h, #24 \n" \ + "sub z26.h, z26.h, #24 \n" \ + "add z27.h, z27.h, #24 \n" +#else +#define YUVTORGB_SVE_SETUP_AR30 YUVTORGB_SVE_SETUP +#endif + #define READYUV444_SVE \ "ld1b {z0.h}, p1/z, [%[src_y]] \n" \ "ld1b {z1.h}, p1/z, [%[src_u]] \n" \ @@ -827,7 +837,7 @@ static inline void I422ToAR30Row_SVE_SC(const uint8_t* src_y, asm volatile( "cnth %[vl] \n" "ptrue p0.b \n" // - YUVTORGB_SVE_SETUP + YUVTORGB_SVE_SETUP_AR30 "dup z23.h, %w[limit] \n" "subs %w[width], %w[width], %w[vl], lsl #1 \n" "b.le 2f \n" diff --git a/include/libyuv/version.h b/include/libyuv/version.h index 361b1f4e3..12eb1503d 100644 --- a/include/libyuv/version.h +++ b/include/libyuv/version.h @@ -11,6 +11,6 @@ #ifndef INCLUDE_LIBYUV_VERSION_H_ #define INCLUDE_LIBYUV_VERSION_H_ -#define LIBYUV_VERSION 1962 +#define LIBYUV_VERSION 1963 #endif // INCLUDE_LIBYUV_VERSION_H_ diff --git a/source/row_common.cc b/source/row_common.cc index df43aecae..d0dcc3b43 100644 --- a/source/row_common.cc +++ b/source/row_common.cc @@ -37,6 +37,9 @@ extern "C" { // LIBYUV_UNLIMITED_BT709 // LIBYUV_UNLIMITED_BT2020 +// This macro fixes the AR30 rounding bias offset (+24 / -24) for 10-bit YUV to RGB conversions: +// LIBYUV_UNBIASED_DATA + // llvm x86 is poor at ternary operator, so use branchless min/max. #define USE_BRANCHLESS 1 @@ -1785,6 +1788,30 @@ MAKEYUVCONSTANTS(V2020, YG, YB, UB, UG, VG, VR) int r16 = y1 + (vi * vr) #endif +#if defined(LIBYUV_UNBIASED_DATA) +#if defined(__aarch64__) || defined(__arm__) || defined(__riscv) +#define LOAD_YUV_CONSTANTS_AR30 \ + int ub = yuvconstants->kUVCoeff[0]; \ + int vr = yuvconstants->kUVCoeff[1]; \ + int ug = yuvconstants->kUVCoeff[2]; \ + int vg = yuvconstants->kUVCoeff[3]; \ + int yg = yuvconstants->kRGBCoeffBias[0]; \ + int bb = yuvconstants->kRGBCoeffBias[1] + 24; \ + int bg = yuvconstants->kRGBCoeffBias[2] - 24; \ + int br = yuvconstants->kRGBCoeffBias[3] + 24 +#else +#define LOAD_YUV_CONSTANTS_AR30 \ + int ub = yuvconstants->kUVToB[0]; \ + int ug = yuvconstants->kUVToG[0]; \ + int vg = yuvconstants->kUVToG[1]; \ + int vr = yuvconstants->kUVToR[1]; \ + int yg = yuvconstants->kYToRgb[0]; \ + int yb = yuvconstants->kYBiasToRgb[0] - 24 +#endif +#else +#define LOAD_YUV_CONSTANTS_AR30 LOAD_YUV_CONSTANTS +#endif + // C reference code that mimics the YUV assembly. // Reads 8 bit YUV and leaves result as 16 bit. static __inline void YuvPixel(uint8_t y, @@ -1810,7 +1837,7 @@ static __inline void YuvPixel8_16(uint8_t y, int* g, int* r, const struct YuvConstants* yuvconstants) { - LOAD_YUV_CONSTANTS; + LOAD_YUV_CONSTANTS_AR30; uint32_t y32 = y * 0x0101; CALC_RGB16; *b = b16; diff --git a/source/row_gcc.cc b/source/row_gcc.cc index e5565bea7..9cc30799a 100644 --- a/source/row_gcc.cc +++ b/source/row_gcc.cc @@ -2270,6 +2270,18 @@ void ARGBToUVMatrixRow_AVX512BW(const uint8_t* src_argb, "movdqa 96(%[yuvconstants]),%%xmm11 \n" \ "movdqa 128(%[yuvconstants]),%%xmm12 \n" +#if defined(LIBYUV_UNBIASED_DATA) +#define YUVTORGB_SETUP_AR30(yuvconstants) \ + YUVTORGB_SETUP(yuvconstants) \ + "pcmpeqb %%xmm0,%%xmm0 \n" \ + "psrlw $14,%%xmm0 \n" \ + "psllw $3,%%xmm0 \n" \ + "psubw %%xmm0,%%xmm12 \n" +#else +#define YUVTORGB_SETUP_AR30(yuvconstants) \ + YUVTORGB_SETUP(yuvconstants) +#endif + // Convert 8 pixels: 8 UV and 8 Y #define YUVTORGB16(yuvconstants) \ "psubb %%xmm13,%%xmm3 \n" \ @@ -2290,6 +2302,7 @@ void ARGBToUVMatrixRow_AVX512BW(const uint8_t* src_argb, #else #define YUVTORGB_SETUP(yuvconstants) +#define YUVTORGB_SETUP_AR30(yuvconstants) // Convert 8 pixels: 8 UV and 8 Y #define YUVTORGB16(yuvconstants) \ @@ -2552,7 +2565,7 @@ void OMITFP I422ToAR30Row_SSSE3(const uint8_t* y_buf, const struct YuvConstants* yuvconstants, int width) { asm volatile ( - YUVTORGB_SETUP(yuvconstants) + YUVTORGB_SETUP_AR30(yuvconstants) "sub %[u_buf],%[v_buf] \n" "pcmpeqb %%xmm5,%%xmm5 \n" // all 1s "movdqa %%xmm5,%%xmm7 \n" @@ -3340,6 +3353,27 @@ void OMITFP I422ToRGBARow_SSSE3(const uint8_t* y_buf, "vmovups (%[dquadsplitperm]),%%zmm17 \n" \ "vmovups (%[unperm]),%%zmm18 \n" +#if defined(LIBYUV_UNBIASED_DATA) +#define YUVTORGB_SETUP_AR30_AVX2(yuvconstants) \ + YUVTORGB_SETUP_AVX2(yuvconstants) \ + "vpcmpeqb %%ymm0,%%ymm0,%%ymm0 \n" \ + "vpsrlw $14,%%ymm0,%%ymm0 \n" \ + "vpsllw $3,%%ymm0,%%ymm0 \n" \ + "vpsubw %%ymm0,%%ymm12,%%ymm12 \n" + +#define YUVTORGB_SETUP_AR30_AVX512BW(yuvconstants) \ + YUVTORGB_SETUP_AVX512BW(yuvconstants) \ + "vpternlogd $0xff,%%zmm0,%%zmm0,%%zmm0 \n" \ + "vpsrlw $14,%%zmm0,%%zmm0 \n" \ + "vpsllw $3,%%zmm0,%%zmm0 \n" \ + "vpsubw %%zmm0,%%zmm12,%%zmm12 \n" +#else +#define YUVTORGB_SETUP_AR30_AVX2(yuvconstants) \ + YUVTORGB_SETUP_AVX2(yuvconstants) +#define YUVTORGB_SETUP_AR30_AVX512BW(yuvconstants) \ + YUVTORGB_SETUP_AVX512BW(yuvconstants) +#endif + #define YUVTORGB16_AVX2(yuvconstants) \ "vpsubb %%ymm13,%%ymm3,%%ymm3 \n" \ "vpmulhuw %%ymm11,%%ymm4,%%ymm4 \n" \ @@ -3369,6 +3403,8 @@ void OMITFP I422ToRGBARow_SSSE3(const uint8_t* y_buf, #else // Convert 16 pixels: 16 UV and 16 Y. #define YUVTORGB_SETUP_AVX2(yuvconstants) +#define YUVTORGB_SETUP_AR30_AVX2(yuvconstants) +#define YUVTORGB_SETUP_AR30_AVX512BW(yuvconstants) #define YUVTORGB16_AVX2(yuvconstants) \ "vpcmpeqb %%ymm0,%%ymm0,%%ymm0 \n" \ "vpabsb %%ymm0,%%ymm0 \n" \ @@ -3818,7 +3854,7 @@ void OMITFP I422ToAR30Row_AVX512BW(const uint8_t* y_buf, const struct YuvConstants* yuvconstants, int width) { asm volatile ( - YUVTORGB_SETUP_AVX512BW(yuvconstants) + YUVTORGB_SETUP_AR30_AVX512BW(yuvconstants) "sub %[u_buf],%[v_buf] \n" "vpternlogd $0xff,%%zmm5,%%zmm5,%%zmm5 \n" // all 1s "vpsrlw $6,%%zmm5,%%zmm7 \n" @@ -3866,7 +3902,7 @@ void OMITFP I422ToAR30Row_AVX2(const uint8_t* y_buf, const struct YuvConstants* yuvconstants, int width) { asm volatile ( - YUVTORGB_SETUP_AVX2(yuvconstants) + YUVTORGB_SETUP_AR30_AVX2(yuvconstants) "sub %[u_buf],%[v_buf] \n" "vpcmpeqb %%ymm5,%%ymm5,%%ymm5 \n" // all 1s "vpsrlw $6,%%ymm5,%%ymm7 \n" diff --git a/source/row_neon64.cc b/source/row_neon64.cc index 7727c0acb..0fe996169 100644 --- a/source/row_neon64.cc +++ b/source/row_neon64.cc @@ -149,6 +149,17 @@ static const uvec8 kNV21InterleavedTable = {1, 1, 5, 5, 9, 9, 13, 13, "ld4r {v28.16b, v29.16b, v30.16b, v31.16b}, [%[kUVCoeff]] \n" \ "ld4r {v24.8h, v25.8h, v26.8h, v27.8h}, [%[kRGBCoeffBias]] \n" +#if defined(LIBYUV_UNBIASED_DATA) +#define YUVTORGB_SETUP_AR30 \ + YUVTORGB_SETUP \ + "movi v2.8h, #24 \n" \ + "add v25.8h, v25.8h, v2.8h \n" \ + "sub v26.8h, v26.8h, v2.8h \n" \ + "add v27.8h, v27.8h, v2.8h \n" +#else +#define YUVTORGB_SETUP_AR30 YUVTORGB_SETUP +#endif + // v16.8h: B // v17.8h: G // v18.8h: R @@ -586,7 +597,7 @@ void I422ToAR30Row_NEON(const uint8_t* src_y, const vec16* rgb_coeff = &yuvconstants->kRGBCoeffBias; const uint16_t limit = 0x3ff0; asm volatile( - YUVTORGB_SETUP + YUVTORGB_SETUP_AR30 "dup v22.8h, %w[limit] \n" "movi v23.8h, #0xc0, lsl #8 \n" // A "1: \n" // diff --git a/source/row_win.cc b/source/row_win.cc index eab4076c5..e35cfce5f 100644 --- a/source/row_win.cc +++ b/source/row_win.cc @@ -1174,7 +1174,13 @@ void I422ToAR30Row_AVX2(const uint8_t* src_y, __m256i ymm_kUVToG = _mm256_loadu_si256((const __m256i*)yuvconstants->kUVToG); __m256i ymm_kUVToR = _mm256_loadu_si256((const __m256i*)yuvconstants->kUVToR); __m256i ymm_kYToRgb = _mm256_loadu_si256((const __m256i*)yuvconstants->kYToRgb); +#if defined(LIBYUV_UNBIASED_DATA) + __m256i ymm_kYBiasToRgb = _mm256_sub_epi16( + _mm256_loadu_si256((const __m256i*)yuvconstants->kYBiasToRgb), + _mm256_set1_epi16(24)); +#else __m256i ymm_kYBiasToRgb = _mm256_loadu_si256((const __m256i*)yuvconstants->kYBiasToRgb); +#endif __m256i ymm_128 = _mm256_set1_epi8((char)0x80); __m256i ymm_3ff0 = _mm256_set1_epi16((short)0x3ff0); __m256i ymm_c000 = _mm256_set1_epi16((short)0xc000); @@ -1723,7 +1729,13 @@ void I422ToAR30Row_AVX512BW(const uint8_t* src_y, __m512i zmm_kUVToG = _mm512_broadcast_i32x4(_mm_loadu_si128((const __m128i*)yuvconstants->kUVToG)); __m512i zmm_kUVToR = _mm512_broadcast_i32x4(_mm_loadu_si128((const __m128i*)yuvconstants->kUVToR)); __m512i zmm_kYToRgb = _mm512_broadcast_i32x4(_mm_loadu_si128((const __m128i*)yuvconstants->kYToRgb)); +#if defined(LIBYUV_UNBIASED_DATA) + __m512i zmm_kYBiasToRgb = _mm512_sub_epi16( + _mm512_broadcast_i32x4(_mm_loadu_si128((const __m128i*)yuvconstants->kYBiasToRgb)), + _mm512_set1_epi16(24)); +#else __m512i zmm_kYBiasToRgb = _mm512_broadcast_i32x4(_mm_loadu_si128((const __m128i*)yuvconstants->kYBiasToRgb)); +#endif __m512i zmm_128 = _mm512_set1_epi8((char)0x80); __m512i zmm_3ff0 = _mm512_set1_epi16((short)0x3ff0); __m512i zmm_fc00 = _mm512_set1_epi16((short)0xfc00); diff --git a/unit_test/convert_argb_test.cc b/unit_test/convert_argb_test.cc index 6b746b2c3..afde0b62f 100644 --- a/unit_test/convert_argb_test.cc +++ b/unit_test/convert_argb_test.cc @@ -2511,10 +2511,10 @@ TEST_F(LibYUVConvertTest, TestH010ToAB30) { ++histogram_b[b10]; ++histogram_g[g10]; ++histogram_r[r10]; - int expected_y = Clamp10(static_cast((i - 64) * 1.164f)); - ASSERT_NEAR(b10, expected_y, 4); - ASSERT_NEAR(g10, expected_y, 4); + int expected_y = Clamp10(static_cast((i - 64) * 1.164f + 0.5)); ASSERT_NEAR(r10, expected_y, 4); + ASSERT_NEAR(g10, expected_y, 4); + ASSERT_NEAR(b10, expected_y, 4); ASSERT_EQ(a2, 3); } @@ -2573,7 +2573,7 @@ TEST_F(LibYUVConvertTest, TestH420ToAR30) { ++histogram_b[b10]; ++histogram_g[g10]; ++histogram_r[r10]; - int expected_y = Clamp10(static_cast((i - 16) * 1.164f * 4.f)); + int expected_y = Clamp10(static_cast((i - 16) * 1.164f * 4.f + 0.5)); ASSERT_NEAR(b10, expected_y, 4); ASSERT_NEAR(g10, expected_y, 4); ASSERT_NEAR(r10, expected_y, 4);