[PATCH v2] libcamera: pipeline: uvcvideo: Map focus controls
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Sep 25 23:55:49 CEST 2024
Hi Kieran,
On Sun, Sep 15, 2024 at 11:39:22AM +0100, Kieran Bingham wrote:
> Quoting Cláudio Paulo (2024-09-14 22:55:49)
> > Added mapping of controls::LensPosition and controls::AfMode to
> > v4l2 controls V4L2_CID_FOCUS_ABSOLUTE and V4L2_CID_FOCUS_AUTO
> > respectively when the device supports them.
> >
> > If not supported, they are not registered.
> >
> > Signed-off-by: Cláudio Paulo <claudio.paulo at makewise.pt>
> > ---
>
> I've tested this and it's functional.
>
> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > Fixed issues pointed out on feedback.
> >
> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 101 +++++++++++++++++--
> > 1 file changed, 93 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index 6b32fa18..11e34e95 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -304,13 +304,26 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
> > cid = V4L2_CID_EXPOSURE_ABSOLUTE;
> > else if (id == controls::AnalogueGain)
> > cid = V4L2_CID_GAIN;
> > + else if (id == controls::LensPosition)
> > + cid = V4L2_CID_FOCUS_ABSOLUTE;
> > + else if (id == controls::AfMode)
> > + cid = V4L2_CID_FOCUS_AUTO;
> > else
> > return -EINVAL;
> >
> > + /* Check if the device supports this control. */
> > + if (controls->idMap()->find(cid) == controls->idMap()->end())
> > + return -EINVAL;
> > +
> > const ControlInfo &v4l2Info = controls->infoMap()->at(cid);
> > - int32_t min = v4l2Info.min().get<int32_t>();
> > - int32_t def = v4l2Info.def().get<int32_t>();
> > - int32_t max = v4l2Info.max().get<int32_t>();
> > +
> > + int32_t min = -1, def = -1, max = -1;
> > + if (v4l2Info.min().type() == ControlTypeInteger32)
> > + min = v4l2Info.min().get<int32_t>();
> > + if (v4l2Info.min().type() == ControlTypeInteger32)
> > + def = v4l2Info.def().get<int32_t>();
> > + if (v4l2Info.min().type() == ControlTypeInteger32)
> > + max = v4l2Info.max().get<int32_t>();
> >
> > /*
> > * See UVCCameraData::addControl() for explanations of the different
> > @@ -333,8 +346,8 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
> >
> > case V4L2_CID_EXPOSURE_AUTO: {
> > int32_t ivalue = value.get<bool>()
> > - ? V4L2_EXPOSURE_APERTURE_PRIORITY
> > - : V4L2_EXPOSURE_MANUAL;
> > + ? V4L2_EXPOSURE_APERTURE_PRIORITY
> > + : V4L2_EXPOSURE_MANUAL;
>
> I'm sorry for this one - but this is a false positive from clang-format
> in checkstyle. I never could coax it into styling this how we like.
I tried to find a way to get clang-format to understand our coding
style, but didn't succeed :-(
> We should remove this hunk from the patch.
>
> > controls->set(V4L2_CID_EXPOSURE_AUTO, ivalue);
> > break;
> > }
> > @@ -358,6 +371,20 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
> > break;
> > }
> >
> > + case V4L2_CID_FOCUS_ABSOLUTE: {
> > + float focusedAt50Cm = 0.15f * (max - min);
> > + float scale = focusedAt50Cm / 2.0f;
> > +
> > + controls->set(cid, static_cast<int32_t>(value.get<float>() * scale + min));
> > + break;
> > + }
> > +
> > + case V4L2_CID_FOCUS_AUTO: {
> > + int32_t ivalue = value.get<int32_t>() != controls::AfModeManual;
> > + controls->set(cid, ivalue);
> > + break;
> > + }
> > +
> > default: {
> > int32_t ivalue = value.get<int32_t>();
> > controls->set(cid, ivalue);
> > @@ -655,14 +682,32 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
> > case V4L2_CID_GAIN:
> > id = &controls::AnalogueGain;
> > break;
> > + case V4L2_CID_FOCUS_ABSOLUTE:
> > + id = &controls::LensPosition;
> > + break;
> > + case V4L2_CID_FOCUS_AUTO:
> > + id = &controls::AfMode;
> > + break;
> > default:
> > return;
> > }
> >
> > /* Map the control info. */
> > - int32_t min = v4l2Info.min().get<int32_t>();
> > - int32_t max = v4l2Info.max().get<int32_t>();
> > - int32_t def = v4l2Info.def().get<int32_t>();
> > + int32_t min = -1, def = -1, max = -1;
> > + if (v4l2Info.min().type() == ControlTypeInteger32)
> > + min = v4l2Info.min().get<int32_t>();
> > + else if (v4l2Info.min().type() == ControlTypeBool)
> > + min = v4l2Info.min().get<bool>();
> > +
> > + if (v4l2Info.def().type() == ControlTypeInteger32)
> > + def = v4l2Info.def().get<int32_t>();
> > + else if (v4l2Info.def().type() == ControlTypeBool)
> > + def = v4l2Info.def().get<bool>();
> > +
> > + if (v4l2Info.max().type() == ControlTypeInteger32)
> > + max = v4l2Info.max().get<int32_t>();
> > + else if (v4l2Info.max().type() == ControlTypeBool)
> > + max = v4l2Info.max().get<bool>();
> >
> > switch (cid) {
> > case V4L2_CID_BRIGHTNESS: {
> > @@ -738,6 +783,46 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
> > };
> > break;
> > }
> > + case V4L2_CID_FOCUS_ABSOLUTE: {
> > + /*
> > + * LensPosition expects values to be in the range of
> > + * [0.0f, maxDioptres], while a value of 0 means focused to
> > + * infinity, 0.5 means focused at 2m, 2 means focused at 50cm
> > + * and maxDioptres will be the closest possible focus which
> > + * will equate to a focus distance of (1 / maxDioptres) metres.
> > + *
> > + * In this case 0.15f (~= 150 / (1023 - 1)) was a value read
> > + * from V4L2_CID_FOCUS_ABSOLUTE control when using a random
> > + * camera and having it autofocus at a distance of about 50cm.
>
> This is a potentially contentious area if it's hardcoding to one
> specific device.
>
> > + * This means the minimum focus distance of this camera should
> > + * be 75mm (0.15 / 2) which equals a maxDioptres value of ~=
> > + * 13.3333 (2 / 0.15).
> > + *
> > + * These values will definitely vary for each different
> > + * hardware. Also, the values might not scale linearly, but
> > + * this implementation assumes they do.
> > + */
>
> I think we'll have to put this in as a \todo rather than a comment to
> express that there is further work required here.
>
> > + float focusedAt50Cm = 0.15f * (max - min);
>
> So it seems like this should be a tunable parameter. But I'm not sure
> how we can handle that...
>
> Perhaps if we can determine some (not insane) default value and let it
> be specified in the upcoming camera configuration files that Milan is
> working on. Or we go and put a full tuning file for UVC ... but that
> seems impractical..
There are thousands of UVC cameras out there, I don't think we can do
that. We may need to expose a new property to indicate that the focus
lens isn't calibrated and that applications can't rely on any particular
mapping between the LensPosition control and focal values.
> But otherwise, this indeed lets the UVC camera start to progress with
> the plumbing.
Indeed :-)
> > + float scale = 2.0f / focusedAt50Cm;
> > +
> > + info = ControlInfo{
> > + { 0.0f },
> > + { (max - min) * scale },
> > + { (def - min) * scale }
> > + };
> > + break;
> > + }
> > + case V4L2_CID_FOCUS_AUTO: {
> > + int32_t manual = controls::AfModeManual;
> > + int32_t continuous = controls::AfModeContinuous;
> > +
> > + info = ControlInfo{
> > + { manual },
> > + { continuous },
> > + { def ? continuous : manual },
> > + };
> > + break;
> > + }
> >
> > default:
> > info = v4l2Info;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list