[libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi: Validate lens controls

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via libcamera-devel wrote:
> Pass the available lens controls to the IPA through the configure() function.
> Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it doesn't, log a
> warning message, and do not advertise focus related controls from the IPA.

Shouldn't this be done at init() time instead of configure() time ? I
don't suppose the presense of a lens controller could change between
different configurations.

> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Nick Hollinghurst <nick.hollinghurst at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/ipa/raspberrypi.mojom       |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           | 21 +++++++++++++++++++
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index 2a4821fbc0ef..bfacd1275bfb 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -38,6 +38,7 @@ struct IPAConfig {
>  	libcamera.SharedFD lsTableHandle;
>  	libcamera.ControlInfoMap sensorControls;
>  	libcamera.ControlInfoMap ispControls;
> +	libcamera.ControlInfoMap lensControls;
>  };
>  
>  struct IPAConfigResult {
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index aa18ed750370..bbf3c7dc4a69 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -131,6 +131,7 @@ private:
>  	void setMode(const IPACameraSensorInfo &sensorInfo);
>  	bool validateSensorControls();
>  	bool validateIspControls();
> +	bool validateLensControls();
>  	void queueRequest(const ControlList &controls);
>  	void returnEmbeddedBuffer(unsigned int bufferId);
>  	void prepareISP(const ISPConfig &data);
> @@ -155,6 +156,7 @@ private:
>  
>  	ControlInfoMap sensorCtrls_;
>  	ControlInfoMap ispCtrls_;
> +	ControlInfoMap lensCtrls_;
>  	bool lensPresent_;
>  	ControlList libcameraMetadata_;
>  
> @@ -394,6 +396,15 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip
>  		return -1;
>  	}
>  
> +	if (lensPresent_) {
> +		lensCtrls_ = ipaConfig.lensControls;
> +		if (!validateLensControls()) {
> +			LOG(IPARPI, Warning) << "Lens validation failed, "
> +					     << "no lens control will be available.";
> +			lensPresent_ = false;
> +		}
> +	}
> +
>  	maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();
>  
>  	/* Setup a metadata ControlList to output metadata. */
> @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()
>  	return true;
>  }
>  
> +bool IPARPi::validateLensControls()
> +{
> +	if (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) == lensCtrls_.end()) {
> +		LOG(IPARPI, Error) << "Unable to find Lens control V4L2_CID_FOCUS_ABSOLUTE";
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  /*
>   * Converting between enums (used in the libcamera API) and the names that
>   * we use to identify different modes. Unfortunately, the conversion tables
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 9dd36cbaea78..249dedcd8c09 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::IPA
>  
>  	ipaConfig.sensorControls = sensor_->controls();
>  	ipaConfig.ispControls = isp_[Isp::Input].dev()->controls();
> +	if (sensor_->focusLens())
> +		ipaConfig.lensControls = sensor_->focusLens()->controls();
>  
>  	/* Always send the user transform to the IPA. */
>  	ipaConfig.transform = static_cast<unsigned int>(config->transform);

-- 
Regards,

Laurent Pinchart