| Message ID | 20250808141315.413839-2-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2025-08-08 15:12:39) > The i.MX8 M Plus has a compression curve inside the compand block. This > curve is necessary to process HDR stitched data and is useful for other > aspects like applying a digital gain to the incoming sensor data. > > Add a basic algorithm for the compression curve. This algorithm has a > hardcoded input width of 20bit and output width of 12bit which matches > the imx8mp pipeline. Only a static gain is supported in this version. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/compress.cpp | 100 +++++++++++++++++++++++++ > src/ipa/rkisp1/algorithms/compress.h | 33 ++++++++ > src/ipa/rkisp1/algorithms/meson.build | 1 + > src/ipa/rkisp1/ipa_context.h | 9 +++ > 4 files changed, 143 insertions(+) > create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp > create mode 100644 src/ipa/rkisp1/algorithms/compress.h > > diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp > new file mode 100644 > index 000000000000..b2fad02404d6 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/compress.cpp If this manages the compand block, should that be the name of the component? > @@ -0,0 +1,100 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * RkISP1 Compression curve > + */ > +#include "compress.h" > + > +#include <linux/videodev2.h> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > + > +#include "libcamera/internal/yaml_parser.h" > + > +#include "linux/rkisp1-config.h" > + > +/** > + * \file compress.h > + */ > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +/** > + * \class Compress > + * \brief RkISP1 Compress curve > + * > + * This algorithm implements support for the compressions curve in the compand > + * block available in the i.MX8 M Plus > + * > + * In its current version it only supports a static gain. This is useful for > + * the agc algorithm to compensate for exposure/gain quantization effects. > + * > + * This algorithm doesn't have any configuration options. It needs to be > + * configured per frame by other algorithms. > + * > + * Other algorithms can check activeState.compress.supported to see if > + * compression is available. If it is available they can configure it per frame > + * using frameContext.compress.enable and frameContext.compress.gain. > + */ > + > +LOG_DEFINE_CATEGORY(RkISP1Compress) > + > +constexpr static int kRkISP1CompressInBits = 20; > +constexpr static int kRkISP1CompressOutBits = 12; > + > +/** > + * \copydoc libcamera::ipa::Algorithm::configure > + */ > +int Compress::configure(IPAContext &context, > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > +{ > + if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS || > + !context.hw->compand) > + LOG(RkISP1Compress, Warning) > + << "Compression is not supported by the hardware or kernel."; > + Shouldn't we ensure context.activeState.compress.supported below stays false if this warning is output ? > + context.activeState.compress.supported = true; > + return 0; > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::prepare > + */ > +void Compress::prepare([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + IPAFrameContext &frameContext, > + RkISP1Params *params) > +{ > + auto comp = params->block<BlockType::CompandCompress>(); > + comp.setEnabled(frameContext.compress.enable); > + Should we also return early if context.activeState.compress.supported is false? I thinks that's probably a 'session configuration' structure item rather than an active state item though ? > + if (!frameContext.compress.enable) > + return; > + > + int xmax = (1 << kRkISP1CompressInBits); > + int ymax = (1 << kRkISP1CompressOutBits); > + int inLogStep = std::log2(xmax / 64); > + > + for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) { > + double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS); > + double y = x * frameContext.compress.gain; > + > + comp->px[i] = inLogStep; > + comp->x[i] = std::min<int>(x * xmax, xmax - 1); > + comp->y[i] = std::min<int>(y * ymax, ymax - 1); > + } > + > + LOG(RkISP1Compress, Debug) << "Compression: " << kRkISP1CompressInBits > + << " bits to " << kRkISP1CompressOutBits > + << " bits gain: " << frameContext.compress.gain; > +} > + > +REGISTER_IPA_ALGORITHM(Compress, "Compress") > + > +} /* namespace ipa::rkisp1::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h > new file mode 100644 > index 000000000000..3006508b2b32 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/compress.h > @@ -0,0 +1,33 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Ideas On Board > + * > + * RkISP1 Gamma out control I think this is compand or compress. > + */ > + > +#pragma once > + > +#include "algorithm.h" > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +class Compress : public Algorithm > +{ > +public: > + Compress() = default; > + ~Compress() = default; > + > + int configure(IPAContext &context, > + const IPACameraSensorInfo &configInfo) override; > + void prepare(IPAContext &context, const uint32_t frame, > + IPAFrameContext &frameContext, > + RkISP1Params *params) override; > + > +private: > + float defaultGamma_; > +}; > + > +} /* namespace ipa::rkisp1::algorithms */ > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > index c66b0b70b82f..2e42a80cf99d 100644 > --- a/src/ipa/rkisp1/algorithms/meson.build > +++ b/src/ipa/rkisp1/algorithms/meson.build > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([ > 'awb.cpp', > 'blc.cpp', > 'ccm.cpp', > + 'compress.cpp', > 'cproc.cpp', > 'dpcc.cpp', > 'dpf.cpp', > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 7ccc7b501aff..37a74215ce19 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -106,6 +106,10 @@ struct IPAActiveState { > Matrix<float, 3, 3> automatic; > } ccm; > > + struct { > + bool supported; > + } compress; > + > struct { > int8_t brightness; > uint8_t contrast; > @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext { > bool update; > } cproc; > > + struct { > + bool enable; > + double gain; > + } compress; > + > struct { > bool denoise; > bool update; > -- > 2.48.1 >
Hi Kieran, Thank you for the review. Quoting Kieran Bingham (2025-08-08 18:10:04) > Quoting Stefan Klug (2025-08-08 15:12:39) > > The i.MX8 M Plus has a compression curve inside the compand block. This > > curve is necessary to process HDR stitched data and is useful for other > > aspects like applying a digital gain to the incoming sensor data. > > > > Add a basic algorithm for the compression curve. This algorithm has a > > hardcoded input width of 20bit and output width of 12bit which matches > > the imx8mp pipeline. Only a static gain is supported in this version. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/ipa/rkisp1/algorithms/compress.cpp | 100 +++++++++++++++++++++++++ > > src/ipa/rkisp1/algorithms/compress.h | 33 ++++++++ > > src/ipa/rkisp1/algorithms/meson.build | 1 + > > src/ipa/rkisp1/ipa_context.h | 9 +++ > > 4 files changed, 143 insertions(+) > > create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp > > create mode 100644 src/ipa/rkisp1/algorithms/compress.h > > > > diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp > > new file mode 100644 > > index 000000000000..b2fad02404d6 > > --- /dev/null > > +++ b/src/ipa/rkisp1/algorithms/compress.cpp > > If this manages the compand block, should that be the name of the > component? > Hm, good point. The compand block is a bit of a outlier. It has three subblocks (CompandExpand, CompandBls, CompandCompress) which can be enabled separately and are also treated as different configs on kernel level. For example the Bls is used in blc.cpp. I honestly don't know the best route here. I can call it compand, but as said BLS is handled elsewhere and I somehow expect expand to also be handler elsewhere as I don't think expansion will be controlled by more than one instance. So writing this I still lean towards "Compress". > > > > @@ -0,0 +1,100 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * RkISP1 Compression curve > > + */ > > +#include "compress.h" > > + > > +#include <linux/videodev2.h> > > + > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/utils.h> > > + > > +#include "libcamera/internal/yaml_parser.h" > > + > > +#include "linux/rkisp1-config.h" > > + > > +/** > > + * \file compress.h > > + */ > > + > > +namespace libcamera { > > + > > +namespace ipa::rkisp1::algorithms { > > + > > +/** > > + * \class Compress > > + * \brief RkISP1 Compress curve > > + * > > + * This algorithm implements support for the compressions curve in the compand > > + * block available in the i.MX8 M Plus > > + * > > + * In its current version it only supports a static gain. This is useful for > > + * the agc algorithm to compensate for exposure/gain quantization effects. > > + * > > + * This algorithm doesn't have any configuration options. It needs to be > > + * configured per frame by other algorithms. > > + * > > + * Other algorithms can check activeState.compress.supported to see if > > + * compression is available. If it is available they can configure it per frame > > + * using frameContext.compress.enable and frameContext.compress.gain. > > + */ > > + > > +LOG_DEFINE_CATEGORY(RkISP1Compress) > > + > > +constexpr static int kRkISP1CompressInBits = 20; > > +constexpr static int kRkISP1CompressOutBits = 12; > > + > > +/** > > + * \copydoc libcamera::ipa::Algorithm::configure > > + */ > > +int Compress::configure(IPAContext &context, > > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > > +{ > > + if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS || > > + !context.hw->compand) > > + LOG(RkISP1Compress, Warning) > > + << "Compression is not supported by the hardware or kernel."; > > + > > Shouldn't we ensure context.activeState.compress.supported below stays > false if this warning is output ? Oh I missed that. Absolutely! > > > > + context.activeState.compress.supported = true; > > + return 0; > > +} > > + > > +/** > > + * \copydoc libcamera::ipa::Algorithm::prepare > > + */ > > +void Compress::prepare([[maybe_unused]] IPAContext &context, > > + [[maybe_unused]] const uint32_t frame, > > + IPAFrameContext &frameContext, > > + RkISP1Params *params) > > +{ > > + auto comp = params->block<BlockType::CompandCompress>(); > > + comp.setEnabled(frameContext.compress.enable); > > + > > Should we also return early if context.activeState.compress.supported is > false? > > I thinks that's probably a 'session configuration' structure item rather > than an active state item though ? > > > + if (!frameContext.compress.enable) > > + return; > > + > > + int xmax = (1 << kRkISP1CompressInBits); > > + int ymax = (1 << kRkISP1CompressOutBits); > > + int inLogStep = std::log2(xmax / 64); > > + > > + for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) { > > + double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS); > > + double y = x * frameContext.compress.gain; > > + > > + comp->px[i] = inLogStep; > > + comp->x[i] = std::min<int>(x * xmax, xmax - 1); > > + comp->y[i] = std::min<int>(y * ymax, ymax - 1); > > + } > > + > > + LOG(RkISP1Compress, Debug) << "Compression: " << kRkISP1CompressInBits > > + << " bits to " << kRkISP1CompressOutBits > > + << " bits gain: " << frameContext.compress.gain; > > +} > > + > > +REGISTER_IPA_ALGORITHM(Compress, "Compress") > > + > > +} /* namespace ipa::rkisp1::algorithms */ > > + > > +} /* namespace libcamera */ > > diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h > > new file mode 100644 > > index 000000000000..3006508b2b32 > > --- /dev/null > > +++ b/src/ipa/rkisp1/algorithms/compress.h > > @@ -0,0 +1,33 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2024, Ideas On Board > > + * > > + * RkISP1 Gamma out control > > I think this is compand or compress. Yep. Fixed for next version. Thanks, Stefan > > > + */ > > + > > +#pragma once > > + > > +#include "algorithm.h" > > + > > +namespace libcamera { > > + > > +namespace ipa::rkisp1::algorithms { > > + > > +class Compress : public Algorithm > > +{ > > +public: > > + Compress() = default; > > + ~Compress() = default; > > + > > + int configure(IPAContext &context, > > + const IPACameraSensorInfo &configInfo) override; > > + void prepare(IPAContext &context, const uint32_t frame, > > + IPAFrameContext &frameContext, > > + RkISP1Params *params) override; > > + > > +private: > > + float defaultGamma_; > > +}; > > + > > +} /* namespace ipa::rkisp1::algorithms */ > > +} /* namespace libcamera */ > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > > index c66b0b70b82f..2e42a80cf99d 100644 > > --- a/src/ipa/rkisp1/algorithms/meson.build > > +++ b/src/ipa/rkisp1/algorithms/meson.build > > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([ > > 'awb.cpp', > > 'blc.cpp', > > 'ccm.cpp', > > + 'compress.cpp', > > 'cproc.cpp', > > 'dpcc.cpp', > > 'dpf.cpp', > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index 7ccc7b501aff..37a74215ce19 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/ipa_context.h > > @@ -106,6 +106,10 @@ struct IPAActiveState { > > Matrix<float, 3, 3> automatic; > > } ccm; > > > > + struct { > > + bool supported; > > + } compress; > > + > > struct { > > int8_t brightness; > > uint8_t contrast; > > @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext { > > bool update; > > } cproc; > > > > + struct { > > + bool enable; > > + double gain; > > + } compress; > > + > > struct { > > bool denoise; > > bool update; > > -- > > 2.48.1 > >
Quoting Stefan Klug (2025-08-11 06:16:37) > Hi Kieran, > > Thank you for the review. > > Quoting Kieran Bingham (2025-08-08 18:10:04) > > Quoting Stefan Klug (2025-08-08 15:12:39) > > > The i.MX8 M Plus has a compression curve inside the compand block. This > > > curve is necessary to process HDR stitched data and is useful for other > > > aspects like applying a digital gain to the incoming sensor data. > > > > > > Add a basic algorithm for the compression curve. This algorithm has a > > > hardcoded input width of 20bit and output width of 12bit which matches > > > the imx8mp pipeline. Only a static gain is supported in this version. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > src/ipa/rkisp1/algorithms/compress.cpp | 100 +++++++++++++++++++++++++ > > > src/ipa/rkisp1/algorithms/compress.h | 33 ++++++++ > > > src/ipa/rkisp1/algorithms/meson.build | 1 + > > > src/ipa/rkisp1/ipa_context.h | 9 +++ > > > 4 files changed, 143 insertions(+) > > > create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp > > > create mode 100644 src/ipa/rkisp1/algorithms/compress.h > > > > > > diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp > > > new file mode 100644 > > > index 000000000000..b2fad02404d6 > > > --- /dev/null > > > +++ b/src/ipa/rkisp1/algorithms/compress.cpp > > > > If this manages the compand block, should that be the name of the > > component? > > > > Hm, good point. The compand block is a bit of a outlier. It has three > subblocks (CompandExpand, CompandBls, CompandCompress) which can be > enabled separately and are also treated as different configs on kernel > level. For example the Bls is used in blc.cpp. I honestly don't know the > best route here. I can call it compand, but as said BLS is handled > elsewhere and I somehow expect expand to also be handler elsewhere as I > don't think expansion will be controlled by more than one instance. So > writing this I still lean towards "Compress". At somepoint we'll expect a separate 'Expand' then perhaps? Anyway, I'm fine with this if you see them as separate/independent function blocks. > > > > > > > > @@ -0,0 +1,100 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2025, Ideas On Board > > > + * > > > + * RkISP1 Compression curve > > > + */ > > > +#include "compress.h" > > > + > > > +#include <linux/videodev2.h> > > > + > > > +#include <libcamera/base/log.h> > > > +#include <libcamera/base/utils.h> > > > + > > > +#include "libcamera/internal/yaml_parser.h" > > > + > > > +#include "linux/rkisp1-config.h" > > > + > > > +/** > > > + * \file compress.h > > > + */ > > > + > > > +namespace libcamera { > > > + > > > +namespace ipa::rkisp1::algorithms { > > > + > > > +/** > > > + * \class Compress > > > + * \brief RkISP1 Compress curve > > > + * > > > + * This algorithm implements support for the compressions curve in the compand > > > + * block available in the i.MX8 M Plus > > > + * > > > + * In its current version it only supports a static gain. This is useful for > > > + * the agc algorithm to compensate for exposure/gain quantization effects. > > > + * > > > + * This algorithm doesn't have any configuration options. It needs to be > > > + * configured per frame by other algorithms. > > > + * > > > + * Other algorithms can check activeState.compress.supported to see if > > > + * compression is available. If it is available they can configure it per frame > > > + * using frameContext.compress.enable and frameContext.compress.gain. > > > + */ > > > + > > > +LOG_DEFINE_CATEGORY(RkISP1Compress) > > > + > > > +constexpr static int kRkISP1CompressInBits = 20; > > > +constexpr static int kRkISP1CompressOutBits = 12; > > > + > > > +/** > > > + * \copydoc libcamera::ipa::Algorithm::configure > > > + */ > > > +int Compress::configure(IPAContext &context, > > > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > > > +{ > > > + if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS || > > > + !context.hw->compand) > > > + LOG(RkISP1Compress, Warning) > > > + << "Compression is not supported by the hardware or kernel."; > > > + > > > > Shouldn't we ensure context.activeState.compress.supported below stays > > false if this warning is output ? > > Oh I missed that. Absolutely! > > > > > > > > + context.activeState.compress.supported = true; > > > + return 0; > > > +} > > > + > > > +/** > > > + * \copydoc libcamera::ipa::Algorithm::prepare > > > + */ > > > +void Compress::prepare([[maybe_unused]] IPAContext &context, > > > + [[maybe_unused]] const uint32_t frame, > > > + IPAFrameContext &frameContext, > > > + RkISP1Params *params) > > > +{ > > > + auto comp = params->block<BlockType::CompandCompress>(); > > > + comp.setEnabled(frameContext.compress.enable); > > > + > > > > Should we also return early if context.activeState.compress.supported is > > false? > > > > I thinks that's probably a 'session configuration' structure item rather > > than an active state item though ? > > > > > + if (!frameContext.compress.enable) > > > + return; > > > + > > > + int xmax = (1 << kRkISP1CompressInBits); > > > + int ymax = (1 << kRkISP1CompressOutBits); > > > + int inLogStep = std::log2(xmax / 64); > > > + > > > + for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) { > > > + double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS); > > > + double y = x * frameContext.compress.gain; > > > + > > > + comp->px[i] = inLogStep; > > > + comp->x[i] = std::min<int>(x * xmax, xmax - 1); > > > + comp->y[i] = std::min<int>(y * ymax, ymax - 1); > > > + } > > > + > > > + LOG(RkISP1Compress, Debug) << "Compression: " << kRkISP1CompressInBits > > > + << " bits to " << kRkISP1CompressOutBits > > > + << " bits gain: " << frameContext.compress.gain; > > > +} > > > + > > > +REGISTER_IPA_ALGORITHM(Compress, "Compress") > > > + > > > +} /* namespace ipa::rkisp1::algorithms */ > > > + > > > +} /* namespace libcamera */ > > > diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h > > > new file mode 100644 > > > index 000000000000..3006508b2b32 > > > --- /dev/null > > > +++ b/src/ipa/rkisp1/algorithms/compress.h > > > @@ -0,0 +1,33 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2024, Ideas On Board > > > + * > > > + * RkISP1 Gamma out control > > > > I think this is compand or compress. > > Yep. Fixed for next version. > > Thanks, > Stefan > > > > > > + */ > > > + > > > +#pragma once > > > + > > > +#include "algorithm.h" > > > + > > > +namespace libcamera { > > > + > > > +namespace ipa::rkisp1::algorithms { > > > + > > > +class Compress : public Algorithm > > > +{ > > > +public: > > > + Compress() = default; > > > + ~Compress() = default; > > > + > > > + int configure(IPAContext &context, > > > + const IPACameraSensorInfo &configInfo) override; > > > + void prepare(IPAContext &context, const uint32_t frame, > > > + IPAFrameContext &frameContext, > > > + RkISP1Params *params) override; > > > + > > > +private: > > > + float defaultGamma_; > > > +}; > > > + > > > +} /* namespace ipa::rkisp1::algorithms */ > > > +} /* namespace libcamera */ > > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > > > index c66b0b70b82f..2e42a80cf99d 100644 > > > --- a/src/ipa/rkisp1/algorithms/meson.build > > > +++ b/src/ipa/rkisp1/algorithms/meson.build > > > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([ > > > 'awb.cpp', > > > 'blc.cpp', > > > 'ccm.cpp', > > > + 'compress.cpp', > > > 'cproc.cpp', > > > 'dpcc.cpp', > > > 'dpf.cpp', > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > > index 7ccc7b501aff..37a74215ce19 100644 > > > --- a/src/ipa/rkisp1/ipa_context.h > > > +++ b/src/ipa/rkisp1/ipa_context.h > > > @@ -106,6 +106,10 @@ struct IPAActiveState { > > > Matrix<float, 3, 3> automatic; > > > } ccm; > > > > > > + struct { > > > + bool supported; > > > + } compress; > > > + > > > struct { > > > int8_t brightness; > > > uint8_t contrast; > > > @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext { > > > bool update; > > > } cproc; > > > > > > + struct { > > > + bool enable; > > > + double gain; > > > + } compress; > > > + > > > struct { > > > bool denoise; > > > bool update; > > > -- > > > 2.48.1 > > >
Morning Stefan - just a couple of tiny things to add to Kieran's review: On 08/08/2025 15:12, Stefan Klug wrote: > The i.MX8 M Plus has a compression curve inside the compand block. This > curve is necessary to process HDR stitched data and is useful for other > aspects like applying a digital gain to the incoming sensor data. > > Add a basic algorithm for the compression curve. This algorithm has a > hardcoded input width of 20bit and output width of 12bit which matches > the imx8mp pipeline. Only a static gain is supported in this version. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/compress.cpp | 100 +++++++++++++++++++++++++ > src/ipa/rkisp1/algorithms/compress.h | 33 ++++++++ > src/ipa/rkisp1/algorithms/meson.build | 1 + > src/ipa/rkisp1/ipa_context.h | 9 +++ > 4 files changed, 143 insertions(+) > create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp > create mode 100644 src/ipa/rkisp1/algorithms/compress.h > > diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp > new file mode 100644 > index 000000000000..b2fad02404d6 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/compress.cpp > @@ -0,0 +1,100 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * RkISP1 Compression curve > + */ > +#include "compress.h" > + > +#include <linux/videodev2.h> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > + > +#include "libcamera/internal/yaml_parser.h" > + > +#include "linux/rkisp1-config.h" > + > +/** > + * \file compress.h > + */ > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +/** > + * \class Compress > + * \brief RkISP1 Compress curve > + * > + * This algorithm implements support for the compressions curve in the compand > + * block available in the i.MX8 M Plus > + * > + * In its current version it only supports a static gain. This is useful for > + * the agc algorithm to compensate for exposure/gain quantization effects. > + * > + * This algorithm doesn't have any configuration options. It needs to be > + * configured per frame by other algorithms. > + * > + * Other algorithms can check activeState.compress.supported to see if > + * compression is available. If it is available they can configure it per frame > + * using frameContext.compress.enable and frameContext.compress.gain. > + */ > + > +LOG_DEFINE_CATEGORY(RkISP1Compress) > + > +constexpr static int kRkISP1CompressInBits = 20; > +constexpr static int kRkISP1CompressOutBits = 12; > + > +/** > + * \copydoc libcamera::ipa::Algorithm::configure > + */ > +int Compress::configure(IPAContext &context, > + [[maybe_unused]] const IPACameraSensorInfo &configInfo) > +{ > + if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS || > + !context.hw->compand) > + LOG(RkISP1Compress, Warning) > + << "Compression is not supported by the hardware or kernel."; > + > + context.activeState.compress.supported = true; > + return 0; > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::prepare > + */ > +void Compress::prepare([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + IPAFrameContext &frameContext, > + RkISP1Params *params) > +{ > + auto comp = params->block<BlockType::CompandCompress>(); > + comp.setEnabled(frameContext.compress.enable); > + > + if (!frameContext.compress.enable) > + return; > + > + int xmax = (1 << kRkISP1CompressInBits); > + int ymax = (1 << kRkISP1CompressOutBits); > + int inLogStep = std::log2(xmax / 64); > + > + for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) { > + double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS); > + double y = x * frameContext.compress.gain; > + > + comp->px[i] = inLogStep; > + comp->x[i] = std::min<int>(x * xmax, xmax - 1); > + comp->y[i] = std::min<int>(y * ymax, ymax - 1); > + } > + > + LOG(RkISP1Compress, Debug) << "Compression: " << kRkISP1CompressInBits > + << " bits to " << kRkISP1CompressOutBits > + << " bits gain: " << frameContext.compress.gain; > +} > + > +REGISTER_IPA_ALGORITHM(Compress, "Compress") > + > +} /* namespace ipa::rkisp1::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h > new file mode 100644 > index 000000000000..3006508b2b32 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/compress.h > @@ -0,0 +1,33 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Ideas On Board s/2024/2025 > + * > + * RkISP1 Gamma out control > + */ > + > +#pragma once > + > +#include "algorithm.h" > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +class Compress : public Algorithm > +{ > +public: > + Compress() = default; > + ~Compress() = default; > + > + int configure(IPAContext &context, > + const IPACameraSensorInfo &configInfo) override; > + void prepare(IPAContext &context, const uint32_t frame, > + IPAFrameContext &frameContext, > + RkISP1Params *params) override; > + > +private: > + float defaultGamma_; This is unused (I guess a hangover from the file copy) > +}; > + > +} /* namespace ipa::rkisp1::algorithms */ > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > index c66b0b70b82f..2e42a80cf99d 100644 > --- a/src/ipa/rkisp1/algorithms/meson.build > +++ b/src/ipa/rkisp1/algorithms/meson.build > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([ > 'awb.cpp', > 'blc.cpp', > 'ccm.cpp', > + 'compress.cpp', > 'cproc.cpp', > 'dpcc.cpp', > 'dpf.cpp', > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 7ccc7b501aff..37a74215ce19 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -106,6 +106,10 @@ struct IPAActiveState { > Matrix<float, 3, 3> automatic; > } ccm; > > + struct { > + bool supported; > + } compress; > + > struct { > int8_t brightness; > uint8_t contrast; > @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext { > bool update; > } cproc; > > + struct { > + bool enable; > + double gain; > + } compress; > + > struct { > bool denoise; > bool update;
diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp new file mode 100644 index 000000000000..b2fad02404d6 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/compress.cpp @@ -0,0 +1,100 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board + * + * RkISP1 Compression curve + */ +#include "compress.h" + +#include <linux/videodev2.h> + +#include <libcamera/base/log.h> +#include <libcamera/base/utils.h> + +#include "libcamera/internal/yaml_parser.h" + +#include "linux/rkisp1-config.h" + +/** + * \file compress.h + */ + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +/** + * \class Compress + * \brief RkISP1 Compress curve + * + * This algorithm implements support for the compressions curve in the compand + * block available in the i.MX8 M Plus + * + * In its current version it only supports a static gain. This is useful for + * the agc algorithm to compensate for exposure/gain quantization effects. + * + * This algorithm doesn't have any configuration options. It needs to be + * configured per frame by other algorithms. + * + * Other algorithms can check activeState.compress.supported to see if + * compression is available. If it is available they can configure it per frame + * using frameContext.compress.enable and frameContext.compress.gain. + */ + +LOG_DEFINE_CATEGORY(RkISP1Compress) + +constexpr static int kRkISP1CompressInBits = 20; +constexpr static int kRkISP1CompressOutBits = 12; + +/** + * \copydoc libcamera::ipa::Algorithm::configure + */ +int Compress::configure(IPAContext &context, + [[maybe_unused]] const IPACameraSensorInfo &configInfo) +{ + if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS || + !context.hw->compand) + LOG(RkISP1Compress, Warning) + << "Compression is not supported by the hardware or kernel."; + + context.activeState.compress.supported = true; + return 0; +} + +/** + * \copydoc libcamera::ipa::Algorithm::prepare + */ +void Compress::prepare([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const uint32_t frame, + IPAFrameContext &frameContext, + RkISP1Params *params) +{ + auto comp = params->block<BlockType::CompandCompress>(); + comp.setEnabled(frameContext.compress.enable); + + if (!frameContext.compress.enable) + return; + + int xmax = (1 << kRkISP1CompressInBits); + int ymax = (1 << kRkISP1CompressOutBits); + int inLogStep = std::log2(xmax / 64); + + for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) { + double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS); + double y = x * frameContext.compress.gain; + + comp->px[i] = inLogStep; + comp->x[i] = std::min<int>(x * xmax, xmax - 1); + comp->y[i] = std::min<int>(y * ymax, ymax - 1); + } + + LOG(RkISP1Compress, Debug) << "Compression: " << kRkISP1CompressInBits + << " bits to " << kRkISP1CompressOutBits + << " bits gain: " << frameContext.compress.gain; +} + +REGISTER_IPA_ALGORITHM(Compress, "Compress") + +} /* namespace ipa::rkisp1::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h new file mode 100644 index 000000000000..3006508b2b32 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/compress.h @@ -0,0 +1,33 @@ +/* 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 Compress : public Algorithm +{ +public: + Compress() = default; + ~Compress() = default; + + int configure(IPAContext &context, + const IPACameraSensorInfo &configInfo) override; + void prepare(IPAContext &context, const uint32_t frame, + IPAFrameContext &frameContext, + RkISP1Params *params) override; + +private: + float defaultGamma_; +}; + +} /* namespace ipa::rkisp1::algorithms */ +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build index c66b0b70b82f..2e42a80cf99d 100644 --- a/src/ipa/rkisp1/algorithms/meson.build +++ b/src/ipa/rkisp1/algorithms/meson.build @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([ 'awb.cpp', 'blc.cpp', 'ccm.cpp', + 'compress.cpp', 'cproc.cpp', 'dpcc.cpp', 'dpf.cpp', diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 7ccc7b501aff..37a74215ce19 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -106,6 +106,10 @@ struct IPAActiveState { Matrix<float, 3, 3> automatic; } ccm; + struct { + bool supported; + } compress; + struct { int8_t brightness; uint8_t contrast; @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext { bool update; } cproc; + struct { + bool enable; + double gain; + } compress; + struct { bool denoise; bool update;
The i.MX8 M Plus has a compression curve inside the compand block. This curve is necessary to process HDR stitched data and is useful for other aspects like applying a digital gain to the incoming sensor data. Add a basic algorithm for the compression curve. This algorithm has a hardcoded input width of 20bit and output width of 12bit which matches the imx8mp pipeline. Only a static gain is supported in this version. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/ipa/rkisp1/algorithms/compress.cpp | 100 +++++++++++++++++++++++++ src/ipa/rkisp1/algorithms/compress.h | 33 ++++++++ src/ipa/rkisp1/algorithms/meson.build | 1 + src/ipa/rkisp1/ipa_context.h | 9 +++ 4 files changed, 143 insertions(+) create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp create mode 100644 src/ipa/rkisp1/algorithms/compress.h