Skip to content

Commit

Permalink
Fix a double scaling issue in Y'CbCr conversion.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sesse committed Dec 13, 2015
1 parent 825c907 commit f71dd67
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 6 deletions.
8 changes: 4 additions & 4 deletions ycbcr.cpp
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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;
Expand Down
80 changes: 78 additions & 2 deletions ycbcr_input_test.cpp
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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

0 comments on commit f71dd67

Please sign in to comment.