[RFC,v3,29/50] ipa: libipa: agc: Calculate vblank and frame duration sooner
diff mbox series

Message ID 20260803131435.153927-30-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: libipa: agc rework
Related show

Commit Message

Barnabás Pőcze Aug. 3, 2026, 1:14 p.m. UTC 
Calculating the vblank and frame duration is problematic in `process()`
because at the moment it is calculated for an already finished frame
based on the new suggested exposure time.

Instead, move the calculation to `prepare()` where the frame's
exposure and gain are finalized.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/libipa/agc.cpp | 29 ++++++++++++++---------------
 src/ipa/libipa/agc.h   |  3 ++-
 2 files changed, 16 insertions(+), 16 deletions(-)

Comments

Jacopo Mondi Aug. 5, 2026, 3:37 p.m. UTC | #1 
Hi Barnabás

On Mon, Aug 03, 2026 at 03:14:14PM +0200, Barnabás Pőcze wrote:
> Calculating the vblank and frame duration is problematic in `process()`
> because at the moment it is calculated for an already finished frame
> based on the new suggested exposure time.
>
> Instead, move the calculation to `prepare()` where the frame's
> exposure and gain are finalized.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/libipa/agc.cpp | 29 ++++++++++++++---------------
>  src/ipa/libipa/agc.h   |  3 ++-
>  2 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index f420dfdce4..69f731569e 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -502,7 +502,7 @@ void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
>  /**
>   * \brief Handle a \a prepare operation
>   */
> -void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameContext)
> +void AgcAlgorithm::prepare(const agc::Session &session, agc::ActiveState &state, agc::FrameContext &frameContext)
>  {
>  	uint32_t activeAutoExposure = state.automatic.exposure;
>  	double activeAutoGain = state.automatic.gain;
> @@ -533,6 +533,19 @@ void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameCont
>  	}
>
>  	frameContext.yTarget = state.automatic.yTarget;
> +
> +	/*
> +	 * Expand the target frame duration so that we do not run faster than
> +	 * the minimum frame duration when we have short exposures.
> +	 */
> +	const auto frameDuration = std::max<uint32_t>(
> +		frameContext.minFrameDuration / session.lineDuration,
> +		frameContext.exposure);


        prepare(X) {
		frameContext.exposure = state.automatic.exposure;
        }

        process(X) {
		state.automatic.exposure = newEv.exposureTime / lineDuration;

        }

Aren't we using the exposure from the previous frame then ?

