[libcamera-devel,v4,05/10] ipa: af_hill_climbing: Add "Windows" metering mode
diff mbox series

Message ID 20230314144834.85193-6-dse@thaumatec.com
State Superseded
Headers show
Series
  • ipa: rkisp1: Add autofocus algorithm
Related show

Commit Message

Daniel Semkowicz March 14, 2023, 2:48 p.m. UTC 
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.

Signed-off-by: Daniel Semkowicz <dse@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(-)

Comments

Jacopo Mondi March 20, 2023, 8:53 p.m. UTC | #1 
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@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
>
Daniel Semkowicz March 21, 2023, 7:45 a.m. UTC | #2 
Hi Jacopo,

On Mon, Mar 20, 2023 at 9:54 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> 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 ?

There is a simple state machine for window mode. This will need to be
duplicated in each platform implementation. This is why I see it fits
better in this place.
Do you see any obstacles that could make this code unusable for other
platforms?

> >
> > Signed-off-by: Daniel Semkowicz <dse@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
> >

Best regards
Daniel

Patch
diff mbox series 

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 */