[PATCH 3/5] libcamera: software_isp: Move color mappings out of debayering

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Apr 26 17:59:06 CEST 2024


Hi Milan,

Thank you for the patch.

On Tue, Apr 23, 2024 at 08:19:58PM +0200, Milan Zamazal wrote:
> Constructing the color mapping tables is related to stats rather than
> debayering, where they are applied.  Let's move the corresponding code
> to stats processing.
> 
> This is a preliminary step towards building this functionality on top of
> libipa/algorithm.h, which should follow.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  .../internal/software_isp/debayer_params.h    | 18 ++--
>  src/ipa/simple/soft_simple.cpp                | 84 ++++++++++++-------
>  src/libcamera/software_isp/debayer.cpp        | 29 ++++---
>  src/libcamera/software_isp/debayer_cpu.cpp    | 41 ++-------
>  src/libcamera/software_isp/debayer_cpu.h      |  9 +-
>  src/libcamera/software_isp/software_isp.cpp   |  4 +-
>  6 files changed, 87 insertions(+), 98 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h
> index 32cd448a..09f4ff00 100644
> --- a/include/libcamera/internal/software_isp/debayer_params.h
> +++ b/include/libcamera/internal/software_isp/debayer_params.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
> - * Copyright (C) 2023, Red Hat Inc.
> + * Copyright (C) 2023, 2024 Red Hat Inc.
>   *
>   * Authors:
>   * Hans de Goede <hdegoede at redhat.com>
> @@ -10,20 +10,20 @@
>  
>  #pragma once
>  
> +#include <array>
> +#include <stdint.h>
> +
>  namespace libcamera {
>  
>  struct DebayerParams {
>  	static constexpr unsigned int kGain10 = 256;
> +	static constexpr unsigned int kRGBLookupSize = 256;
>  
> -	unsigned int gainR;
> -	unsigned int gainG;
> -	unsigned int gainB;
> +	using ColorLookupTable = std::array<uint8_t, kRGBLookupSize>;
>  
> -	float gamma;
> -	/**
> -	 * \brief Level of the black point, 0..255, 0 is no correction.
> -	 */
> -	unsigned int blackLevel;
> +	ColorLookupTable red;
> +	ColorLookupTable green;
> +	ColorLookupTable blue;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index 30956a19..c5085f71 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -5,6 +5,7 @@
>   * soft_simple.cpp - Simple Software Image Processing Algorithm module
>   */
>  
> +#include <math.h>
>  #include <numeric>
>  #include <stdint.h>
>  #include <sys/mman.h>
> @@ -84,6 +85,10 @@ private:
>  	ControlInfoMap sensorInfoMap_;
>  	BlackLevel blackLevel_;
>  
> +	static constexpr unsigned int kGammaLookupSize = 1024;
> +	std::array<uint8_t, kGammaLookupSize> gammaTable_;
> +	int lastBlackLevel_ = -1;
> +
>  	int32_t exposureMin_, exposureMax_;
>  	int32_t exposure_;
>  	double againMin_, againMax_, againMinStep_;
> @@ -246,40 +251,59 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>  	if (ignoreUpdates_ > 0)
>  		blackLevel_.update(histogram);
>  	const uint8_t blackLevel = blackLevel_.get();
> -	params_->blackLevel = blackLevel;
>  
>  	/*
>  	 * Calculate red and blue gains for AWB.
>  	 * Clamp max gain at 4.0, this also avoids 0 division.
>  	 */
> -	{
> -		const uint64_t nPixels = std::accumulate(
> -			histogram.begin(), histogram.end(), 0);
> -		auto subtractBlackLevel = [blackLevel, nPixels](
> -						  uint64_t sum, uint64_t div)
> -			-> uint64_t {
> -			uint64_t reducedSum = sum - blackLevel * nPixels / div;
> -			uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel);
> -			return spreadSum;
> -		};
> -		const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4);
> -		const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2);
> -		const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4);
> -
> -		if (sumR <= sumG / 4)
> -			params_->gainR = 1024;
> -		else
> -			params_->gainR = 256 * sumG / sumR;
> -
> -		if (sumB <= sumG / 4)
> -			params_->gainB = 1024;
> -		else
> -			params_->gainB = 256 * sumG / sumB;
> +	const uint64_t nPixels = std::accumulate(
> +		histogram.begin(), histogram.end(), 0);
> +	auto subtractBlackLevel = [blackLevel, nPixels](
> +					  uint64_t sum, uint64_t div)
> +		-> uint64_t {
> +		const uint64_t reducedSum = sum - blackLevel * nPixels / div;
> +		const uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel);
> +		return spreadSum;
> +	};
> +	const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4);
> +	const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2);
> +	const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4);

