[PATCH 5/5] ipa: rkisp1: agc: Plumb mode-selection and frame duration controls

Dan Scally dan.scally at ideasonboard.com
Wed Apr 10 17:00:44 CEST 2024


Hi Paul

On 05/04/2024 15:47, Paul Elder wrote:
> Plumb controls for setting metering mode, exposure mode, constraint
> mode, and frame duration limits. Also report them as available controls,
> as well as in metadata.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>   src/ipa/rkisp1/algorithms/agc.cpp     | 67 ++++++++++++++++++++-------
>   src/ipa/rkisp1/algorithms/agc.h       |  5 ++
>   src/ipa/rkisp1/algorithms/algorithm.h |  2 +
>   src/ipa/rkisp1/ipa_context.h          |  4 ++
>   src/ipa/rkisp1/rkisp1.cpp             | 10 ++++
>   5 files changed, 72 insertions(+), 16 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index a1b6eb39..ed4d6330 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -11,6 +11,7 @@
>   #include <chrono>
>   #include <cmath>
>   #include <tuple>
> +#include <vector>
>   
>   #include <libcamera/base/log.h>
>   #include <libcamera/base/utils.h>
> @@ -49,6 +50,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
>   		return -EINVAL;
>   	}
>   
> +	std::vector<ControlValue> availableMeteringModes;
>   	for (const auto &[key, value] : yamlMeteringModes.asDict()) {
>   		if (controls::AeMeteringModeNameValueMap.find(key) ==
>   		    controls::AeMeteringModeNameValueMap.end()) {
> @@ -67,9 +69,14 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
>   			return -ENODATA;
>   		}
>   
> -		meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
> +		int32_t control = controls::AeMeteringModeNameValueMap.at(key);
> +		meteringModes_[control] = weights;
> +		availableMeteringModes.push_back(control);
>   	}
>   
> +	Algorithm::controls_[&controls::AeMeteringMode] =
> +		ControlInfo(availableMeteringModes);
> +
>   	return 0;
>   }
>   
> @@ -131,9 +138,25 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
>   
>   	context.ctrlMap.merge(controls());
>   
> +	defaultConstraintMode_ = constraintModes().begin()->first;
> +	defaultExposureMode_ = exposureModeHelpers().begin()->first;
> +	defaultMeteringMode_ = meteringModes_.begin()->first;
> +
> +	Algorithm::controls_.merge(ControlInfoMap::Map(controls()));
> +
>   	return 0;
>   }
>   
> +void Agc::configureExposureModeHelpers(IPAContext &context, utils::Duration maxShutterSpeed)
> +{
> +	for (auto &[id, helper] : exposureModeHelpers()) {
> +		helper->configure(context.configuration.sensor.minShutterSpeed,
> +				  maxShutterSpeed,
> +				  context.configuration.sensor.minAnalogueGain,
> +				  context.configuration.sensor.maxAnalogueGain);
> +	}
> +}
> +
>   /**
>    * \brief Configure the AGC given a configInfo
>    * \param[in] context The shared IPA context
> @@ -153,12 +176,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>   	context.activeState.agc.manual.dgain = 1;
>   	context.activeState.agc.autoEnabled = !context.configuration.raw;
>   
> -	/*
> -	* \todo We should use the first available mode rather than assume that
> -	* the "Normal" modes are present in tuning data.
> -	*/
> -	context.activeState.agc.constraintMode = controls::ConstraintNormal;
> -	context.activeState.agc.exposureMode = controls::ExposureNormal;
> +	context.activeState.agc.constraintMode = defaultConstraintMode_;
> +	context.activeState.agc.exposureMode = defaultExposureMode_;
>   
>   	/*
>   	 * Define the measurement window for AGC as a centered rectangle
> @@ -169,13 +188,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>   	context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;
>   	context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;
>   
> -	for (auto &[id, helper] : exposureModeHelpers()) {
> -		/* \todo Run this again when FrameDurationLimits is passed in */
> -		helper->configure(context.configuration.sensor.minShutterSpeed,
> -				  context.configuration.sensor.maxShutterSpeed,
> -				  context.configuration.sensor.minAnalogueGain,
> -				  context.configuration.sensor.maxAnalogueGain);
> -	}
> +	configureExposureModeHelpers(context, context.configuration.sensor.maxShutterSpeed);
>   
>   	return 0;
>   }
> @@ -223,6 +236,20 @@ void Agc::queueRequest(IPAContext &context,
>   		frameContext.agc.exposure = agc.manual.exposure;
>   		frameContext.agc.gain = agc.manual.gain;
>   	}
> +
> +	const auto &meteringMode = controls.get(controls::AeMeteringMode);
> +	frameContext.agc.meteringMode = meteringMode.value_or(defaultMeteringMode_);
> +
> +	const auto &exposureMode = controls.get(controls::AeExposureMode);
> +	frameContext.agc.exposureMode = exposureMode.value_or(defaultExposureMode_);
> +
> +	const auto &constraintMode = controls.get(controls::AeConstraintMode);
> +	frameContext.agc.constraintMode = constraintMode.value_or(defaultConstraintMode_);


