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 :)
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 ?
> * .
> * \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
>