> +	frameContext.vblank = frameDuration - session.sensor.outputSize.height;
> +
> +	/* Update frame duration accounting for line length quantization. */
> +	frameContext.frameDuration =
> +		(session.sensor.outputSize.height + frameContext.vblank) * session.lineDuration;
>  }
>
>  /**
> @@ -543,7 +556,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>  			   ControlList &metadata)
>  {
>  	const utils::Duration &lineDuration = session.lineDuration;
> -	utils::Duration newExposureTime = {};
>
>  	if (params) {
>  		ASSERT(session.autoAllowed);
> @@ -603,8 +615,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>  		state.automatic.digitalGain = newEv.digitalGain;
>  		state.automatic.yTarget = newEv.yTarget;
>
> -		newExposureTime = newEv.exposureTime;
> -
>  		LOG(Agc, Debug)
>  			<< "exposure-time:" << utils::Duration(state.automatic.exposure * lineDuration)
>  			<< " analogue-gain:" << state.automatic.gain
> @@ -612,17 +622,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>  			<< " digital-gain:" << state.automatic.digitalGain;
>  	}
>
> -	/*
> -	 * Expand the target frame duration so that we do not run faster than
> -	 * the minimum frame duration when we have short exposures.
> -	 */
> -	const auto frameDuration = std::max(frameContext.minFrameDuration, newExposureTime);
> -	frameContext.vblank = (frameDuration / lineDuration) - session.sensor.outputSize.height;
> -
> -	/* Update frame duration accounting for line length quantization. */
> -	frameContext.frameDuration =
> -		(session.sensor.outputSize.height + frameContext.vblank) * lineDuration;
> -
>  	metadata.set(controls::AnalogueGain, frameContext.gain);
>  	metadata.set(controls::ExposureTime,
>  		     utils::Duration(lineDuration * frameContext.exposure).get<std::micro>());
> diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
> index b0f811e93d..e96c6926b2 100644
> --- a/src/ipa/libipa/agc.h
> +++ b/src/ipa/libipa/agc.h
> @@ -126,7 +126,8 @@ public:
>  	void queueRequest(const agc::Session &session, agc::ActiveState &state,
>  			  agc::FrameContext &frameContext, const ControlList &controls);
>
> -	void prepare(agc::ActiveState &state, agc::FrameContext &frameContext);
> +	void prepare(const agc::Session &session, agc::ActiveState &state,
> +		     agc::FrameContext &frameContext);
>
>  	struct ProcessParams {
>  		const AgcMeanLuminance::Traits &traits;
> --
> 2.55.0
>
Barnabás Pőcze Aug. 5, 2026, 3:44 p.m. UTC | #2 
2026. 08. 05. 17:37 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Mon, Aug 03, 2026 at 03:14:14PM +0200, Barnabás Pőcze wrote:
>> Calculating the vblank and frame duration is problematic in `process()`
>> because at the moment it is calculated for an already finished frame
>> based on the new suggested exposure time.
>>
>> Instead, move the calculation to `prepare()` where the frame's
>> exposure and gain are finalized.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   src/ipa/libipa/agc.cpp | 29 ++++++++++++++---------------
>>   src/ipa/libipa/agc.h   |  3 ++-
>>   2 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
>> index f420dfdce4..69f731569e 100644
>> --- a/src/ipa/libipa/agc.cpp
>> +++ b/src/ipa/libipa/agc.cpp
>> @@ -502,7 +502,7 @@ void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
>>   /**
>>    * \brief Handle a \a prepare operation
>>    */
>> -void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameContext)
>> +void AgcAlgorithm::prepare(const agc::Session &session, agc::ActiveState &state, agc::FrameContext &frameContext)
>>   {
>>   	uint32_t activeAutoExposure = state.automatic.exposure;
>>   	double activeAutoGain = state.automatic.gain;
>> @@ -533,6 +533,19 @@ void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameCont
>>   	}
>>
>>   	frameContext.yTarget = state.automatic.yTarget;
>> +
>> +	/*
>> +	 * Expand the target frame duration so that we do not run faster than
>> +	 * the minimum frame duration when we have short exposures.
>> +	 */
>> +	const auto frameDuration = std::max<uint32_t>(
>> +		frameContext.minFrameDuration / session.lineDuration,
>> +		frameContext.exposure);
> 
> 
>          prepare(X) {
> 		frameContext.exposure = state.automatic.exposure;
>          }
> 
>          process(X) {
> 		state.automatic.exposure = newEv.exposureTime / lineDuration;
> 
>          }
> 
> Aren't we using the exposure from the previous frame then ?

I'm pretty sure not. The exposure and gain for a given frame are "finalized" in `prepare()`.
`process()` just updates the most up-to-date suggestion after the given frame has finished,
and this suggestion will be used in `prepare()` for the later frames.


