[libcamera-devel,v2,1/2] camera_sensor: ipa: core: Add CFA pattern to IPACameraSensorInfo
diff mbox series

Message ID 20230602132358.16314-2-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Mono sensor improvements
Related show

Commit Message

Naushir Patuck June 2, 2023, 1:23 p.m. UTC
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@raspberrypi.com>
---
 include/libcamera/ipa/core.mojom | 10 ++++++++++
 src/libcamera/camera_sensor.cpp  |  3 +++
 2 files changed, 13 insertions(+)

Comments

Laurent Pinchart June 2, 2023, 3:48 p.m. UTC | #1
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@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@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,

Patch
diff mbox series

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.
+ */
+
 /**
  * \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,