[libcamera-devel] [PATCH v4 09/10] rkisp1: Control camera lens position from IPA

[Jacopo Mondi]

Hi Daniel

On Tue, Mar 14, 2023 at 03:48:33PM +0100, Daniel Semkowicz wrote:
> Allow control of lens position from the IPA, by setting corresponding
> af fields in the IPAFrameContext structure. Controls are then passed to
> the pipeline handler, which sets the lens position in CameraLens.
>
> Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> ---
>  include/libcamera/ipa/rkisp1.mojom       |  1 +
>  src/ipa/rkisp1/rkisp1.cpp                |  8 ++++++++
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++++++++++++
>  3 files changed, 26 insertions(+)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index bf6e9141..c3ed87aa 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -39,5 +39,6 @@ interface IPARkISP1Interface {
>  interface IPARkISP1EventInterface {
>  	paramsBufferReady(uint32 frame);
>  	setSensorControls(uint32 frame, libcamera.ControlList sensorControls);
> +	setLensControls(libcamera.ControlList lensControls);
>  	metadataReady(uint32 frame, libcamera.ControlList metadata);
>  };
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 4b30844f..4aef2e30 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -469,6 +469,14 @@ void IPARkISP1::setControls(unsigned int frame)
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
>
>  	setSensorControls.emit(frame, ctrls);
> +
> +	if (lensControls_ && context_.activeState.af.applyLensCtrls) {
> +		context_.activeState.af.applyLensCtrls = false;
> +		ControlList lensCtrls(*lensControls_);
> +		lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE,
> +			      context_.activeState.af.lensPosition);
> +		setLensControls.emit(lensCtrls);
> +	}
>  }
>
>  } /* namespace ipa::rkisp1 */
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 83fb6287..1dc3f957 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -114,6 +114,7 @@ private:
>  	void paramFilled(unsigned int frame);
>  	void setSensorControls(unsigned int frame,
>  			       const ControlList &sensorControls);
> +	void setLensControls(const ControlList &lensControls);
>
>  	void metadataReady(unsigned int frame, const ControlList &metadata);
>  };
> @@ -340,6 +341,7 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  		return -ENOENT;
>
>  	ipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);
> +	ipa_->setLensControls.connect(this, &RkISP1CameraData::setLensControls);
>  	ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);
>  	ipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);
>
> @@ -403,6 +405,21 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
>  	delayedCtrls_->push(sensorControls);
>  }
>
> +void RkISP1CameraData::setLensControls(const ControlList &lensControls)
> +{
> +	CameraLens *focusLens = sensor_->focusLens();
> +	if (!focusLens)
> +		return;
> +
> +	if (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))
> +		return;

As you correctly pointed out in the reply to my review of 2/10,
Am I wrong thinking that if any of the two above condition is false
we don't get a valid CameraLens * from the CameraSensor, hence we
never initialize ipaConfig.lensControls, effecively disabling lens
support in the IPA ?

	CameraLens *lens = data->sensor_->focusLens();
	if (lens)
		ipaConfig.lensControls = lens->controls();

> +
> +	const ControlValue &focusValue =
> +		lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);
> +
> +	focusLens->setFocusPosition(focusValue.get<int32_t>());
> +}
> +
>  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
>  {
>  	RkISP1FrameInfo *info = frameInfo_.find(frame);
> --
> 2.39.2
>