[libcamera-devel,v3,5/8] ipa: rkisp1: Add AF algorithm basing on common AfHillClimbing class

18144 diff mbox series

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@thaumatec.com>
---
 src/ipa/rkisp1/algorithms/af.cpp      | 101 ++++++++++++++++++++++++++
 src/ipa/rkisp1/algorithms/af.h        |  42 +++++++++++
 src/ipa/rkisp1/algorithms/meson.build |   1 +
 3 files changed, 144 insertions(+)
 create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/af.h

Hi Daniel,

Thank you for progressing AF support on RKISP1, and in particular
rebasing all this after all the rework to the algorithm designs.

Quoting Daniel Semkowicz via libcamera-devel (2023-01-19 08:41:09)
> 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@thaumatec.com>
> ---
>  src/ipa/rkisp1/algorithms/af.cpp      | 101 ++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/af.h        |  42 +++++++++++
>  src/ipa/rkisp1/algorithms/meson.build |   1 +
>  3 files changed, 144 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..c2a321cd
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/af.cpp
> @@ -0,0 +1,101 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Theobroma Systems
> + *
> + * af.cpp - RkISP1 AF hill climbing based control algorithm
> + */
> +
> +#include "af.h"
> +
> +/**
> + * \file af.h
> + */
> +
> +namespace libcamera::ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Af
> + * \brief AF control algorithm
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Af)
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Af::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> +{
> +       waitFramesLens_ = tuningData["wait-frames-lens"].get<uint32_t>(1);

I am weary that perhaps many of the other tuning file parameters may not
be well documented, but I'd like to fix that.

Would it be possible to prepare some documentation on the accepted
tuning parameters handled by this algorithm, perhaps collected as a
single documentation block at the top of the file please?


> +
> +       LOG(RkISP1Af, Debug) << "waitFramesLens_: " << waitFramesLens_;
> +
> +       return initBase(tuningData);

I guess we may need some common documentation for the bases
implementation here too.

> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::configure
> + */
> +int Af::configure([[maybe_unused]] IPAContext &context,
> +                 [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> +{
> +       return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Af::queueRequest([[maybe_unused]] IPAContext &context,
> +                     const uint32_t frame,
> +                     [[maybe_unused]] IPAFrameContext &frameContext,
> +                     const ControlList &controls)
> +{
> +       queueRequestBase(frame, controls);
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void Af::prepare([[maybe_unused]] IPAContext &context,
> +                [[maybe_unused]] const uint32_t frame,
> +                [[maybe_unused]] IPAFrameContext &frameContext,
> +                [[maybe_unused]] rkisp1_params_cfg *params)
> +{
> +}

I think algorithm methods are optional. If they're not needed they don't
need to be defined in the header or the class.

The base/virtual implementation in libipa/algorithm.h will cover it so
we don't need to duplicate here with a no-op implementation.

Same for Af::configure()?

> +
> +/**
> + * \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
> +                            << ", Luminance: " << luminance;
> +
> +       uint32_t lensPosition = processAutofocus(sharpness);
> +
> +       if (lensPosition != context.activeState.af.lensPosition) {
> +               context.activeState.af.lensPosition = lensPosition;
> +               context.activeState.af.applyLensCtrls = true;
> +               setFramesToSkip(waitFramesLens_);
> +       }
> +}
> +
> +void Af::setMeteringMode([[maybe_unused]] controls::AfMeteringEnum metering)
> +{
> +       LOG(RkISP1Af, Error) << __FUNCTION__ << " not implemented!";
> +}
> +
> +void Af::setWindows([[maybe_unused]] Span<const Rectangle> windows)
> +{
> +       LOG(RkISP1Af, Error) << __FUNCTION__ << " not implemented!";
> +}
> +
> +REGISTER_IPA_ALGORITHM(Af, "Af")
> +
> +} /* namespace libcamera::ipa::rkisp1::algorithms */
> diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h
> new file mode 100644
> index 00000000..882be952
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/af.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Theobroma Systems
> + *
> + * af.h - RkISP1 AF hill climbing based control algorithm
> + */
> +
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include "libipa/algorithms/af_hill_climbing.h"
> +
> +#include "algorithm.h"
> +
> +namespace libcamera::ipa::rkisp1::algorithms {
> +
> +class Af : public ipa::common::algorithms::AfHillClimbing, public Algorithm
> +{
> +public:
> +       Af() = default;
> +       ~Af() = default;
> +
> +       int init(IPAContext &context, const YamlObject &tuningData) override;
> +       int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> +       void queueRequest(IPAContext &context, const uint32_t frame,
> +                         IPAFrameContext &frameContext, const ControlList &controls) override;
> +       void prepare(IPAContext &context, const uint32_t frame,
> +                    IPAFrameContext &frameContext, rkisp1_params_cfg *params) override;
> +       void process(IPAContext &context, const uint32_t frame,
> +                    IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats,
> +                    ControlList &metadata) override;
> +
> +private:
> +       void setMeteringMode(controls::AfMeteringEnum metering) final;
> +       void setWindows(Span<const Rectangle> windows) final;

These two methods are private and not called in this patch. Maybe just
add them when they are needed rather than adding unused stubs that get
updated later.



> +
> +       /* Wait number of frames after changing lens position */
> +       uint32_t waitFramesLens_;
> +};
> +
> +} /* namespace libcamera::ipa::rkisp1::algorithms */
> 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',
> -- 
> 2.39.0
>