From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Thu, 2 Dec 2021 12:15:11 +0000 (+0100)
Subject: Use exact sRGB matrix values.
X-Git-Tag: 1.7.0~6
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=0b1705581552217b0e387bd687d65e4e3410ab91

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 2b8a0ca..c14a889 100644
--- 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 1395140..b268ce0 100644
--- 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 1bd19f9..40067c6 100644
--- 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 {