[1/5] ipa: rkisp1: agc: Read histogram weights from tuning file

19851 diff mbox series

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@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/agc.cpp | 80 +++++++++++++++++++++++++++++--
 src/ipa/rkisp1/algorithms/agc.h   |  6 +++
 2 files changed, 83 insertions(+), 3 deletions(-)

Hi Paul,

thanks for the patch.

On Fri, Apr 05, 2024 at 11:47:25PM +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@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 80 +++++++++++++++++++++++++++++--
>  src/ipa/rkisp1/algorithms/agc.h   |  6 +++
>  2 files changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 1cd977a0..fd47ba4e 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"
>  
>  /**
> @@ -42,6 +44,62 @@ static constexpr double kMinAnalogueGain = 1.0;
>  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
>  static constexpr utils::Duration kMaxShutterSpeed = 60ms;
>  
> +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;
> +		}
> +
> +		std::vector<uint8_t> weights =
> +			value.getList<uint8_t>().value_or(std::vector<uint8_t>{});
> +		if (weights.size() != context.hw->numHistogramWeights) {
> +			LOG(RkISP1Agc, Error)
> +				<< "Invalid '" << key << "' values: expected "
> +				<< context.hw->numHistogramWeights
> +				<< " elements, got " << weights.size();
> +			return -ENODATA;
> +		}
> +
> +		meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
> +	}

Kieran already mentioned that. What about adding the matrix metering
mode (+ warning) in case meteringModes_ is still empty.

> +
> +	return 0;
> +}
> +
> +uint8_t Agc::predivider(Size &size)
> +{
> +	static const std::vector<std::pair<unsigned int, unsigned int>>
> +		pixelCountThresholds = {
> +			{  640 *  480,  3 },
> +			{  800 *  600,  4 },
> +			{ 1024 *  768,  5 },
> +			{ 1280 *  960,  6 },
> +			{ 1600 * 1200,  8 },
> +			{ 2048 * 1536, 10 },
> +			{ 2600 * 1950, 12 },
> +			{ 4096 * 3072, 16 },
> +		};

Where do these values come from? What would happen if we keep the predivider
of 4?

> +
> +	unsigned long pixels = size.width * size.height;
> +	for (auto &pair : pixelCountThresholds)
> +		if (pixels < pair.first)
> +			return pair.second;
> +
> +	return 24;
> +}
> +
>  Agc::Agc()
>  {
>  	supportsRaw_ = true;
> @@ -72,6 +130,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;
> @@ -177,6 +239,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;
>  
> @@ -195,14 +258,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 311d4e94..43e3d5b2 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -44,6 +44,10 @@ 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);
>  	void parseStatistics(const rkisp1_stat_buffer *stats,
> @@ -52,6 +56,8 @@ private:
>  
>  	Histogram hist_;
>  	Span<const uint8_t> expMeans_;
> +
> +	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
>  };
>  
>  } /* namespace ipa::rkisp1::algorithms */
> -- 
> 2.39.2
>