[PATCH] libcamera: software_isp: Assign colour spaces in configurations

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Thu, Mar 20, 2025 at 05:26:24PM +0100, Milan Zamazal wrote:
> StreamConfiguration's should have colorSpace set.  This is not the case
> in the simple pipeline.  Let's set it there, basically mimicking what
> rpi and rkisp1 pipelines do (having a common helper may be a
> consideration).
> 
> This also fixes a crash in `cam' due to accessing an unset colorSpace.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf3..62db96ca 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>
> @@ -1196,11 +1197,24 @@ 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;
>  
> +		switch (role) {
> +		case StreamRole::Raw:
> +			cfg.colorSpace = ColorSpace::Raw;
> +			break;
> +		case StreamRole::StillCapture:
> +		case StreamRole::Viewfinder:
> +			cfg.colorSpace = ColorSpace::Sycc;
> +			break;
> +		case StreamRole::VideoRecording:
> +			cfg.colorSpace = ColorSpace::Rec709;
> +			break;
> +		}
> +

Doesn't this need to depend on the hardware ? For instance
ColorSpace::Rec709 uses limited range, so it only makes sense with YUV
formats. When using a YUV sensor you may be able to produce that colour
space, but the software ISP doesn't support outputting YUV.

You also need colour space handling in
SimpleCameraConfiguration::validate().

>  		config->addConfiguration(cfg);
>  	}
>  

-- 
Regards,

Laurent Pinchart