[libcamera-devel] [PATCH v2 1/2] camera_sensor: ipa: core: Add CFA pattern to IPACameraSensorInfo

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Fri, Jun 02, 2023 at 02:23:57PM +0100, Naushir Patuck via libcamera-devel wrote:
> Add a new cfaPattern field to the IPACameraSensorInfo to pass the
> CFA/Bayer pattern for the current sensor configuration to the IPA.
> This field takes a value from properties::draft::ColorFilterArrangementEnum.
> 
> Populate cfaPattern in CameraSensor::sensorInfo(), called by the
> pipeline handler before it calls ipa->init() and ipa->config().
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/ipa/core.mojom | 10 ++++++++++
>  src/libcamera/camera_sensor.cpp  |  3 +++
>  2 files changed, 13 insertions(+)

I like how we can do all this without touching any of the pipeline
handlers :-)

> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
> index 5cef340d014c..af02efacf335 100644
> --- a/include/libcamera/ipa/core.mojom
> +++ b/include/libcamera/ipa/core.mojom
> @@ -140,6 +140,15 @@ module libcamera;
>   * image sensor
>   */
>  
> +/**
> + * \var IPACameraSensorInfo::cfaPattern
> + * \brief The arrangement of colour filters on the image sensor
> + *
> + * This takes a value defined by properties::draft::ColorFilterArrangementEnum.
> + * For non-Bayer colour sensors, the cfaPattern will be set to
> + * properties::draft::ColorFilterArrangementEnum::RGB.

That's fine for now, but I think it would be nice to eventually add
support for optional fields in mojom. Let's record this as a TODO:

 * \todo Make this variable optional once mojom supports it, instead of using
 * RGB for sensors that don't have a CFA.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + */
> +
>  /**
>   * \var IPACameraSensorInfo::activeAreaSize
>   * \brief The size of the pixel array active area of the sensor
> @@ -229,6 +238,7 @@ struct IPACameraSensorInfo {
>  	string model;
>  
>  	uint32 bitsPerPixel;
> +	uint32 cfaPattern;
>  
>  	Size activeAreaSize;
>  	Rectangle analogCrop;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 06f315210240..60bf87b49e6e 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -956,6 +956,9 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
>  	info->bitsPerPixel = format.bitsPerPixel();
>  	info->outputSize = format.size;
>  
> +	std::optional<int32_t> cfa = properties_.get(properties::draft::ColorFilterArrangement);
> +	info->cfaPattern = cfa ? *cfa : properties::draft::RGB;
> +
>  	/*
>  	 * Retrieve the pixel rate, line length and minimum/maximum frame
>  	 * duration through V4L2 controls. Support for the V4L2_CID_PIXEL_RATE,

-- 
Regards,

Laurent Pinchart