[libcamera-devel] [PATCH v6 07/10] ipa: rkisp1: af: Add "Windows" Metering mode
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Tue Apr 4 12:53:26 CEST 2023
Hi Daniel
On Tue, Apr 04, 2023 at 11:39:46AM +0200, Daniel Semkowicz wrote:
> Hi Jacopo,
>
> On Mon, Apr 3, 2023 at 3:59 PM Jacopo Mondi
> <jacopo.mondi at ideasonboard.com> wrote:
> >
> > Hi Daniel
> >
> > On Fri, Mar 31, 2023 at 10:19:27AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > > Add platform related code for configuring auto focus window on the
> > > rkisp1. Connect to the windowUpdateRequested() signal exposed by the
> > > AfHillClimbing and configure the window on each signal emission.
> > > This enables support of AfMetering and AfWindows controls on the rkisp1
> > > platform.
> > >
> > > Currently, only one window is enabled, but ISP allows up to three
> > > of them.
> > >
> > > Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> > > ---
> > > src/ipa/rkisp1/algorithms/af.cpp | 64 +++++++++++++++++++++++++++++++-
> > > src/ipa/rkisp1/algorithms/af.h | 13 ++++++-
> > > 2 files changed, 75 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp
> > > index fde924d4..b6f6eee4 100644
> > > --- a/src/ipa/rkisp1/algorithms/af.cpp
> > > +++ b/src/ipa/rkisp1/algorithms/af.cpp
> > > @@ -32,16 +32,43 @@ namespace ipa::rkisp1::algorithms {
> > > * amount of time on each movement. This parameter should be set according
> > > * to the worst case - the number of frames it takes to move lens between
> > > * limit positions.
> > > + * - **isp-threshold:** Threshold used for minimizing the influence of noise.
> > > + * This affects the ISP sharpness calculation.
> > > + * - **isp-var-shift:** The number of bits for the shift operation at the end
> > > + * of the calculation chain. This affects the ISP sharpness calculation.
> >
> > I wonder if the introduction of these values belong to this patch or
> > not. I guess they affect the AF algorithm globally, as a default
> > window has to be programmed to have it working (something that happens in this
> > patch, you might say :)
>
> You are right, these parameters should be moved to the previous commit.
> I found that if no AF window was configured, ISP has a default one
> (probably maximum AF window size), but this is not documented. This
> way rkisp1 AF algo should work even without this commit.
>
> >
> > Regardless of that, have you been able to identify with a little more
> > accuracy how these values should be computed ? For the threshold I
> > guess the explanation is somewhat clear, but the var-shift parameter I
> > wonder how one should compute it ? As I read it, the var-shift value
> > represents the number number of bits to right-shift the pixel values
> > before summing them to avoid overflows of the afm_sum register (32
> > bits). How did you come up with a value of 4 in your configuration
> > file ? Does this value depend on the window size or does it depend on
> > the sensor in use ?
>
> I came up with 4 just by experimenting with my setup. It worked even
> with 0, but I shifted it a little bit for safety.
> There is very little documentation on this, so unfortunately I know
> only as much as you.
>
a 4 position bit-shift does halve the information for an 8-bit Bayer
sensor... I presume this is safe when it comes to overflow but reduces
the accuracy.
Ideally this value would be tuned according to the number of pixels
that will be summed (per window ??) and the sensor's bit depth. By
dividing (2^32-1 / num_pixels) you would get what the maximum allowed
value per pixel would be and then you would have to adust right shift
enough to make sure all your pixel values staty in that limit.
I'm not asking to do this right now, but I wonder if these values
should really come from configuration file or should be computed here
(hard-coded for the moment).
Did you ever experienced overflow ?
> >
> > > * .
> > > * \sa libcamera::ipa::algorithms::AfHillClimbing for additional tuning
> > > * parameters.
> > > *
> > > * \todo Model the lens delay as number of frames required for the lens position
> > > * to stabilize in the CameraLens class.
> > > + * \todo Check if requested window size is valid. RkISP supports AF window size
> > > + * few pixels smaller than sensor output size.
> > > + * \todo Implement support for all available AF windows. RkISP supports up to 3
> > > + * AF windows.
> > > */
> > >
> > > LOG_DEFINE_CATEGORY(RkISP1Af)
> > >
> > > +namespace {
> > > +
> > > +constexpr rkisp1_cif_isp_window rectangleToIspWindow(const Rectangle &rectangle)
> > > +{
> > > + return rkisp1_cif_isp_window{
> > > + .h_offs = static_cast<uint16_t>(rectangle.x),
> > > + .v_offs = static_cast<uint16_t>(rectangle.y),
> > > + .h_size = static_cast<uint16_t>(rectangle.width),
> > > + .v_size = static_cast<uint16_t>(rectangle.height)
> > > + };
> > > +}
> > > +
> > > +} /* namespace */
> > > +
> > > +Af::Af()
> > > +{
> > > + af.windowUpdateRequested.connect(this, &Af::updateCurrentWindow);
> > > +}
> > > +
> > > /**
> > > * \copydoc libcamera::ipa::Algorithm::init
> > > */
> > > @@ -54,8 +81,12 @@ int Af::init(IPAContext &context, const YamlObject &tuningData)
> > > }
> > >
> > > waitFramesLens_ = tuningData["wait-frames-lens"].get<uint32_t>(1);
> > > + ispThreshold_ = tuningData["isp-threshold"].get<uint32_t>(128);
> > > + ispVarShift_ = tuningData["isp-var-shift"].get<uint32_t>(4);
> > >
> > > - LOG(RkISP1Af, Debug) << "waitFramesLens_: " << waitFramesLens_;
> > > + LOG(RkISP1Af, Debug) << "waitFramesLens_: " << waitFramesLens_
> > > + << ", ispThreshold_: " << ispThreshold_
> > > + << ", ispVarShift_: " << ispVarShift_;
> > >
> > > return af.init(lensConfig->minFocusPosition,
> > > lensConfig->maxFocusPosition, tuningData);
> > > @@ -89,6 +120,32 @@ void Af::queueRequest([[maybe_unused]] IPAContext &context,
> > > af.queueRequest(controls);
> > > }
> > >
> > > +/**
> > > + * \copydoc libcamera::ipa::Algorithm::prepare
> > > + */
> > > +void Af::prepare([[maybe_unused]] IPAContext &context,
> > > + [[maybe_unused]] const uint32_t frame,
> > > + [[maybe_unused]] IPAFrameContext &frameContext,
> > > + rkisp1_params_cfg *params)
> > > +{
> > > + if (updateWindow_) {
> >
> > or
> > if (!updateWindow_)
> > return;
> >
> > > + params->meas.afc_config.num_afm_win = 1;
> > > + params->meas.afc_config.thres = ispThreshold_;
> > > + params->meas.afc_config.var_shift = ispVarShift_;
> > > + params->meas.afc_config.afm_win[0] =
> > > + rectangleToIspWindow(*updateWindow_);
> > > +
> > > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AFC;
> > > + params->module_ens |= RKISP1_CIF_ISP_MODULE_AFC;
> > > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_AFC;
> > > +
> > > + updateWindow_.reset();
> > > +
> > > + /* Wait one frame for the ISP to apply changes. */
> > > + af.skipFrames(1);
> > > + }
> > > +}
> > > +
> > > /**
> > > * \copydoc libcamera::ipa::Algorithm::process
> > > */
> > > @@ -114,6 +171,11 @@ void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > > }
> > > }
> > >
> > > +void Af::updateCurrentWindow(const Rectangle &window)
> > > +{
> > > + updateWindow_ = window;
> > > +}
> > > +
> > > REGISTER_IPA_ALGORITHM(Af, "Af")
> > >
> > > } /* namespace ipa::rkisp1::algorithms */
> > > diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h
> > > index 3ba66d38..6f5adb19 100644
> > > --- a/src/ipa/rkisp1/algorithms/af.h
> > > +++ b/src/ipa/rkisp1/algorithms/af.h
> > > @@ -20,21 +20,32 @@ namespace ipa::rkisp1::algorithms {
> > > class Af : public Algorithm
> > > {
> > > public:
> > > + Af();
> > > +
> > > int init(IPAContext &context, const YamlObject &tuningData) override;
> > > int configure(IPAContext &context,
> > > const IPACameraSensorInfo &configInfo) override;
> > > void queueRequest(IPAContext &context, uint32_t frame,
> > > IPAFrameContext &frameContext,
> > > const ControlList &controls) override;
> > > + void prepare(IPAContext &context, uint32_t frame,
> > > + IPAFrameContext &frameContext,
> > > + rkisp1_params_cfg *params) override;
> > > void process(IPAContext &context, uint32_t frame,
> > > IPAFrameContext &frameContext,
> > > const rkisp1_stat_buffer *stats,
> > > ControlList &metadata) override;
> > >
> > > private:
> > > + void updateCurrentWindow(const Rectangle &window);
> > > +
> > > ipa::algorithms::AfHillClimbing af;
> > >
> > > - /* Wait number of frames after changing lens position */
> > > + std::optional<Rectangle> updateWindow_;
> > > + uint32_t ispThreshold_ = 0;
> > > + uint32_t ispVarShift_ = 0;
> > > +
> > > + /* Wait number of frames after changing lens position. */
> > > uint32_t waitFramesLens_ = 0;
> > > };
> > >
> > > --
> > > 2.39.2
> > >
>
> Best regards
> Daniel
More information about the libcamera-devel
mailing list