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

[Robert Mader]

Thanks for the patch!

Tested-by: Robert Mader <robert.mader at collabora.com>

Would love to see it land so 
https://patchwork.libcamera.org/patch/21865/ can be rebased and land on top

On 29.10.24 12:25, Stanislaw Gruszka wrote:
> 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.
>
> 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));
>   

-- 
Robert Mader
Consultant Software Developer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718