> 
>> +	frameContext.vblank = frameDuration - session.sensor.outputSize.height;
>> +
>> +	/* Update frame duration accounting for line length quantization. */
>> +	frameContext.frameDuration =
>> +		(session.sensor.outputSize.height + frameContext.vblank) * session.lineDuration;
>>   }
>>
>>   /**
>> @@ -543,7 +556,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>>   			   ControlList &metadata)
>>   {
>>   	const utils::Duration &lineDuration = session.lineDuration;
>> -	utils::Duration newExposureTime = {};
>>
>>   	if (params) {
>>   		ASSERT(session.autoAllowed);
>> @@ -603,8 +615,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>>   		state.automatic.digitalGain = newEv.digitalGain;
>>   		state.automatic.yTarget = newEv.yTarget;
>>
>> -		newExposureTime = newEv.exposureTime;
>> -
>>   		LOG(Agc, Debug)
>>   			<< "exposure-time:" << utils::Duration(state.automatic.exposure * lineDuration)
>>   			<< " analogue-gain:" << state.automatic.gain
>> @@ -612,17 +622,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>>   			<< " digital-gain:" << state.automatic.digitalGain;
>>   	}
>>
>> -	/*
>> -	 * Expand the target frame duration so that we do not run faster than
>> -	 * the minimum frame duration when we have short exposures.
>> -	 */
>> -	const auto frameDuration = std::max(frameContext.minFrameDuration, newExposureTime);
>> -	frameContext.vblank = (frameDuration / lineDuration) - session.sensor.outputSize.height;
>> -
>> -	/* Update frame duration accounting for line length quantization. */
>> -	frameContext.frameDuration =
>> -		(session.sensor.outputSize.height + frameContext.vblank) * lineDuration;
>> -
>>   	metadata.set(controls::AnalogueGain, frameContext.gain);
>>   	metadata.set(controls::ExposureTime,
>>   		     utils::Duration(lineDuration * frameContext.exposure).get<std::micro>());
>> diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
>> index b0f811e93d..e96c6926b2 100644
>> --- a/src/ipa/libipa/agc.h
>> +++ b/src/ipa/libipa/agc.h
>> @@ -126,7 +126,8 @@ public:
>>   	void queueRequest(const agc::Session &session, agc::ActiveState &state,
>>   			  agc::FrameContext &frameContext, const ControlList &controls);
>>
>> -	void prepare(agc::ActiveState &state, agc::FrameContext &frameContext);
>> +	void prepare(const agc::Session &session, agc::ActiveState &state,
>> +		     agc::FrameContext &frameContext);
>>
>>   	struct ProcessParams {
>>   		const AgcMeanLuminance::Traits &traits;
>> --
>> 2.55.0
>>
Jacopo Mondi Aug. 5, 2026, 4:04 p.m. UTC | #3 
Hi Barnabás

On Wed, Aug 05, 2026 at 05:44:11PM +0200, Barnabás Pőcze wrote:
> 2026. 08. 05. 17:37 keltezéssel, Jacopo Mondi írta:
> > Hi Barnabás
> >
> > On Mon, Aug 03, 2026 at 03:14:14PM +0200, Barnabás Pőcze wrote:
> > > Calculating the vblank and frame duration is problematic in `process()`
> > > because at the moment it is calculated for an already finished frame
> > > based on the new suggested exposure time.
> > >
> > > Instead, move the calculation to `prepare()` where the frame's
> > > exposure and gain are finalized.
> > >
> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > > ---
> > >   src/ipa/libipa/agc.cpp | 29 ++++++++++++++---------------
> > >   src/ipa/libipa/agc.h   |  3 ++-
> > >   2 files changed, 16 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> > > index f420dfdce4..69f731569e 100644
> > > --- a/src/ipa/libipa/agc.cpp
> > > +++ b/src/ipa/libipa/agc.cpp
> > > @@ -502,7 +502,7 @@ void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
> > >   /**
> > >    * \brief Handle a \a prepare operation
> > >    */
> > > -void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameContext)
> > > +void AgcAlgorithm::prepare(const agc::Session &session, agc::ActiveState &state, agc::FrameContext &frameContext)
> > >   {
> > >   	uint32_t activeAutoExposure = state.automatic.exposure;
> > >   	double activeAutoGain = state.automatic.gain;
> > > @@ -533,6 +533,19 @@ void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameCont
> > >   	}
> > >
> > >   	frameContext.yTarget = state.automatic.yTarget;
> > > +
> > > +	/*
> > > +	 * Expand the target frame duration so that we do not run faster than
> > > +	 * the minimum frame duration when we have short exposures.
> > > +	 */
> > > +	const auto frameDuration = std::max<uint32_t>(
> > > +		frameContext.minFrameDuration / session.lineDuration,
> > > +		frameContext.exposure);
> >
> >
> >          prepare(X) {
> > 		frameContext.exposure = state.automatic.exposure;
> >          }
> >
> >          process(X) {
> > 		state.automatic.exposure = newEv.exposureTime / lineDuration;
> >
> >          }
> >
> > Aren't we using the exposure from the previous frame then ?
>
> I'm pretty sure not. The exposure and gain for a given frame are "finalized" in `prepare()`.
> `process()` just updates the most up-to-date suggestion after the given frame has finished,
> and this suggestion will be used in `prepare()` for the later frames.
>

There is still something a little weird for me here

The IPA does

        computeParams(X) {
                for (const auto &algo : algorithms())
                        algo->prepare(context_, frame, frameContext, &params);
        }


        processStats(X) {
                for (const auto &a : algorithms()) {
                        Algorithm *algo = static_cast<Algorithm *>(a.get());
                        if (algo->disabled_)
                                continue;
                        algo->process(context_, frame, frameContext, stats, metadata);
                }

                setControls(frame);
     }

algo->prepare sets the exposure time and gain in the frame context
propagating there the ones from the active state that was set at the
(X-1) process(), right ?

indeed, if you move the vblank calculation to prepare() at least frame
duration, exposure and gain will all come from the same state, but
isn't it the previous one ?

Possibly just me not fully understanding this, and possibily not an
issue with this patch ?

Thanks
   j