The above belongs to the previous patch, including adding the const
keyword to the local variables in subtractBlackLevel().

> +
> +	/* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */
> +	unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;
> +	unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;
> +	/* Green gain and gamma values are fixed */
> +	constexpr unsigned int gainG = 256;
> +	/* gamma == 1.0 means no correction */
> +	constexpr float gamma = 0.5;
> +
> +	/* Update the gamma table if needed */
> +	if (blackLevel != lastBlackLevel_) {
> +		const unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;
> +		std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);
> +		const float divisor = kGammaLookupSize - blackIndex - 1.0;
> +		for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
> +			gammaTable_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, gamma);
> +
> +		lastBlackLevel_ = blackLevel;
>  	}
>  
> -	/* Green gain and gamma values are fixed */
> -	params_->gainG = 256;
> -	params_->gamma = 0.5;
> +	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> +		constexpr unsigned int div =
> +			DebayerParams::kRGBLookupSize * DebayerParams::kGain10 /
> +			kGammaLookupSize;
> +		unsigned int idx;
> +
> +		/* Apply gamma after gain! */
> +		idx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });
> +		params_->red[i] = gammaTable_[idx];
> +
> +		idx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });
> +		params_->green[i] = gammaTable_[idx];
> +
> +		idx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });
> +		params_->blue[i] = gammaTable_[idx];
> +	}
>  
>  	setIspParams.emit();
>  
> @@ -300,7 +324,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>  	 * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf
>  	 */
>  	const unsigned int blackLevelHistIdx =
> -		params_->blackLevel / (256 / SwIspStats::kYHistogramSize);
> +		blackLevel / (256 / SwIspStats::kYHistogramSize);
>  	const unsigned int histogramSize =
>  		SwIspStats::kYHistogramSize - blackLevelHistIdx;
>  	const unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;
> @@ -348,8 +372,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>  
>  	LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV
>  			    << " exp " << exposure_ << " again " << again_
> -			    << " gain R/B " << params_->gainR << "/" << params_->gainB
> -			    << " black level " << params_->blackLevel;
> +			    << " gain R/B " << gainR << "/" << gainB
> +			    << " black level " << blackLevel;
>  }
>  
>  void IPASoftSimple::updateExposure(double exposureMSV)
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index 1c035e9b..ac438a33 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -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, 2024 Red Hat Inc.
>   *
>   * Authors:
>   * Hans de Goede <hdegoede at redhat.com>
> @@ -24,29 +24,28 @@ namespace libcamera {
>   */
>  
>  /**
> - * \var DebayerParams::gainR
> - * \brief Red gain
> - *
> - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
> + * \var DebayerParams::kRGBLookupSize
> + * \brief Size of a color lookup table
>   */
>  
>  /**
> - * \var DebayerParams::gainG
> - * \brief Green gain
> - *
> - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
> + * \typedef DebayerParams::ColorLookupTable
> + * \brief Type of the lookup tables for red, green, blue values
>   */
>  
>  /**
> - * \var DebayerParams::gainB
> - * \brief Blue gain
> - *
> - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
> + * \var DebayerParams::red
> + * \brief Lookup table for red color, mapping input values to output values
> + */
> +
> +/**
> + * \var DebayerParams::green
> + * \brief Lookup table for green color, mapping input values to output values
>   */
>  
>  /**
> - * \var DebayerParams::gamma
> - * \brief Gamma correction, 1.0 is no correction
> + * \var DebayerParams::blue
> + * \brief Lookup table for blue color, mapping input values to output values
>   */
>  
>  /**
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 88d6578b..8b2b2f40 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -11,7 +11,6 @@
>  
>  #include "debayer_cpu.h"
>  
> -#include <math.h>
>  #include <stdlib.h>
>  #include <time.h>
>  
> @@ -47,9 +46,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
>  	 */
>  	enableInputMemcpy_ = true;
>  
> -	/* Initialize gamma to 1.0 curve */
> -	for (unsigned int i = 0; i < kGammaLookupSize; i++)
> -		gamma_[i] = i / (kGammaLookupSize / kRGBLookupSize);
> +	/* Initialize color lookup tables */
> +	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)
> +		red_[i] = green_[i] = blue_[i] = i;
>  
>  	for (unsigned int i = 0; i < kMaxLineBuffers; i++)
>  		lineBuffers_[i] = nullptr;
> @@ -698,37 +697,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
>  		clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);
>  	}
>  
> -	/* Apply DebayerParams */
> -	if (params.gamma != gammaCorrection_ || params.blackLevel != blackLevel_) {
> -		const unsigned int blackIndex =
> -			params.blackLevel * kGammaLookupSize / 256;
> -		std::fill(gamma_.begin(), gamma_.begin() + blackIndex, 0);
> -		const float divisor = kGammaLookupSize - blackIndex - 1.0;
> -		for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
> -			gamma_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, params.gamma);
> -
> -		gammaCorrection_ = params.gamma;
> -		blackLevel_ = params.blackLevel;
> -	}
> -
> -	if (swapRedBlueGains_)
> -		std::swap(params.gainR, params.gainB);
> -
> -	for (unsigned int i = 0; i < kRGBLookupSize; i++) {
> -		constexpr unsigned int div =
> -			kRGBLookupSize * DebayerParams::kGain10 / kGammaLookupSize;
> -		unsigned int idx;
> -
> -		/* Apply gamma after gain! */
> -		idx = std::min({ i * params.gainR / div, (kGammaLookupSize - 1) });
> -		red_[i] = gamma_[idx];
> -
> -		idx = std::min({ i * params.gainG / div, (kGammaLookupSize - 1) });
> -		green_[i] = gamma_[idx];
> -
> -		idx = std::min({ i * params.gainB / div, (kGammaLookupSize - 1) });
> -		blue_[i] = gamma_[idx];
> -	}
> +	green_ = params.green;
> +	red_ = swapRedBlueGains_ ? params.blue : params.red;
> +	blue_ = swapRedBlueGains_ ? params.red : params.blue;
>  
>  	/* Copy metadata from the input buffer */
>  	FrameMetadata &metadata = output->_d()->metadata();
> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
> index 689c1075..47373426 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -122,15 +122,12 @@ private:
>  	void process2(const uint8_t *src, uint8_t *dst);
>  	void process4(const uint8_t *src, uint8_t *dst);
>  
> -	static constexpr unsigned int kGammaLookupSize = 1024;
> -	static constexpr unsigned int kRGBLookupSize = 256;
>  	/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */
>  	static constexpr unsigned int kMaxLineBuffers = 5;
>  
> -	std::array<uint8_t, kGammaLookupSize> gamma_;
> -	std::array<uint8_t, kRGBLookupSize> red_;
> -	std::array<uint8_t, kRGBLookupSize> green_;
> -	std::array<uint8_t, kRGBLookupSize> blue_;
> +	DebayerParams::ColorLookupTable red_;
> +	DebayerParams::ColorLookupTable green_;
> +	DebayerParams::ColorLookupTable blue_;
>  	debayerFn debayer0_;
>  	debayerFn debayer1_;
>  	debayerFn debayer2_;
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index e4e56086..3e07453d 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -63,9 +63,7 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
>   * handler
>   */
>  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
> -	: debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10,
> -			  DebayerParams::kGain10, 0.5f, 0 },
> -	  dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)
> +	: dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)
>  {
>  	if (!dmaHeap_.isValid()) {
>  		LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object";

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list