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

[Kieran Bingham]

Quoting Laurent Pinchart (2021-12-03 14:17:35)
> 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.

I can, I kind of prefer it when the line above is broken over two lines
though:

		const ControlValue &focusValue =
			lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);

		focusLens->setFocusPostion(focusValue.get<int32_t>());

vs

		const ControlValue &focusValue = lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);
		focusLens->setFocusPostion(focusValue.get<int32_t>());

That takes us to 92 chars on the FOCUS_ABSOLUTE,
so if you prefer without the blank line, I'll move that up. It's still
within our hard-line-lenght-limit.

> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks
--
Kieran


> 
> > -             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