[libcamera-devel] [PATCH v6 05/10] ipa: af_hill_climbing: Add "Windows" metering mode
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Tue Apr 4 12:59:11 CEST 2023
Hi Daniel
cc RPi folks which originally introduced the AF controls and which
have recently been working a new implementation
On Tue, Apr 04, 2023 at 11:06:54AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> 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 presume so.
I should check how RPi handles windowing
> >
> > 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?
>
Good point. Do we need an AfWindowMaximum ?
RPi any opinions here ?
> > 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