[libcamera-devel] [PATCH v4 05/10] ipa: af_hill_climbing: Add "Windows" metering mode

[Jacopo Mondi]

Hi Daniel

On Tue, Mar 14, 2023 at 03:48:29PM +0100, Daniel Semkowicz wrote:
> Add support for setting user defined auto focus window in the
> AfHillClimbing. This enables usage of AfMetering and AfWindows controls.
> Each time, there is a need for changing the window configuration in the
> ISP, the signal is emitted. Platform layer that wants to use
> the "Windows" metering mode, needs to connect to this signal
> and configure the ISP on each emission.
>
> Currently only one window is supported.

I wonder... shouldn't window hadling be left to the platform layer ?
In other words, should the "common" Af::queueRequest() parse the
windowing controls and then signal the platform layers, or either
should parsing of windowing controls be done in
RkISP1::Af::queueRequest() before (or after) having called the common
class implementation of queueRequest ?
>
> Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> ---
>  .../libipa/algorithms/af_hill_climbing.cpp    | 52 +++++++++++++++++--
>  src/ipa/libipa/algorithms/af_hill_climbing.h  | 12 ++++-
>  2 files changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.cpp b/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> index 2b99afef..3421f240 100644
> --- a/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> +++ b/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> @@ -83,15 +83,28 @@ int AfHillClimbing::init(const YamlObject &tuningData)
>   * \brief Configure the AfHillClimbing with sensor and lens information
>   * \param[in] minFocusPosition Minimum position supported by camera lens
>   * \param[in] maxFocusPosition Maximum position supported by camera lens
> + * \param[in] outputSize Camera sensor output size
>   *
>   * This method should be called in the libcamera::ipa::Algorithm::configure()
>   * method of the platform layer.
>   */
>  void AfHillClimbing::configure(int32_t minFocusPosition,
> -			       int32_t maxFocusPosition)
> +			       int32_t maxFocusPosition,
> +			       const Size &outputSize)
>  {
>  	minLensPosition_ = minFocusPosition;
>  	maxLensPosition_ = maxFocusPosition;
> +
> +	/*
> +	 * Default AF window of 3/4 size of the camera sensor output,
> +	 * placed at the center
> +	 */
> +	defaultWindow_ = Rectangle(outputSize.width / 8,
> +				   outputSize.height / 8,
> +				   3 * outputSize.width / 4,
> +				   3 * outputSize.height / 4);
> +
> +	windowUpdateRequested.emit(defaultWindow_);
>  }
>
>  /**
> @@ -165,6 +178,14 @@ void AfHillClimbing::setFramesToSkip(uint32_t n)
>  		framesToSkip_ = n;
>  }
>
> +/**
> + * \var AfHillClimbing::windowUpdateRequested
> + * \brief Signal emitted when change in AF window was requested
> + *
> + * Platform layer supporting AF windows should connect to this signal
> + * and configure the ISP with new window on each emition.
> + */
> +
>  void AfHillClimbing::setMode(controls::AfModeEnum mode)
>  {
>  	if (mode == mode_)
> @@ -190,14 +211,35 @@ void AfHillClimbing::setSpeed([[maybe_unused]] controls::AfSpeedEnum speed)
>  	LOG(Af, Error) << __FUNCTION__ << " not implemented!";
>  }
>
> -void AfHillClimbing::setMeteringMode([[maybe_unused]] controls::AfMeteringEnum metering)
> +void AfHillClimbing::setMeteringMode(controls::AfMeteringEnum metering)
>  {
> -	LOG(Af, Error) << __FUNCTION__ << " not implemented!";
> +	if (metering == meteringMode_)
> +		return;
> +
> +	if (metering == controls::AfMeteringWindows) {
> +		windowUpdateRequested.emit(userWindow_);
> +	} else {
> +		windowUpdateRequested.emit(defaultWindow_);
> +	}
> +
> +	meteringMode_ = metering;
>  }
>
> -void AfHillClimbing::setWindows([[maybe_unused]] Span<const Rectangle> windows)
> +void AfHillClimbing::setWindows(Span<const Rectangle> windows)
>  {
> -	LOG(Af, Error) << __FUNCTION__ << " not implemented!";
> +	if (windows.size() != 1) {
> +		LOG(Af, Error) << "Only one AF window is supported";
> +		return;
> +	}
> +
> +	/* \todo Check if window size is valid for ISP */
> +
> +	LOG(Af, Debug) << "setWindows: " << windows[0];
> +
> +	userWindow_ = windows[0];
> +
> +	if (meteringMode_ == controls::AfMeteringWindows)
> +		windowUpdateRequested.emit(userWindow_);
>  }
>
>  void AfHillClimbing::setTrigger(controls::AfTriggerEnum trigger)
> diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.h b/src/ipa/libipa/algorithms/af_hill_climbing.h
> index b361e5a1..4f507010 100644
> --- a/src/ipa/libipa/algorithms/af_hill_climbing.h
> +++ b/src/ipa/libipa/algorithms/af_hill_climbing.h
> @@ -10,6 +10,7 @@
>  #pragma once
>
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/signal.h>
>
>  #include "af.h"
>
> @@ -25,13 +26,16 @@ class AfHillClimbing : public Af
>  {
>  public:
>  	int init(const YamlObject &tuningData);
> -	void configure(int32_t minFocusPosition, int32_t maxFocusPosition);
> +	void configure(int32_t minFocusPosition, int32_t maxFocusPosition,
> +		       const Size &outputSize);
>  	int32_t process(double currentContrast);
>  	void setFramesToSkip(uint32_t n);
>
>  	controls::AfStateEnum state() final { return state_; }
>  	controls::AfPauseStateEnum pauseState() final { return pauseState_; }
>
> +	Signal<const Rectangle &> windowUpdateRequested;
> +
>  private:
>  	void setMode(controls::AfModeEnum mode) final;
>  	void setRange(controls::AfRangeEnum range) final;
> @@ -54,6 +58,7 @@ private:
>  	controls::AfModeEnum mode_ = controls::AfModeManual;
>  	controls::AfStateEnum state_ = controls::AfStateIdle;
>  	controls::AfPauseStateEnum pauseState_ = controls::AfPauseStateRunning;
> +	controls::AfMeteringEnum meteringMode_ = controls::AfMeteringAuto;
>
>  	/* VCM step configuration. It is the current setting of the VCM step. */
>  	int32_t lensPosition_ = 0;
> @@ -87,6 +92,11 @@ private:
>
>  	/* Max ratio of variance change, 0.0 < maxChange_ < 1.0 */
>  	double maxChange_;
> +
> +	/* Default AF window */
> +	Rectangle defaultWindow_;
> +	/* AF window set by user using AF_WINDOWS control */
> +	Rectangle userWindow_;
>  };
>
>  } /* namespace ipa::common::algorithms */
> --
> 2.39.2
>