[PATCH] ipa: rpi: Fix for incorrectly reported max shutter speed

[Naushir Patuck]

Hi Laurent,

On Wed, 1 May 2024 at 16:53, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> Thank you for the patch.
>
> On Fri, Apr 26, 2024 at 12:18:15PM +0100, Naushir Patuck wrote:
> > The maximum shutter speed calculation in the cam-helper relied on
> > the frame duration limits being correctly set in the cam-helper's mode
> > structure. This was not the case on first startup, so the maximum
> > shutter speed reported back via the ControlInfo was incorrect.
> >
> > Fix this by setting up the camera mode in the cam-helper before querying
> > for the max shutter value.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/ipa/rpi/common/ipa_base.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/ipa/rpi/common/ipa_base.cpp
> b/src/ipa/rpi/common/ipa_base.cpp
> > index 149a133ab662..1d12262bda01 100644
> > --- a/src/ipa/rpi/common/ipa_base.cpp
> > +++ b/src/ipa/rpi/common/ipa_base.cpp
> > @@ -592,6 +592,12 @@ void IpaBase::setMode(const IPACameraSensorInfo
> &sensorInfo)
> >       mode_.minAnalogueGain =
> helper_->gain(gainCtrl.min().get<int32_t>());
> >       mode_.maxAnalogueGain =
> helper_->gain(gainCtrl.max().get<int32_t>());
> >
> > +     /*
> > +      * We need to give the helper the min/max frame durations so it
> can calculate
> > +      * the correct exposure limits below.
> > +      */
> > +     helper_->setCameraMode(mode_);
> > +
>
> Don't you end up doing this twice, once here, and once in
> IpaBase::configure(), which calls IpaBase::setMode() ?
>

Yes, it does happen twice.  The other option would be to pass a const
reference to mode_ into the helper through setMode(), allowing us only do
it once.  But that was a bit more involved over this more trivial fix.  I
can change to do that if folks prefer.

Naush


> >       /* Shutter speed is calculated based on the limits of the frame
> durations. */
> >       mode_.minShutter =
> helper_->exposure(shutterCtrl.min().get<int32_t>(), mode_.minLineLength);
> >       mode_.maxShutter = Duration::max();
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240501/bb282e3e/attachment.htm>