From: Steinar H. Gunderson Date: Sun, 13 Dec 2015 12:26:01 +0000 (+0100) Subject: Fix a double scaling issue in Y'CbCr conversion. X-Git-Tag: 1.3.0~16 X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=f71dd67f6a2abca2bb95eb3e0902f9fdcf8e8ed9 Fix a double scaling issue in Y'CbCr conversion. We multiplied by 224/219 once too many, causing some small accuracy issues. Furthermore, we also did this for full-range Y'CbCr, which obviously is wrong. The issue was so small that the unit tests kept on passing (its investigation was prompted by a test that failed on AMD cards, which is a separate issue). After this, the Rec. 601 matrices match Wikipedia exactly, both for limited and full range. Added unit tests for this. --- diff --git a/ycbcr.cpp b/ycbcr.cpp index dc223e7..5005912 100644 --- a/ycbcr.cpp +++ b/ycbcr.cpp @@ -1,5 +1,5 @@ -// Note: This file does not have its own unit test; it is tested mainly -// through YCbCrInput's unit tests. +// Note: These functions are tested in ycbcr_input_test.cpp; both through some +// direct matrix tests, but most of all through YCbCrInput's unit tests. #include #include @@ -116,12 +116,12 @@ void compute_ycbcr_matrix(YCbCrFormat ycbcr_format, float* offset, Matrix3d* ycb rgb_to_ycbcr(0,1) = coeff[1]; rgb_to_ycbcr(0,2) = coeff[2]; - float cb_fac = (224.0 / 219.0) / (coeff[0] + coeff[1] + 1.0f - coeff[2]); + float cb_fac = 1.0 / (coeff[0] + coeff[1] + 1.0f - coeff[2]); rgb_to_ycbcr(1,0) = -coeff[0] * cb_fac; rgb_to_ycbcr(1,1) = -coeff[1] * cb_fac; rgb_to_ycbcr(1,2) = (1.0f - coeff[2]) * cb_fac; - float cr_fac = (224.0 / 219.0) / (1.0f - coeff[0] + coeff[1] + coeff[2]); + float cr_fac = 1.0 / (1.0f - coeff[0] + coeff[1] + coeff[2]); rgb_to_ycbcr(2,0) = (1.0f - coeff[0]) * cr_fac; rgb_to_ycbcr(2,1) = -coeff[1] * cr_fac; rgb_to_ycbcr(2,2) = -coeff[2] * cr_fac; diff --git a/ycbcr_input_test.cpp b/ycbcr_input_test.cpp index 109178f..463cb7e 100644 --- a/ycbcr_input_test.cpp +++ b/ycbcr_input_test.cpp @@ -1,8 +1,11 @@ -// Unit tests for YCbCrInput. +// Unit tests for YCbCrInput. Also tests the matrix functions in ycbcr.cpp directly. #include #include +#include +#include + #include "effect_chain.h" #include "gtest/gtest.h" #include "test_util.h" @@ -352,7 +355,7 @@ TEST(YCbCrInputTest, Subsampling420WithNonCenteredSamples) { // Y'CbCr isn't 100% accurate (the input values are rounded), // so we need some leeway. - expect_equal(expected_data, out_data, width, height, 0.01, 0.001); + expect_equal(expected_data, out_data, width, height, 0.01, 0.0012); } // Yes, some 4:2:2 formats actually have this craziness. @@ -612,4 +615,77 @@ TEST(YCbCrInputTest, ExternalTexture) { expect_equal(expected_data, out_data, 4 * width, height, 0.025, 0.002); } +TEST(YCbCrTest, WikipediaRec601ForwardMatrix) { + YCbCrFormat ycbcr_format; + ycbcr_format.luma_coefficients = YCBCR_REC_601; + ycbcr_format.full_range = false; + ycbcr_format.num_levels = 256; + + float offset[3]; + Eigen::Matrix3d ycbcr_to_rgb; + compute_ycbcr_matrix(ycbcr_format, offset, &ycbcr_to_rgb); + + Eigen::Matrix3d rgb_to_ycbcr = ycbcr_to_rgb.inverse() * 255.0; + + // Values from https://en.wikipedia.org/wiki/YCbCr#ITU-R_BT.601_conversion. + EXPECT_NEAR( 65.481, rgb_to_ycbcr(0,0), 1e-3); + EXPECT_NEAR( 128.553, rgb_to_ycbcr(0,1), 1e-3); + EXPECT_NEAR( 24.966, rgb_to_ycbcr(0,2), 1e-3); + + EXPECT_NEAR( -37.797, rgb_to_ycbcr(1,0), 1e-3); + EXPECT_NEAR( -74.203, rgb_to_ycbcr(1,1), 1e-3); + EXPECT_NEAR( 112.000, rgb_to_ycbcr(1,2), 1e-3); + + EXPECT_NEAR( 112.000, rgb_to_ycbcr(2,0), 1e-3); + EXPECT_NEAR( -93.786, rgb_to_ycbcr(2,1), 1e-3); + EXPECT_NEAR( -18.214, rgb_to_ycbcr(2,2), 1e-3); + + EXPECT_NEAR( 16.0, offset[0] * 255.0, 1e-3); + EXPECT_NEAR(128.0, offset[1] * 255.0, 1e-3); + EXPECT_NEAR(128.0, offset[2] * 255.0, 1e-3); +} + +TEST(YCbCrTest, WikipediaJPEGMatrices) { + YCbCrFormat ycbcr_format; + ycbcr_format.luma_coefficients = YCBCR_REC_601; + ycbcr_format.full_range = true; + ycbcr_format.num_levels = 256; + + float offset[3]; + Eigen::Matrix3d ycbcr_to_rgb; + compute_ycbcr_matrix(ycbcr_format, offset, &ycbcr_to_rgb); + + // Values from https://en.wikipedia.org/wiki/YCbCr#JPEG_conversion. + EXPECT_NEAR( 1.00000, ycbcr_to_rgb(0,0), 1e-5); + EXPECT_NEAR( 0.00000, ycbcr_to_rgb(0,1), 1e-5); + EXPECT_NEAR( 1.40200, ycbcr_to_rgb(0,2), 1e-5); + + EXPECT_NEAR( 1.00000, ycbcr_to_rgb(1,0), 1e-5); + EXPECT_NEAR(-0.34414, ycbcr_to_rgb(1,1), 1e-5); + EXPECT_NEAR(-0.71414, ycbcr_to_rgb(1,2), 1e-5); + + EXPECT_NEAR( 1.00000, ycbcr_to_rgb(2,0), 1e-5); + EXPECT_NEAR( 1.77200, ycbcr_to_rgb(2,1), 1e-5); + EXPECT_NEAR( 0.00000, ycbcr_to_rgb(2,2), 1e-5); + + Eigen::Matrix3d rgb_to_ycbcr = ycbcr_to_rgb.inverse(); + + // Same. + EXPECT_NEAR( 0.299000, rgb_to_ycbcr(0,0), 1e-6); + EXPECT_NEAR( 0.587000, rgb_to_ycbcr(0,1), 1e-6); + EXPECT_NEAR( 0.114000, rgb_to_ycbcr(0,2), 1e-6); + + EXPECT_NEAR(-0.168736, rgb_to_ycbcr(1,0), 1e-6); + EXPECT_NEAR(-0.331264, rgb_to_ycbcr(1,1), 1e-6); + EXPECT_NEAR( 0.500000, rgb_to_ycbcr(1,2), 1e-6); + + EXPECT_NEAR( 0.500000, rgb_to_ycbcr(2,0), 1e-6); + EXPECT_NEAR(-0.418688, rgb_to_ycbcr(2,1), 1e-6); + EXPECT_NEAR(-0.081312, rgb_to_ycbcr(2,2), 1e-6); + + EXPECT_NEAR( 0.0, offset[0] * 255.0, 1e-3); + EXPECT_NEAR(128.0, offset[1] * 255.0, 1e-3); + EXPECT_NEAR(128.0, offset[2] * 255.0, 1e-3); +} + } // namespace movit