[libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF algorithm based on AfHillClimbing

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Apr 26 06:04:17 CEST 2023


On Wed, Apr 26, 2023 at 06:46:58AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Fri, Mar 31, 2023 at 10:19:26AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > Add contrast based auto focus implementation for rkisp1 platform using
> > the common AfHillClimbing algorithm.
> > 
> > Rockchip ISP AF block allows calculation of sharpness and luminance
> > in up to three user defined windows. If no windows are set, there are
> > some default settings applied for the first window and exposed through
> > the driver. For each frame, use the sharpness value calculated for this
> > default window and feed the hill climbing algorithm with them. Then set
> > the lens position to value calculated by the algorithm.
> > 
> > Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> > ---
> >  src/ipa/rkisp1/algorithms/af.cpp      | 121 ++++++++++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/af.h        |  43 +++++++++
> >  src/ipa/rkisp1/algorithms/meson.build |   1 +
> >  src/ipa/rkisp1/ipa_context.cpp        |  13 +++
> >  src/ipa/rkisp1/ipa_context.h          |   5 ++
> >  5 files changed, 183 insertions(+)
> >  create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/af.h
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp
> > new file mode 100644
> > index 00000000..fde924d4
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/af.cpp
> > @@ -0,0 +1,121 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2023, Theobroma Systems
> > + *
> > + * af.cpp - RkISP1 AF hill climbing based control algorithm
> 
> s/climbing based/climbing-based/
> 
> > + */
> > +
> > +#include "af.h"
> > +
> > +/**
> > + * \file af.h
> 
> Missing \brief
> 
> > + */
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +/**
> > + * \class Af
> > + * \brief AF contrast based hill climbing control algorithm for RkISP platforms
> 
> s/contrast based/contrast-based/
> 
> > + *
> > + * Auto focus algorithm for RkISP platforms, based on the common hill climbing
> > + * auto focus implementation (libcamera::ipa::algorithms::AfHillClimbing).
> > + *
> > + * This is the platform layer of the algorithm.
> > + *
> > + * Tuning file parameters:
> > + * - **wait-frames-lens:** Number of frames that should be skipped when lens
> 
> rkisp1 tuning files use camelCase for parameter names.
> 
> > + *   position is changed. Lens movement takes some time and statistics measured
> > + *   during the lens movement are unstable. Currently there is no way to know
> > + *   when lens movement finished and this is a workaround for this. Wait a fixed
> > + *   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.
> > + * .
> 
> Stray period ?
> 
> > + * \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.
> 
> More than that, we should take the lens movement into account in the
> algorithm to avoid skipping frames at all.
> 
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(RkISP1Af)
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int Af::init(IPAContext &context, const YamlObject &tuningData)
> > +{
> > +	const auto &lensConfig = context.configuration.lens;
> > +	if (!lensConfig) {
> > +		LOG(RkISP1Af, Error) << "Lens not found";
> > +		return 1;
> > +	}
> > +
> > +	waitFramesLens_ = tuningData["wait-frames-lens"].get<uint32_t>(1);
> > +
> > +	LOG(RkISP1Af, Debug) << "waitFramesLens_: " << waitFramesLens_;
> > +
> > +	return af.init(lensConfig->minFocusPosition,
> > +		       lensConfig->maxFocusPosition, tuningData);
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::configure
> > + */
> > +int Af::configure(IPAContext &context,
> > +		  const IPACameraSensorInfo &configInfo)
> > +{
> > +	/*
> > +	 * Lens position is unknown at the startup, so initialize
> > +	 * the current position to something out of range.
> > +	 */
> > +	context.activeState.af.lensPosition =
> > +		context.configuration.lens->maxFocusPosition + 1;
> > +
> > +	af.configure(configInfo.outputSize);
> > +	return 0;
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::queueRequest
> > + */
> > +void Af::queueRequest([[maybe_unused]] IPAContext &context,
> > +		      [[maybe_unused]] const uint32_t frame,
> > +		      [[maybe_unused]] IPAFrameContext &frameContext,
> > +		      const ControlList &controls)
> > +{
> > +	af.queueRequest(controls);
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::process
> > + */
> > +void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > +		 [[maybe_unused]] IPAFrameContext &frameContext,
> > +		 const rkisp1_stat_buffer *stats,
> > +		 [[maybe_unused]] ControlList &metadata)
> > +{
> > +	uint32_t sharpness = stats->params.af.window[0].sum;
> > +	uint32_t luminance = stats->params.af.window[0].lum;
> > +
> > +	LOG(RkISP1Af, Debug)
> > +		<< "lensPosition: " << context.activeState.af.lensPosition
> > +		<< ", Sharpness: " << sharpness
> 
> Maybe contrast, as everything in this series talks about contrast-based
> AF ?
> 
> > +		<< ", Luminance: " << luminance;
> 
> The luminance isn't used, so you could drop it.

Or better, you should use it :-) The luminance is meant to normalize the
sharpness values.

> > +
> > +	int32_t newLensPosition = af.process(sharpness);
> > +
> > +	if (newLensPosition != context.activeState.af.lensPosition) {
> > +		context.activeState.af.lensPosition = newLensPosition;
> > +		context.activeState.af.applyLensCtrls = true;
> > +		af.skipFrames(waitFramesLens_);
> 
> I'm tempted to implement frame skipping in this class instead of the 
> ipa::algorithms::AfHillClimbing class. It would be simpler, we could
> just skip calling process(). On the other hand, I suppose
> ipa::algorithms::AfHillClimbing will need to change when we'll improve
> the mechanism that takes lens movement into account, and it's hard to
> predict how the interface will change.
> 
> > +	}
> > +}
> 
> The IPA module should also report the lens position in metadata.
> 
> > +
> > +REGISTER_IPA_ALGORITHM(Af, "Af")
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h
> > new file mode 100644
> > index 00000000..3ba66d38
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/af.h
> > @@ -0,0 +1,43 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2023, Theobroma Systems
> > + *
> > + * af.h - RkISP1 AF hill climbing based control algorithm
> 
> s/climbing based/climbing-based/
> 
> > + */
> > +
> > +#pragma once
> > +
> > +#include <linux/rkisp1-config.h>
> > +
> > +#include "libipa/algorithms/af_hill_climbing.h"
> > +
> > +#include "algorithm.h"
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +class Af : public Algorithm
> > +{
> > +public:
> > +	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 process(IPAContext &context, uint32_t frame,
> > +		     IPAFrameContext &frameContext,
> > +		     const rkisp1_stat_buffer *stats,
> > +		     ControlList &metadata) override;
> > +
> > +private:
> > +	ipa::algorithms::AfHillClimbing af;
> 
> s/af/af_/
> 
> > +
> > +	/* Wait number of frames after changing lens position */
> > +	uint32_t waitFramesLens_ = 0;
> > +};
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> > index 93a48329..ab7e44f3 100644
> > --- a/src/ipa/rkisp1/algorithms/meson.build
> > +++ b/src/ipa/rkisp1/algorithms/meson.build
> > @@ -1,6 +1,7 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> >  rkisp1_ipa_algorithms = files([
> > +    'af.cpp',
> >      'agc.cpp',
> >      'awb.cpp',
> >      'blc.cpp',
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index aea99299..d80a7b1b 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -139,6 +139,19 @@ namespace libcamera::ipa::rkisp1 {
> >   * member may be read by any algorithm, but shall only be written by its owner.
> >   */
> >  
> > +/**
> > + * \var IPAActiveState::af
> > + * \brief State for the Automatic Focus Control algorithm
> > + *
> > + * \var IPAActiveState::af.lensPosition
> > + * \brief Lens position calculated by the AF algorithm
> > + *
> > + * \var IPAActiveState::af.applyLensCtrls
> > + * \brief Whether the lens position should be applied
> > + *
> > + * If true, IPA should send new controls to the PH to set new lens position.
> > + */
> > +
> >  /**
> >   * \var IPAActiveState::agc
> >   * \brief State for the Automatic Gain Control algorithm
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 65b3fbfe..3dcc5aa0 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -62,6 +62,11 @@ struct IPASessionConfiguration {
> >  };
> >  
> >  struct IPAActiveState {
> > +	struct {
> > +		int32_t lensPosition;
> > +		bool applyLensCtrls;
> 
> I suppose applyLens will be used later in the series.
> 
> > +	} af;
> > +
> >  	struct {
> >  		struct {
> >  			uint32_t exposure;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list