[libcamera-devel] [PATCH v6 05/10] ipa: af_hill_climbing: Add "Windows" metering mode
Daniel Semkowicz
dse at thaumatec.com
Tue Apr 4 11:06:54 CEST 2023
Hi Jacopo,
On Mon, Apr 3, 2023 at 3:07 PM Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Daniel
> sorry I have neglected the Windows related patches in previous
> reviews as I hoped someone would have shared an opinion on the
> Signal-based mechanism.
>
> I don't have better ideas to propose, so let's go with it
>
> On Fri, Mar 31, 2023 at 10:19:25AM +0200, Daniel Semkowicz via libcamera-devel 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.
> >
> > Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> > ---
> > .../libipa/algorithms/af_hill_climbing.cpp | 62 +++++++++++++++++--
> > src/ipa/libipa/algorithms/af_hill_climbing.h | 10 +++
> > 2 files changed, 66 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.cpp b/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> > index 244b8803..0fb17df3 100644
> > --- a/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> > +++ b/src/ipa/libipa/algorithms/af_hill_climbing.cpp
> > @@ -63,9 +63,8 @@ LOG_DEFINE_CATEGORY(Af)
> > * movement range rather than coarse search result.
> > * \todo Implement setRange.
> > * \todo Implement setSpeed.
> > - * \todo Implement setMeteringMode.
> > - * \todo Implement setWindows.
> > * \todo Implement the AfPauseDeferred mode.
> > + * \todo Implement support for multiple AF windows.
> > */
> >
> > /**
> > @@ -99,6 +98,27 @@ int AfHillClimbing::init(int32_t minFocusPosition, int32_t maxFocusPosition,
> > return 0;
> > }
> >
> > +/**
> > + * \brief Configure the AfHillClimbing with sensor information
> > + * \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(const Size &outputSize)
>
> According to the AfWindows control definition
>
> Sets the focus windows used by the AF algorithm when AfMetering is set
> to AfMeteringWindows. The units used are pixels within the rectangle
> returned by the ScalerCropMaximum property.
>
> The final sensor's output size can be obtained by downscaling/cropping
> a larger portion of the pixel array. The portion of the pixel array
> processed to obtain the final image is named AnalogueCropRectangle and
> all valid crop rectangles lie inside it.
>
> Most of our controls, such as ScalerCrop, are expressed with the
> AnalogueCrop as reference, to allow expressing them regardless of the
> current output size. It's up to the pipeline handler to re-scale any
> valid control in the current configured output.
For the 1024x768 stream, I have the following parameters:
- activeAreaSize: 2592x1944
- analogCrop: (0, 0)/2592x1944
- outputSize: 1296x972
When using analogCrop, I will also need to scale the AF window
size when configuring the rkisp1_params_cfg. rkisp1_params_cfg takes
values in reference to the ISP input size
(IPACameraSensorInfo::outputSize).
Based on what you wrote earlier, I understand that it is supposed to be
like that?
>
> I'm not 100% sure why we decided to use ScalerCropMaximum here, most
> probably to allow pipeline handler express any additional processing
> margins required by any cropping/scaling that happens on the ISP.
>
> However as the RkISP1 pipeline doesn't express any of those margin
> yet, I would simply use here the sensor's analogue crop, which is part
> of the IPACameraSensorInfo you already use in the platform layer.
>
I have some concerns about the ScalerCropMaximum.
What if the ISP have different size margins for cropping and auto-focus
windows? I see there are margins defined for the AF window, but nothing
about the cropping margins in the RK3399 documentation. In this case,
it will not be a problem, but what if some ISP will have margins for
cropping, but no margins for AF window?
> To remove ambiguities I would call the parameter here "maxWindow" or
> something similar.
>
> I would also initialize userWindow_ to the same value, so that if an
> application switches to AfMeteringWindows mode the rectangle is at
> least initialized to something.
Good point!
>
>
> > +{
> > + /*
> > + * Default AF window of 3/4 size of the camera sensor output,
> > + * placed at the center
> > + */
> > + defaultWindow_ = Rectangle(static_cast<int>(outputSize.width / 8),
> > + static_cast<int>(outputSize.height / 8),
> > + 3 * outputSize.width / 4,
> > + 3 * outputSize.height / 4);
> > +
> > + windowUpdateRequested.emit(defaultWindow_);
> > +}
> > +
> > /**
> > * \brief Run the auto focus algorithm loop
> > * \param[in] currentContrast New value of contrast measured for current frame
> > @@ -185,6 +205,14 @@ void AfHillClimbing::skipFrames(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_)
> > @@ -210,14 +238,36 @@ void AfHillClimbing::setSpeed([[maybe_unused]] controls::AfSpeedEnum speed)
> > LOG(Af, Error) << "setSpeed() not implemented!";
> > }
> >
> > -void AfHillClimbing::setMeteringMode([[maybe_unused]] controls::AfMeteringEnum metering)
> > +void AfHillClimbing::setMeteringMode(controls::AfMeteringEnum metering)
> > {
> > - LOG(Af, Error) << "setMeteringMode() not implemented!";
> > + if (metering == meteringMode_)
> > + return;
> > +
> > + meteringMode_ = metering;
> > +
> > + switch (metering) {
> > + case controls::AfMeteringAuto:
> > + windowUpdateRequested.emit(defaultWindow_);
> > + break;
> > + case controls::AfMeteringWindows:
> > + windowUpdateRequested.emit(userWindow_);
> > + break;
> > + }
> > }
> >
> > -void AfHillClimbing::setWindows([[maybe_unused]] Span<const Rectangle> windows)
> > +void AfHillClimbing::setWindows(Span<const Rectangle> windows)
> > {
> > - LOG(Af, Error) << "setWindows() not implemented!";
> > + if (windows.size() != 1) {
> > + LOG(Af, Error) << "Only one AF window is supported";
> > + return;
> > + }
>
>
> Should this be an hard error or should we just want and continue by
> only considering the first available rectangle ?
I will change it to a warning that only the first window was used.
>
> > +
> > + 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 2147939b..0f7c65db 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"
> >
> > @@ -26,12 +27,15 @@ class AfHillClimbing : public Af
> > public:
> > int init(int32_t minFocusPosition, int32_t maxFocusPosition,
> > const YamlObject &tuningData);
> > + void configure(const Size &outputSize);
> > int32_t process(double currentContrast);
> > void skipFrames(uint32_t n);
> >
> > controls::AfStateEnum state() override { return state_; }
> > controls::AfPauseStateEnum pauseState() override { return pauseState_; }
> >
> > + Signal<const Rectangle &> windowUpdateRequested;
> > +
> > private:
> > void setMode(controls::AfModeEnum mode) override;
> > void setRange(controls::AfRangeEnum range) override;
> > @@ -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;
> >
> > /* Current focus lens position. */
> > int32_t lensPosition_ = 0;
> > @@ -84,6 +89,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::algorithms */
> > --
> > 2.39.2
> >
Best regards
Daniel
More information about the libcamera-devel
mailing list