[libcamera-devel] [PATCH v1 2/3] ipa: raspberrypi: Populate SensorLimits

Naushir Patuck naush at raspberrypi.com
Fri Mar 24 15:54:21 CET 2023


Hi Jacopo,

Thank you for the feedback.


On Fri, 24 Mar 2023 at 14:23, Jacopo Mondi <jacopo.mondi at ideasonboard.com>
wrote:

> Hi Naush
>
> On Wed, Mar 22, 2023 at 04:13:16PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > Populate all the fields of the SensorLimits structure in configure().
> > This allows us to use the cached values instead of re-computing them
> > on every frame.
> >
> > For the gain -> code convertion, ensure we clamp to the analogue gain
> > limits set in the SensorLimits structure.
>
> I wonder if this can't be done by expanding your CameraMode structure.
> CameraMode already combines information from the SensorInfo and could
> very well include information from sensorControls.
>
> This would avoid introducing another type to move it around across
> different layers of your IPA.
>
> Have you considered that and decided you prefer introducing
> SensorLimits ?
>

That's a great idea!  It definitely fits well in the CameraMode structure.
If there are no objection from David, I'll rework the series to move these
fields into CameraMode.

Regards,
Naush


>
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/raspberrypi.cpp | 67 ++++++++++++++++-------------
> >  1 file changed, 38 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index c3b2c375036f..e8d8023bcfe7 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -224,8 +224,8 @@ private:
> >       Duration minFrameDuration_;
> >       Duration maxFrameDuration_;
> >
> > -     /* Maximum gain code for the sensor. */
> > -     uint32_t maxSensorGainCode_;
> > +     /* Mode specific sensor gain/exposure limits. */
> > +     RPiController::AgcAlgorithm::SensorLimits sensorLimits_;
> >
> >       /* Track the frame length times over FrameLengthsQueueSize frames.
> */
> >       std::deque<Duration> frameLengths_;
> > @@ -439,8 +439,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);
> >
> > @@ -473,6 +471,28 @@ int IPARPi::configure(const IPACameraSensorInfo
> &sensorInfo, const IPAConfig &ip
> >       /* Pass the camera mode to the CamHelper to setup algorithms. */
> >       helper_->setCameraMode(mode_);
> >
> > +     /*
> > +      * Store the sensor gain and shutter limits for the mode.
> > +      *
> > +      * The maximum shutter value are calculated from the frame
> duration limit
> > +      * as V4L2 will restrict the maximum control value based on the
> current
> > +      * VBLANK value.
> > +      *
> > +      * These limits get set in the AGC algorithm through
> applyFrameDurations()
> > +      * below.
> > +      */
> > +     const ControlInfo &gainCtrl =
> sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN);
> > +     const ControlInfo &shutterCtrl =
> sensorCtrls_.at(V4L2_CID_EXPOSURE);
> > +
> > +     sensorLimits_.minAnalogueGain =
> helper_->gain(gainCtrl.min().get<int32_t>());
> > +     sensorLimits_.maxAnalogueGain =
> helper_->gain(gainCtrl.max().get<int32_t>());
> > +     sensorLimits_.minFrameDuration = mode_.minFrameLength *
> mode_.minLineLength;
> > +     sensorLimits_.maxFrameDuration = mode_.maxFrameLength *
> mode_.maxLineLength;
> > +     sensorLimits_.minShutter =
> helper_->exposure(shutterCtrl.min().get<int32_t>(), mode_.minLineLength);
> > +     sensorLimits_.maxShutter = Duration::max();
> > +     helper_->getBlanking(sensorLimits_.maxShutter,
> > +                          sensorLimits_.minFrameDuration,
> sensorLimits_.maxFrameDuration);
> > +
> >       /*
> >        * Initialise this ControlList correctly, even if empty, in case
> the IPA is
> >        * running is isolation mode (passing the ControlList through the
> IPC layer).
> > @@ -501,26 +521,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>(sensorLimits_.minFrameDuration.get<std::micro>()),
> > +
>  static_cast<int64_t>(sensorLimits_.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>(sensorLimits_.minAnalogueGain),
> > +
>  static_cast<float>(sensorLimits_.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>(sensorLimits_.minShutter.get<std::micro>()),
> > +
>  static_cast<int32_t>(sensorLimits_.maxShutter.get<std::micro>()));
> >
> >       /* Declare Autofocus controls, only if we have a controllable lens
> */
> >       if (lensPresent_)
> > @@ -1480,9 +1491,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.
> > @@ -1490,9 +1498,9 @@ void IPARPi::applyFrameDurations(Duration
> minFrameDuration, Duration maxFrameDur
> >       minFrameDuration_ = minFrameDuration ? minFrameDuration :
> defaultMaxFrameDuration;
> >       maxFrameDuration_ = maxFrameDuration ? maxFrameDuration :
> defaultMinFrameDuration;
> >       minFrameDuration_ = std::clamp(minFrameDuration_,
> > -                                    minSensorFrameDuration,
> maxSensorFrameDuration);
> > +                                    sensorLimits_.minFrameDuration,
> sensorLimits_.maxFrameDuration);
> >       maxFrameDuration_ = std::clamp(maxFrameDuration_,
> > -                                    minSensorFrameDuration,
> maxSensorFrameDuration);
> > +                                    sensorLimits_.minFrameDuration,
> sensorLimits_.maxFrameDuration);
> >       maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
> >
> >       /* Return the validated limits via metadata. */
> > @@ -1505,17 +1513,18 @@ void IPARPi::applyFrameDurations(Duration
> minFrameDuration, Duration maxFrameDur
> >        * getBlanking() will update maxShutter with the largest exposure
> >        * value possible.
> >        */
> > -     RPiController::AgcAlgorithm::SensorLimits limits;
> > -     limits.maxShutter = Duration::max();
> > -     helper_->getBlanking(limits.maxShutter, minFrameDuration_,
> maxFrameDuration_);
> > +     sensorLimits_.maxShutter = Duration::max();
> > +     helper_->getBlanking(sensorLimits_.maxShutter, minFrameDuration_,
> maxFrameDuration_);
> >
> >       RPiController::AgcAlgorithm *agc =
> dynamic_cast<RPiController::AgcAlgorithm *>(
> >               controller_.getAlgorithm("agc"));
> > -     agc->setSensorLimits(limits);
> > +     agc->setSensorLimits(sensorLimits_);
> >  }
> >
> >  void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList
> &ctrls)
> >  {
> > +     const int32_t minGainCode =
> helper_->gainCode(sensorLimits_.minAnalogueGain);
> > +     const int32_t maxGainCode =
> helper_->gainCode(sensorLimits_.maxAnalogueGain);
> >       int32_t gainCode = helper_->gainCode(agcStatus->analogueGain);
> >
> >       /*
> > @@ -1523,7 +1532,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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230324/e89cc2f7/attachment.htm>


More information about the libcamera-devel mailing list