[PATCH v2 06/10] ipa: mali-c55: Add BLC Algorithm

[Kieran Bingham]

Quoting Daniel Scally (2024-07-09 15:49:46)
> From: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> 
> Add a Black Level Correction algorithm.
> 
> Acked-by: Nayden Kanchev  <nayden.kanchev at arm.com>
> Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
> Changes in v2:
> 
>         - Use the union rather than reinterpret_cast<>() to abstract the block
> 
>  src/ipa/mali-c55/algorithms/blc.cpp     | 121 ++++++++++++++++++++++++
>  src/ipa/mali-c55/algorithms/blc.h       |  40 ++++++++
>  src/ipa/mali-c55/algorithms/meson.build |   1 +
>  3 files changed, 162 insertions(+)
>  create mode 100644 src/ipa/mali-c55/algorithms/blc.cpp
>  create mode 100644 src/ipa/mali-c55/algorithms/blc.h
> 
> diff --git a/src/ipa/mali-c55/algorithms/blc.cpp b/src/ipa/mali-c55/algorithms/blc.cpp
> new file mode 100644
> index 00000000..aebb9dc4
> --- /dev/null
> +++ b/src/ipa/mali-c55/algorithms/blc.cpp
> @@ -0,0 +1,121 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board

Perhaps a refresh on dates...

> + *
> + * blc.cpp - Mali-C55 sensor offset (black level) correction

And drop blc.cpp

> + */
> +
> +#include "blc.h"
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/control_ids.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +/**
> + * \file blc.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55::algorithms {
> +
> +/**
> + * \class BlackLevelCorrection
> + * \brief MaliC55 Black Level Correction control
> + *
> + * todo

What's todo here ? Lets do it or remove it...

> + */
> +
> +LOG_DEFINE_CATEGORY(MaliC55Blc)
> +
> +BlackLevelCorrection::BlackLevelCorrection()
> +       : tuningParameters_(false)
> +{
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,
> +                              const YamlObject &tuningData)
> +{
> +       offset00 = tuningData["offset00"].get<uint32_t>(256);
> +       offset01 = tuningData["offset01"].get<uint32_t>(256);
> +       offset10 = tuningData["offset10"].get<uint32_t>(256);
> +       offset11 = tuningData["offset11"].get<uint32_t>(256);
> +
> +       if (offset00 > kMaxOffset || offset01 > kMaxOffset ||
> +           offset10 > kMaxOffset || offset11 > kMaxOffset) {
> +               LOG(MaliC55Blc, Error) << "Invalid black level offsets";
> +               return -EINVAL;
> +       }
> +

I think in here we can also check/read a black level from the
CameraSensorHelper, but take precedence from the tuning file if it's
present.


> +       tuningParameters_ = true;
> +
> +       LOG(MaliC55Blc, Debug)
> +               << "Black levels: 00 " << offset00 << ", 01 " << offset01
> +               << ", 10 " << offset10 << ", 11 " << offset11;
> +
> +       return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,
> +                                  const uint32_t frame,
> +                                  [[maybe_unused]] IPAFrameContext &frameContext,
> +                                  mali_c55_params_buffer *params)
> +{
> +       mali_c55_params_block block;
> +       block.data = &params->data[params->total_size];
> +
> +       if (frame > 0)
> +               return;
> +
> +       if (!tuningParameters_)
> +               return;
> +
> +       block.header->type = MALI_C55_PARAM_BLOCK_SENSOR_OFFS;
> +       block.header->enabled = true;
> +       block.header->size = sizeof(mali_c55_params_sensor_off_preshading);
> +
> +       block.sensor_offs->chan00 = offset00;
> +       block.sensor_offs->chan01 = offset01;
> +       block.sensor_offs->chan10 = offset10;
> +       block.sensor_offs->chan11 = offset11;
> +
> +       params->total_size += block.header->size;
> +}
> +
> +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,
> +                                  [[maybe_unused]] const uint32_t frame,
> +                                  [[maybe_unused]] IPAFrameContext &frameContext,
> +                                  [[maybe_unused]] const mali_c55_stats_buffer *stats,
> +                                  ControlList &metadata)
> +{
> +       /*
> +        * Black Level Offsets in tuning data need to be 20-bit, whereas the
> +        * metadata expects values from a 16-bit range. Right-shift to remove
> +        * the 4 least significant bits.
> +        *
> +        * The black levels should be reported in the order R, Gr, Gb, B. We
> +        * ignore that here given we're using matching values so far, but it
> +        * would be safer to check the sensor's bayer order.
> +        *
> +        * \todo Account for bayer order.
> +        */
> +       metadata.set(controls::SensorBlackLevels, {
> +               static_cast<int32_t>(offset00 >> 4),
> +               static_cast<int32_t>(offset01 >> 4),
> +               static_cast<int32_t>(offset10 >> 4),
> +               static_cast<int32_t>(offset11 >> 4),
> +       });
> +}
> +
> +REGISTER_IPA_ALGORITHM(BlackLevelCorrection, "BlackLevelCorrection")
> +
> +} /* namespace ipa::mali_c55::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/mali-c55/algorithms/blc.h b/src/ipa/mali-c55/algorithms/blc.h
> new file mode 100644
> index 00000000..e73056e0
> --- /dev/null
> +++ b/src/ipa/mali-c55/algorithms/blc.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board

Sounds old ;-)

> + *
> + * blc.h - Mali-C55 sensor offset (black level) correction

I think we've dropped adding filenames perhaps since this was started,
as they just get out of date, and don't add value.


But otherwise, this is a pretty simplistic algo module.

With updates where helpful,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> + */
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55::algorithms {
> +
> +class BlackLevelCorrection : public Algorithm
> +{
> +public:
> +       BlackLevelCorrection();
> +       ~BlackLevelCorrection() = default;
> +
> +       int init(IPAContext &context, const YamlObject &tuningData) override;
> +       void prepare(IPAContext &context, const uint32_t frame,
> +                    IPAFrameContext &frameContext,
> +                    mali_c55_params_buffer *params) override;
> +       void process(IPAContext &context, const uint32_t frame,
> +                    IPAFrameContext &frameContext,
> +                    const mali_c55_stats_buffer *stats,
> +                    ControlList &metadata) override;
> +
> +private:
> +       static constexpr uint32_t kMaxOffset = 0xfffff;
> +
> +       bool tuningParameters_;
> +       uint32_t offset00;
> +       uint32_t offset01;
> +       uint32_t offset10;
> +       uint32_t offset11;
> +};
> +
> +} /* namespace ipa::mali_c55::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/mali-c55/algorithms/meson.build b/src/ipa/mali-c55/algorithms/meson.build
> index f2203b15..d84432b9 100644
> --- a/src/ipa/mali-c55/algorithms/meson.build
> +++ b/src/ipa/mali-c55/algorithms/meson.build
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  mali_c55_ipa_algorithms = files([
> +    'blc.cpp',
>  ])
> -- 
> 2.34.1
>