>
> >
> > > +	frameContext.vblank = frameDuration - session.sensor.outputSize.height;
> > > +
> > > +	/* Update frame duration accounting for line length quantization. */
> > > +	frameContext.frameDuration =
> > > +		(session.sensor.outputSize.height + frameContext.vblank) * session.lineDuration;
> > >   }
> > >
> > >   /**
> > > @@ -543,7 +556,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
> > >   			   ControlList &metadata)
> > >   {
> > >   	const utils::Duration &lineDuration = session.lineDuration;
> > > -	utils::Duration newExposureTime = {};
> > >
> > >   	if (params) {
> > >   		ASSERT(session.autoAllowed);
> > > @@ -603,8 +615,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
> > >   		state.automatic.digitalGain = newEv.digitalGain;
> > >   		state.automatic.yTarget = newEv.yTarget;
> > >
> > > -		newExposureTime = newEv.exposureTime;
> > > -
> > >   		LOG(Agc, Debug)
> > >   			<< "exposure-time:" << utils::Duration(state.automatic.exposure * lineDuration)
> > >   			<< " analogue-gain:" << state.automatic.gain
> > > @@ -612,17 +622,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
> > >   			<< " digital-gain:" << state.automatic.digitalGain;
> > >   	}
> > >
> > > -	/*
> > > -	 * Expand the target frame duration so that we do not run faster than
> > > -	 * the minimum frame duration when we have short exposures.
> > > -	 */
> > > -	const auto frameDuration = std::max(frameContext.minFrameDuration, newExposureTime);
> > > -	frameContext.vblank = (frameDuration / lineDuration) - session.sensor.outputSize.height;
> > > -
> > > -	/* Update frame duration accounting for line length quantization. */
> > > -	frameContext.frameDuration =
> > > -		(session.sensor.outputSize.height + frameContext.vblank) * lineDuration;
> > > -
> > >   	metadata.set(controls::AnalogueGain, frameContext.gain);
> > >   	metadata.set(controls::ExposureTime,
> > >   		     utils::Duration(lineDuration * frameContext.exposure).get<std::micro>());
> > > diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
> > > index b0f811e93d..e96c6926b2 100644
> > > --- a/src/ipa/libipa/agc.h
> > > +++ b/src/ipa/libipa/agc.h
> > > @@ -126,7 +126,8 @@ public:
> > >   	void queueRequest(const agc::Session &session, agc::ActiveState &state,
> > >   			  agc::FrameContext &frameContext, const ControlList &controls);
> > >
> > > -	void prepare(agc::ActiveState &state, agc::FrameContext &frameContext);
> > > +	void prepare(const agc::Session &session, agc::ActiveState &state,
> > > +		     agc::FrameContext &frameContext);
> > >
> > >   	struct ProcessParams {
> > >   		const AgcMeanLuminance::Traits &traits;
> > > --
> > > 2.55.0
> > >
>
Barnabás Pőcze Aug. 6, 2026, 10:17 a.m. UTC | #4 
2026. 08. 05. 18:04 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Wed, Aug 05, 2026 at 05:44:11PM +0200, Barnabás Pőcze wrote:
>> 2026. 08. 05. 17:37 keltezéssel, Jacopo Mondi írta:
>>> Hi Barnabás
>>>
>>> On Mon, Aug 03, 2026 at 03:14:14PM +0200, Barnabás Pőcze wrote:
>>>> Calculating the vblank and frame duration is problematic in `process()`
>>>> because at the moment it is calculated for an already finished frame
>>>> based on the new suggested exposure time.
>>>>
>>>> Instead, move the calculation to `prepare()` where the frame's
>>>> exposure and gain are finalized.
>>>>
>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>> ---
>>>>    src/ipa/libipa/agc.cpp | 29 ++++++++++++++---------------
>>>>    src/ipa/libipa/agc.h   |  3 ++-
>>>>    2 files changed, 16 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
>>>> index f420dfdce4..69f731569e 100644
>>>> --- a/src/ipa/libipa/agc.cpp
>>>> +++ b/src/ipa/libipa/agc.cpp
>>>> @@ -502,7 +502,7 @@ void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
>>>>    /**
>>>>     * \brief Handle a \a prepare operation
>>>>     */
>>>> -void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameContext)
>>>> +void AgcAlgorithm::prepare(const agc::Session &session, agc::ActiveState &state, agc::FrameContext &frameContext)
>>>>    {
>>>>    	uint32_t activeAutoExposure = state.automatic.exposure;
>>>>    	double activeAutoGain = state.automatic.gain;
>>>> @@ -533,6 +533,19 @@ void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameCont
>>>>    	}
>>>>
>>>>    	frameContext.yTarget = state.automatic.yTarget;
>>>> +
>>>> +	/*
>>>> +	 * Expand the target frame duration so that we do not run faster than
>>>> +	 * the minimum frame duration when we have short exposures.
>>>> +	 */
>>>> +	const auto frameDuration = std::max<uint32_t>(
>>>> +		frameContext.minFrameDuration / session.lineDuration,
>>>> +		frameContext.exposure);
>>>
>>>
>>>           prepare(X) {
>>> 		frameContext.exposure = state.automatic.exposure;
>>>           }
>>>
>>>           process(X) {
>>> 		state.automatic.exposure = newEv.exposureTime / lineDuration;
>>>
>>>           }
>>>
>>> Aren't we using the exposure from the previous frame then ?
>>
>> I'm pretty sure not. The exposure and gain for a given frame are "finalized" in `prepare()`.
>> `process()` just updates the most up-to-date suggestion after the given frame has finished,
>> and this suggestion will be used in `prepare()` for the later frames.
>>
> 
> There is still something a little weird for me here
> 
> The IPA does
> 
>          computeParams(X) {
>                  for (const auto &algo : algorithms())
>                          algo->prepare(context_, frame, frameContext, &params);
>          }
> 
> 
>          processStats(X) {
>                  for (const auto &a : algorithms()) {
>                          Algorithm *algo = static_cast<Algorithm *>(a.get());
>                          if (algo->disabled_)
>                                  continue;
>                          algo->process(context_, frame, frameContext, stats, metadata);
>                  }
> 
>                  setControls(frame);
>       }
> 
> algo->prepare sets the exposure time and gain in the frame context
> propagating there the ones from the active state that was set at the
> (X-1) process(), right ?

