[libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1: Implement color space support

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 ++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 03bbe6b467ae..25fbf9f1a0a9 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -18,6 +18,7 @@
 #include <libcamera/base/utils.h>
 
 #include <libcamera/camera.h>
+#include <libcamera/color_space.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
 #include <libcamera/framebuffer.h>
@@ -416,11 +417,13 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 {
 	const CameraSensor *sensor = data_->sensor_.get();
 	unsigned int pathCount = data_->selfPath_ ? 2 : 1;
-	Status status = Valid;
+	Status status;
 
 	if (config_.empty())
 		return Invalid;
 
+	status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);
+
 	if (transform != Transform::Identity) {
 		transform = Transform::Identity;
 		status = Adjusted;
@@ -547,21 +550,44 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
 	if (roles.empty())
 		return config;
 
+	/*
+	 * As the ISP can't output different color spaces for the main and self
+	 * path, pick a sensible default color space based on the role of the
+	 * first stream and use it for all streams.
+	 */
+	std::optional<ColorSpace> colorSpace;
+
 	bool mainPathAvailable = true;
 	bool selfPathAvailable = data->selfPath_;
+
 	for (const StreamRole role : roles) {
 		bool useMainPath;
 
 		switch (role) {
-		case StreamRole::StillCapture: {
+		case StreamRole::StillCapture:
 			useMainPath = mainPathAvailable;
+			/* JPEG encoders typically expect sYCC. */
+			if (!colorSpace)
+				colorSpace = ColorSpace::Sycc;
 			break;
-		}
+
 		case StreamRole::Viewfinder:
-		case StreamRole::VideoRecording: {
 			useMainPath = !selfPathAvailable;
+			/*
+			 * sYCC is the YCbCr encoding of sRGB, which is commonly
+			 * used by displays.
+			 */
+			if (!colorSpace)
+				colorSpace = ColorSpace::Sycc;
 			break;
-		}
+
+		case StreamRole::VideoRecording:
+			useMainPath = !selfPathAvailable;
+			/* Rec. 709 is a good default for HD video recording. */
+			if (!colorSpace)
+				colorSpace = ColorSpace::Rec709;
+			break;
+
 		default:
 			LOG(RkISP1, Warning)
 				<< "Requested stream role not supported: " << role;
@@ -580,6 +606,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
 			selfPathAvailable = false;
 		}
 
+		cfg.colorSpace = colorSpace;
 		config->addConfiguration(cfg);
 	}
 
@@ -642,6 +669,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	if (ret < 0)
 		return ret;
 
+	format.colorSpace = config->at(0).colorSpace;
 	ret = isp_->setFormat(2, &format);
 	if (ret < 0)
 		return ret;
-- 
Regards,

Laurent Pinchart