Use exact sRGB matrix values.

https://photosauce.net/blog/post/what-makes-srgb-a-special-color-space
alerted me to that sRGB does not use the exact same matrix as Rec. 709;
it demands specific, rounded-off values. The difference should be minute,
but now we are at least exact in the forward direction.

Note that this isn't an ABI break per se, even though the definition of the
COLORSPACE_sRGB enum changes; old software will simply still continue
to treat it as Rec. 709, while newly-compiled software will get the
corrected version.

diff --git a/colorspace_conversion_effect.cpp b/colorspace_conversion_effect.cpp
index 2b8a0ca329fd34b3d535fca0cbdfb56584b8ff1d..c14a8898f9ab2e10588479dc4601a37e34257ab4 100644 (file)
--- a/colorspace_conversion_effect.cpp
+++ b/colorspace_conversion_effect.cpp
@@ -38,12 +38,28 @@ Matrix3d ColorspaceConversionEffect::get_xyz_matrix(Colorspace space)
        if (space == COLORSPACE_XYZ) {
                return Matrix3d::Identity();
        }
+       if (space == COLORSPACE_sRGB) {
+               // sRGB is not defined by the color primaries, but by concrete
+               // forward and inverse matrices that are rounded-off versions
+               // of the Rec. 709 color space (see
+               // https://photosauce.net/blog/post/what-makes-srgb-a-special-color-space).
+               // We're not compliant with the inverse matrix, since we'd be
+               // too accurate (sRGB is specified for 8-bit only); however,
+               // results should be very close in practice (and even closer to
+               // scRGB's inverse matrix, which is a higher-accuracy inversion of
+               // the same forward matrix).
+               return Matrix3d{
+                       { 0.4124, 0.3576, 0.1805 },
+                       { 0.2126, 0.7152, 0.0722 },
+                       { 0.0193, 0.1192, 0.9505 }
+               };
+       }
 
        double x_R, x_G, x_B;
        double y_R, y_G, y_B;
 
        switch (space) {
-       case COLORSPACE_REC_709:  // And sRGB.
+       case COLORSPACE_REC_709:
                x_R = rec709_x_R; x_G = rec709_x_G; x_B = rec709_x_B;
                y_R = rec709_y_R; y_G = rec709_y_G; y_B = rec709_y_B;
                break;

diff --git a/colorspace_conversion_effect_test.cpp b/colorspace_conversion_effect_test.cpp
index 1395140c81a480a910f8f8691efdae5256e50b04..b268ce00caf92c8ce88cce81e277510166783732 100644 (file)
--- a/colorspace_conversion_effect_test.cpp
+++ b/colorspace_conversion_effect_test.cpp
@@ -41,7 +41,7 @@ TEST(ColorspaceConversionEffectTest, sRGB_Primaries) {
        };
        float out_data[4 * 5];
 
-       EffectChainTester tester(data, 1, 5, FORMAT_RGBA_POSTMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR);
+       EffectChainTester tester(data, 1, 5, FORMAT_RGBA_POSTMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR, GL_RGBA32F);
        tester.run(out_data, GL_RGBA, COLORSPACE_XYZ, GAMMA_LINEAR);
 
        // Black should stay black.
@@ -87,6 +87,21 @@ TEST(ColorspaceConversionEffectTest, sRGB_Primaries) {
        EXPECT_NEAR(0.150, blue_x, 1e-3);
        EXPECT_NEAR(0.060, blue_y, 1e-3);
        EXPECT_FLOAT_EQ(1.0f, out_data[4 * 4 + 3]);
+
+       // The forward matrix should be exactly as specified in the standard,
+       // up to floating-point precision. (We're not compliant with the
+       // inverse matrix, but we should be very close.)
+       EXPECT_FLOAT_EQ(0.4124f, out_data[2 * 4 + 0]);
+       EXPECT_FLOAT_EQ(0.2126f, out_data[2 * 4 + 1]);
+       EXPECT_FLOAT_EQ(0.0193f, out_data[2 * 4 + 2]);
+
+       EXPECT_FLOAT_EQ(0.3576f, out_data[3 * 4 + 0]);
+       EXPECT_FLOAT_EQ(0.7152f, out_data[3 * 4 + 1]);
+       EXPECT_FLOAT_EQ(0.1192f, out_data[3 * 4 + 2]);
+
+       EXPECT_FLOAT_EQ(0.1805f, out_data[4 * 4 + 0]);
+       EXPECT_FLOAT_EQ(0.0722f, out_data[4 * 4 + 1]);
+       EXPECT_FLOAT_EQ(0.9505f, out_data[4 * 4 + 2]);
 }
 
 TEST(ColorspaceConversionEffectTest, Rec601_525_Primaries) {

diff --git a/image_format.h b/image_format.h
index 1bd19f953e6ddc27102baf601229a44eb8b33cbd..40067c656782a38017920a7e649c03bb4f34cd3b 100644 (file)
--- a/image_format.h
+++ b/image_format.h
@@ -27,12 +27,12 @@ enum MovitPixelFormat {
 
 enum Colorspace {
        COLORSPACE_INVALID = -1,  // For internal use.
-       COLORSPACE_sRGB = 0,
-       COLORSPACE_REC_709 = 0,  // Same as sRGB.
+       COLORSPACE_REC_709 = 0,
        COLORSPACE_REC_601_525 = 1,
        COLORSPACE_REC_601_625 = 2,
        COLORSPACE_XYZ = 3,  // Mostly useful for testing and debugging.
        COLORSPACE_REC_2020 = 4,
+       COLORSPACE_sRGB = 5,  // Used to be same as COLORSPACE_REC_709.
 };
 
 enum GammaCurve {