[libcamera-devel] [PATCH] libcamera: ipu3: Only process focus if we have a lens

[Jacopo Mondi]

Hi Kieran,

On Fri, Dec 03, 2021 at 04:17:35PM +0200, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Fri, Dec 03, 2021 at 12:53:59PM +0000, Kieran Bingham wrote:
> > If there is no lens detected by the system, then we will not be able to
> > set the control, so we can skip processing of the list.
> >
> > Furthermore, if the IPA has not set a V4L2_CID_FOCUS_ABSOLUTE control,
> > then a warning will be printed as the lensControls.get() will not
> > succeed.
> >
> > Break out of the control parsing when there is no CameraLens
> > device, or if there is no absolute focus control set by the IPA.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 1215bdb84224..16380d2091b2 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -1242,13 +1242,19 @@ void IPU3CameraData::queueFrameAction(unsigned int id,
> >  		const ControlList &sensorControls = action.sensorControls;
> >  		delayedCtrls_->push(sensorControls);
> >
> > +		CameraLens *focusLens = cio2_.sensor()->focusLens();
> > +		if (!focusLens)
> > +			break;
> > +
> >  		const ControlList lensControls = action.lensControls;
> > +		if (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))
> > +			break;
> > +
> >  		const ControlValue &focusValue =
> >  			lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);
> >
>
> I'd drop this blank line.

Was about to suggest the same!

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > -		CameraLens *focusLens = cio2_.sensor()->focusLens();
> > -		if (focusLens && !focusValue.isNone())
> > -			focusLens->setFocusPostion(focusValue.get<int32_t>());
> > +		focusLens->setFocusPostion(focusValue.get<int32_t>());
> > +
> >  		break;
> >  	}
> >  	case ipa::ipu3::ActionParamFilled: {
>
> --
> Regards,
>
> Laurent Pinchart