[libcamera-devel] [PATCH v2 09/11] ipa: af_hill_climb: Skip the first frame after triggering auto focus

[Jacopo Mondi]

Hi Daniel,
 +cc rpi which might find the series interesting

On Wed, Jul 13, 2022 at 10:43:15AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> When AFTrigger is received, AF algorithm internal state is reset to
> initial values. If reset happens between frames, then first contrast
> value that arrives after this, is interpreted as contrast for initial
> position. This is wrong, because it was measured for lens position

I still don't get why the focus position should be reset...

But the control description is under-specified in this case, so we
should probably decide what to do and also specify that.

I sounds natural to me to assume that when the mode moves from Auto to
Manual the last computed value is kept until the application doesn't
provide a LensPosition. When switching from manual to auto the last
applied value is kept until an AF scan is request with AfTriggerStart.

Does this sound reasonable ?

> before the reset. Skip this first frame to allow correct contrast
> measure for the initial lens position.
>
> Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> ---
>  .../libipa/algorithms/af_hill_climbing.cpp    |  8 +++++++
>  src/ipa/libipa/algorithms/af_hill_climbing.h  | 23 ++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.cpp b/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> index f666c6c2..28d09176 100644
> --- a/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> +++ b/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> @@ -84,6 +84,14 @@ namespace ipa::common::algorithms {
>   * \return New lens position calculated by AF algorithm
>   */
>
> +/**
> + * \fn AfHillClimbing::setFramesToSkip()
> + * \brief Request AF to skip n frames
> + * \param[in] n Number of frames to be skipped
> + *
> + * Requested number of frames will not be used for AF calculation.
> + */
> +
>  } /* namespace ipa::common::algorithms */
>
>  } /* namespace libcamera */
> diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.h b/src/ipa/libipa/algorithms/af_hill_climbing.h
> index db9fc058..e251f3eb 100644
> --- a/src/ipa/libipa/algorithms/af_hill_climbing.h
> +++ b/src/ipa/libipa/algorithms/af_hill_climbing.h
> @@ -29,7 +29,7 @@ public:
>  		  lensPosition_(0), bestPosition_(0), currentContrast_(0.0),
>  		  previousContrast_(0.0), maxContrast_(0.0), maxStep_(0),
>  		  coarseCompleted_(false), fineCompleted_(false),
> -		  lowStep_(0), highStep_(kMaxFocusSteps)
> +		  lowStep_(0), highStep_(kMaxFocusSteps), framesToSkip_(0)
>  	{
>  	}
>
> @@ -92,6 +92,9 @@ protected:
>  	{
>  		currentContrast_ = currentContrast;
>
> +		if (shouldSkipFrame())
> +			return lensPosition_;
> +
>  		/* If we are in a paused state, we won't process the stats */
>  		if (pauseState_ == libcamera::controls::AfPauseStatePaused)
>  			return lensPosition_;
> @@ -113,6 +116,12 @@ protected:
>  		return lensPosition_;
>  	}
>
> +	void setFramesToSkip(uint32_t n)
> +	{
> +		if (n > framesToSkip_)
> +			framesToSkip_ = n;
> +	}
> +
>  private:
>  	void afCoarseScan()
>  	{
> @@ -191,6 +200,7 @@ private:
>  		coarseCompleted_ = false;
>  		fineCompleted_ = false;
>  		maxContrast_ = 0.0;
> +		setFramesToSkip(1);
>  	}
>
>  	bool afIsOutOfFocus()
> @@ -206,6 +216,16 @@ private:
>  			return false;
>  	}
>
> +	bool shouldSkipFrame()
> +	{
> +		if (framesToSkip_ > 0) {
> +			framesToSkip_--;
> +			return true;
> +		}
> +
> +		return false;
> +	}
> +
>  	controls::AfModeEnum mode_;
>  	controls::AfStateEnum state_;
>  	controls::AfPauseStateEnum pauseState_;
> @@ -229,6 +249,7 @@ private:
>
>  	uint32_t lowStep_;
>  	uint32_t highStep_;
> +	uint32_t framesToSkip_;
>
>  	/*
>  	* Maximum focus steps of the VCM control
> --
> 2.34.1
>