[RFC,v2] libcamera: software_isp: Assign colour spaces in configurations
diff mbox series

Message ID 20250324171900.40326-1-mzamazal@redhat.com
State New
Headers show
Series
  • [RFC,v2] libcamera: software_isp: Assign colour spaces in configurations
Related show

Commit Message

Milan Zamazal March 24, 2025, 5:19 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.
- 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 | 25 +++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 6e039bf3..07cf7c11 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"
@@ -1088,8 +1090,24 @@  CameraConfiguration::Status SimpleCameraConfiguration::validate()
 		if (cfg.pixelFormat != pixelFormat) {
 			LOG(SimplePipeline, Debug) << "Adjusting pixel format";
 			cfg.pixelFormat = pixelFormat;
+			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);
+		}
 
 		if (!pipeConfig_->outputSizes.contains(cfg.size)) {
 			Size adjustedSize = pipeConfig_->captureSize;
@@ -1196,11 +1214,16 @@  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)
+			cfg.colorSpace = ColorSpace::Raw;
+		else if (data->swIsp_)
+			cfg.colorSpace = ColorSpace::Srgb;
+
 		config->addConfiguration(cfg);
 	}