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 dc223e7441c845cf7c1449e439862c5732efc71f..50059125dc4a3c866dff4ef08e82916d94ff7a43 100644 (file)
--- 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 <Eigen/Core>
 #include <Eigen/LU>
@@ -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 109178f09e2fa8e20d805830285989437aefd7c6..463cb7e2d3771180c0d8fe06a65e62bd7931678e 100644 (file)
--- 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 <epoxy/gl.h>
 #include <stddef.h>
 
+#include <Eigen/Core>
+#include <Eigen/LU>
+
 #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