[v8,01/10] libcamera: software_isp: Assign colour spaces in configurations
diff mbox series

Message ID 20250702121024.38728-2-mzamazal@redhat.com
State New
Headers show
Series
  • Enable raw streams with software ISP
Related show

Commit Message

Milan Zamazal July 2, 2025, 12:09 p.m. UTC
StreamConfiguration's should have colorSpace set.  This is not the case
in the simple pipeline.  Let's set it there.  This also fixes a crash in
`cam' due to accessing an unset colorSpace.

The colour space is assigned as follows:

- If raw role is requested, then the colour space must be raw,
  regardless of the stream format.
- Otherwise, if software ISP is used, the default colour space is
  set to Srgb (YcbcrEncoding::None, Range::Full).
- Otherwise, if a converter is used, the default colour space is left
  unset.
- Then in configuration validation, if the pixel format is changed, the
  colour space is set according to the new pixel format.
- If the colour space is still unset, it is set according to the
  resulting pixel format.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index efb07051b..d45480fe7 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -25,6 +25,7 @@ 
 #include <libcamera/base/log.h>
 
 #include <libcamera/camera.h>
+#include <libcamera/color_space.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
@@ -35,6 +36,7 @@ 
 #include "libcamera/internal/converter.h"
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
+#include "libcamera/internal/formats.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/software_isp/software_isp.h"
@@ -1206,6 +1208,28 @@  CameraConfiguration::Status SimpleCameraConfiguration::validate()
 		if (cfg.pixelFormat != pixelFormat) {
 			LOG(SimplePipeline, Debug) << "Adjusting pixel format";
 			cfg.pixelFormat = pixelFormat;
+			/*
+			 * Do not touch the colour space for raw requested roles.
+			 * Even if the pixel format is non-raw (whatever it means), we
+			 * shouldn't try to interpret the colour space of raw data.
+			 */
+			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)
+				cfg.colorSpace->adjust(pixelFormat);
+			status = Adjusted;
+		}
+		if (!cfg.colorSpace) {
+			const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
+			switch (info.colourEncoding) {
+			case PixelFormatInfo::ColourEncodingRGB:
+				cfg.colorSpace = ColorSpace::Srgb;
+				break;
+			case libcamera::PixelFormatInfo::ColourEncodingYUV:
+				cfg.colorSpace = ColorSpace::Sycc;
+				break;
+			default:
+				cfg.colorSpace = ColorSpace::Raw;
+			}
+			cfg.colorSpace->adjust(pixelFormat);
 			status = Adjusted;
 		}
 
@@ -1314,11 +1338,23 @@  SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo
 	 *
 	 * \todo Implement a better way to pick the default format
 	 */
-	for ([[maybe_unused]] StreamRole role : roles) {
+	for (StreamRole role : roles) {
 		StreamConfiguration cfg{ StreamFormats{ formats } };
 		cfg.pixelFormat = formats.begin()->first;
 		cfg.size = formats.begin()->second[0].max;
 
+		if (role == StreamRole::Raw) {
+			/* Enforce raw colour space for raw roles. */
+			cfg.colorSpace = ColorSpace::Raw;
+		} else if (data->swIsp_) {
+			/*
+			 * This is what software ISP currently produces. It may be arguable
+			 * whether this applies also when CCM is not used but even then it's
+			 * still likely to be a reasonable setting.
+			 */
+			cfg.colorSpace = ColorSpace::Srgb;
+		}
+
 		config->addConfiguration(cfg);
 	}