| Message ID | 20240603140806.90045-4-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2024-06-03 15:06:30) > Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which > closely resembles sRGB. No seperate sRGB mode was implemented because the pwl > that models the gamma curve is so coarse that there is basically no difference > between srgb and gamma=2.2. The default can be overridden within the tuning > file or set at runtime using the gamma control. > > The gamma algorithm is not enabled by default. This will be done in future > tuning file updates. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > v2 -> v3: > - Squashed with patch 1/4 from previous series > - Renamed from Gamma to GammaOutCorrection > - Read default gamma value from tuning file > - Some stylistic fixes from review > > src/ipa/rkisp1/algorithms/goc.cpp | 161 ++++++++++++++++++++++++++ > src/ipa/rkisp1/algorithms/goc.h | 48 ++++++++ > src/ipa/rkisp1/algorithms/meson.build | 1 + > src/ipa/rkisp1/ipa_context.h | 4 + > 4 files changed, 214 insertions(+) > create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp > create mode 100644 src/ipa/rkisp1/algorithms/goc.h > > diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp > new file mode 100644 > index 00000000..875c82e5 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/goc.cpp > @@ -0,0 +1,161 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Ideas On Board > + * > + * RkISP1 Gamma out control > + */ > +#include "goc.h" > + > +#include <cmath> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > + > +#include <libcamera/control_ids.h> > + > +#include "libcamera/internal/yaml_parser.h" > + > +#include "linux/rkisp1-config.h" > + > +/** > + * \file goc.h > + */ > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +/** > + * \class GammaOutCorrection > + * \brief RkISP1 Gamma out correction > + * > + * This algorithm implements the gamma out curve for the RkISP1. > + * It defaults to a gamma value of 2.2 > + * As gamma is internally represented as a piecewise linear function with only > + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal. > + * Therefore sRGB gamma was not implemented as special case. > + * > + * Useful links: > + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm > + * https://en.wikipedia.org/wiki/SRGB > + */ > + > +LOG_DEFINE_CATEGORY(RkISP1Gamma) > + > +const double kDefaultGamma = 2.2; > + > +GammaOutCorrection::GammaOutCorrection() > + : gamma_(0) > +{ > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::init > + */ > +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const YamlObject &tuningData) > +{ > + context.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, 2.2f); > + defaultGamma_ = tuningData["gamma"].get<double>(kDefaultGamma); > + > + if (context.hw->numGammaOutSamples != > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) { > + LOG(RkISP1Gamma, Error) > + << "Gamma is not implemented for RkISP1 V12"; Is that because the size is different? Are there other differences that prevent us supporting it ? > + return -EINVAL; > + } > + > + return 0; > +} > + > +/** > + * \brief Configure the Gamma given a configInfo > + * \param[in] context The shared IPA context > + * \param[in] configInfo The IPA configuration data > + * > + * \return 0 > + */ > +int GammaOutCorrection::configure(IPAContext &context, > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > +{ > + context.activeState.gamma = defaultGamma_; > + return 0; > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::queueRequest > + */ > +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + IPAFrameContext &frameContext, > + const ControlList &controls) > +{ > + const auto &gamma = controls.get(controls::Gamma); > + if (gamma) { > + /* > + * \todo This is not correct as it updates the current state with a > + * future value. But it does no harm at the moment an allows us to > + * track the last active gamma > + */ > + context.activeState.gamma = *gamma; > + LOG(RkISP1Gamma, Debug) << "Set gamma to " << *gamma; > + } > + > + frameContext.gamma = context.activeState.gamma; > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::prepare > + */ > +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context, > + const uint32_t frame, > + [[maybe_unused]] IPAFrameContext &frameContext, > + rkisp1_params_cfg *params) > +{ > + ASSERT(context.hw->numGammaOutSamples == > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); > + > + /* > + * The logarithmic segments as specified in the reference. > + * Plus an additional 0 to make the loop easier Is the '0' still required now that this uses utils::enumerate(segments)? > + */ > + std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { > + 64, 64, 64, 64, 128, 128, 128, 128, 256, > + 256, 256, 512, 512, 512, 512, 512, 0 > + }; Presumably we'd have different segments for V12 ? > + auto gamma_y = params->others.goc_config.gamma_y; > + > + if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) > + return; > + > + gamma_ = frameContext.gamma; > + > + unsigned x = 0; > + for (auto [i, size] : utils::enumerate(segments)) { > + gamma_y[i] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; > + x += size; Assuming the '0' is actually valid, as I think the last iteration through this loop will still have added x by 512 anyway, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + } > + > + params->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC; > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC; > + params->module_ens |= RKISP1_CIF_ISP_MODULE_GOC; > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::process > + */ > +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + IPAFrameContext &frameContext, > + [[maybe_unused]] const rkisp1_stat_buffer *stats, > + ControlList &metadata) > +{ > + metadata.set(controls::Gamma, frameContext.gamma); > +} > + > +REGISTER_IPA_ALGORITHM(GammaOutCorrection, "GammaOutCorrection") > + > +} /* namespace ipa::rkisp1::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h > new file mode 100644 > index 00000000..d45e4194 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/goc.h > @@ -0,0 +1,48 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Ideas On Board > + * > + * RkISP1 Gamma out control > + */ > + > +#pragma once > + > +#include "algorithm.h" > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +class GammaOutCorrection : public Algorithm > +{ > +public: > + GammaOutCorrection(); > + ~GammaOutCorrection() = 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: > + /* > + * \todo: gamma_ holds the current state of the hardware. context.activeState > + * can not be used as that holds the "future state" after applying all known > + * requests. The intended usage of activeState needs to be clarified. > + */ > + double gamma_; > + > + double defaultGamma_; > +}; > + > +} /* namespace ipa::rkisp1::algorithms */ > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > index 93a48329..6ee71a9b 100644 > --- a/src/ipa/rkisp1/algorithms/meson.build > +++ b/src/ipa/rkisp1/algorithms/meson.build > @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([ > 'dpcc.cpp', > 'dpf.cpp', > 'filter.cpp', > + 'goc.cpp', > 'gsl.cpp', > 'lsc.cpp', > ]) > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index bd02a7a2..5252e222 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -104,6 +104,8 @@ struct IPAActiveState { > uint8_t denoise; > uint8_t sharpness; > } filter; > + > + double gamma; > }; > > struct IPAFrameContext : public FrameContext { > @@ -146,6 +148,8 @@ struct IPAFrameContext : public FrameContext { > uint32_t exposure; > double gain; > } sensor; > + > + double gamma; > }; > > struct IPAContext { > -- > 2.43.0 >
Hi Kieran, thanks for the review. On Mon, Jun 03, 2024 at 04:57:25PM +0100, Kieran Bingham wrote: > Quoting Stefan Klug (2024-06-03 15:06:30) > > Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which > > closely resembles sRGB. No seperate sRGB mode was implemented because the pwl > > that models the gamma curve is so coarse that there is basically no difference > > between srgb and gamma=2.2. The default can be overridden within the tuning > > file or set at runtime using the gamma control. > > > > The gamma algorithm is not enabled by default. This will be done in future > > tuning file updates. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > v2 -> v3: > > - Squashed with patch 1/4 from previous series > > - Renamed from Gamma to GammaOutCorrection > > - Read default gamma value from tuning file > > - Some stylistic fixes from review > > > > src/ipa/rkisp1/algorithms/goc.cpp | 161 ++++++++++++++++++++++++++ > > src/ipa/rkisp1/algorithms/goc.h | 48 ++++++++ > > src/ipa/rkisp1/algorithms/meson.build | 1 + > > src/ipa/rkisp1/ipa_context.h | 4 + > > 4 files changed, 214 insertions(+) > > create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp > > create mode 100644 src/ipa/rkisp1/algorithms/goc.h > > > > diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp > > new file mode 100644 > > index 00000000..875c82e5 > > --- /dev/null > > +++ b/src/ipa/rkisp1/algorithms/goc.cpp > > @@ -0,0 +1,161 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2024, Ideas On Board > > + * > > + * RkISP1 Gamma out control > > + */ > > +#include "goc.h" > > + > > +#include <cmath> > > + > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/utils.h> > > + > > +#include <libcamera/control_ids.h> > > + > > +#include "libcamera/internal/yaml_parser.h" > > + > > +#include "linux/rkisp1-config.h" > > + > > +/** > > + * \file goc.h > > + */ > > + > > +namespace libcamera { > > + > > +namespace ipa::rkisp1::algorithms { > > + > > +/** > > + * \class GammaOutCorrection > > + * \brief RkISP1 Gamma out correction > > + * > > + * This algorithm implements the gamma out curve for the RkISP1. > > + * It defaults to a gamma value of 2.2 > > + * As gamma is internally represented as a piecewise linear function with only > > + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal. > > + * Therefore sRGB gamma was not implemented as special case. > > + * > > + * Useful links: > > + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm > > + * https://en.wikipedia.org/wiki/SRGB > > + */ > > + > > +LOG_DEFINE_CATEGORY(RkISP1Gamma) > > + > > +const double kDefaultGamma = 2.2; > > + > > +GammaOutCorrection::GammaOutCorrection() > > + : gamma_(0) > > +{ > > +} > > + > > +/** > > + * \copydoc libcamera::ipa::Algorithm::init > > + */ > > +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context, > > + [[maybe_unused]] const YamlObject &tuningData) > > +{ > > + context.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, 2.2f); > > + defaultGamma_ = tuningData["gamma"].get<double>(kDefaultGamma); > > + > > + if (context.hw->numGammaOutSamples != > > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) { > > + LOG(RkISP1Gamma, Error) > > + << "Gamma is not implemented for RkISP1 V12"; > > Is that because the size is different? Are there other differences that > prevent us supporting it ? Yes, and I have no documentation on the segment sizes for the logarithmic case. (And no device to test it). > > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * \brief Configure the Gamma given a configInfo > > + * \param[in] context The shared IPA context > > + * \param[in] configInfo The IPA configuration data > > + * > > + * \return 0 > > + */ > > +int GammaOutCorrection::configure(IPAContext &context, > > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > > +{ > > + context.activeState.gamma = defaultGamma_; > > + return 0; > > +} > > + > > +/** > > + * \copydoc libcamera::ipa::Algorithm::queueRequest > > + */ > > +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context, > > + [[maybe_unused]] const uint32_t frame, > > + IPAFrameContext &frameContext, > > + const ControlList &controls) > > +{ > > + const auto &gamma = controls.get(controls::Gamma); > > + if (gamma) { > > + /* > > + * \todo This is not correct as it updates the current state with a > > + * future value. But it does no harm at the moment an allows us to > > + * track the last active gamma > > + */ > > + context.activeState.gamma = *gamma; > > + LOG(RkISP1Gamma, Debug) << "Set gamma to " << *gamma; > > + } > > + > > + frameContext.gamma = context.activeState.gamma; > > +} > > + > > +/** > > + * \copydoc libcamera::ipa::Algorithm::prepare > > + */ > > +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context, > > + const uint32_t frame, > > + [[maybe_unused]] IPAFrameContext &frameContext, > > + rkisp1_params_cfg *params) > > +{ > > + ASSERT(context.hw->numGammaOutSamples == > > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); > > + > > + /* > > + * The logarithmic segments as specified in the reference. > > + * Plus an additional 0 to make the loop easier > > Is the '0' still required now that this uses utils::enumerate(segments)? The segments define the spacing between the kneepoints. So there is one segment less than kneepoints. Therefore the 0 is still needed. > > > + */ > > + std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { > > + 64, 64, 64, 64, 128, 128, 128, 128, 256, > > + 256, 256, 512, 512, 512, 512, 512, 0 > > + }; > > Presumably we'd have different segments for V12 ? Yes, exactly. And I don't know the sizes. > > > + auto gamma_y = params->others.goc_config.gamma_y; > > + > > + if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) > > + return; > > + > > + gamma_ = frameContext.gamma; > > + > > + unsigned x = 0; > > + for (auto [i, size] : utils::enumerate(segments)) { > > + gamma_y[i] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; > > + x += size; > > Assuming the '0' is actually valid, as I think the last iteration > through this loop will still have added x by 512 anyway, > But without the 0 the loop would run one iteration too short and I would need to duplicate the gamma_y = ... oh wait in our usecase gamma_y[0] is always zero. I can start with gamma_y[1] and remove the 0. Dang. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Thanks :-) > > > + } > > + > > + params->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; > > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC; > > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC; > > + params->module_ens |= RKISP1_CIF_ISP_MODULE_GOC; > > +} > > + > > +/** > > + * \copydoc libcamera::ipa::Algorithm::process > > + */ > > +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context, > > + [[maybe_unused]] const uint32_t frame, > > + IPAFrameContext &frameContext, > > + [[maybe_unused]] const rkisp1_stat_buffer *stats, > > + ControlList &metadata) > > +{ > > + metadata.set(controls::Gamma, frameContext.gamma); > > +} > > + > > +REGISTER_IPA_ALGORITHM(GammaOutCorrection, "GammaOutCorrection") > > + > > +} /* namespace ipa::rkisp1::algorithms */ > > + > > +} /* namespace libcamera */ > > diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h > > new file mode 100644 > > index 00000000..d45e4194 > > --- /dev/null > > +++ b/src/ipa/rkisp1/algorithms/goc.h > > @@ -0,0 +1,48 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2024, Ideas On Board > > + * > > + * RkISP1 Gamma out control > > + */ > > + > > +#pragma once > > + > > +#include "algorithm.h" > > + > > +namespace libcamera { > > + > > +namespace ipa::rkisp1::algorithms { > > + > > +class GammaOutCorrection : public Algorithm > > +{ > > +public: > > + GammaOutCorrection(); > > + ~GammaOutCorrection() = 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: > > + /* > > + * \todo: gamma_ holds the current state of the hardware. context.activeState > > + * can not be used as that holds the "future state" after applying all known > > + * requests. The intended usage of activeState needs to be clarified. > > + */ > > + double gamma_; > > + > > + double defaultGamma_; > > +}; > > + > > +} /* namespace ipa::rkisp1::algorithms */ > > +} /* namespace libcamera */ > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > > index 93a48329..6ee71a9b 100644 > > --- a/src/ipa/rkisp1/algorithms/meson.build > > +++ b/src/ipa/rkisp1/algorithms/meson.build > > @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([ > > 'dpcc.cpp', > > 'dpf.cpp', > > 'filter.cpp', > > + 'goc.cpp', > > 'gsl.cpp', > > 'lsc.cpp', > > ]) > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index bd02a7a2..5252e222 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/ipa_context.h > > @@ -104,6 +104,8 @@ struct IPAActiveState { > > uint8_t denoise; > > uint8_t sharpness; > > } filter; > > + > > + double gamma; > > }; > > > > struct IPAFrameContext : public FrameContext { > > @@ -146,6 +148,8 @@ struct IPAFrameContext : public FrameContext { > > uint32_t exposure; > > double gain; > > } sensor; > > + > > + double gamma; > > }; > > > > struct IPAContext { > > -- > > 2.43.0 > >
Hi Kieran, On Mon, Jun 03, 2024 at 07:21:09PM +0200, Stefan Klug wrote: > Hi Kieran, > > thanks for the review. > > On Mon, Jun 03, 2024 at 04:57:25PM +0100, Kieran Bingham wrote: > > Quoting Stefan Klug (2024-06-03 15:06:30) > > > Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which > > > closely resembles sRGB. No seperate sRGB mode was implemented because the pwl > > > that models the gamma curve is so coarse that there is basically no difference > > > between srgb and gamma=2.2. The default can be overridden within the tuning > > > file or set at runtime using the gamma control. > > > > > > The gamma algorithm is not enabled by default. This will be done in future > > > tuning file updates. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > v2 -> v3: > > > - Squashed with patch 1/4 from previous series > > > - Renamed from Gamma to GammaOutCorrection > > > - Read default gamma value from tuning file > > > - Some stylistic fixes from review > > > > > > src/ipa/rkisp1/algorithms/goc.cpp | 161 ++++++++++++++++++++++++++ > > > src/ipa/rkisp1/algorithms/goc.h | 48 ++++++++ > > > src/ipa/rkisp1/algorithms/meson.build | 1 + > > > src/ipa/rkisp1/ipa_context.h | 4 + > > > 4 files changed, 214 insertions(+) > > > create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp > > > create mode 100644 src/ipa/rkisp1/algorithms/goc.h > > > > > > diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp > > > new file mode 100644 > > > index 00000000..875c82e5 > > > --- /dev/null > > > +++ b/src/ipa/rkisp1/algorithms/goc.cpp > > > @@ -0,0 +1,161 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2024, Ideas On Board > > > + * > > > + * RkISP1 Gamma out control > > > + */ > > > +#include "goc.h" > > > + > > > +#include <cmath> > > > + > > > +#include <libcamera/base/log.h> > > > +#include <libcamera/base/utils.h> > > > + > > > +#include <libcamera/control_ids.h> > > > + > > > +#include "libcamera/internal/yaml_parser.h" > > > + > > > +#include "linux/rkisp1-config.h" > > > + > > > +/** > > > + * \file goc.h > > > + */ > > > + > > > +namespace libcamera { > > > + > > > +namespace ipa::rkisp1::algorithms { > > > + > > > +/** > > > + * \class GammaOutCorrection > > > + * \brief RkISP1 Gamma out correction > > > + * > > > + * This algorithm implements the gamma out curve for the RkISP1. > > > + * It defaults to a gamma value of 2.2 > > > + * As gamma is internally represented as a piecewise linear function with only > > > + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal. > > > + * Therefore sRGB gamma was not implemented as special case. > > > + * > > > + * Useful links: > > > + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm > > > + * https://en.wikipedia.org/wiki/SRGB > > > + */ > > > + > > > +LOG_DEFINE_CATEGORY(RkISP1Gamma) > > > + > > > +const double kDefaultGamma = 2.2; > > > + > > > +GammaOutCorrection::GammaOutCorrection() > > > + : gamma_(0) > > > +{ > > > +} > > > + > > > +/** > > > + * \copydoc libcamera::ipa::Algorithm::init > > > + */ > > > +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context, > > > + [[maybe_unused]] const YamlObject &tuningData) > > > +{ > > > + context.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, 2.2f); > > > + defaultGamma_ = tuningData["gamma"].get<double>(kDefaultGamma); > > > + > > > + if (context.hw->numGammaOutSamples != > > > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) { > > > + LOG(RkISP1Gamma, Error) > > > + << "Gamma is not implemented for RkISP1 V12"; > > > > Is that because the size is different? Are there other differences that > > prevent us supporting it ? > > Yes, and I have no documentation on the segment sizes for the > logarithmic case. (And no device to test it). > > > > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +/** > > > + * \brief Configure the Gamma given a configInfo > > > + * \param[in] context The shared IPA context > > > + * \param[in] configInfo The IPA configuration data > > > + * > > > + * \return 0 > > > + */ > > > +int GammaOutCorrection::configure(IPAContext &context, > > > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > > > +{ > > > + context.activeState.gamma = defaultGamma_; > > > + return 0; > > > +} > > > + > > > +/** > > > + * \copydoc libcamera::ipa::Algorithm::queueRequest > > > + */ > > > +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context, > > > + [[maybe_unused]] const uint32_t frame, > > > + IPAFrameContext &frameContext, > > > + const ControlList &controls) > > > +{ > > > + const auto &gamma = controls.get(controls::Gamma); > > > + if (gamma) { > > > + /* > > > + * \todo This is not correct as it updates the current state with a > > > + * future value. But it does no harm at the moment an allows us to > > > + * track the last active gamma > > > + */ > > > + context.activeState.gamma = *gamma; > > > + LOG(RkISP1Gamma, Debug) << "Set gamma to " << *gamma; > > > + } > > > + > > > + frameContext.gamma = context.activeState.gamma; > > > +} > > > + > > > +/** > > > + * \copydoc libcamera::ipa::Algorithm::prepare > > > + */ > > > +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context, > > > + const uint32_t frame, > > > + [[maybe_unused]] IPAFrameContext &frameContext, > > > + rkisp1_params_cfg *params) > > > +{ > > > + ASSERT(context.hw->numGammaOutSamples == > > > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); > > > + > > > + /* > > > + * The logarithmic segments as specified in the reference. > > > + * Plus an additional 0 to make the loop easier > > > > Is the '0' still required now that this uses utils::enumerate(segments)? > > The segments define the spacing between the kneepoints. So there is one > segment less than kneepoints. Therefore the 0 is still needed. > > > > > > + */ > > > + std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { > > > + 64, 64, 64, 64, 128, 128, 128, 128, 256, > > > + 256, 256, 512, 512, 512, 512, 512, 0 > > > + }; > > > > Presumably we'd have different segments for V12 ? > > Yes, exactly. And I don't know the sizes. > > > > > > + auto gamma_y = params->others.goc_config.gamma_y; > > > + > > > + if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) > > > + return; > > > + > > > + gamma_ = frameContext.gamma; > > > + > > > + unsigned x = 0; > > > + for (auto [i, size] : utils::enumerate(segments)) { > > > + gamma_y[i] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; > > > + x += size; > > > > Assuming the '0' is actually valid, as I think the last iteration > > through this loop will still have added x by 512 anyway, > > > > But without the 0 the loop would run one iteration too short and I would > need to duplicate the gamma_y = ... oh wait in our usecase gamma_y[0] is > always zero. I can start with gamma_y[1] and remove the 0. Dang. I did that. The code looks like this: ---- /* The logarithmic segments as specified in the reference. */ std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { 64, 64, 64, 64, 128, 128, 128, 128, 256, 256, 256, 512, 512, 512, 512, 512 }; auto gamma_y = params->others.goc_config.gamma_y; if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) return; gamma_ = frameContext.gamma; unsigned x = 0; gamma_y[0] = 0; for (auto [i, size] : utils::enumerate(segments)) { x += size; gamma_y[i+1] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; } --- Is that really better, with that speciall handling of gamma_y[0]? Regards, Stefan > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Thanks :-) > > > > > > + } > > > + > > > + params->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; > > > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC; > > > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC; > > > + params->module_ens |= RKISP1_CIF_ISP_MODULE_GOC; > > > +} > > > + > > > +/** > > > + * \copydoc libcamera::ipa::Algorithm::process > > > + */ > > > +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context, > > > + [[maybe_unused]] const uint32_t frame, > > > + IPAFrameContext &frameContext, > > > + [[maybe_unused]] const rkisp1_stat_buffer *stats, > > > + ControlList &metadata) > > > +{ > > > + metadata.set(controls::Gamma, frameContext.gamma); > > > +} > > > + > > > +REGISTER_IPA_ALGORITHM(GammaOutCorrection, "GammaOutCorrection") > > > + > > > +} /* namespace ipa::rkisp1::algorithms */ > > > + > > > +} /* namespace libcamera */ > > > diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h > > > new file mode 100644 > > > index 00000000..d45e4194 > > > --- /dev/null > > > +++ b/src/ipa/rkisp1/algorithms/goc.h > > > @@ -0,0 +1,48 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2024, Ideas On Board > > > + * > > > + * RkISP1 Gamma out control > > > + */ > > > + > > > +#pragma once > > > + > > > +#include "algorithm.h" > > > + > > > +namespace libcamera { > > > + > > > +namespace ipa::rkisp1::algorithms { > > > + > > > +class GammaOutCorrection : public Algorithm > > > +{ > > > +public: > > > + GammaOutCorrection(); > > > + ~GammaOutCorrection() = 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: > > > + /* > > > + * \todo: gamma_ holds the current state of the hardware. context.activeState > > > + * can not be used as that holds the "future state" after applying all known > > > + * requests. The intended usage of activeState needs to be clarified. > > > + */ > > > + double gamma_; > > > + > > > + double defaultGamma_; > > > +}; > > > + > > > +} /* namespace ipa::rkisp1::algorithms */ > > > +} /* namespace libcamera */ > > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > > > index 93a48329..6ee71a9b 100644 > > > --- a/src/ipa/rkisp1/algorithms/meson.build > > > +++ b/src/ipa/rkisp1/algorithms/meson.build > > > @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([ > > > 'dpcc.cpp', > > > 'dpf.cpp', > > > 'filter.cpp', > > > + 'goc.cpp', > > > 'gsl.cpp', > > > 'lsc.cpp', > > > ]) > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > > index bd02a7a2..5252e222 100644 > > > --- a/src/ipa/rkisp1/ipa_context.h > > > +++ b/src/ipa/rkisp1/ipa_context.h > > > @@ -104,6 +104,8 @@ struct IPAActiveState { > > > uint8_t denoise; > > > uint8_t sharpness; > > > } filter; > > > + > > > + double gamma; > > > }; > > > > > > struct IPAFrameContext : public FrameContext { > > > @@ -146,6 +148,8 @@ struct IPAFrameContext : public FrameContext { > > > uint32_t exposure; > > > double gain; > > > } sensor; > > > + > > > + double gamma; > > > }; > > > > > > struct IPAContext { > > > -- > > > 2.43.0 > > >
Quoting Stefan Klug (2024-06-04 08:34:55) > Hi Kieran, > > On Mon, Jun 03, 2024 at 07:21:09PM +0200, Stefan Klug wrote: > > Hi Kieran, > > > > thanks for the review. > > > > On Mon, Jun 03, 2024 at 04:57:25PM +0100, Kieran Bingham wrote: > > > Quoting Stefan Klug (2024-06-03 15:06:30) > > > > Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which > > > > closely resembles sRGB. No seperate sRGB mode was implemented because the pwl > > > > that models the gamma curve is so coarse that there is basically no difference > > > > between srgb and gamma=2.2. The default can be overridden within the tuning > > > > file or set at runtime using the gamma control. > > > > > > > > The gamma algorithm is not enabled by default. This will be done in future > > > > tuning file updates. > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > v2 -> v3: > > > > - Squashed with patch 1/4 from previous series > > > > - Renamed from Gamma to GammaOutCorrection > > > > - Read default gamma value from tuning file > > > > - Some stylistic fixes from review > > > > > > > > src/ipa/rkisp1/algorithms/goc.cpp | 161 ++++++++++++++++++++++++++ > > > > src/ipa/rkisp1/algorithms/goc.h | 48 ++++++++ > > > > src/ipa/rkisp1/algorithms/meson.build | 1 + > > > > src/ipa/rkisp1/ipa_context.h | 4 + > > > > 4 files changed, 214 insertions(+) > > > > create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp > > > > create mode 100644 src/ipa/rkisp1/algorithms/goc.h > > > > > > > > diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp > > > > new file mode 100644 > > > > index 00000000..875c82e5 > > > > --- /dev/null > > > > +++ b/src/ipa/rkisp1/algorithms/goc.cpp > > > > @@ -0,0 +1,161 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2024, Ideas On Board > > > > + * > > > > + * RkISP1 Gamma out control > > > > + */ > > > > +#include "goc.h" > > > > + > > > > +#include <cmath> > > > > + > > > > +#include <libcamera/base/log.h> > > > > +#include <libcamera/base/utils.h> > > > > + > > > > +#include <libcamera/control_ids.h> > > > > + > > > > +#include "libcamera/internal/yaml_parser.h" > > > > + > > > > +#include "linux/rkisp1-config.h" > > > > + > > > > +/** > > > > + * \file goc.h > > > > + */ > > > > + > > > > +namespace libcamera { > > > > + > > > > +namespace ipa::rkisp1::algorithms { > > > > + > > > > +/** > > > > + * \class GammaOutCorrection > > > > + * \brief RkISP1 Gamma out correction > > > > + * > > > > + * This algorithm implements the gamma out curve for the RkISP1. > > > > + * It defaults to a gamma value of 2.2 > > > > + * As gamma is internally represented as a piecewise linear function with only > > > > + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal. > > > > + * Therefore sRGB gamma was not implemented as special case. > > > > + * > > > > + * Useful links: > > > > + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm > > > > + * https://en.wikipedia.org/wiki/SRGB > > > > + */ > > > > + > > > > +LOG_DEFINE_CATEGORY(RkISP1Gamma) > > > > + > > > > +const double kDefaultGamma = 2.2; > > > > + > > > > +GammaOutCorrection::GammaOutCorrection() > > > > + : gamma_(0) > > > > +{ > > > > +} > > > > + > > > > +/** > > > > + * \copydoc libcamera::ipa::Algorithm::init > > > > + */ > > > > +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context, > > > > + [[maybe_unused]] const YamlObject &tuningData) > > > > +{ > > > > + context.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, 2.2f); > > > > + defaultGamma_ = tuningData["gamma"].get<double>(kDefaultGamma); > > > > + > > > > + if (context.hw->numGammaOutSamples != > > > > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) { > > > > + LOG(RkISP1Gamma, Error) > > > > + << "Gamma is not implemented for RkISP1 V12"; > > > > > > Is that because the size is different? Are there other differences that > > > prevent us supporting it ? > > > > Yes, and I have no documentation on the segment sizes for the > > logarithmic case. (And no device to test it). > > > > > > > > > + return -EINVAL; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/** > > > > + * \brief Configure the Gamma given a configInfo > > > > + * \param[in] context The shared IPA context > > > > + * \param[in] configInfo The IPA configuration data > > > > + * > > > > + * \return 0 > > > > + */ > > > > +int GammaOutCorrection::configure(IPAContext &context, > > > > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > > > > +{ > > > > + context.activeState.gamma = defaultGamma_; > > > > + return 0; > > > > +} > > > > + > > > > +/** > > > > + * \copydoc libcamera::ipa::Algorithm::queueRequest > > > > + */ > > > > +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context, > > > > + [[maybe_unused]] const uint32_t frame, > > > > + IPAFrameContext &frameContext, > > > > + const ControlList &controls) > > > > +{ > > > > + const auto &gamma = controls.get(controls::Gamma); > > > > + if (gamma) { > > > > + /* > > > > + * \todo This is not correct as it updates the current state with a > > > > + * future value. But it does no harm at the moment an allows us to > > > > + * track the last active gamma > > > > + */ > > > > + context.activeState.gamma = *gamma; > > > > + LOG(RkISP1Gamma, Debug) << "Set gamma to " << *gamma; > > > > + } > > > > + > > > > + frameContext.gamma = context.activeState.gamma; > > > > +} > > > > + > > > > +/** > > > > + * \copydoc libcamera::ipa::Algorithm::prepare > > > > + */ > > > > +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context, > > > > + const uint32_t frame, > > > > + [[maybe_unused]] IPAFrameContext &frameContext, > > > > + rkisp1_params_cfg *params) > > > > +{ > > > > + ASSERT(context.hw->numGammaOutSamples == > > > > + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); > > > > + > > > > + /* > > > > + * The logarithmic segments as specified in the reference. > > > > + * Plus an additional 0 to make the loop easier > > > > > > Is the '0' still required now that this uses utils::enumerate(segments)? > > > > The segments define the spacing between the kneepoints. So there is one > > segment less than kneepoints. Therefore the 0 is still needed. > > > > > > > > > + */ > > > > + std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { > > > > + 64, 64, 64, 64, 128, 128, 128, 128, 256, > > > > + 256, 256, 512, 512, 512, 512, 512, 0 > > > > + }; > > > > > > Presumably we'd have different segments for V12 ? > > > > Yes, exactly. And I don't know the sizes. > > > > > > > > > + auto gamma_y = params->others.goc_config.gamma_y; > > > > + > > > > + if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) > > > > + return; > > > > + > > > > + gamma_ = frameContext.gamma; > > > > + > > > > + unsigned x = 0; > > > > + for (auto [i, size] : utils::enumerate(segments)) { > > > > + gamma_y[i] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; > > > > + x += size; > > > > > > Assuming the '0' is actually valid, as I think the last iteration > > > through this loop will still have added x by 512 anyway, > > > > > > > But without the 0 the loop would run one iteration too short and I would > > need to duplicate the gamma_y = ... oh wait in our usecase gamma_y[0] is > > always zero. I can start with gamma_y[1] and remove the 0. Dang. > > I did that. The code looks like this: > ---- > > /* The logarithmic segments as specified in the reference. */ > std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { > 64, 64, 64, 64, 128, 128, 128, 128, 256, > 256, 256, 512, 512, 512, 512, 512 > }; > auto gamma_y = params->others.goc_config.gamma_y; > > if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) > return; > > gamma_ = frameContext.gamma; > > unsigned x = 0; > gamma_y[0] = 0; > for (auto [i, size] : utils::enumerate(segments)) { > x += size; > gamma_y[i+1] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; > } > > --- > > Is that really better, with that speciall handling of gamma_y[0]? Maybe not, you can choose whichever you prefer ;-) -- Kieran
diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp new file mode 100644 index 00000000..875c82e5 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/goc.cpp @@ -0,0 +1,161 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Ideas On Board + * + * RkISP1 Gamma out control + */ +#include "goc.h" + +#include <cmath> + +#include <libcamera/base/log.h> +#include <libcamera/base/utils.h> + +#include <libcamera/control_ids.h> + +#include "libcamera/internal/yaml_parser.h" + +#include "linux/rkisp1-config.h" + +/** + * \file goc.h + */ + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +/** + * \class GammaOutCorrection + * \brief RkISP1 Gamma out correction + * + * This algorithm implements the gamma out curve for the RkISP1. + * It defaults to a gamma value of 2.2 + * As gamma is internally represented as a piecewise linear function with only + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal. + * Therefore sRGB gamma was not implemented as special case. + * + * Useful links: + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm + * https://en.wikipedia.org/wiki/SRGB + */ + +LOG_DEFINE_CATEGORY(RkISP1Gamma) + +const double kDefaultGamma = 2.2; + +GammaOutCorrection::GammaOutCorrection() + : gamma_(0) +{ +} + +/** + * \copydoc libcamera::ipa::Algorithm::init + */ +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const YamlObject &tuningData) +{ + context.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, 2.2f); + defaultGamma_ = tuningData["gamma"].get<double>(kDefaultGamma); + + if (context.hw->numGammaOutSamples != + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) { + LOG(RkISP1Gamma, Error) + << "Gamma is not implemented for RkISP1 V12"; + return -EINVAL; + } + + return 0; +} + +/** + * \brief Configure the Gamma given a configInfo + * \param[in] context The shared IPA context + * \param[in] configInfo The IPA configuration data + * + * \return 0 + */ +int GammaOutCorrection::configure(IPAContext &context, + [[maybe_unused]] const IPACameraSensorInfo &configInfo) +{ + context.activeState.gamma = defaultGamma_; + return 0; +} + +/** + * \copydoc libcamera::ipa::Algorithm::queueRequest + */ +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const uint32_t frame, + IPAFrameContext &frameContext, + const ControlList &controls) +{ + const auto &gamma = controls.get(controls::Gamma); + if (gamma) { + /* + * \todo This is not correct as it updates the current state with a + * future value. But it does no harm at the moment an allows us to + * track the last active gamma + */ + context.activeState.gamma = *gamma; + LOG(RkISP1Gamma, Debug) << "Set gamma to " << *gamma; + } + + frameContext.gamma = context.activeState.gamma; +} + +/** + * \copydoc libcamera::ipa::Algorithm::prepare + */ +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context, + const uint32_t frame, + [[maybe_unused]] IPAFrameContext &frameContext, + rkisp1_params_cfg *params) +{ + ASSERT(context.hw->numGammaOutSamples == + RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); + + /* + * The logarithmic segments as specified in the reference. + * Plus an additional 0 to make the loop easier + */ + std::array<unsigned, RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10> segments = { + 64, 64, 64, 64, 128, 128, 128, 128, 256, + 256, 256, 512, 512, 512, 512, 512, 0 + }; + auto gamma_y = params->others.goc_config.gamma_y; + + if (frame > 0 && std::fabs(frameContext.gamma - gamma_) < 0.001) + return; + + gamma_ = frameContext.gamma; + + unsigned x = 0; + for (auto [i, size] : utils::enumerate(segments)) { + gamma_y[i] = std::pow(x / 4096.0, 1.0 / gamma_) * 1023.0; + x += size; + } + + params->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC; + params->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC; + params->module_ens |= RKISP1_CIF_ISP_MODULE_GOC; +} + +/** + * \copydoc libcamera::ipa::Algorithm::process + */ +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const uint32_t frame, + IPAFrameContext &frameContext, + [[maybe_unused]] const rkisp1_stat_buffer *stats, + ControlList &metadata) +{ + metadata.set(controls::Gamma, frameContext.gamma); +} + +REGISTER_IPA_ALGORITHM(GammaOutCorrection, "GammaOutCorrection") + +} /* namespace ipa::rkisp1::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h new file mode 100644 index 00000000..d45e4194 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/goc.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Ideas On Board + * + * RkISP1 Gamma out control + */ + +#pragma once + +#include "algorithm.h" + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +class GammaOutCorrection : public Algorithm +{ +public: + GammaOutCorrection(); + ~GammaOutCorrection() = 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: + /* + * \todo: gamma_ holds the current state of the hardware. context.activeState + * can not be used as that holds the "future state" after applying all known + * requests. The intended usage of activeState needs to be clarified. + */ + double gamma_; + + double defaultGamma_; +}; + +} /* namespace ipa::rkisp1::algorithms */ +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build index 93a48329..6ee71a9b 100644 --- a/src/ipa/rkisp1/algorithms/meson.build +++ b/src/ipa/rkisp1/algorithms/meson.build @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([ 'dpcc.cpp', 'dpf.cpp', 'filter.cpp', + 'goc.cpp', 'gsl.cpp', 'lsc.cpp', ]) diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index bd02a7a2..5252e222 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -104,6 +104,8 @@ struct IPAActiveState { uint8_t denoise; uint8_t sharpness; } filter; + + double gamma; }; struct IPAFrameContext : public FrameContext { @@ -146,6 +148,8 @@ struct IPAFrameContext : public FrameContext { uint32_t exposure; double gain; } sensor; + + double gamma; }; struct IPAContext {
Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which closely resembles sRGB. No seperate sRGB mode was implemented because the pwl that models the gamma curve is so coarse that there is basically no difference between srgb and gamma=2.2. The default can be overridden within the tuning file or set at runtime using the gamma control. The gamma algorithm is not enabled by default. This will be done in future tuning file updates. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- v2 -> v3: - Squashed with patch 1/4 from previous series - Renamed from Gamma to GammaOutCorrection - Read default gamma value from tuning file - Some stylistic fixes from review src/ipa/rkisp1/algorithms/goc.cpp | 161 ++++++++++++++++++++++++++ src/ipa/rkisp1/algorithms/goc.h | 48 ++++++++ src/ipa/rkisp1/algorithms/meson.build | 1 + src/ipa/rkisp1/ipa_context.h | 4 + 4 files changed, 214 insertions(+) create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp create mode 100644 src/ipa/rkisp1/algorithms/goc.h