Wouldn't this mean the modes revert to default unless we pass in those controls with every request? 
I have this same operation as:


frameContext.agc.exposureMode = exposureMode.value_or(frameContext.agc.exposureMode)


so that we keep the already configured value instead.

> +
> +	const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
> +	frameContext.agc.maxShutterSpeed = frameDurationLimits
> +					 ? std::chrono::milliseconds((*frameDurationLimits).back())
> +					 : 60ms;
>   }
>   
>   /**
> @@ -262,8 +289,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>   		params->meas.hst_config.hist_weight,
>   		context.hw->numHistogramWeights
>   	};
> -	/* \todo Get this from control */
> -	std::vector<uint8_t> &modeWeights = meteringModes_.at(controls::MeteringMatrix);
> +	std::vector<uint8_t> &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);
>   	std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());
>   
>   	std::stringstream str;
> @@ -290,6 +316,7 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>   				     * frameContext.sensor.exposure;
>   	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>   	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> +	metadata.set(controls::AeEnable, frameContext.agc.autoEnabled);
>   
>   	/* \todo Use VBlank value calculated from each frame exposure. */
>   	uint32_t vTotal = context.configuration.sensor.size.height
> @@ -297,6 +324,10 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>   	utils::Duration frameDuration = context.configuration.sensor.lineDuration
>   				      * vTotal;
>   	metadata.set(controls::FrameDuration, frameDuration.get<std::micro>());
> +
> +	metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
> +	metadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);
> +	metadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);
>   }
>   
>   void Agc::parseStatistics(const rkisp1_stat_buffer *stats,
> @@ -378,6 +409,10 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>   
>   	ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);
>   
> +	utils::Duration maxShutterSpeed = std::min(context.configuration.sensor.maxShutterSpeed,
> +						   frameContext.agc.maxShutterSpeed);
> +	configureExposureModeHelpers(context, maxShutterSpeed);
> +
>   	parseStatistics(stats, context);
>   
>   	/*
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index 43e3d5b2..c05ba4be 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -47,6 +47,7 @@ private:
>   	int parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
>   			       const char *prop);
>   	uint8_t predivider(Size &size);
> +	void configureExposureModeHelpers(IPAContext &context, utils::Duration maxShutterSpeed);
>   
>   	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>   			  ControlList &metadata);
> @@ -58,6 +59,10 @@ private:
>   	Span<const uint8_t> expMeans_;
>   
>   	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
> +
> +	int32_t defaultConstraintMode_;
> +	int32_t defaultExposureMode_;
> +	int32_t defaultMeteringMode_;
>   };
>   
>   } /* namespace ipa::rkisp1::algorithms */
> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
> index 9454c9a1..c3a002b8 100644
> --- a/src/ipa/rkisp1/algorithms/algorithm.h
> +++ b/src/ipa/rkisp1/algorithms/algorithm.h
> @@ -25,6 +25,8 @@ public:
>   
>   	bool disabled_;
>   	bool supportsRaw_;
> +
> +	ControlInfoMap::Map controls_;
>   };
>   
>   } /* namespace ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index a70c7eb3..dc876da0 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -114,6 +114,10 @@ struct IPAFrameContext : public FrameContext {
>   		double gain;
>   		double dgain;
>   		bool autoEnabled;
> +		int32_t meteringMode;
> +		int32_t exposureMode;
> +		int32_t constraintMode;
> +		utils::Duration maxShutterSpeed;
>   	} agc;
>   
>   	struct {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d66dfdd7..3654b5a6 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -80,6 +80,7 @@ private:
>   	std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
>   
>   	ControlInfoMap sensorControls_;
> +	ControlInfoMap::Map algoControls_;
>   
>   	/* Interface to the Camera Helper */
>   	std::unique_ptr<CameraSensorHelper> camHelper_;
> @@ -193,6 +194,14 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>   	if (ret)
>   		return ret;
>   
> +	for (auto const &a : algorithms()) {
> +		Algorithm *algo = static_cast<Algorithm *>(a.get());
> +
> +		/* \todo Avoid merging duplicate controls */
> +		if (!algo->controls_.empty())
> +			algoControls_.merge(ControlInfoMap::Map(algo->controls_));
> +	}
> +
>   	/* Initialize controls. */
>   	updateControls(sensorInfo, sensorControls, ipaControls);
>   
> @@ -377,6 +386,7 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
>   			       ControlInfoMap *ipaControls)
>   {
>   	ControlInfoMap::Map ctrlMap = rkisp1Controls;
> +	ctrlMap.merge(algoControls_);
>   
>   	/*
>   	 * Compute exposure time limits from the V4L2_CID_EXPOSURE control


More information about the libcamera-devel mailing list