| Message ID | 20260126104256.119697-4-rick.w.ten.wolde@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2026. 01. 26. 11:42 keltezéssel, Rick ten Wolde írta: > From: Xander Pronk <xander.c.pronk@gmail.com> > > Add LSC algorithm module. > > Co-authored-by: Rick ten Wolde <rick_libcamera@wolde.info> > Signed-off-by: Rick ten Wolde <rick_libcamera@wolde.info> > Signed-off-by: Xander Pronk <xander.c.pronk@gmail.com> > --- > src/ipa/simple/algorithms/lsc.cpp | 69 +++++++++++++++++++++++++++ > src/ipa/simple/algorithms/lsc.h | 49 +++++++++++++++++++ > src/ipa/simple/algorithms/meson.build | 1 + > 3 files changed, 119 insertions(+) > create mode 100644 src/ipa/simple/algorithms/lsc.cpp > create mode 100644 src/ipa/simple/algorithms/lsc.h > > diff --git a/src/ipa/simple/algorithms/lsc.cpp b/src/ipa/simple/algorithms/lsc.cpp > new file mode 100644 > index 00000000..95783e4e > --- /dev/null > +++ b/src/ipa/simple/algorithms/lsc.cpp > @@ -0,0 +1,69 @@ > +#include "lsc.h" > + > +#include <iostream> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > + > +#include <libcamera/control_ids.h> At least the preceding two includes seem unnecessary. > + > +#include "libcamera/internal/matrix.h" > + > + > +namespace libcamera { > + > +namespace ipa::soft::algorithms { > + > +LOG_DEFINE_CATEGORY(IPASoftLsc) Empty line after this. > +int Lsc::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData) > +{ > + int ret_r = lsc_r.readYaml(tuningData["grids"], "ct", "r"); > + int ret_g = lsc_r.readYaml(tuningData["grids"], "ct", "g"); > + int ret_b = lsc_r.readYaml(tuningData["grids"], "ct", "b"); > + > + if (ret_r < 0 || ret_g < 0 || ret_b < 0) { > + LOG(IPASoftLsc, Error) > + << "Failed to parse 'lsc' parameter from tuning file."; > + return -1; This should a valid error code, e.g. `return -EINVAL`. > + } > + > + return 0; > +} > + > +int Lsc::configure([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const IPAConfigInfo &configInfo) > +{ > + return 0; > +} > + > + > +void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, > + [[maybe_unused]] IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params) > +{ > + unsigned int ct = context.activeState.awb.temperatureK; > + if (ct == 0) > + ct = 2700; > + const Matrix<uint8_t, 16, 16> matrix_r = lsc_r.getInterpolated(ct); > + const Matrix<uint8_t, 16, 16> matrix_g = lsc_r.getInterpolated(ct); > + const Matrix<uint8_t, 16, 16> matrix_b = lsc_r.getInterpolated(ct); > + > + for (unsigned long i = 0; i < matrix_r.data().size(); ++i) { > + params->LSC_red[i] = matrix_r.data()[i]; > + params->LSC_green[i] = matrix_g.data()[i]; > + params->LSC_blue[i] = matrix_b.data()[i]; > + } I suggest using 3 `std::copy` calls instead. > +} > + > +void Lsc::process([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] const SwIspStats *stats, > + [[maybe_unused]] ControlList &metadata) > +{ > +} If empty, it's probably better to omit this completely. > + > +REGISTER_IPA_ALGORITHM(Lsc, "Lsc") > + > +} /* namespace ipa::soft::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/simple/algorithms/lsc.h b/src/ipa/simple/algorithms/lsc.h > new file mode 100644 > index 00000000..8a3123ad > --- /dev/null > +++ b/src/ipa/simple/algorithms/lsc.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024-2025, Red Hat Inc. > + * > + * Color correction matrix The above needs adjustment. > + */ > + > +#pragma once > + > +#include <optional> Seems unnecessary? > + > +#include "libcamera/internal/matrix.h" > + > +#include <libipa/interpolator.h> > + > +#include "algorithm.h" > + > +namespace libcamera { > + > +namespace ipa::soft::algorithms { > + > + > +class Lsc : public Algorithm > +{ > +public: > + Lsc() = default; > + ~Lsc() = default; > + > + int init(IPAContext &context, const YamlObject &tuningData) override; > + int configure(IPAContext &context, > + const IPAConfigInfo &configInfo) override; > + void prepare(IPAContext &context, > + const uint32_t frame, > + IPAFrameContext &frameContext, > + DebayerParams *params) override; > + void process(IPAContext &context, const uint32_t frame, > + IPAFrameContext &frameContext, > + const SwIspStats *stats, > + ControlList &metadata) override; > + > +private: > + Interpolator<Matrix<uint8_t, 16, 16>> lsc_r; > + Interpolator<Matrix<uint8_t, 16, 16>> lsc_g; > + Interpolator<Matrix<uint8_t, 16, 16>> lsc_b; Camel case again. Regards, Barnabás Pőcze > +}; > + > +} /* namespace ipa::soft::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build > index 2d0adb05..9cfc8030 100644 > --- a/src/ipa/simple/algorithms/meson.build > +++ b/src/ipa/simple/algorithms/meson.build > @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([ > 'blc.cpp', > 'ccm.cpp', > 'lut.cpp', > + 'lsc.cpp', > ])
Hi, thank you for the patch. Rick ten Wolde <rick.w.ten.wolde@gmail.com> writes: > From: Xander Pronk <xander.c.pronk@gmail.com> > > Add LSC algorithm module. > > Co-authored-by: Rick ten Wolde <rick_libcamera@wolde.info> > Signed-off-by: Rick ten Wolde <rick_libcamera@wolde.info> > Signed-off-by: Xander Pronk <xander.c.pronk@gmail.com> > --- > src/ipa/simple/algorithms/lsc.cpp | 69 +++++++++++++++++++++++++++ > src/ipa/simple/algorithms/lsc.h | 49 +++++++++++++++++++ > src/ipa/simple/algorithms/meson.build | 1 + > 3 files changed, 119 insertions(+) > create mode 100644 src/ipa/simple/algorithms/lsc.cpp > create mode 100644 src/ipa/simple/algorithms/lsc.h > > diff --git a/src/ipa/simple/algorithms/lsc.cpp b/src/ipa/simple/algorithms/lsc.cpp > new file mode 100644 > index 00000000..95783e4e > --- /dev/null > +++ b/src/ipa/simple/algorithms/lsc.cpp > @@ -0,0 +1,69 @@ > +#include "lsc.h" > + > +#include <iostream> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > + > +#include <libcamera/control_ids.h> > + > +#include "libcamera/internal/matrix.h" > + > + > +namespace libcamera { > + > +namespace ipa::soft::algorithms { > + > +LOG_DEFINE_CATEGORY(IPASoftLsc) > +int Lsc::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData) > +{ > + int ret_r = lsc_r.readYaml(tuningData["grids"], "ct", "r"); > + int ret_g = lsc_r.readYaml(tuningData["grids"], "ct", "g"); > + int ret_b = lsc_r.readYaml(tuningData["grids"], "ct", "b"); > + > + if (ret_r < 0 || ret_g < 0 || ret_b < 0) { > + LOG(IPASoftLsc, Error) > + << "Failed to parse 'lsc' parameter from tuning file."; > + return -1; > + } > + > + return 0; > +} > + > +int Lsc::configure([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const IPAConfigInfo &configInfo) > +{ > + return 0; > +} > + > + > +void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, > + [[maybe_unused]] IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params) > +{ > + unsigned int ct = context.activeState.awb.temperatureK; > + if (ct == 0) > + ct = 2700; Why exactly this value? It should be explained somewhere. I was thinking whether it would be better to disable the correction until the temperature is available, but this wouldn't work when white balance algorithm is disabled. I'm not sure whether the initial temperature value is guaranteed to be 0. I think we should have some common initial temperature value set, to at least to use the same value in different algorithms (the other one is currently Ccm). > + const Matrix<uint8_t, 16, 16> matrix_r = lsc_r.getInterpolated(ct); > + const Matrix<uint8_t, 16, 16> matrix_g = lsc_r.getInterpolated(ct); > + const Matrix<uint8_t, 16, 16> matrix_b = lsc_r.getInterpolated(ct); > + > + for (unsigned long i = 0; i < matrix_r.data().size(); ++i) { > + params->LSC_red[i] = matrix_r.data()[i]; > + params->LSC_green[i] = matrix_g.data()[i]; > + params->LSC_blue[i] = matrix_b.data()[i]; > + } > +} > + > +void Lsc::process([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] const SwIspStats *stats, > + [[maybe_unused]] ControlList &metadata) > +{ > +} > + > +REGISTER_IPA_ALGORITHM(Lsc, "Lsc") > + > +} /* namespace ipa::soft::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/simple/algorithms/lsc.h b/src/ipa/simple/algorithms/lsc.h > new file mode 100644 > index 00000000..8a3123ad > --- /dev/null > +++ b/src/ipa/simple/algorithms/lsc.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024-2025, Red Hat Inc. > + * > + * Color correction matrix > + */ > + > +#pragma once > + > +#include <optional> > + > +#include "libcamera/internal/matrix.h" > + > +#include <libipa/interpolator.h> > + > +#include "algorithm.h" > + > +namespace libcamera { > + > +namespace ipa::soft::algorithms { > + > + > +class Lsc : public Algorithm > +{ > +public: > + Lsc() = default; > + ~Lsc() = default; > + > + int init(IPAContext &context, const YamlObject &tuningData) override; > + int configure(IPAContext &context, > + const IPAConfigInfo &configInfo) override; > + void prepare(IPAContext &context, > + const uint32_t frame, > + IPAFrameContext &frameContext, > + DebayerParams *params) override; > + void process(IPAContext &context, const uint32_t frame, > + IPAFrameContext &frameContext, > + const SwIspStats *stats, > + ControlList &metadata) override; > + > +private: > + Interpolator<Matrix<uint8_t, 16, 16>> lsc_r; > + Interpolator<Matrix<uint8_t, 16, 16>> lsc_g; > + Interpolator<Matrix<uint8_t, 16, 16>> lsc_b; > +}; > + > +} /* namespace ipa::soft::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build > index 2d0adb05..9cfc8030 100644 > --- a/src/ipa/simple/algorithms/meson.build > +++ b/src/ipa/simple/algorithms/meson.build > @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([ > 'blc.cpp', > 'ccm.cpp', > 'lut.cpp', > + 'lsc.cpp', > ])
diff --git a/src/ipa/simple/algorithms/lsc.cpp b/src/ipa/simple/algorithms/lsc.cpp new file mode 100644 index 00000000..95783e4e --- /dev/null +++ b/src/ipa/simple/algorithms/lsc.cpp @@ -0,0 +1,69 @@ +#include "lsc.h" + +#include <iostream> + +#include <libcamera/base/log.h> +#include <libcamera/base/utils.h> + +#include <libcamera/control_ids.h> + +#include "libcamera/internal/matrix.h" + + +namespace libcamera { + +namespace ipa::soft::algorithms { + +LOG_DEFINE_CATEGORY(IPASoftLsc) +int Lsc::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData) +{ + int ret_r = lsc_r.readYaml(tuningData["grids"], "ct", "r"); + int ret_g = lsc_r.readYaml(tuningData["grids"], "ct", "g"); + int ret_b = lsc_r.readYaml(tuningData["grids"], "ct", "b"); + + if (ret_r < 0 || ret_g < 0 || ret_b < 0) { + LOG(IPASoftLsc, Error) + << "Failed to parse 'lsc' parameter from tuning file."; + return -1; + } + + return 0; +} + +int Lsc::configure([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const IPAConfigInfo &configInfo) +{ + return 0; +} + + +void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, + [[maybe_unused]] IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params) +{ + unsigned int ct = context.activeState.awb.temperatureK; + if (ct == 0) + ct = 2700; + const Matrix<uint8_t, 16, 16> matrix_r = lsc_r.getInterpolated(ct); + const Matrix<uint8_t, 16, 16> matrix_g = lsc_r.getInterpolated(ct); + const Matrix<uint8_t, 16, 16> matrix_b = lsc_r.getInterpolated(ct); + + for (unsigned long i = 0; i < matrix_r.data().size(); ++i) { + params->LSC_red[i] = matrix_r.data()[i]; + params->LSC_green[i] = matrix_g.data()[i]; + params->LSC_blue[i] = matrix_b.data()[i]; + } +} + +void Lsc::process([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const uint32_t frame, + [[maybe_unused]] IPAFrameContext &frameContext, + [[maybe_unused]] const SwIspStats *stats, + [[maybe_unused]] ControlList &metadata) +{ +} + +REGISTER_IPA_ALGORITHM(Lsc, "Lsc") + +} /* namespace ipa::soft::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/simple/algorithms/lsc.h b/src/ipa/simple/algorithms/lsc.h new file mode 100644 index 00000000..8a3123ad --- /dev/null +++ b/src/ipa/simple/algorithms/lsc.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024-2025, Red Hat Inc. + * + * Color correction matrix + */ + +#pragma once + +#include <optional> + +#include "libcamera/internal/matrix.h" + +#include <libipa/interpolator.h> + +#include "algorithm.h" + +namespace libcamera { + +namespace ipa::soft::algorithms { + + +class Lsc : public Algorithm +{ +public: + Lsc() = default; + ~Lsc() = default; + + int init(IPAContext &context, const YamlObject &tuningData) override; + int configure(IPAContext &context, + const IPAConfigInfo &configInfo) override; + void prepare(IPAContext &context, + const uint32_t frame, + IPAFrameContext &frameContext, + DebayerParams *params) override; + void process(IPAContext &context, const uint32_t frame, + IPAFrameContext &frameContext, + const SwIspStats *stats, + ControlList &metadata) override; + +private: + Interpolator<Matrix<uint8_t, 16, 16>> lsc_r; + Interpolator<Matrix<uint8_t, 16, 16>> lsc_g; + Interpolator<Matrix<uint8_t, 16, 16>> lsc_b; +}; + +} /* namespace ipa::soft::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build index 2d0adb05..9cfc8030 100644 --- a/src/ipa/simple/algorithms/meson.build +++ b/src/ipa/simple/algorithms/meson.build @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([ 'blc.cpp', 'ccm.cpp', 'lut.cpp', + 'lsc.cpp', ])