[libcamera-devel] [PATCH v2 2/4] ipa: raspberrypi: Use the new sensor limits fields in CameraMode

[David Plowman]

Hi Naush

Thanks for the patch.

On Mon, 27 Mar 2023 at 10:34, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Use the new analogue gain and shutter speed limit fields in the ipa
> code when reporting back the control value limits and calculating the
> analogue gain code to use. This also replaces the now unused (and
> removed) maxSensorGainCode_ field.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

LGTM.

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 37 +++++++++--------------------
>  1 file changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 3f1afb846420..c10e57e07ab0 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -224,9 +224,6 @@ private:
>         Duration minFrameDuration_;
>         Duration maxFrameDuration_;
>
> -       /* Maximum gain code for the sensor. */
> -       uint32_t maxSensorGainCode_;
> -
>         /* Track the frame length times over FrameLengthsQueueSize frames. */
>         std::deque<Duration> frameLengths_;
>         Duration lastTimeout_;
> @@ -455,8 +452,6 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip
>                 }
>         }
>
> -       maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();
> -
>         /* Setup a metadata ControlList to output metadata. */
>         libcameraMetadata_ = ControlList(controls::controls);
>
> @@ -517,26 +512,17 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip
>          * based on the current sensor mode.
>          */
>         ControlInfoMap::Map ctrlMap = ipaControls;
> -       const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> -       const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;
>         ctrlMap[&controls::FrameDurationLimits] =
> -               ControlInfo(static_cast<int64_t>(minSensorFrameDuration.get<std::micro>()),
> -                           static_cast<int64_t>(maxSensorFrameDuration.get<std::micro>()));
> +               ControlInfo(static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()),
> +                           static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()));
>
>         ctrlMap[&controls::AnalogueGain] =
> -               ControlInfo(1.0f, static_cast<float>(helper_->gain(maxSensorGainCode_)));
> -
> -       /*
> -        * Calculate the max exposure limit from the frame duration limit as V4L2
> -        * will limit the maximum control value based on the current VBLANK value.
> -        */
> -       Duration maxShutter = Duration::max();
> -       helper_->getBlanking(maxShutter, minSensorFrameDuration, maxSensorFrameDuration);
> -       const uint32_t exposureMin = sensorCtrls_.at(V4L2_CID_EXPOSURE).min().get<int32_t>();
> +               ControlInfo(static_cast<float>(mode_.minAnalogueGain),
> +                           static_cast<float>(mode_.maxAnalogueGain));
>
>         ctrlMap[&controls::ExposureTime] =
> -               ControlInfo(static_cast<int32_t>(helper_->exposure(exposureMin, mode_.minLineLength).get<std::micro>()),
> -                           static_cast<int32_t>(maxShutter.get<std::micro>()));
> +               ControlInfo(static_cast<int32_t>(mode_.minShutter.get<std::micro>()),
> +                           static_cast<int32_t>(mode_.maxShutter.get<std::micro>()));
>
>         /* Declare Autofocus controls, only if we have a controllable lens */
>         if (lensPresent_)
> @@ -1496,9 +1482,6 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
>
>  void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration)
>  {
> -       const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> -       const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;
> -
>         /*
>          * This will only be applied once AGC recalculations occur.
>          * The values may be clamped based on the sensor mode capabilities as well.
> @@ -1506,9 +1489,9 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>         minFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration;
>         maxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration;
>         minFrameDuration_ = std::clamp(minFrameDuration_,
> -                                      minSensorFrameDuration, maxSensorFrameDuration);
> +                                      mode_.minFrameDuration, mode_.maxFrameDuration);
>         maxFrameDuration_ = std::clamp(maxFrameDuration_,
> -                                      minSensorFrameDuration, maxSensorFrameDuration);
> +                                      mode_.minFrameDuration, mode_.maxFrameDuration);
>         maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
>
>         /* Return the validated limits via metadata. */
> @@ -1531,6 +1514,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>
>  void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>  {
> +       const int32_t minGainCode = helper_->gainCode(mode_.minAnalogueGain);
> +       const int32_t maxGainCode = helper_->gainCode(mode_.maxAnalogueGain);
>         int32_t gainCode = helper_->gainCode(agcStatus->analogueGain);
>
>         /*
> @@ -1538,7 +1523,7 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>          * 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_);
> +       gainCode = std::clamp<int32_t>(gainCode, minGainCode, maxGainCode);
>
>         /* getBlanking might clip exposure time to the fps limits. */
>         Duration exposure = agcStatus->shutterTime;
> --
> 2.34.1
>