[libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the maximum sensor gain used

[Kieran Bingham]

Hi Naush,

Quoting Naushir Patuck (2022-01-24 10:31:06)
> Limit the gain code to the maximum value reported by the sensor controls when
> sending to DelayedControls. The AGC algorithm will handle a lower gain used by
> the sensor, provided provided it knows the actual gain used. This change ensures
> that DelayedControls will never report an unclipped gain used.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 0ed41385018a..a72d516f84ee 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -173,6 +173,9 @@ private:
>         /* Frame duration (1/fps) limits. */
>         Duration minFrameDuration_;
>         Duration maxFrameDuration_;
> +
> +       /* Maximum gain code for the sensor. */
> +       uint32_t maxSensorGainCode_;

Any risk of needing to clamp the minimum value too?


>  };
>  
>  int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)
> @@ -357,6 +360,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>                 return -1;
>         }
>  
> +       maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();
> +
>         /* Setup a metadata ControlList to output metadata. */
>         libcameraMetadata_ = ControlList(controls::controls);
>  
> @@ -1113,6 +1118,13 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>  {
>         int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);
>  
> +       /*
> +        * Ensure anything larger than the max gain code will not be passed to
> +        * DelayedControls. The AGC will correctly handle a lower gain returned
> +        * by the sensor, provided it knows the actual gain used.
> +        */
> +       gainCode = std::min<int32_t>(gainCode, maxSensorGainCode_);
> +

Sounds ok to me though.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>         /* GetVBlanking might clip exposure time to the fps limits. */
>         Duration exposure = agcStatus->shutter_time;
>         int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_);
> -- 
> 2.25.1
>