[PATCH v3 1/3] ipa: rkisp1: agc: Read histogram weights from tuning file

Stefan Klug stefan.klug at ideasonboard.com
Fri May 17 12:31:22 CEST 2024


Hi Paul,

thanks for the patch.

On Fri, May 17, 2024 at 05:07:59PM +0900, Paul Elder wrote:
> Add support to the rkisp1 AGC to read histogram weights from the tuning
> file. As controls for selecting the metering mode are not yet supported,
> for now hardcode the matrix metering mode, which is the same as what the
> AGC previously hardcoded.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v3:
> - support the new tuning file layout for interpolated matrices
> - support both v10 and v12
> 
> Changes in v2:
> - add default metering mode if none are read successfully from the
>   tuning file
> - compute the predivider instead of using a table
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 94 ++++++++++++++++++++++++++++++-
>  src/ipa/rkisp1/algorithms/agc.h   |  6 ++
>  2 files changed, 97 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 5aad5c7c2..4af397bdc 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -17,6 +17,8 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/ipa/core_ipa_interface.h>
>  
> +#include "libcamera/internal/yaml_parser.h"
> +
>  #include "libipa/histogram.h"
>  
>  /**
> @@ -36,6 +38,76 @@ namespace ipa::rkisp1::algorithms {
>  
>  LOG_DEFINE_CATEGORY(RkISP1Agc)
>  
> +int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> +			    const char *prop)
> +{
> +	const YamlObject &yamlMeteringModes = tuningData[prop];
> +	if (!yamlMeteringModes.isDictionary()) {
> +		LOG(RkISP1Agc, Error)
> +			<< "'" << prop << "' parameter not found in tuning file";
> +		return -EINVAL;
> +	}
> +
> +	for (const auto &[key, value] : yamlMeteringModes.asDict()) {
> +		if (controls::AeMeteringModeNameValueMap.find(key) ==
> +		    controls::AeMeteringModeNameValueMap.end()) {
> +			LOG(RkISP1Agc, Warning)
> +				<< "Skipping unknown metering mode '" << key << "'";
> +			continue;
> +		}
> +
> +		for (const auto &[version, matrix] : value.asDict()) {
> +			std::vector<uint8_t> weights =
> +				matrix.getList<uint8_t>().value_or(std::vector<uint8_t>{});
> +			if (weights.size() != context.hw->numHistogramWeights)
> +				continue;
> +
> +			LOG(RkISP1Agc, Debug)
> +				<< "Matched metering matrix mode "
> +				<< key << ", version " << version;
> +
> +			meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
> +		}
> +	}
> +
> +	if (meteringModes_.empty()) {
> +		int32_t meteringModeId = controls::AeMeteringModeNameValueMap.at("MeteringMatrix");
> +		std::vector<uint8_t> weights(context.hw->numHistogramWeights, 1);
> +
> +		meteringModes_[meteringModeId] = weights;
> +	}
> +
> +	return 0;
> +}
> +
> +uint8_t Agc::predivider(Size &size)
> +{
> +	/*
> +	 * The maximum number of pixels that could potentially be in one bin is
> +	 * if all the pixels of the image are in it, multiplied by 3 for the
> +	 * three color channels. The counter for each bin is 16 bits wide, so
> +	 * `factor` thus contains the number of times we'd wrap around. This is
> +	 * obviously the number of pixels that we need to skip to make sure
> +	 * that we don't wrap around, but we compute the square root of it
> +	 * instead, as the skip that we need to program is for both the x and y
> +	 * directions.
> +	 *
> +	 * There's a bit of extra rounding math to make sure the rounding goes
> +	 * the correct direction so that the square of the step is big enough
> +	 * to encompass the `factor` number of pixels that we need to skip.
> +	 */

I like that...

THe rest look also good to me.

Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com> 

Cheers,
Stefan

> +	double factor = size.width * size.height * 3 / 65535.0;
> +	double root = std::sqrt(factor);
> +	uint8_t ret;
> +
> +	if (std::pow(std::floor(root), 2) + 0.01 < factor)
> +		ret = static_cast<uint8_t>(std::ceil(root));
> +	else
> +		ret = static_cast<uint8_t>(std::floor(root));
> +
> +	return std::clamp<uint8_t>(ret, 3, 127);
> +}
> +
>  Agc::Agc()
>  {
>  	supportsRaw_ = true;
> @@ -59,6 +131,10 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
>  	if (ret)
>  		return ret;
>  
> +	ret = parseMeteringModes(context, tuningData, "AeMeteringMode");
> +	if (ret)
> +		return ret;
> +
>  	context.ctrlMap.merge(controls());
>  
>  	return 0;
> @@ -160,6 +236,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>  		frameContext.agc.gain = context.activeState.agc.automatic.gain;
>  	}
>  
> +	/* \todo Remove this when we can set the below with controls */
>  	if (frame > 0)
>  		return;
>  
> @@ -178,14 +255,25 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>  	params->meas.hst_config.meas_window = context.configuration.agc.measureWindow;
>  	/* Produce the luminance histogram. */
>  	params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;
> +
>  	/* Set an average weighted histogram. */
>  	Span<uint8_t> weights{
>  		params->meas.hst_config.hist_weight,
>  		context.hw->numHistogramWeights
>  	};
> -	std::fill(weights.begin(), weights.end(), 1);
> -	/* Step size can't be less than 3. */
> -	params->meas.hst_config.histogram_predivider = 4;
> +	/* \todo Get this from control */
> +	std::vector<uint8_t> &modeWeights = meteringModes_.at(controls::MeteringMatrix);
> +	std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());
> +
> +	std::stringstream str;
> +	str << "Histogram weights : ";
> +	for (size_t i = 0; i < context.hw->numHistogramWeights; i++)
> +		str << (int)params->meas.hst_config.hist_weight[i] << " ";
> +	LOG(RkISP1Agc, Debug) << str.str();
> +
> +	/* \todo Add a control for this? */
> +	params->meas.hst_config.histogram_predivider =
> +		predivider(context.configuration.sensor.size);
>  
>  	/* Update the configuration for histogram. */
>  	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index f2f5b59d0..77d944237 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -44,11 +44,17 @@ public:
>  		     ControlList &metadata) override;
>  
>  private:
> +	int parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> +			       const char *prop);
> +	uint8_t predivider(Size &size);
> +
>  	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>  			  ControlList &metadata);
>  	double estimateLuminance(double gain) const override;
>  
>  	Span<const uint8_t> expMeans_;
> +
> +	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
>  };
>  
>  } /* namespace ipa::rkisp1::algorithms */
> -- 
> 2.39.2
> 


More information about the libcamera-devel mailing list