[libcamera-devel,v3,1/2] libcamera: camera_sensor: Print warning when orientation is unknown

11371 diff mbox series

Print a warning when the orientation of a sensor is unknown. The
location property is still defaulted to external.

Also add a recommended controls list, similar to the optional and
mandatory controls list, to handle controls in a similar situation in
the future.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
Changes in v3:
- add recommendedControls

Changes in v2:
- expand the warning message
---
 src/libcamera/camera_sensor.cpp | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Hi Paul,

On Wed, Feb 24, 2021 at 06:48:31PM +0900, Paul Elder wrote:
> Print a warning when the orientation of a sensor is unknown. The
> location property is still defaulted to external.
>
> Also add a recommended controls list, similar to the optional and
> mandatory controls list, to handle controls in a similar situation in
> the future.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
> Changes in v3:
> - add recommendedControls
>
> Changes in v2:
> - expand the warning message
> ---
>  src/libcamera/camera_sensor.cpp | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index c9e8d49b..8a1b9bd2 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -276,12 +276,13 @@ int CameraSensor::init()
>
>  int CameraSensor::validateSensorDriver()
>  {
> +	int err = 0;
> +
>  	/*
>  	 * Optional controls are used to register optional sensor properties. If
>  	 * not present, some values will be defaulted.
>  	 */
>  	static constexpr uint32_t optionalControls[] = {
> -		V4L2_CID_CAMERA_ORIENTATION,
>  		V4L2_CID_CAMERA_SENSOR_ROTATION,
>  	};
>
> @@ -293,6 +294,23 @@ int CameraSensor::validateSensorDriver()
>  				<< " not supported";
>  	}
>
> +	/*
> +	 * Recommended controls are similar to optional controls, but will
> +	 * become mandatory in the near future. Be loud if they're missing.
> +	 */
> +	static constexpr uint32_t recommendedControls[] = {
> +		V4L2_CID_CAMERA_ORIENTATION,
> +	};
> +
> +	for (uint32_t ctrl : recommendedControls) {
> +		if (!controls.count(ctrl)) {
> +			LOG(CameraSensor, Warning)
> +				<< "Recommended V4L2 control " << utils::hex(ctrl)
> +				<< " not supported";
> +			err = -EINVAL;
> +		}
> +	}
> +
>  	/*
>  	 * Make sure the required selection targets are supported.
>  	 *
> @@ -303,7 +321,6 @@ int CameraSensor::validateSensorDriver()
>  	 * \todo Make support for selection targets mandatory as soon as all
>  	 * test platforms have been updated.
>  	 */
> -	int err = 0;
>  	Rectangle rect;
>  	int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);
>  	if (ret) {
> @@ -446,6 +463,8 @@ int CameraSensor::initProperties()
>  			break;
>  		}
>  	} else {
> +		LOG(CameraSensor, Warning)
> +			<< "Failed to retrieve the camera location, setting to External";
>  		propertyValue = properties::CameraLocationExternal;
>  	}
>  	properties_.set(properties::Location, propertyValue);
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Paul,

Thank you for the patch.

On Wed, Feb 24, 2021 at 06:48:31PM +0900, Paul Elder wrote:
> Print a warning when the orientation of a sensor is unknown. The
> location property is still defaulted to external.
> 
> Also add a recommended controls list, similar to the optional and
> mandatory controls list, to handle controls in a similar situation in
> the future.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
> Changes in v3:
> - add recommendedControls
> 
> Changes in v2:
> - expand the warning message
> ---
>  src/libcamera/camera_sensor.cpp | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index c9e8d49b..8a1b9bd2 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -276,12 +276,13 @@ int CameraSensor::init()
>  
>  int CameraSensor::validateSensorDriver()
>  {
> +	int err = 0;
> +
>  	/*
>  	 * Optional controls are used to register optional sensor properties. If
>  	 * not present, some values will be defaulted.
>  	 */
>  	static constexpr uint32_t optionalControls[] = {
> -		V4L2_CID_CAMERA_ORIENTATION,
>  		V4L2_CID_CAMERA_SENSOR_ROTATION,
>  	};
>  
> @@ -293,6 +294,23 @@ int CameraSensor::validateSensorDriver()
>  				<< " not supported";
>  	}
>  
> +	/*
> +	 * Recommended controls are similar to optional controls, but will
> +	 * become mandatory in the near future. Be loud if they're missing.
> +	 */
> +	static constexpr uint32_t recommendedControls[] = {
> +		V4L2_CID_CAMERA_ORIENTATION,
> +	};
> +
> +	for (uint32_t ctrl : recommendedControls) {
> +		if (!controls.count(ctrl)) {
> +			LOG(CameraSensor, Warning)
> +				<< "Recommended V4L2 control " << utils::hex(ctrl)
> +				<< " not supported";
> +			err = -EINVAL;
> +		}
> +	}
> +
>  	/*
>  	 * Make sure the required selection targets are supported.
>  	 *
> @@ -303,7 +321,6 @@ int CameraSensor::validateSensorDriver()
>  	 * \todo Make support for selection targets mandatory as soon as all
>  	 * test platforms have been updated.
>  	 */
> -	int err = 0;
>  	Rectangle rect;
>  	int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);
>  	if (ret) {
> @@ -446,6 +463,8 @@ int CameraSensor::initProperties()
>  			break;
>  		}
>  	} else {
> +		LOG(CameraSensor, Warning)
> +			<< "Failed to retrieve the camera location, setting to External";
>  		propertyValue = properties::CameraLocationExternal;
>  	}
>  	properties_.set(properties::Location, propertyValue);