If frame X-1 has already fully completed, then yes. But it may be some
other previously completed frame, specifically, it will be based on the
latest frame for which statistics have been processed.


> 
> indeed, if you move the vblank calculation to prepare() at least frame
> duration, exposure and gain will all come from the same state, but
> isn't it the previous one ?

I suppose you could say that, but for sensor parameters specifically,
there is not much else one can do. One cannot use the statistics from
frame X to configure the sensor parameters for frame X, so it has to
be some function of all the results from previous frames. Or maybe
I misunderstand what you mean?


> 
> Possibly just me not fully understanding this, and possibily not an
> issue with this patch ?
> 
> Thanks
>     j
> 
>>
>>>
>>>> +	frameContext.vblank = frameDuration - session.sensor.outputSize.height;
>>>> +
>>>> +	/* Update frame duration accounting for line length quantization. */
>>>> +	frameContext.frameDuration =
>>>> +		(session.sensor.outputSize.height + frameContext.vblank) * session.lineDuration;
>>>>    }
>>>>
>>>>    /**
>>>> @@ -543,7 +556,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>>>>    			   ControlList &metadata)
>>>>    {
>>>>    	const utils::Duration &lineDuration = session.lineDuration;
>>>> -	utils::Duration newExposureTime = {};
>>>>
>>>>    	if (params) {
>>>>    		ASSERT(session.autoAllowed);
>>>> @@ -603,8 +615,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>>>>    		state.automatic.digitalGain = newEv.digitalGain;
>>>>    		state.automatic.yTarget = newEv.yTarget;
>>>>
>>>> -		newExposureTime = newEv.exposureTime;
>>>> -
>>>>    		LOG(Agc, Debug)
>>>>    			<< "exposure-time:" << utils::Duration(state.automatic.exposure * lineDuration)
>>>>    			<< " analogue-gain:" << state.automatic.gain
>>>> @@ -612,17 +622,6 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>>>>    			<< " digital-gain:" << state.automatic.digitalGain;
>>>>    	}
>>>>
>>>> -	/*
>>>> -	 * Expand the target frame duration so that we do not run faster than
>>>> -	 * the minimum frame duration when we have short exposures.
>>>> -	 */
>>>> -	const auto frameDuration = std::max(frameContext.minFrameDuration, newExposureTime);
>>>> -	frameContext.vblank = (frameDuration / lineDuration) - session.sensor.outputSize.height;
>>>> -
>>>> -	/* Update frame duration accounting for line length quantization. */
>>>> -	frameContext.frameDuration =
>>>> -		(session.sensor.outputSize.height + frameContext.vblank) * lineDuration;
>>>> -
>>>>    	metadata.set(controls::AnalogueGain, frameContext.gain);
>>>>    	metadata.set(controls::ExposureTime,
>>>>    		     utils::Duration(lineDuration * frameContext.exposure).get<std::micro>());
>>>> diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
>>>> index b0f811e93d..e96c6926b2 100644
>>>> --- a/src/ipa/libipa/agc.h
>>>> +++ b/src/ipa/libipa/agc.h
>>>> @@ -126,7 +126,8 @@ public:
>>>>    	void queueRequest(const agc::Session &session, agc::ActiveState &state,
>>>>    			  agc::FrameContext &frameContext, const ControlList &controls);
>>>>
>>>> -	void prepare(agc::ActiveState &state, agc::FrameContext &frameContext);
>>>> +	void prepare(const agc::Session &session, agc::ActiveState &state,
>>>> +		     agc::FrameContext &frameContext);
>>>>
>>>>    	struct ProcessParams {
>>>>    		const AgcMeanLuminance::Traits &traits;
>>>> --
>>>> 2.55.0
>>>>
>>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
index f420dfdce4..69f731569e 100644
--- a/src/ipa/libipa/agc.cpp
+++ b/src/ipa/libipa/agc.cpp
@@ -502,7 +502,7 @@  void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
 /**
  * \brief Handle a \a prepare operation
  */
-void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameContext)
+void AgcAlgorithm::prepare(const agc::Session &session, agc::ActiveState &state, agc::FrameContext &frameContext)
 {
 	uint32_t activeAutoExposure = state.automatic.exposure;
 	double activeAutoGain = state.automatic.gain;
@@ -533,6 +533,19 @@  void AgcAlgorithm::prepare(agc::ActiveState &state, agc::FrameContext &frameCont
 	}
 
 	frameContext.yTarget = state.automatic.yTarget;
+
+	/*
+	 * Expand the target frame duration so that we do not run faster than
+	 * the minimum frame duration when we have short exposures.
+	 */
+	const auto frameDuration = std::max<uint32_t>(
+		frameContext.minFrameDuration / session.lineDuration,
+		frameContext.exposure);
+	frameContext.vblank = frameDuration - session.sensor.outputSize.height;
+
+	/* Update frame duration accounting for line length quantization. */
+	frameContext.frameDuration =
+		(session.sensor.outputSize.height + frameContext.vblank) * session.lineDuration;
 }
 
 /**
@@ -543,7 +556,6 @@  void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
 			   ControlList &metadata)
 {
 	const utils::Duration &lineDuration = session.lineDuration;
-	utils::Duration newExposureTime = {};
 
 	if (params) {
 		ASSERT(session.autoAllowed);
@@ -603,8 +615,6 @@  void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
 		state.automatic.digitalGain = newEv.digitalGain;
 		state.automatic.yTarget = newEv.yTarget;
 
-		newExposureTime = newEv.exposureTime;
-
 		LOG(Agc, Debug)
 			<< "exposure-time:" << utils::Duration(state.automatic.exposure * lineDuration)
 			<< " analogue-gain:" << state.automatic.gain
@@ -612,17 +622,6 @@  void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
 			<< " digital-gain:" << state.automatic.digitalGain;
 	}
 
-	/*
-	 * Expand the target frame duration so that we do not run faster than
-	 * the minimum frame duration when we have short exposures.
-	 */
-	const auto frameDuration = std::max(frameContext.minFrameDuration, newExposureTime);
-	frameContext.vblank = (frameDuration / lineDuration) - session.sensor.outputSize.height;
-
-	/* Update frame duration accounting for line length quantization. */
-	frameContext.frameDuration =
-		(session.sensor.outputSize.height + frameContext.vblank) * lineDuration;
-
 	metadata.set(controls::AnalogueGain, frameContext.gain);
 	metadata.set(controls::ExposureTime,
 		     utils::Duration(lineDuration * frameContext.exposure).get<std::micro>());
diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
index b0f811e93d..e96c6926b2 100644
--- a/src/ipa/libipa/agc.h
+++ b/src/ipa/libipa/agc.h
@@ -126,7 +126,8 @@  public:
 	void queueRequest(const agc::Session &session, agc::ActiveState &state,
 			  agc::FrameContext &frameContext, const ControlList &controls);
 
-	void prepare(agc::ActiveState &state, agc::FrameContext &frameContext);
+	void prepare(const agc::Session &session, agc::ActiveState &state,
+		     agc::FrameContext &frameContext);
 
 	struct ProcessParams {
 		const AgcMeanLuminance::Traits &traits;