[PATCH v2 09/10] ipa: mali-c55: Add Lens Shading Correction algorithm

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Oct 9 17:43:59 CEST 2024


Quoting Daniel Scally (2024-07-09 15:49:49)
> Add a lens shading correction algorithm to the mali-c55 IPA. This
> algorithm parses tables from Yaml in a easy to follow format before
> munging them into Arm's interleaved mesh to be copied to the ISP.
> A colour temperature estimate from the AGC statistics is used to
> select the appropriate table to apply; this can be some interpolation
> of two tables, in which case the colour temperature estimate is also
> used to derive the coefficient that does the blending.
> 
> Acked-by: Nayden Kanchev <nayden.kanchev at arm.com>
> Co-developed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
> Changes in v2:
> 
>         - Use the union rather than reinterpret_cast<>() to abstract the block
> 
>  src/ipa/mali-c55/algorithms/lsc.cpp     | 216 ++++++++++++++++++++++++
>  src/ipa/mali-c55/algorithms/lsc.h       |  45 +++++
>  src/ipa/mali-c55/algorithms/meson.build |   1 +
>  3 files changed, 262 insertions(+)
>  create mode 100644 src/ipa/mali-c55/algorithms/lsc.cpp
>  create mode 100644 src/ipa/mali-c55/algorithms/lsc.h
> 
> diff --git a/src/ipa/mali-c55/algorithms/lsc.cpp b/src/ipa/mali-c55/algorithms/lsc.cpp
> new file mode 100644
> index 00000000..8d574779
> --- /dev/null
> +++ b/src/ipa/mali-c55/algorithms/lsc.cpp
> @@ -0,0 +1,216 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board Oy
> + *
> + * lsc.cpp - Mali-C55 Lens shading correction algorithm
> + */
> +
> +#include "lsc.h"
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55::algorithms {
> +
> +LOG_DEFINE_CATEGORY(MaliC55Lsc)
> +
> +Lsc::Lsc()
> +{
> +}

I don't think we need to declare emtpy/default constructors?

> +
> +int Lsc::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> +{
> +       if (!tuningData.contains("meshScale")) {
> +               LOG(MaliC55Lsc, Error) << "meshScale missing from tuningData";
> +               return -EINVAL;
> +       }
> +
> +       meshScale_ = tuningData["meshScale"].get<uint32_t>(0);
> +
> +       const YamlObject &yamlSets = tuningData["sets"];
> +       if (!yamlSets.isList()) {
> +               LOG(MaliC55Lsc, Error) << "LSC tables missing or invalid";
> +               return -EINVAL;
> +       }
> +
> +       size_t tableSize = 0;
> +       const auto &sets = yamlSets.asList();
> +       for (const auto &yamlSet : sets) {
> +               uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +               if (!ct) {
> +                       LOG(MaliC55Lsc, Error) << "Invalid colour temperature";
> +                       return -EINVAL;
> +               }
> +
> +               if (std::count(colourTemperatures_.begin(),
> +                              colourTemperatures_.end(), ct)) {
> +                       LOG(MaliC55Lsc, Error)
> +                               << "Multiple sets found for colour temperature";
> +                       return -EINVAL;
> +               }
> +
> +               std::vector<uint8_t> rTable =
> +                       yamlSet["r"].getList<uint8_t>().value_or(std::vector<uint8_t>{});
> +               std::vector<uint8_t> gTable =
> +                       yamlSet["g"].getList<uint8_t>().value_or(std::vector<uint8_t>{});
> +               std::vector<uint8_t> bTable =
> +                       yamlSet["b"].getList<uint8_t>().value_or(std::vector<uint8_t>{});
> +
> +               /*
> +                * Some validation to do; only 16x16 and 32x32 tables of
> +                * coefficients are acceptable, and all tables across all of the
> +                * sets must be the same size. The first time we encounter a
> +                * table we check that it is an acceptable size and if so make
> +                * sure all other tables are of equal size.
> +                */
> +               if (!tableSize) {
> +                       if (rTable.size() != 256 && rTable.size() != 1024) {
> +                               LOG(MaliC55Lsc, Error)
> +                                       << "Invalid table size for colour temperature " << ct;
> +                               return -EINVAL;
> +                       }
> +                       tableSize = rTable.size();
> +               }
> +
> +               if (rTable.size() != tableSize ||
> +                   gTable.size() != tableSize ||
> +                   bTable.size() != tableSize) {
> +                       LOG(MaliC55Lsc, Error)
> +                               << "Invalid or mismatched table size for colour temperature " << ct;
> +                       return -EINVAL;
> +               }
> +
> +               if (colourTemperatures_.size() >= 3) {
> +                       LOG(MaliC55Lsc, Error)
> +                               << "A maximum of 3 colour temperatures are supported";
> +                       return -EINVAL;
> +               }
> +
> +               for (unsigned int i = 0; i < tableSize; i++) {
> +                       mesh_[kRedOffset + i] |=
> +                               (rTable[i] << (colourTemperatures_.size() * 8));
> +                       mesh_[kGreenOffset + i] |=
> +                               (gTable[i] << (colourTemperatures_.size() * 8));
> +                       mesh_[kBlueOffset + i] |=
> +                               (bTable[i] << (colourTemperatures_.size() * 8));
> +               }
> +
> +               colourTemperatures_.push_back(ct);
> +       }
> +

A comment here about meshSize_ would be helpful. I have no idea what
this next block of code is doing or why.


Seems that's all I've got in here for now.

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

> +       if (tableSize == 256)
> +               meshSize_ = 15;
> +       else
> +               meshSize_ = 31;
> +
> +       return 0;
> +}
> +
> +size_t Lsc::fillConfigParamsBlock(mali_c55_params_block block) const
> +{
> +       block.header->type = MALI_C55_PARAM_MESH_SHADING_CONFIG;
> +       block.header->enabled = true;
> +       block.header->size = sizeof(struct mali_c55_params_mesh_shading_config);
> +
> +       block.shading_config->mesh_show = false;
> +       block.shading_config->mesh_scale = meshScale_;
> +       block.shading_config->mesh_page_r = 0;
> +       block.shading_config->mesh_page_g = 1;
> +       block.shading_config->mesh_page_b = 2;
> +       block.shading_config->mesh_width = meshSize_;
> +       block.shading_config->mesh_height = meshSize_;
> +
> +       std::copy(mesh_.begin(), mesh_.end(), block.shading_config->mesh);
> +
> +       return block.header->size;
> +}
> +
> +size_t Lsc::fillSelectionParamsBlock(mali_c55_params_block block, uint8_t bank,
> +                                    uint8_t alpha) const
> +{
> +       block.header->type = MALI_C55_PARAM_MESH_SHADING_SELECTION;
> +       block.header->enabled = true;
> +       block.header->size = sizeof(struct mali_c55_params_mesh_shading_selection);
> +
> +       block.shading_selection->mesh_alpha_bank_r = bank;
> +       block.shading_selection->mesh_alpha_bank_g = bank;
> +       block.shading_selection->mesh_alpha_bank_b = bank;
> +       block.shading_selection->mesh_alpha_r = alpha;
> +       block.shading_selection->mesh_alpha_g = alpha;
> +       block.shading_selection->mesh_alpha_b = alpha;
> +       block.shading_selection->mesh_strength = 0x1000; /* Otherwise known as 1.0 */
> +
> +       return block.header->size;
> +}
> +
> +std::tuple<uint8_t, uint8_t> Lsc::findBankAndAlpha(uint32_t ct) const
> +{
> +       unsigned int i;
> +
> +       ct = std::clamp<uint32_t>(ct, colourTemperatures_.front(),
> +                                 colourTemperatures_.back());
> +
> +       for (i = 0; i < colourTemperatures_.size() - 1; i++) {
> +               if (ct >= colourTemperatures_[i] &&
> +                   ct <= colourTemperatures_[i + 1])
> +                       break;
> +       }
> +
> +       /*
> +        * With the clamping, we're guaranteed an index into colourTemperatures_
> +        * that's <= colourTemperatures_.size() - 1.
> +        */
> +       uint8_t alpha = (255 * (ct - colourTemperatures_[i])) /
> +                       (colourTemperatures_[i + 1] - colourTemperatures_[i]);
> +
> +       return { i, alpha };
> +}
> +
> +void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +                 [[maybe_unused]] IPAFrameContext &frameContext,
> +                 mali_c55_params_buffer *params)
> +{
> +       /*
> +        * For each frame we assess the colour temperature of the **last** frame
> +        * and then select an appropriately blended table of coefficients based
> +        * on that ct. As a bit of a shortcut, if we've only a single table the
> +        * handling is somewhat simpler; if it's the first frame we just select
> +        * that table and if we're past the first frame then we can just do
> +        * nothing - the config will never change.
> +        */
> +       uint32_t temperatureK = context.activeState.agc.temperatureK;
> +       uint8_t bank, alpha;
> +
> +       if (colourTemperatures_.size() == 1) {
> +               if (frame > 0)
> +                       return;
> +
> +               bank = 0;
> +               alpha = 0;
> +       } else {
> +               std::tie(bank, alpha) = findBankAndAlpha(temperatureK);
> +       }
> +
> +       mali_c55_params_block block;
> +       block.data = &params->data[params->total_size];
> +
> +       params->total_size += fillSelectionParamsBlock(block, bank, alpha);
> +
> +       if (frame > 0)
> +               return;
> +
> +       /*
> +        * If this is the first frame, we need to load the parsed coefficient
> +        * tables from tuning data to the ISP.
> +        */
> +       block.data = &params->data[params->total_size];
> +       params->total_size += fillConfigParamsBlock(block);
> +}
> +
> +REGISTER_IPA_ALGORITHM(Lsc, "Lsc")
> +
> +} /* namespace ipa::mali_c55::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/mali-c55/algorithms/lsc.h b/src/ipa/mali-c55/algorithms/lsc.h
> new file mode 100644
> index 00000000..ed356f4a
> --- /dev/null
> +++ b/src/ipa/mali-c55/algorithms/lsc.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board Oy
> + *
> + * lsc.h - Mali-C55 Lens shading correction algorithm
> + */
> +
> +#include <map>
> +#include <tuple>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55::algorithms {
> +
> +class Lsc : public Algorithm
> +{
> +public:
> +       Lsc();
> +       ~Lsc() = 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;
> +private:
> +       static constexpr unsigned int kRedOffset = 0;
> +       static constexpr unsigned int kGreenOffset = 1024;
> +       static constexpr unsigned int kBlueOffset = 2048;
> +
> +       size_t fillConfigParamsBlock(mali_c55_params_block block) const;
> +       size_t fillSelectionParamsBlock(mali_c55_params_block block,
> +                                       uint8_t bank, uint8_t alpha) const;
> +       std::tuple<uint8_t, uint8_t> findBankAndAlpha(uint32_t ct) const;
> +
> +       std::vector<uint32_t> mesh_ = std::vector<uint32_t>(3072);
> +       std::vector<uint32_t> colourTemperatures_;
> +       uint32_t meshScale_;
> +       uint32_t meshSize_;
> +};
> +
> +} /* 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 f11791aa..1665da07 100644
> --- a/src/ipa/mali-c55/algorithms/meson.build
> +++ b/src/ipa/mali-c55/algorithms/meson.build
> @@ -4,4 +4,5 @@ mali_c55_ipa_algorithms = files([
>      'agc.cpp',
>      'awb.cpp',
>      'blc.cpp',
> +    'lsc.cpp',
>  ])
> -- 
> 2.34.1
>


More information about the libcamera-devel mailing list