[v1,16/35] ipa: rkisp1: agc: Process frame duration at the right time

24782 diff mbox series

The frame duration and vblank should not be calculated during process()
but within prepare(), where the data for that frame get's computed.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/agc.cpp | 20 ++++++++++----------
 src/ipa/rkisp1/algorithms/agc.h   |  3 +--
 2 files changed, 11 insertions(+), 12 deletions(-)

Quoting Stefan Klug (2025-10-24 17:50:40)
> The frame duration and vblank should not be calculated during process()
> but within prepare(), where the data for that frame get's computed.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 20 ++++++++++----------
>  src/ipa/rkisp1/algorithms/agc.h   |  3 +--
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index c67106339ef8..5ce77b7672f2 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -383,6 +383,12 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>  
>         frameContext.agc.yTarget = context.activeState.agc.automatic.yTarget;
>  
> +       /*
> +        * Expand the target frame duration so that we do not run faster than
> +        * the minimum frame duration when we have short exposures.
> +        */
> +       processFrameDuration(context, frameContext);
> +
>         if (frame > 0 && !frameContext.agc.updateMetering)
>                 return;
>  
> @@ -511,11 +517,13 @@ double Agc::estimateLuminance(double gain) const
>   * Compute and populate vblank from the target frame duration.
>   */
>  void Agc::processFrameDuration(IPAContext &context,
> -                              IPAFrameContext &frameContext,
> -                              utils::Duration frameDuration)
> +                              IPAFrameContext &frameContext)
>  {
>         IPACameraSensorInfo &sensorInfo = context.sensorInfo;
>         utils::Duration lineDuration = context.configuration.sensor.lineDuration;
> +       utils::Duration frameDuration = frameContext.agc.exposure * lineDuration;
> +
> +       frameDuration = std::max(frameDuration, frameContext.agc.minFrameDuration);

Looks good to me.

>  
>         frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;
>  
> @@ -539,8 +547,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>                   ControlList &metadata)
>  {
>         if (!stats) {
> -               processFrameDuration(context, frameContext,
> -                                    frameContext.agc.minFrameDuration);

This hasn't been replaced... oh it's in the next patch. I think it should be
squashed then so that we don't break in between patches. It's not a complex
patch.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

>                 fillMetadata(context, frameContext, metadata);
>                 return;
>         }
> @@ -641,12 +647,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         activeState.agc.automatic.gain = aGain;
>         activeState.agc.automatic.quantizationGain = qGain;
>         activeState.agc.automatic.yTarget = effectiveYTarget();
> -       /*
> -        * Expand the target frame duration so that we do not run faster than
> -        * the minimum frame duration when we have short exposures.
> -        */
> -       processFrameDuration(context, frameContext,
> -                            std::max(frameContext.agc.minFrameDuration, newExposureTime));
>  
>         fillMetadata(context, frameContext, metadata);
>         expMeans_ = {};
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index 7867eed9c4e3..4432711f43af 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -51,8 +51,7 @@ private:
>                           ControlList &metadata);
>         double estimateLuminance(double gain) const override;
>         void processFrameDuration(IPAContext &context,
> -                                 IPAFrameContext &frameContext,
> -                                 utils::Duration frameDuration);
> +                                 IPAFrameContext &frameContext);
>  
>         Span<const uint8_t> expMeans_;
>         Span<const uint8_t> weights_;
> -- 
> 2.48.1
>