[PATCH v5 14/18] libcamera: software_isp: Move color handling to an algorithm module
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Sep 6 15:02:27 CEST 2024
On Fri, Sep 06, 2024 at 12:41:07PM +0200, Milan Zamazal wrote:
> Laurent Pinchart writes:
> > On Fri, Aug 30, 2024 at 09:25:50AM +0200, Milan Zamazal wrote:
> >> After black level handling has been moved to an algorithm module, white
> >> balance and the construction of color tables can be moved to algorithm
> >> modules too.
> >>
> >> This time, the moved code is split between stats processing and
> >> parameter construction methods. It is also split to three algorithm
> >> modules:
> >>
> >> - Gamma table computation. This is actually independent of color
> >> handling.
> >>
> >> - White balance computation.
> >>
> >> - Color lookup tables construction. While this applies the color gains
> >> computed by the white balance algorithm, it is not directly related to
> >> white balance. We may want to modify the color lookup tables in
> >> future according to other parameters than just gamma and white balance
> >> gains.
> >>
> >> This is the only part of the software ISP algorithms that sets the
> >> parameters so emitting setIspParams can be moved to prepare() method.
> >>
> >> A more natural representation of the gains (and black level) would be
> >> floating point numbers. This is not done here in order to minimize
> >> changes in code movements. It will be addressed in a followup patch.
> >>
> >> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >> ---
> >> src/ipa/simple/algorithms/awb.cpp | 70 +++++++++++++++++++++++++++
> >> src/ipa/simple/algorithms/awb.h | 38 +++++++++++++++
> >> src/ipa/simple/algorithms/gamma.cpp | 64 ++++++++++++++++++++++++
> >> src/ipa/simple/algorithms/gamma.h | 42 ++++++++++++++++
> >> src/ipa/simple/algorithms/lut.cpp | 57 ++++++++++++++++++++++
> >> src/ipa/simple/algorithms/lut.h | 37 ++++++++++++++
> >> src/ipa/simple/algorithms/meson.build | 3 ++
> >> src/ipa/simple/data/uncalibrated.yaml | 3 ++
> >> src/ipa/simple/ipa_context.cpp | 30 ++++++++++++
> >> src/ipa/simple/ipa_context.h | 12 +++++
> >> src/ipa/simple/soft_simple.cpp | 66 ++-----------------------
> >> 11 files changed, 360 insertions(+), 62 deletions(-)
> >> create mode 100644 src/ipa/simple/algorithms/awb.cpp
> >> create mode 100644 src/ipa/simple/algorithms/awb.h
> >> create mode 100644 src/ipa/simple/algorithms/gamma.cpp
> >> create mode 100644 src/ipa/simple/algorithms/gamma.h
> >> create mode 100644 src/ipa/simple/algorithms/lut.cpp
> >> create mode 100644 src/ipa/simple/algorithms/lut.h
> >>
> >> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
> >> new file mode 100644
> >> index 00000000..4b49996a
> >> --- /dev/null
> >> +++ b/src/ipa/simple/algorithms/awb.cpp
> >> @@ -0,0 +1,70 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2024, Red Hat Inc.
> >> + *
> >> + * Auto white balance
> >> + */
> >> +
> >> +#include "awb.h"
> >> +
> >> +#include <numeric>
> >> +#include <stdint.h>
> >> +
> >> +#include <libcamera/base/log.h>
> >> +
> >> +#include "simple/ipa_context.h"
> >> +
> >> +namespace libcamera {
> >> +
> >> +LOG_DEFINE_CATEGORY(IPASoftAwb)
> >> +
> >> +namespace ipa::soft::algorithms {
> >> +
> >> +int Awb::init(IPAContext &context,
> >> + [[maybe_unused]] const YamlObject &tuningData)
> >> +{
> >> + auto &gains = context.activeState.gains;
> >> + gains.red = gains.green = gains.blue = 256;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +void Awb::process(IPAContext &context,
> >> + [[maybe_unused]] const uint32_t frame,
> >> + [[maybe_unused]] IPAFrameContext &frameContext,
> >> + const SwIspStats *stats,
> >> + [[maybe_unused]] ControlList &metadata)
> >> +{
> >> + const SwIspStats::Histogram &histogram = stats->yHistogram;
> >> + const uint8_t blackLevel = context.activeState.black.level;
> >> +
> >> + /*
> >> + * Black level must be subtracted to get the correct AWB ratios, they
> >> + * would be off if they were computed from the whole brightness range
> >> + * rather than from the sensor range.
> >> + */
> >> + const uint64_t nPixels = std::accumulate(
> >> + histogram.begin(), histogram.end(), 0);
> >> + const uint64_t offset = blackLevel * nPixels;
> >> + const uint64_t sumR = stats->sumR_ - offset / 4;
> >> + const uint64_t sumG = stats->sumG_ - offset / 2;
> >> + const uint64_t sumB = stats->sumB_ - offset / 4;
> >> +
> >> + /*
> >> + * Calculate red and blue gains for AWB.
> >> + * Clamp max gain at 4.0, this also avoids 0 division.
> >> + * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
> >> + */
> >> + auto &gains = context.activeState.gains;
> >> + gains.red = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;
> >> + gains.blue = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;
> >> + /* Green gain is fixed to 256 */
> >> +
> >> + LOG(IPASoftAwb, Debug) << "gain R/B " << gains.red << "/" << gains.blue;
> >> +}
> >> +
> >> +REGISTER_IPA_ALGORITHM(Awb, "Awb")
> >> +
> >> +} /* namespace ipa::soft::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h
> >> new file mode 100644
> >> index 00000000..c0b88bec
> >> --- /dev/null
> >> +++ b/src/ipa/simple/algorithms/awb.h
> >> @@ -0,0 +1,38 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2024, Red Hat Inc.
> >> + *
> >> + * Auto white balance
> >> + */
> >> +
> >> +#pragma once
> >> +
> >> +#include <libcamera/controls.h>
> >> +
> >> +#include "libcamera/internal/software_isp/swisp_stats.h"
> >> +
> >> +#include "algorithm.h"
> >> +#include "ipa_context.h"
> >
> > As with the previous patch, I think you can drop all the includes except
> > algorithm.h. Same for the other algorithms.
> >
> >> +
> >> +namespace libcamera {
> >> +
> >> +namespace ipa::soft::algorithms {
> >> +
> >> +class Awb : public Algorithm
> >> +{
> >> +public:
> >> + Awb() = default;
> >> + ~Awb() = default;
> >> +
> >> + int init(IPAContext &context, const YamlObject &tuningData)
> >> + override;
> >
> > No need to split this on two lines. Same below.
> >
> >> + void process(IPAContext &context,
> >> + const uint32_t frame,
> >> + IPAFrameContext &frameContext,
> >> + const SwIspStats *stats,
> >> + ControlList &metadata) override;
> >> +};
> >> +
> >> +} /* namespace ipa::soft::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/simple/algorithms/gamma.cpp b/src/ipa/simple/algorithms/gamma.cpp
> >> new file mode 100644
> >> index 00000000..0b8ec5ee
> >> --- /dev/null
> >> +++ b/src/ipa/simple/algorithms/gamma.cpp
> >> @@ -0,0 +1,64 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2024, Red Hat Inc.
> >> + *
> >> + * Gamma table construction
> >> + */
> >> +
> >> +#include "gamma.h"
> >> +
> >> +#include <algorithm>
> >> +#include <cmath>
> >> +#include <stdint.h>
> >> +
> >> +#include <libcamera/base/log.h>
> >> +
> >> +#include "simple/ipa_context.h"
> >> +
> >> +namespace libcamera {
> >> +
> >> +namespace ipa::soft::algorithms {
> >> +
> >> +int Gamma::init(IPAContext &context,
> >> + [[maybe_unused]] const YamlObject &tuningData)
> >> +{
> >> + /* Gamma value is fixed */
> >> + context.configuration.gamma = 0.5;
> >> + updateGammaTable(context);
> >
> > Same question as before, does this belong to .init() ? Actually, given
> > that prepare() will always be called to calculate ISP parameters, do you
> > even need to initialize the gamme table ?
>
> The update of the gamma table in prepare() is conditional and I wouldn't
> rely on the initial values of the variables used in the condition;
> making sure the table is initialized is safer and it doesn't harm to do
> it in init(). Once the gamma table is initialized, we can rely on the
> conditional update in prepare(), so there is no need to update the gamma
> table in configure() again.
Then I think it should be initialized in configure() instead, to avoid
retaining state between capture sessions.
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +void Gamma::updateGammaTable(IPAContext &context)
> >> +{
> >> + auto &gammaTable = context.activeState.gamma.gammaTable;
> >> + auto blackLevel = context.activeState.black.level;
> >> + const unsigned int blackIndex =
> >> + blackLevel * IPAActiveState::kGammaLookupSize / 256;
> >
> > Use gammaTable.size() instead of kGammaLookupSize.
> >
> >> + std::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);
> >> + const float divisor = kGammaLookupSize - blackIndex - 1.0;
> >> + for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
> >
> > Same here.
> >
> >> + gammaTable[i] = UINT8_MAX * powf((i - blackIndex) / divisor,
> >
> > std::powf
>
> std::pow actually, I suppose.
Yes.
> >> + context.configuration.gamma);
> >> + context.activeState.gamma.blackLevel = blackLevel;
> >> +}
> >> +
> >> +void Gamma::prepare(IPAContext &context,
> >> + [[maybe_unused]] const uint32_t frame,
> >> + [[maybe_unused]] IPAFrameContext &frameContext,
> >> + [[maybe_unused]] DebayerParams *params)
> >> +{
> >> + /*
> >> + * Update the gamma table if needed. This means if black level changes and
> >> + * since the black level gets updated only if a lower value is observed,
> >> + * it's not permanently prone to minor fluctuations or rounding errors.
> >
> > Please reflow to 80 columns.
> >
> >> + */
> >> + if (context.activeState.gamma.blackLevel != context.activeState.black.level)
> >> + updateGammaTable(context);
> >> +}
> >> +
> >> +REGISTER_IPA_ALGORITHM(Gamma, "Gamma")
> >> +
> >> +} /* namespace ipa::soft::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/simple/algorithms/gamma.h b/src/ipa/simple/algorithms/gamma.h
> >> new file mode 100644
> >> index 00000000..43e88e87
> >> --- /dev/null
> >> +++ b/src/ipa/simple/algorithms/gamma.h
> >> @@ -0,0 +1,42 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2024, Red Hat Inc.
> >> + *
> >> + * Gamma table construction
> >> + */
> >> +
> >> +#pragma once
> >> +
> >> +#include <libcamera/controls.h>
> >> +
> >> +#include "libcamera/internal/software_isp/debayer_params.h"
> >> +
> >> +#include "algorithm.h"
> >> +#include "ipa_context.h"
> >> +
> >> +namespace libcamera {
> >> +
> >> +namespace ipa::soft::algorithms {
> >> +
> >> +static constexpr unsigned int kGammaLookupSize = 1024;
> >> +
> >> +class Gamma : public Algorithm
> >> +{
> >> +public:
> >> + Gamma() = default;
> >> + ~Gamma() = default;
> >> +
> >> + int init(IPAContext &context, const YamlObject &tuningData)
> >> + override;
> >> + void prepare(IPAContext &context,
> >> + const uint32_t frame,
> >> + IPAFrameContext &frameContext,
> >> + DebayerParams *params) override;
> >> +
> >> +private:
> >> + void updateGammaTable(IPAContext &context);
> >> +};
> >> +
> >> +} /* namespace ipa::soft::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
> >> new file mode 100644
> >> index 00000000..748f10aa
> >> --- /dev/null
> >> +++ b/src/ipa/simple/algorithms/lut.cpp
> >> @@ -0,0 +1,57 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2024, Red Hat Inc.
> >> + *
> >> + * Lookup table construction
> >> + */
> >> +
> >> +#include "lut.h"
> >> +
> >> +#include <algorithm>
> >> +#include <stdint.h>
> >> +
> >> +#include <libcamera/base/log.h>
> >> +
> >> +#include "simple/ipa_context.h"
> >> +
> >> +#include "gamma.h"
> >> +
> >> +namespace libcamera {
> >> +
> >> +namespace ipa::soft::algorithms {
> >> +
> >> +int Lut::init(IPAContext &context,
> >> + [[maybe_unused]] const YamlObject &tuningData)
> >> +{
> >> + auto &gains = context.activeState.gains;
> >> + gains.red = gains.green = gains.blue = 1.0;
> >
> > That looks wrong, the Awb algorithm does this already.
>
> Indeed, it doesn't belong here.
>
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +void Lut::prepare(IPAContext &context,
> >> + [[maybe_unused]] const uint32_t frame,
> >> + [[maybe_unused]] IPAFrameContext &frameContext,
> >> + DebayerParams *params)
> >> +{
> >> + auto &gains = context.activeState.gains;
> >> + auto &gammaTable = context.activeState.gamma.gammaTable;
> >> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> >> + constexpr unsigned int div =
> >> + static_cast<double>(DebayerParams::kRGBLookupSize) * 256 / kGammaLookupSize;
> >> + /* Apply gamma after gain! */
> >> + unsigned int idx;
> >> + idx = std::min({ i * gains.red / div, kGammaLookupSize - 1 });
> >> + params->red[i] = gammaTable[idx];
> >> + idx = std::min({ i * gains.green / div, kGammaLookupSize - 1 });
> >> + params->green[i] = gammaTable[idx];
> >> + idx = std::min({ i * gains.blue / div, kGammaLookupSize - 1 });
> >> + params->blue[i] = gammaTable[idx];
> >> + }
> >
> > Why is this a separate algorithm, why isn't it part of Gamma ?
>
> Well, constructing the gamma table and the color lookup tables are two
> different things. While the gammaTable looks like a utility just for
> the color lookup tables here, it may not always be the case, for
> instance contrast adjustment may be implemented by modifying the gamma
> table.
>
> But I don't have a strong opinion on this and I can merge the two.
I'd start simple and merge the two, and split them later if needed.
> >> +}
> >> +
> >> +REGISTER_IPA_ALGORITHM(Lut, "Lut")
> >> +
> >> +} /* namespace ipa::soft::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/simple/algorithms/lut.h b/src/ipa/simple/algorithms/lut.h
> >> new file mode 100644
> >> index 00000000..5b5e554e
> >> --- /dev/null
> >> +++ b/src/ipa/simple/algorithms/lut.h
> >> @@ -0,0 +1,37 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2024, Red Hat Inc.
> >> + *
> >> + * Lookup table construction
> >> + */
> >> +
> >> +#pragma once
> >> +
> >> +#include <libcamera/controls.h>
> >> +
> >> +#include "libcamera/internal/software_isp/debayer_params.h"
> >> +
> >> +#include "algorithm.h"
> >> +#include "ipa_context.h"
> >> +
> >> +namespace libcamera {
> >> +
> >> +namespace ipa::soft::algorithms {
> >> +
> >> +class Lut : public Algorithm
> >> +{
> >> +public:
> >> + Lut() = default;
> >> + ~Lut() = default;
> >> +
> >> + int init(IPAContext &context, const YamlObject &tuningData)
> >> + override;
> >> + void prepare(IPAContext &context,
> >> + const uint32_t frame,
> >> + IPAFrameContext &frameContext,
> >> + DebayerParams *params) override;
> >> +};
> >> +
> >> +} /* namespace ipa::soft::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build
> >> index 1f63c220..68e5aa58 100644
> >> --- a/src/ipa/simple/algorithms/meson.build
> >> +++ b/src/ipa/simple/algorithms/meson.build
> >> @@ -1,5 +1,8 @@
> >> # SPDX-License-Identifier: CC0-1.0
> >>
> >> soft_simple_ipa_algorithms = files([
> >> + 'awb.cpp',
> >> 'blc.cpp',
> >> + 'gamma.cpp',
> >> + 'lut.cpp',
> >> ])
> >> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml
> >> index e0d03d96..deaea6b1 100644
> >> --- a/src/ipa/simple/data/uncalibrated.yaml
> >> +++ b/src/ipa/simple/data/uncalibrated.yaml
> >> @@ -4,4 +4,7 @@
> >> version: 1
> >> algorithms:
> >> - BlackLevel:
> >> + - Gamma:
> >> + - Awb:
> >> + - Lut:
> >> ...
> >> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp
> >> index 268cff32..5fa492cb 100644
> >> --- a/src/ipa/simple/ipa_context.cpp
> >> +++ b/src/ipa/simple/ipa_context.cpp
> >> @@ -50,6 +50,11 @@ namespace libcamera::ipa::soft {
> >> * \brief The current state of IPA algorithms
> >> */
> >>
> >> +/**
> >> + * \var IPASessionConfiguration::gamma
> >> + * \brief Gamma value to be used in the raw image processing
> >> + */
> >> +
> >> /**
> >> * \var IPAActiveState::black
> >> * \brief Context for the Black Level algorithm
> >> @@ -58,4 +63,29 @@ namespace libcamera::ipa::soft {
> >> * \brief Current determined black level
> >> */
> >>
> >> +/**
> >> + * \var IPAActiveState::gains
> >> + * \brief Context for gains in the Colors algorithm
> >> + *
> >> + * \var IPAActiveState::gains.red
> >> + * \brief Gain of red color
> >> + *
> >> + * \var IPAActiveState::gains.green
> >> + * \brief Gain of green color
> >> + *
> >> + * \var IPAActiveState::gains.blue
> >> + * \brief Gain of blue color
> >> + */
> >> +
> >> +/**
> >> + * \var IPAActiveState::gamma
> >> + * \brief Context for gamma in the Colors algorithm
> >> + *
> >> + * \var IPAActiveState::gamma.gammaTable
> >> + * \brief Computed gamma table
> >> + *
> >> + * \var IPAActiveState::gamma.blackLevel
> >> + * \brief Black level used for the gamma table computation
> >> + */
> >> +
> >> } /* namespace libcamera::ipa::soft */
> >> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> >> index 22156569..f8b8834d 100644
> >> --- a/src/ipa/simple/ipa_context.h
> >> +++ b/src/ipa/simple/ipa_context.h
> >> @@ -7,6 +7,7 @@
> >>
> >> #pragma once
> >>
> >> +#include <array>
> >> #include <stdint.h>
> >>
> >> #include <libipa/fc_queue.h>
> >> @@ -16,12 +17,23 @@ namespace libcamera {
> >> namespace ipa::soft {
> >>
> >> struct IPASessionConfiguration {
> >> + float gamma;
> >> };
> >>
> >> struct IPAActiveState {
> >> struct {
> >> uint8_t level;
> >> } black;
> >
> > Blank line. Same below.
> >
> >> + struct {
> >> + unsigned int red;
> >> + unsigned int green;
> >> + unsigned int blue;
> >> + } gains;
> >> + static constexpr unsigned int kGammaLookupSize = 1024;
> >
> > This duplicates the kGammaLookupSize from
> > src/ipa/simple/algorithms/gamma.h.
>
> Yes. The constant in gamma.h is actually not needed when replacing it
> with size() calls, I'll remove it.
>
> >> + struct {
> >> + std::array<double, kGammaLookupSize> gammaTable;
> >> + uint8_t blackLevel;
> >> + } gamma;
> >> };
> >>
> >> struct IPAFrameContext : public FrameContext {
> >> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> >> index a1bd2f0c..8f01bf7d 100644
> >> --- a/src/ipa/simple/soft_simple.cpp
> >> +++ b/src/ipa/simple/soft_simple.cpp
> >> @@ -5,8 +5,6 @@
> >> * Simple Software Image Processing Algorithm module
> >> */
> >>
> >> -#include <cmath>
> >> -#include <numeric>
> >> #include <stdint.h>
> >> #include <sys/mman.h>
> >>
> >> @@ -93,9 +91,6 @@ private:
> >> std::unique_ptr<CameraSensorHelper> camHelper_;
> >> ControlInfoMap sensorInfoMap_;
> >>
> >> - static constexpr unsigned int kGammaLookupSize = 1024;
> >> - std::array<uint8_t, kGammaLookupSize> gammaTable_;
> >> - int lastBlackLevel_ = -1;
> >> /* Local parameter storage */
> >> struct IPAContext context_;
> >>
> >> @@ -283,6 +278,7 @@ void IPASoftSimple::prepare(const uint32_t frame)
> >> IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> >> for (auto const &algo : algorithms())
> >> algo->prepare(context_, frame, frameContext, params_);
> >> + setIspParams.emit();
> >> }
> >>
> >> void IPASoftSimple::processStats(const uint32_t frame,
> >> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(const uint32_t frame,
> >> for (auto const &algo : algorithms())
> >> algo->process(context_, frame, frameContext, stats_, metadata);
> >>
> >> - SwIspStats::Histogram histogram = stats_->yHistogram;
> >> - const uint8_t blackLevel = context_.activeState.black.level;
> >> -
> >> - /*
> >> - * Black level must be subtracted to get the correct AWB ratios, they
> >> - * would be off if they were computed from the whole brightness range
> >> - * rather than from the sensor range.
> >> - */
> >> - const uint64_t nPixels = std::accumulate(
> >> - histogram.begin(), histogram.end(), 0);
> >> - const uint64_t offset = blackLevel * nPixels;
> >> - const uint64_t sumR = stats_->sumR_ - offset / 4;
> >> - const uint64_t sumG = stats_->sumG_ - offset / 2;
> >> - const uint64_t sumB = stats_->sumB_ - offset / 4;
> >> -
> >> - /*
> >> - * Calculate red and blue gains for AWB.
> >> - * Clamp max gain at 4.0, this also avoids 0 division.
> >> - * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
> >> - */
> >> - const unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;
> >> - const unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;
> >> - /* Green gain and gamma values are fixed */
> >> - constexpr unsigned int gainG = 256;
> >> -
> >> - /* Update the gamma table if needed */
> >> - if (blackLevel != lastBlackLevel_) {
> >> - constexpr float gamma = 0.5;
> >> - const unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;
> >> - std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);
> >> - const float divisor = kGammaLookupSize - blackIndex - 1.0;
> >> - for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
> >> - gammaTable_[i] = UINT8_MAX *
> >> - std::pow((i - blackIndex) / divisor, gamma);
> >> -
> >> - lastBlackLevel_ = blackLevel;
> >> - }
> >> -
> >> - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> >> - constexpr unsigned int div =
> >> - DebayerParams::kRGBLookupSize * 256 / kGammaLookupSize;
> >> - unsigned int idx;
> >> -
> >> - /* Apply gamma after gain! */
> >> - idx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });
> >> - params_->red[i] = gammaTable_[idx];
> >> -
> >> - idx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });
> >> - params_->green[i] = gammaTable_[idx];
> >> -
> >> - idx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });
> >> - params_->blue[i] = gammaTable_[idx];
> >> - }
> >> -
> >> - setIspParams.emit();
> >> -
> >> /* \todo Switch to the libipa/algorithm.h API someday. */
> >>
> >> /*
> >> @@ -372,6 +312,7 @@ void IPASoftSimple::processStats(const uint32_t frame,
> >> * Calculate Mean Sample Value (MSV) according to formula from:
> >> * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf
> >> */
> >> + const uint8_t blackLevel = context_.activeState.black.level;
> >> const unsigned int blackLevelHistIdx =
> >> blackLevel / (256 / SwIspStats::kYHistogramSize);
> >> const unsigned int histogramSize =
> >> @@ -421,7 +362,8 @@ void IPASoftSimple::processStats(const uint32_t frame,
> >>
> >> LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV
> >> << " exp " << exposure_ << " again " << again_
> >> - << " gain R/B " << gainR << "/" << gainB
> >> + << " gain R/B " << context_.activeState.gains.red
> >> + << "/" << context_.activeState.gains.blue
> >
> > That's already printed in Awb::process(), does it need to be duplicated
> > here ?
>
> I don't think so, I'll remove it here.
>
> >> << " black level " << static_cast<unsigned int>(blackLevel);
> >> }
> >>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list