[v3,09/22] libcamera: software_isp: Move useful items from DebayerCpu to Debayer base class

25295 diff mbox series

The DebayerCpu class has a number of variables, embedded structures and
methods which are useful to DebayerGpu implementation.

Move relevant variables and methods to base class.

Since we want to call setParams() from the GPUISP and reuse the code in
the existing CPUISP as a first step, we need to move all of the
dependent variables in DebayerCPU to the Debayer base class including
LookupTable and redCcm_.

The DebayerEGL class will ultimately be able to consume both the CCM and
non-CCM data.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/libcamera/software_isp/debayer.cpp     | 149 +++++++++++++++++++++
 src/libcamera/software_isp/debayer.h       |  35 ++++-
 src/libcamera/software_isp/debayer_cpu.cpp |   2 +-
 src/libcamera/software_isp/debayer_cpu.h   |  41 +-----
 4 files changed, 185 insertions(+), 42 deletions(-)

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> The DebayerCpu class has a number of variables, embedded structures and
> methods which are useful to DebayerGpu implementation.
>
> Move relevant variables and methods to base class.
>
> Since we want to call setParams() from the GPUISP and reuse the code in
> the existing CPUISP as a first step, we need to move all of the
> dependent variables in DebayerCPU to the Debayer base class including
> LookupTable and redCcm_.
>
> The DebayerEGL class will ultimately be able to consume both the CCM and
> non-CCM data.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  src/libcamera/software_isp/debayer.cpp     | 149 +++++++++++++++++++++
>  src/libcamera/software_isp/debayer.h       |  35 ++++-
>  src/libcamera/software_isp/debayer_cpu.cpp |   2 +-
>  src/libcamera/software_isp/debayer_cpu.h   |  41 +-----
>  4 files changed, 185 insertions(+), 42 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index e9e18c488..1e3f08673 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -18,6 +18,12 @@ namespace libcamera {
>   * \brief Struct to hold the debayer parameters.
>   */
>  
> +/**
> + * \fn Debayer::Debayer(const GlobalConfiguration &configuration)
> + * \brief Construct a Debayer object
> + * \param[in] configuration Global configuration reference
> + */
> +
>  /**
>   * \var DebayerParams::kRGBLookupSize
>   * \brief Size of a color lookup table
> @@ -167,6 +173,24 @@ Debayer::~Debayer()
>   */
>  
>  /**
> + * \fn const SharedFD &Debayer::getStatsFD()
> + * \brief Get the file descriptor for the statistics.

I think \brief lines (here and below) shouldn't contain final dots but I
happily leave dealing with this to the maintainers.

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> + *
> + * This file descriptor provides access to the output statistics buffer
> + * associated with the current debayering process.
> + *
> + * \return The file descriptor pointing to the statistics data.
> + */
> +
> +/**
> + * \fn unsigned int Debayer::frameSize()
> + * \brief Get the output frame size.
> + *
> + * \return The total output frame size in bytes as configured for the
> + * current stream.
> + */
> +
> + /**
>   * \var Signal<FrameBuffer *> Debayer::inputBufferReady
>   * \brief Signals when the input buffer is ready
>   */
> @@ -176,4 +200,129 @@ Debayer::~Debayer()
>   * \brief Signals when the output buffer is ready
>   */
>  
> +/**
> + * \struct Debayer::DebayerInputConfig
> + * \brief Structure describing the incoming Bayer parameters.
> + *
> + * The DebayerInputConfig structure defines the characteristics of the raw
> + * Bayer frame being processed, including:
> + *  - The Bayer pattern dimensions (\ref patternSize)
> + *  - Memory layout parameters such as stride and bytes per pixel (\ref bpp)
> + *  - A list of supported output pixel formats.
> + *
> + * \var Debayer::DebayerInputConfig::patternSize
> + * Size of the Bayer pattern in pixels. For standard Bayer formats such as
> + * BGGR, GRBG, GBRG, and RGGB, this is typically 2×2 pixels.
> + *
> + * \var Debayer::DebayerInputConfig::bpp
> + * Number of bytes used per pixel in memory. This reflects storage size,
> + * not precision.
> + *
> + * \var Debayer::DebayerInputConfig::stride
> + * Line stride in bytes for the Bayer input frame.
> + *
> + * \var Debayer::DebayerInputConfig::outputFormats
> + * List of pixel formats supported as output for this input configuration.
> + */
> +
> +/**
> + * \struct Debayer::DebayerOutputConfig
> + * \brief Structure describing the output frame configuration.
> + *
> + * Defines how the output of the debayer process is laid out in memory.
> + * It includes per-pixel size, stride, and total frame size.
> + *
> + * \var Debayer::DebayerOutputConfig::bpp
> + * Bytes used per pixel in the output format.
> + *
> + * \var Debayer::DebayerOutputConfig::stride
> + * Line stride in bytes for the output frame.
> + *
> + * \var Debayer::DebayerOutputConfig::frameSize
> + * Total frame size in bytes for the output buffer.
> + */
> +
> +/**
> + * \var Debayer::inputConfig_
> + * \brief Input configuration parameters for the current debayer operation.
> + *
> + * Holds metadata describing the incoming Bayer image layout, including
> + * pattern size, bytes per pixel, stride, and supported output formats.
> + * Populated during configuration.
> + */
> +
> +/**
> + * \var Debayer::outputConfig_
> + * \brief Output configuration data for the debayered frame.
> + *
> + * Contains bytes per pixel, stride, and total frame size for the
> + * output image buffer. Set during stream configuration.
> + */
> +
> +/**
> + * \var Debayer::red_
> + * \brief Lookup table for red channel gain and correction values.
> + *
> + * This table provides precomputed per-pixel or per-intensity
> + * correction values for the red color channel used during debayering.
> + */
> +
> +/**
> + * \var Debayer::green_
> + * \brief Lookup table for green channel gain and correction values.
> + *
> + * This table provides precomputed per-pixel or per-intensity
> + * correction values for the green color channel used during debayering.
> + */
> +
> +/**
> + * \var Debayer::blue_
> + * \brief Lookup table for blue channel gain and correction values.
> + *
> + * This table provides precomputed per-pixel or per-intensity
> + * correction values for the blue color channel used during debayering.
> + */
> +
> +/**
> + * \var Debayer::redCcm_
> + * \brief Red channel Color Correction Matrix (CCM) lookup table.
> + *
> + * Contains coefficients for green channel color correction.
> + */
> +
> +/**
> + * \var Debayer::greenCcm_
> + * \brief Green channel Color Correction Matrix (CCM) lookup table.
> + *
> + * Contains coefficients for green channel color correction.
> + */
> +
> +/**
> + * \var Debayer::blueCcm_
> + * \brief Blue channel Color Correction Matrix (CCM) lookup table.
> + *
> + * Contains coefficients for blue channel color correction.
> + */
> +
> +/**
> + * \var Debayer::gammaLut_
> + * \brief Gamma correction lookup table.
> + */
> +
> +/**
> + * \var Debayer::swapRedBlueGains_
> + * \brief Flag indicating whether red and blue channel gains should be swapped.
> + *
> + * Used when the Bayer pattern order indicates that red/blue color channels are
> + * reversed.
> + */
> +
> +/**
> + * \var Debayer::bench_
> + * \brief Benchmarking utility instance for performance measurements.
> + *
> + * Used internally to track timing and performance metrics during
> + * debayer processing.
> + */
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
> index ba033d440..b562985a6 100644
> --- a/src/libcamera/software_isp/debayer.h
> +++ b/src/libcamera/software_isp/debayer.h
> @@ -14,11 +14,14 @@
>  #include <stdint.h>
>  
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/object.h>
>  #include <libcamera/base/signal.h>
>  
>  #include <libcamera/geometry.h>
>  #include <libcamera/stream.h>
>  
> +#include "libcamera/internal/global_configuration.h"
> +#include "libcamera/internal/software_isp/benchmark.h"
>  #include "libcamera/internal/software_isp/debayer_params.h"
>  
>  namespace libcamera {
> @@ -27,9 +30,10 @@ class FrameBuffer;
>  
>  LOG_DECLARE_CATEGORY(Debayer)
>  
> -class Debayer
> +class Debayer : public Object
>  {
>  public:
> +	Debayer (const GlobalConfiguration &configuration) : bench_(configuration) {};
>  	virtual ~Debayer() = 0;
>  
>  	virtual int configure(const StreamConfiguration &inputCfg,
> @@ -45,9 +49,38 @@ public:
>  
>  	virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;
>  
> +	virtual const SharedFD &getStatsFD() = 0;
> +
> +	unsigned int frameSize() { return outputConfig_.frameSize; }
> +
>  	Signal<FrameBuffer *> inputBufferReady;
>  	Signal<FrameBuffer *> outputBufferReady;
>  
> +	struct DebayerInputConfig {
> +		Size patternSize;
> +		unsigned int bpp;
> +		unsigned int stride;
> +		std::vector<PixelFormat> outputFormats;
> +	};
> +
> +	struct DebayerOutputConfig {
> +		unsigned int bpp;
> +		unsigned int stride;
> +		unsigned int frameSize;
> +	};
> +
> +	DebayerInputConfig inputConfig_;
> +	DebayerOutputConfig outputConfig_;
> +	DebayerParams::LookupTable red_;
> +	DebayerParams::LookupTable green_;
> +	DebayerParams::LookupTable blue_;
> +	DebayerParams::CcmLookupTable redCcm_;
> +	DebayerParams::CcmLookupTable greenCcm_;
> +	DebayerParams::CcmLookupTable blueCcm_;
> +	DebayerParams::LookupTable gammaLut_;
> +	bool swapRedBlueGains_;
> +	Benchmark bench_;
> +
>  private:
>  	virtual Size patternSize(PixelFormat inputFormat) = 0;
>  };
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index b92c6a904..e55599f09 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -42,7 +42,7 @@ namespace libcamera {
>   * \param[in] configuration The global configuration
>   */
>  DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfiguration &configuration)
> -	: stats_(std::move(stats)), bench_(configuration)
> +	: Debayer(configuration), stats_(std::move(stats))
>  {
>  	/*
>  	 * Reading from uncached buffers may be very slow.
> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
> index aff32491e..ecc4f9dd0 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -17,16 +17,14 @@
>  
>  #include <libcamera/base/object.h>
>  
> -#include "libcamera/internal/software_isp/benchmark.h"
>  #include "libcamera/internal/bayer_format.h"
> -#include "libcamera/internal/global_configuration.h"
>  #include "libcamera/internal/software_isp/swstats_cpu.h"
>  
>  #include "debayer.h"
>  
>  namespace libcamera {
>  
> -class DebayerCpu : public Debayer, public Object
> +class DebayerCpu : public Debayer
>  {
>  public:
>  	DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfiguration &configuration);
> @@ -41,21 +39,8 @@ public:
>  	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
>  	void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params);
>  	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
> -
> -	/**
> -	 * \brief Get the file descriptor for the statistics
> -	 *
> -	 * \return the file descriptor pointing to the statistics
> -	 */
>  	const SharedFD &getStatsFD() { return stats_->getStatsFD(); }
>  
> -	/**
> -	 * \brief Get the output frame size
> -	 *
> -	 * \return The output frame size
> -	 */
> -	unsigned int frameSize() { return outputConfig_.frameSize; }
> -
>  private:
>  	/**
>  	 * \brief Called to debayer 1 line of Bayer input data to output format
> @@ -112,19 +97,6 @@ private:
>  	template<bool addAlphaByte, bool ccmEnabled>
>  	void debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[]);
>  
> -	struct DebayerInputConfig {
> -		Size patternSize;
> -		unsigned int bpp; /* Memory used per pixel, not precision */
> -		unsigned int stride;
> -		std::vector<PixelFormat> outputFormats;
> -	};
> -
> -	struct DebayerOutputConfig {
> -		unsigned int bpp; /* Memory used per pixel, not precision */
> -		unsigned int stride;
> -		unsigned int frameSize;
> -	};
> -
>  	int getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config);
>  	int getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config);
>  	int setupStandardBayerOrder(BayerFormat::Order order);
> @@ -140,20 +112,11 @@ private:
>  	/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */
>  	static constexpr unsigned int kMaxLineBuffers = 5;
>  
> -	DebayerParams::LookupTable red_;
> -	DebayerParams::LookupTable green_;
> -	DebayerParams::LookupTable blue_;
> -	DebayerParams::CcmLookupTable redCcm_;
> -	DebayerParams::CcmLookupTable greenCcm_;
> -	DebayerParams::CcmLookupTable blueCcm_;
> -	DebayerParams::LookupTable gammaLut_;
>  	debayerFn debayer0_;
>  	debayerFn debayer1_;
>  	debayerFn debayer2_;
>  	debayerFn debayer3_;
>  	Rectangle window_;
> -	DebayerInputConfig inputConfig_;
> -	DebayerOutputConfig outputConfig_;
>  	std::unique_ptr<SwStatsCpu> stats_;
>  	std::vector<uint8_t> lineBuffers_[kMaxLineBuffers];
>  	unsigned int lineBufferLength_;
> @@ -161,8 +124,6 @@ private:
>  	unsigned int lineBufferIndex_;
>  	unsigned int xShift_; /* Offset of 0/1 applied to window_.x */
>  	bool enableInputMemcpy_;
> -	bool swapRedBlueGains_;
> -	Benchmark bench_;
>  };
>  
>  } /* namespace libcamera */