[PATCH 1/2] libcamera: software_isp: Initialize exposure+gain before agc calculations

[Milan Zamazal]

Hi Stanislaw,

thank you for the fix.

Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com> writes:

> On my setup, since commit fb8ad13d ("libcamera: software_isp: Move exposure+gain
> to an algorithm module"), at start camera output stays very dark for dozen
> of seconds, and then later slowly gets to normal. This is because existing
> sensor exposure+gain settings are not used at start. We save initial
> values in frameContext but in the agc algorithm we use IPA context.

I think this was indeed just a mistake.

Reviewed-by: Milan Zamazal <mzamazal at redhat.com>

> Fix the problem by using in frameContext sensor values, since we already
> use those in blc algorithm and change exposure type to int32_t to
> unnecessary castings.
>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
> ---
>  src/ipa/simple/algorithms/agc.cpp | 8 ++++----
>  src/ipa/simple/algorithms/agc.h   | 2 +-
>  src/ipa/simple/algorithms/blc.h   | 2 +-
>  src/ipa/simple/ipa_context.h      | 2 +-
>  src/ipa/simple/soft_simple.cpp    | 4 ++--
>  5 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp
> index df92edd7..72aade14 100644
> --- a/src/ipa/simple/algorithms/agc.cpp
> +++ b/src/ipa/simple/algorithms/agc.cpp
> @@ -39,7 +39,7 @@ Agc::Agc()
>  {
>  }
>  
> -void Agc::updateExposure(IPAContext &context, double exposureMSV)
> +void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)
>  {
>  	/*
>  	 * kExpDenominator of 10 gives ~10% increment/decrement;
> @@ -50,8 +50,8 @@ void Agc::updateExposure(IPAContext &context, double exposureMSV)
>  	static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;
>  
>  	double next;
> -	int32_t &exposure = context.activeState.agc.exposure;
> -	double &again = context.activeState.agc.again;
> +	int32_t &exposure = frameContext.sensor.exposure;
> +	double &again = frameContext.sensor.gain;
>  
>  	if (exposureMSV < kExposureOptimal - kExposureSatisfactory) {
>  		next = exposure * kExpNumeratorUp / kExpDenominator;
> @@ -129,7 +129,7 @@ void Agc::process(IPAContext &context,
>  	}
>  
>  	float exposureMSV = (denom == 0 ? 0 : static_cast<float>(num) / denom);
> -	updateExposure(context, exposureMSV);
> +	updateExposure(context, frameContext, exposureMSV);
>  }
>  
>  REGISTER_IPA_ALGORITHM(Agc, "Agc")
> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h
> index ad5fca9f..112d9f5a 100644
> --- a/src/ipa/simple/algorithms/agc.h
> +++ b/src/ipa/simple/algorithms/agc.h
> @@ -25,7 +25,7 @@ public:
>  		     ControlList &metadata) override;
>  
>  private:
> -	void updateExposure(IPAContext &context, double exposureMSV);
> +	void updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);
>  };
>  
>  } /* namespace ipa::soft::algorithms */
> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h
> index 2cf2a877..67c688ae 100644
> --- a/src/ipa/simple/algorithms/blc.h
> +++ b/src/ipa/simple/algorithms/blc.h
> @@ -27,7 +27,7 @@ public:
>  		     ControlList &metadata) override;
>  
>  private:
> -	uint32_t exposure_;
> +	int32_t exposure_;
>  	double gain_;
>  };
>  
> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> index fd121eeb..c510c436 100644
> --- a/src/ipa/simple/ipa_context.h
> +++ b/src/ipa/simple/ipa_context.h
> @@ -53,7 +53,7 @@ struct IPAActiveState {
>  
>  struct IPAFrameContext : public FrameContext {
>  	struct {
> -		uint32_t exposure;
> +		int32_t exposure;
>  		double gain;
>  	} sensor;
>  };
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index c8ad55a2..dfacd6aa 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -310,8 +310,8 @@ void IPASoftSimple::processStats(const uint32_t frame,
>  
>  	ControlList ctrls(sensorInfoMap_);
>  
> -	auto &againNew = context_.activeState.agc.again;
> -	ctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);
> +	auto &againNew = frameContext.sensor.gain;
> +	ctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN,
>  		  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(againNew) : againNew));