[libcamera-devel,v2,4/9] libcamera: camera_sensor: Register ColorFilterArrangement
diff mbox series

Message ID 20201218164754.81422-5-jacopo@jmondi.org
State Accepted
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: Introduce sensor database
Related show

Commit Message

Jacopo Mondi Dec. 18, 2020, 4:47 p.m. UTC
Inspect the list of media bus codes supported by the camera sensor
in order to deduce the color filter arrangement and register the
ColorFilterArrangement draft property.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/camera_sensor.cpp | 36 +++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Dec. 23, 2020, 4:04 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Fri, Dec 18, 2020 at 05:47:49PM +0100, Jacopo Mondi wrote:
> Inspect the list of media bus codes supported by the camera sensor
> in order to deduce the color filter arrangement and register the
> ColorFilterArrangement draft property.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp | 36 +++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 1628ba9c892b..2b78eb4cb530 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -17,6 +17,7 @@
>  
>  #include <libcamera/property_ids.h>
>  
> +#include "libcamera/internal/bayer_format.h"
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/sysfs.h"
>  #include "libcamera/internal/utils.h"
> @@ -178,10 +179,6 @@ int CameraSensor::init()
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = initProperties();
> -	if (ret)
> -		return ret;
> -
>  	/* Enumerate, sort and cache media bus codes and sizes. */
>  	formats_ = subdev_->formats(pad_);
>  	if (formats_.empty()) {
> @@ -210,6 +207,10 @@ int CameraSensor::init()
>  	 */
>  	resolution_ = sizes_.back();
>  
> +	ret = initProperties();
> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }
>  
> @@ -307,6 +308,33 @@ int CameraSensor::initProperties()
>  		properties_.set(properties::PixelArrayActiveAreas, { crop });
>  	}
>  
> +	/* Color filter arrangement, default to the generic 'RGB' */

s/arrangement/array pattern/ (CFA stands for color filter array)

> +	int32_t cfa = properties::draft::RGB;

Would it make sense, for non-Bayer sensors, to not register the property
at all ? For Android, android.sensor.info.colorFilterArrangement is only
mandatory when raw capture is supported, so it shouldn't cause any
issue. For the libcamera native API, reporting RGB for, for instance,
YUV sensors seems a bit of a weird match.

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

> +	for (const auto &format : formats_) {
> +		unsigned int mbusCode = format.first;
> +		BayerFormat bayerFormat(mbusCode);
> +		if (!bayerFormat.isValid())
> +			continue;
> +
> +		/* Report the CFA ordering of the first valid RAW Bayer format. */
> +		switch (bayerFormat.order) {
> +		case BayerFormat::BGGR:
> +			cfa = properties::draft::BGGR;
> +			break;
> +		case BayerFormat::GBRG:
> +			cfa = properties::draft::GBRG;
> +			break;
> +		case BayerFormat::GRBG:
> +			cfa = properties::draft::GRBG;
> +			break;
> +		case BayerFormat::RGGB:
> +			cfa = properties::draft::RGGB;
> +			break;
> +		}
> +		break;
> +	}
> +	properties_.set(properties::draft::ColorFilterArrangement, cfa);
> +
>  	return 0;
>  }
>

Patch
diff mbox series

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 1628ba9c892b..2b78eb4cb530 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -17,6 +17,7 @@ 
 
 #include <libcamera/property_ids.h>
 
+#include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/formats.h"
 #include "libcamera/internal/sysfs.h"
 #include "libcamera/internal/utils.h"
@@ -178,10 +179,6 @@  int CameraSensor::init()
 	if (ret < 0)
 		return ret;
 
-	ret = initProperties();
-	if (ret)
-		return ret;
-
 	/* Enumerate, sort and cache media bus codes and sizes. */
 	formats_ = subdev_->formats(pad_);
 	if (formats_.empty()) {
@@ -210,6 +207,10 @@  int CameraSensor::init()
 	 */
 	resolution_ = sizes_.back();
 
+	ret = initProperties();
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -307,6 +308,33 @@  int CameraSensor::initProperties()
 		properties_.set(properties::PixelArrayActiveAreas, { crop });
 	}
 
+	/* Color filter arrangement, default to the generic 'RGB' */
+	int32_t cfa = properties::draft::RGB;
+	for (const auto &format : formats_) {
+		unsigned int mbusCode = format.first;
+		BayerFormat bayerFormat(mbusCode);
+		if (!bayerFormat.isValid())
+			continue;
+
+		/* Report the CFA ordering of the first valid RAW Bayer format. */
+		switch (bayerFormat.order) {
+		case BayerFormat::BGGR:
+			cfa = properties::draft::BGGR;
+			break;
+		case BayerFormat::GBRG:
+			cfa = properties::draft::GBRG;
+			break;
+		case BayerFormat::GRBG:
+			cfa = properties::draft::GRBG;
+			break;
+		case BayerFormat::RGGB:
+			cfa = properties::draft::RGGB;
+			break;
+		}
+		break;
+	}
+	properties_.set(properties::draft::ColorFilterArrangement, cfa);
+
 	return 0;
 }