[libcamera-devel] [PATCH v3 04/13] ipa: rkisp1: Add support for manual gain and exposure

Jacopo Mondi jacopo at jmondi.org
Wed Oct 26 16:26:25 CEST 2022


Hi Laurent

On Mon, Oct 24, 2022 at 03:03:47AM +0300, Laurent Pinchart via libcamera-devel wrote:
> From: Paul Elder <paul.elder at ideasonboard.com>
>
> Add support for manual gain and exposure in the rkisp1 IPA.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  include/libcamera/ipa/rkisp1.mojom       |  2 +-
>  src/ipa/rkisp1/algorithms/agc.cpp        | 62 +++++++++++++++++++++---
>  src/ipa/rkisp1/algorithms/agc.h          |  4 ++
>  src/ipa/rkisp1/ipa_context.h             | 13 ++++-
>  src/ipa/rkisp1/rkisp1.cpp                | 17 +++++++
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  2 +-
>  6 files changed, 89 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index eaf3955e4096..b526450d9949 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -10,7 +10,7 @@ import "include/libcamera/ipa/core.mojom";
>
>  interface IPARkISP1Interface {
>  	init(libcamera.IPASettings settings,
> -	     uint32 hwRevision)
> +	     uint32 hwRevision, libcamera.ControlInfoMap sensorControls)
>  		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
>  	start() => (int32 ret);
>  	stop();
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 169afe372803..377a3e8a0efd 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -74,9 +74,14 @@ Agc::Agc()
>  int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>  {
>  	/* Configure the default exposure and gain. */
> -	context.activeState.agc.gain = std::max(context.configuration.sensor.minAnalogueGain,
> -						kMinAnalogueGain);
> -	context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;
> +	context.activeState.agc.automatic.gain =
> +		std::max(context.configuration.sensor.minAnalogueGain,
> +			 kMinAnalogueGain);
> +	context.activeState.agc.automatic.exposure =
> +		10ms / context.configuration.sensor.lineDuration;
> +	context.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;
> +	context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;
> +	context.activeState.agc.autoEnabled = true;
>
>  	/*
>  	 * According to the RkISP1 documentation:
> @@ -108,14 +113,57 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>  	return 0;
>  }
>
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Agc::queueRequest(IPAContext &context,
> +		       [[maybe_unused]] const uint32_t frame,
> +		       IPAFrameContext &frameContext,
> +		       const ControlList &controls)
> +{
> +	auto &agc = context.activeState.agc;
> +
> +	const auto &agcEnable = controls.get(controls::AeEnable);
> +	if (agcEnable && *agcEnable != agc.autoEnabled) {
> +		agc.autoEnabled = *agcEnable;
> +
> +		LOG(RkISP1Agc, Debug)
> +			<< (*agcEnable ? "Enabling" : "Disabling") << " AGC";
> +	}
> +
> +	const auto &exposure = controls.get(controls::ExposureTime);
> +	if (exposure && !agc.autoEnabled) {
> +		agc.manual.exposure = *exposure;
> +
> +		LOG(RkISP1Agc, Debug)
> +			<< "Set exposure to: " << agc.manual.exposure;
> +	}
> +
> +	const auto &gain = controls.get(controls::AnalogueGain);
> +	if (gain && !agc.autoEnabled) {
> +		agc.manual.gain = *gain;
> +
> +		LOG(RkISP1Agc, Debug) << "Set gain to: " << agc.manual.gain;
> +	}
> +
> +	frameContext.agc.autoEnabled = agc.autoEnabled;
> +
> +	if (!frameContext.agc.autoEnabled) {
> +		frameContext.agc.exposure = agc.manual.exposure;
> +		frameContext.agc.gain = agc.manual.gain;
> +	}
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
>  void Agc::prepare(IPAContext &context, const uint32_t frame,
>  		  IPAFrameContext &frameContext, rkisp1_params_cfg *params)
>  {
> -	frameContext.agc.exposure = context.activeState.agc.exposure;
> -	frameContext.agc.gain = context.activeState.agc.gain;
> +	if (frameContext.agc.autoEnabled) {
> +		frameContext.agc.exposure = context.activeState.agc.automatic.exposure;
> +		frameContext.agc.gain = context.activeState.agc.automatic.gain;
> +	}
>
>  	if (frame > 0)
>  		return;
> @@ -263,8 +311,8 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
>  			      << stepGain;
>
>  	/* Update the estimated exposure and gain. */
> -	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> -	activeState.agc.gain = stepGain;
> +	activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
> +	activeState.agc.automatic.gain = stepGain;
>  }
>
>  /**
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index da4d2d4e8359..a228d0c37768 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -26,6 +26,10 @@ public:
>  	~Agc() = default;
>
>  	int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> +	void queueRequest(IPAContext &context,
> +			  const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
>  	void prepare(IPAContext &context, const uint32_t frame,
>  		     IPAFrameContext &frameContext,
>  		     rkisp1_params_cfg *params) override;
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 3e47ac663c58..b9b2065328d6 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -54,8 +54,16 @@ struct IPASessionConfiguration {
>
>  struct IPAActiveState {
>  	struct {
> -		uint32_t exposure;
> -		double gain;
> +		struct {
> +			uint32_t exposure;
> +			double gain;
> +		} manual;
> +		struct {
> +			uint32_t exposure;
> +			double gain;
> +		} automatic;
> +
> +		bool autoEnabled;
>  	} agc;
>
>  	struct {
> @@ -96,6 +104,7 @@ struct IPAFrameContext : public FrameContext {
>  	struct {
>  		uint32_t exposure;
>  		double gain;
> +		bool autoEnabled;
>  	} agc;
>
>  	struct {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 538e42cb6f7f..5c041ded0db4 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -50,6 +50,7 @@ public:
>  	IPARkISP1();
>
>  	int init(const IPASettings &settings, unsigned int hwRevision,
> +		 const ControlInfoMap &sensorControls,
>  		 ControlInfoMap *ipaControls) override;
>  	int start() override;
>  	void stop() override;
> @@ -116,6 +117,7 @@ std::string IPARkISP1::logPrefix() const
>  }
>
>  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> +		    const ControlInfoMap &sensorControls,
>  		    ControlInfoMap *ipaControls)
>  {
>  	/* \todo Add support for other revisions */
> @@ -184,6 +186,21 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>
>  	/* Return the controls handled by the IPA. */
>  	ControlInfoMap::Map ctrlMap = rkisp1Controls;
> +
> +	auto exposureTimeInfo = sensorControls.find(V4L2_CID_EXPOSURE);
> +	if (exposureTimeInfo != sensorControls.end()) {
> +		ctrlMap.emplace(&controls::ExposureTime,
> +				ControlInfo(exposureTimeInfo->second.min(),
> +					    exposureTimeInfo->second.max()));
> +	}
> +
> +	auto analogueGainInfo = sensorControls.find(V4L2_CID_ANALOGUE_GAIN);
> +	if (analogueGainInfo != sensorControls.end()) {
> +		ctrlMap.emplace(&controls::AnalogueGain,
> +				ControlInfo(static_cast<float>(analogueGainInfo->second.min().get<int32_t>()),
> +					    static_cast<float>(analogueGainInfo->second.max().get<int32_t>())));
> +	}
> +

At configure() time

	const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
	if (itExp == ctrls_.end()) {
		LOG(IPARkISP1, Error) << "Can't find exposure control";
		return -EINVAL;
	}

	const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
	if (itGain == ctrls_.end()) {
		LOG(IPARkISP1, Error) << "Can't find gain control";
		return -EINVAL;
	}

Could well fail earlier at init() time ?

>  	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
>
>  	return 0;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 455ee2a0a711..1418dc9a47fb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -348,7 +348,7 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  	}
>
>  	int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> -			     &controlInfo_);
> +			     sensor_->controls(), &controlInfo_);
>  	if (ret < 0) {
>  		LOG(RkISP1, Error) << "IPA initialization failure";
>  		return ret;
> --
> Regards,
>
> Laurent Pinchart
>


More information about the libcamera-devel mailing list