[PATCH v5 09/10] libcamera: software_isp: Track whether CCM is enabled
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Feb 3 02:52:05 CET 2025
Hi Milan,
Thank you for the patch.
On Thu, Jan 30, 2025 at 07:14:46PM +0100, Milan Zamazal wrote:
> Applying color correction matrix (CCM) in software ISP is optional due
> to performance reasons. CCM is applied if and only if `Ccm' algorithm
> is present in the tuning file.
>
> Software ISP debayering is a performance critical piece of code and we
> do not want to use dynamic conditionals there. Therefore we pass
> information about CCM application to debayering configuration and let it
> select the right versions of debayering functions using templates. This
> is a trick similar to the previously used one for adding or not adding
> an alpha channel to the output.
>
> Debayering gets this information but it ignores it in this patch.
> Actual processing with CCM is added in the followup patch.
>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
> .../internal/software_isp/software_isp.h | 1 +
> include/libcamera/ipa/soft.mojom | 2 +-
> src/ipa/simple/algorithms/ccm.cpp | 2 +
> src/ipa/simple/ipa_context.h | 1 +
> src/ipa/simple/soft_simple.cpp | 8 +++-
> src/libcamera/software_isp/debayer.cpp | 3 +-
> src/libcamera/software_isp/debayer.h | 3 +-
> src/libcamera/software_isp/debayer_cpu.cpp | 44 ++++++++++++-------
> src/libcamera/software_isp/debayer_cpu.h | 29 ++++++------
> src/libcamera/software_isp/software_isp.cpp | 5 ++-
> 10 files changed, 61 insertions(+), 37 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index d51b03fd..156fde96 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -97,6 +97,7 @@ private:
> SharedMemObject<DebayerParams> sharedParams_;
> DebayerParams debayerParams_;
> DmaBufAllocator dmaHeap_;
> + bool ccmEnabled_;
>
> std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
> };
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index d52e6f1a..ede74413 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -17,7 +17,7 @@ interface IPASoftInterface {
> libcamera.SharedFD fdStats,
> libcamera.SharedFD fdParams,
> libcamera.ControlInfoMap sensorCtrlInfoMap)
> - => (int32 ret, libcamera.ControlInfoMap ipaControls);
> + => (int32 ret, libcamera.ControlInfoMap ipaControls, bool ccmEnabled);
> start() => (int32 ret);
> stop();
> configure(IPAConfigInfo configInfo)
> diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp
> index 069a12f8..c50eb17d 100644
> --- a/src/ipa/simple/algorithms/ccm.cpp
> +++ b/src/ipa/simple/algorithms/ccm.cpp
> @@ -34,6 +34,8 @@ int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData
> return ret;
> }
>
> + context.ccmEnabled = true;
> +
> return 0;
> }
>
> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> index bd6c66d8..40c13731 100644
> --- a/src/ipa/simple/ipa_context.h
> +++ b/src/ipa/simple/ipa_context.h
> @@ -82,6 +82,7 @@ struct IPAContext {
> IPAActiveState activeState;
> FCQueue<IPAFrameContext> frameContexts;
> ControlInfoMap::Map ctrlMap;
> + bool ccmEnabled;
> };
>
> } /* namespace ipa::soft */
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index b26e4e15..a87c6cdd 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -51,7 +51,8 @@ public:
> const SharedFD &fdStats,
> const SharedFD &fdParams,
> const ControlInfoMap &sensorInfoMap,
> - ControlInfoMap *ipaControls) override;
> + ControlInfoMap *ipaControls,
> + bool *ccmEnabled) override;
> int configure(const IPAConfigInfo &configInfo) override;
>
> int start() override;
> @@ -89,7 +90,8 @@ int IPASoftSimple::init(const IPASettings &settings,
> const SharedFD &fdStats,
> const SharedFD &fdParams,
> const ControlInfoMap &sensorInfoMap,
> - ControlInfoMap *ipaControls)
> + ControlInfoMap *ipaControls,
> + bool *ccmEnabled)
> {
> camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> if (!camHelper_) {
> @@ -125,6 +127,8 @@ int IPASoftSimple::init(const IPASettings &settings,
> if (ret)
> return ret;
>
> + *ccmEnabled = context_.ccmEnabled;
> +
> params_ = nullptr;
> stats_ = nullptr;
>
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index f0b83261..45fe6960 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -57,10 +57,11 @@ Debayer::~Debayer()
> }
>
> /**
> - * \fn int Debayer::configure(const StreamConfiguration &inputCfg, const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs)
> + * \fn int Debayer::configure(const StreamConfiguration &inputCfg, const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs, bool ccmEnabled)
If there are no overloads of the function, you can write
* \fn int Debayer::configure()
and doxygen will figure it out.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> * \brief Configure the debayer object according to the passed in parameters
> * \param[in] inputCfg The input configuration
> * \param[in] outputCfgs The output configurations
> + * \param[in] ccmEnabled Whether a color correction matrix is applied
> *
> * \return 0 on success, a negative errno on failure
> */
> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
> index d7ca060d..ba033d44 100644
> --- a/src/libcamera/software_isp/debayer.h
> +++ b/src/libcamera/software_isp/debayer.h
> @@ -33,7 +33,8 @@ public:
> virtual ~Debayer() = 0;
>
> virtual int configure(const StreamConfiguration &inputCfg,
> - const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) = 0;
> + const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
> + bool ccmEnabled) = 0;
>
> virtual std::vector<PixelFormat> formats(PixelFormat inputFormat) = 0;
>
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 01cfb36b..0cd03a8f 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -114,7 +114,7 @@ DebayerCpu::~DebayerCpu() = default;
> (prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)), \
> curr[x] / (div))
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer8_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> DECLARE_SRC_POINTERS(uint8_t)
> @@ -125,7 +125,7 @@ void DebayerCpu::debayer8_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer8_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> DECLARE_SRC_POINTERS(uint8_t)
> @@ -136,7 +136,7 @@ void DebayerCpu::debayer8_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer10_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> DECLARE_SRC_POINTERS(uint16_t)
> @@ -148,7 +148,7 @@ void DebayerCpu::debayer10_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer10_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> DECLARE_SRC_POINTERS(uint16_t)
> @@ -160,7 +160,7 @@ void DebayerCpu::debayer10_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer12_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> DECLARE_SRC_POINTERS(uint16_t)
> @@ -172,7 +172,7 @@ void DebayerCpu::debayer12_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer12_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> DECLARE_SRC_POINTERS(uint16_t)
> @@ -184,7 +184,7 @@ void DebayerCpu::debayer12_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> const int widthInBytes = window_.width * 5 / 4;
> @@ -210,7 +210,7 @@ void DebayerCpu::debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> const int widthInBytes = window_.width * 5 / 4;
> @@ -231,7 +231,7 @@ void DebayerCpu::debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> const int widthInBytes = window_.width * 5 / 4;
> @@ -252,7 +252,7 @@ void DebayerCpu::debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[])
> }
> }
>
> -template<bool addAlphaByte>
> +template<bool addAlphaByte, bool ccmEnabled>
> void DebayerCpu::debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[])
> {
> const int widthInBytes = window_.width * 5 / 4;
> @@ -368,11 +368,17 @@ int DebayerCpu::setupStandardBayerOrder(BayerFormat::Order order)
> return 0;
> }
>
> -#define SET_DEBAYER_METHODS(method0, method1) \
> - debayer0_ = addAlphaByte ? &DebayerCpu::method0<true> : &DebayerCpu::method0<false>; \
> - debayer1_ = addAlphaByte ? &DebayerCpu::method1<true> : &DebayerCpu::method1<false>;
> -
> -int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputFormat)
> +#define SET_DEBAYER_METHODS(method0, method1) \
> + debayer0_ = addAlphaByte \
> + ? (ccmEnabled ? &DebayerCpu::method0<true, true> : &DebayerCpu::method0<true, false>) \
> + : (ccmEnabled ? &DebayerCpu::method0<false, true> : &DebayerCpu::method0<false, false>); \
> + debayer1_ = addAlphaByte \
> + ? (ccmEnabled ? &DebayerCpu::method1<true, true> : &DebayerCpu::method1<true, false>) \
> + : (ccmEnabled ? &DebayerCpu::method1<false, true> : &DebayerCpu::method1<false, false>);
> +
> +int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat,
> + PixelFormat outputFormat,
> + bool ccmEnabled)
> {
> BayerFormat bayerFormat =
> BayerFormat::fromPixelFormat(inputFormat);
> @@ -464,7 +470,8 @@ int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputF
> }
>
> int DebayerCpu::configure(const StreamConfiguration &inputCfg,
> - const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs)
> + const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
> + bool ccmEnabled)
> {
> if (getInputConfig(inputCfg.pixelFormat, inputConfig_) != 0)
> return -EINVAL;
> @@ -503,7 +510,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,
> return -EINVAL;
> }
>
> - if (setDebayerFunctions(inputCfg.pixelFormat, outputCfg.pixelFormat) != 0)
> + int ret = setDebayerFunctions(inputCfg.pixelFormat,
> + outputCfg.pixelFormat,
> + ccmEnabled);
> + if (ret != 0)
> return -EINVAL;
>
> window_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) &
> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
> index 2c47e7c6..21c08a2d 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -1,7 +1,7 @@
> /* SPDX-License-Identifier: LGPL-2.1-or-later */
> /*
> * Copyright (C) 2023, Linaro Ltd
> - * Copyright (C) 2023, Red Hat Inc.
> + * Copyright (C) 2023-2025 Red Hat Inc.
> *
> * Authors:
> * Hans de Goede <hdegoede at redhat.com>
> @@ -31,7 +31,8 @@ public:
> ~DebayerCpu();
>
> int configure(const StreamConfiguration &inputCfg,
> - const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs);
> + const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
> + bool ccmEnabled);
> Size patternSize(PixelFormat inputFormat);
> std::vector<PixelFormat> formats(PixelFormat input);
> std::tuple<unsigned int, unsigned int>
> @@ -85,28 +86,28 @@ private:
> using debayerFn = void (DebayerCpu::*)(uint8_t *dst, const uint8_t *src[]);
>
> /* 8-bit raw bayer format */
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer8_BGBG_BGR888(uint8_t *dst, const uint8_t *src[]);
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer8_GRGR_BGR888(uint8_t *dst, const uint8_t *src[]);
> /* unpacked 10-bit raw bayer format */
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer10_BGBG_BGR888(uint8_t *dst, const uint8_t *src[]);
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer10_GRGR_BGR888(uint8_t *dst, const uint8_t *src[]);
> /* unpacked 12-bit raw bayer format */
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer12_BGBG_BGR888(uint8_t *dst, const uint8_t *src[]);
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer12_GRGR_BGR888(uint8_t *dst, const uint8_t *src[]);
> /* CSI-2 packed 10-bit raw bayer format (all the 4 orders) */
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[]);
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[]);
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[]);
> - template<bool addAlphaByte>
> + template<bool addAlphaByte, bool ccmEnabled>
> void debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[]);
>
> struct DebayerInputConfig {
> @@ -125,7 +126,9 @@ private:
> int getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config);
> int getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config);
> int setupStandardBayerOrder(BayerFormat::Order order);
> - int setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputFormat);
> + int setDebayerFunctions(PixelFormat inputFormat,
> + PixelFormat outputFormat,
> + bool ccmEnabled);
> void setupInputMemcpy(const uint8_t *linePointers[]);
> void shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);
> void memcpyNextLine(const uint8_t *linePointers[]);
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 44baf200..b03c4baa 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -128,7 +128,8 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> debayer_->getStatsFD(),
> sharedParams_.fd(),
> sensor->controls(),
> - ipaControls);
> + ipaControls,
> + &ccmEnabled_);
> if (ret) {
> LOG(SoftwareIsp, Error) << "IPA init failed";
> debayer_.reset();
> @@ -240,7 +241,7 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
> if (ret < 0)
> return ret;
>
> - return debayer_->configure(inputCfg, outputCfgs);
> + return debayer_->configure(inputCfg, outputCfgs, ccmEnabled_);
> }
>
> /**
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list