[{"id":31367,"web_url":"https://patchwork.libcamera.org/comment/31367/","msgid":"<20240925235929.GA10330@pendragon.ideasonboard.com>","date":"2024-09-25T23:59:29","subject":"Re: [PATCH v3] libcamera: pipeline: uvcvideo: Map focus controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Cláudio,\n\nThank you for the patch.\n\nOn Sun, Sep 15, 2024 at 02:31:11PM +0000, Cláudio Paulo wrote:\n> Added mapping of controls::LensPosition and controls::AfMode to\n> v4l2 controls V4L2_CID_FOCUS_ABSOLUTE and V4L2_CID_FOCUS_AUTO\n\ns/v4l2/V4L2/\n\n> respectively when the device supports them.\n> \n> If not supported, they are not registered.\n> \n> Signed-off-by: Cláudio Paulo <claudio.paulo@makewise.pt>\n> ---\n> Fixed more issues raised in review.\n> \n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 97 ++++++++++++++++++--\n>  1 file changed, 91 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 6b32fa18..401d3052 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -304,13 +304,26 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n>  \t\tcid = V4L2_CID_EXPOSURE_ABSOLUTE;\n>  \telse if (id == controls::AnalogueGain)\n>  \t\tcid = V4L2_CID_GAIN;\n> +\telse if (id == controls::LensPosition)\n> +\t\tcid = V4L2_CID_FOCUS_ABSOLUTE;\n> +\telse if (id == controls::AfMode)\n> +\t\tcid = V4L2_CID_FOCUS_AUTO;\n>  \telse\n>  \t\treturn -EINVAL;\n>  \n> +\t/* Check if the device supports this control. */\n> +\tif (controls->idMap()->find(cid) == controls->idMap()->end())\n> +\t\treturn -EINVAL;\n> +\n>  \tconst ControlInfo &v4l2Info = controls->infoMap()->at(cid);\n> -\tint32_t min = v4l2Info.min().get<int32_t>();\n> -\tint32_t def = v4l2Info.def().get<int32_t>();\n> -\tint32_t max = v4l2Info.max().get<int32_t>();\n> +\n> +\tint32_t min = -1, def = -1, max = -1;\n> +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> +\t\tmin = v4l2Info.min().get<int32_t>();\n> +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> +\t\tdef = v4l2Info.def().get<int32_t>();\n> +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> +\t\tmax = v4l2Info.max().get<int32_t>();\n>  \n>  \t/*\n>  \t * See UVCCameraData::addControl() for explanations of the different\n> @@ -358,6 +371,21 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n>  \t\tbreak;\n>  \t}\n>  \n> +\tcase V4L2_CID_FOCUS_ABSOLUTE: {\n> +\t\tfloat focusedAt50Cm = 0.15f * (max - min);\n> +\t\tfloat scale = focusedAt50Cm / 2.0f;\n> +\n> +\t\tfloat fvalue = value.get<float>() * scale + min;\n> +\t\tcontrols->set(cid, static_cast<int32_t>(lroundf(fvalue)));\n\nYou should use std::lround() here. Please remember to replace <math.h>\nwith <cmath> at the top of the file.\n\n> +\t\tbreak;\n> +\t}\n> +\n> +\tcase V4L2_CID_FOCUS_AUTO: {\n> +\t\tint32_t ivalue = value.get<int32_t>() != controls::AfModeManual;\n> +\t\tcontrols->set(cid, ivalue);\n> +\t\tbreak;\n> +\t}\n> +\n>  \tdefault: {\n>  \t\tint32_t ivalue = value.get<int32_t>();\n>  \t\tcontrols->set(cid, ivalue);\n> @@ -655,14 +683,32 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n>  \tcase V4L2_CID_GAIN:\n>  \t\tid = &controls::AnalogueGain;\n>  \t\tbreak;\n> +\tcase V4L2_CID_FOCUS_ABSOLUTE:\n> +\t\tid = &controls::LensPosition;\n> +\t\tbreak;\n> +\tcase V4L2_CID_FOCUS_AUTO:\n> +\t\tid = &controls::AfMode;\n> +\t\tbreak;\n>  \tdefault:\n>  \t\treturn;\n>  \t}\n>  \n>  \t/* Map the control info. */\n> -\tint32_t min = v4l2Info.min().get<int32_t>();\n> -\tint32_t max = v4l2Info.max().get<int32_t>();\n> -\tint32_t def = v4l2Info.def().get<int32_t>();\n> +\tint32_t min = -1, def = -1, max = -1;\n> +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> +\t\tmin = v4l2Info.min().get<int32_t>();\n> +\telse if (v4l2Info.min().type() == ControlTypeBool)\n> +\t\tmin = v4l2Info.min().get<bool>();\n> +\n> +\tif (v4l2Info.def().type() == ControlTypeInteger32)\n> +\t\tdef = v4l2Info.def().get<int32_t>();\n> +\telse if (v4l2Info.def().type() == ControlTypeBool)\n> +\t\tdef = v4l2Info.def().get<bool>();\n> +\n> +\tif (v4l2Info.max().type() == ControlTypeInteger32)\n> +\t\tmax = v4l2Info.max().get<int32_t>();\n> +\telse if (v4l2Info.max().type() == ControlTypeBool)\n> +\t\tmax = v4l2Info.max().get<bool>();\n\nAs the min, max and def values all have the same type, you can write\n\n\tif (v4l2Info.min().type() == ControlTypeInteger32) {\n\t\tmin = v4l2Info.min().get<int32_t>();\n\t\tdef = v4l2Info.def().get<int32_t>();\n\t\tmax = v4l2Info.max().get<int32_t>();\n\t} else if (v4l2Info.min().type() == ControlTypeBool) {\n\t\tmin = v4l2Info.min().get<bool>();\n\t\tdef = v4l2Info.def().get<bool>();\n\t\tmax = v4l2Info.max().get<bool>();\n\t}\n\n>  \n>  \tswitch (cid) {\n>  \tcase V4L2_CID_BRIGHTNESS: {\n> @@ -738,6 +784,45 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n>  \t\t};\n>  \t\tbreak;\n>  \t}\n\nAdd a blank line here.\n\n> +\tcase V4L2_CID_FOCUS_ABSOLUTE: {\n> +\t\t/*\n> +\t\t * LensPosition expects values to be in the range of\n> +\t\t * [0.0f, maxDioptres], while a value of 0 means focused to\n> +\t\t * infinity, 0.5 means focused at 2m, 2 means focused at 50cm\n> +\t\t * and maxDioptres will be the closest possible focus which\n> +\t\t * will equate to a focus distance of (1 / maxDioptres) metres.\n> +\t\t *\n> +\t\t * \\todo These values will definitely vary for each different\n> +\t\t * hardware, so this should be tunable somehow. In this case\n> +\t\t * 0.15f (~= 150 / (1023 - 1)) was a value read from\n> +\t\t * V4L2_CID_FOCUS_ABSOLUTE control when using a random camera\n> +\t\t * and having it autofocus at a distance of about 50cm.\n> +\t\t * This means the minimum focus distance of this camera should\n> +\t\t * be 75mm (0.15 / 2) which equals a maxDioptres value of ~=\n> +\t\t * 13.3333 (2 / 0.15). Also, the values might not scale\n> +\t\t * linearly, but this implementation assumes they do.\n> +\t\t */\n> +\t\tfloat focusedAt50Cm = 0.15f * (max - min);\n> +\t\tfloat scale = 2.0f / focusedAt50Cm;\n> +\n> +\t\tinfo = ControlInfo{\n> +\t\t\t{ 0.0f },\n> +\t\t\t{ (max - min) * scale },\n> +\t\t\t{ (def - min) * scale }\n> +\t\t};\n> +\t\tbreak;\n> +\t}\n\nAnd here too.\n\n> +\tcase V4L2_CID_FOCUS_AUTO: {\n> +\t\tint32_t manual = controls::AfModeManual;\n> +\t\tint32_t continuous = controls::AfModeContinuous;\n> +\n> +\t\tinfo = ControlInfo{\n> +\t\t\t{ manual },\n> +\t\t\t{ continuous },\n> +\t\t\t{ def ? continuous : manual },\n\nHow about hardcoding the default to continuous, to make it less\ndevice-specific ? Then you could write\n\n\t\tinfo = ControlInfo{\n\t\t\t{ controls::AfModeManual },\n\t\t\t{ controls::AfModeContinuous },\n\t\t\t{ controls::AfModeContinuous },\n\t\t};\n\nBut for enumerated controls, you should use the ControlInfo constructor\nthat takes a list of supported values:\n\n\t\tinfo = ControlInfo{\n\t\t\tstd::array<ControlValue, 2>{\n\t\t\t\tstatic_cast<int32_t>(controls::AfModeManual),\n\t\t\t\tstatic_cast<int32_t>(controls::AfModeContinuous),\n\t\t\t},\n\t\t\t{ static_cast<int32_t>(controls::AfModeContinuous) },\n\t\t};\n\nThis is a bit complicated. I've posted a patch (\"\") that let's you\nsimplify the construct:\n\n\t\tinfo = ControlInfo{\n\t\t\tstd::array<ControlValue, 2>{\n\t\t\t\tcontrols::AfModeManual,\n\t\t\t\tcontrols::AfModeContinuous,\n\t\t\t},\n\t\t\t{ controls::AfModeContinuous },\n\t\t};\n\nThis being said, I think you should also check what the minimum and\nmaximum values are for the V4L2 control. It's conceivable that a UVC\ndevice would implement the V4L2_CID_FOCUS_AUTO control but only supports\nmanual focus, or auto-focus. I think the following should work.\n\n\t\tstd::vector<ControlValue> values;\n\n\t\tif (!min || !max)\n\t\t\tvalues.push_back(controls::AfModeManual);\n\t\tif (min ||max)\n\t\t\tvalues.push_back(controls::AfModeContinuous);\n\n\t\tinfo = ControlInfo{ values, values.back() };\n\t\tbreak;\n\nWith these changes (please test them, as I may have made a mistake), I\nthink we can merge the next version.\n\n> +\t\t};\n> +\t\tbreak;\n> +\t}\n>  \n>  \tdefault:\n>  \t\tinfo = v4l2Info;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 53D74C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 25 Sep 2024 23:59:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80CA86350E;\n\tThu, 26 Sep 2024 01:59:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0409634FB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Sep 2024 01:59:31 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD51883D;\n\tThu, 26 Sep 2024 01:58:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XxCMFbuz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727308684;\n\tbh=z5dWNmqjFh5a82Q/QwzB5qM83GufhKnvqGt+99P+4pU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XxCMFbuzGfnKAvCAcUE2RUOLz/BBLc0B1c4pmh7OkWTp7E7MboTX/s1REoqV61LWB\n\tA4wh29S+yPyTb1EUTXLk6iV1/tNfZjRxfNtLbPf8V8Yoe1XiunT5mOFZoVMCP4S1Lg\n\tBHzGKtqmc5RZo/Auiz9JKVZeVvj39bxpl3g/mEgA=","Date":"Thu, 26 Sep 2024 02:59:29 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Cl=C3=A1udio?= Paulo <claudio.paulo@makewise.pt>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3] libcamera: pipeline: uvcvideo: Map focus controls","Message-ID":"<20240925235929.GA10330@pendragon.ideasonboard.com>","References":"<01020191f6184753-fa7f549e-0b7c-4f7a-9bcf-c9389c9f6fd1-000000@eu-west-1.amazonses.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<01020191f6184753-fa7f549e-0b7c-4f7a-9bcf-c9389c9f6fd1-000000@eu-west-1.amazonses.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31368,"web_url":"https://patchwork.libcamera.org/comment/31368/","msgid":"<20240926002515.GA27736@pendragon.ideasonboard.com>","date":"2024-09-26T00:25:15","subject":"Re: [PATCH v3] libcamera: pipeline: uvcvideo: Map focus controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Sep 26, 2024 at 02:59:29AM +0300, Laurent Pinchart wrote:\n> Hi Cláudio,\n> \n> Thank you for the patch.\n> \n> On Sun, Sep 15, 2024 at 02:31:11PM +0000, Cláudio Paulo wrote:\n> > Added mapping of controls::LensPosition and controls::AfMode to\n> > v4l2 controls V4L2_CID_FOCUS_ABSOLUTE and V4L2_CID_FOCUS_AUTO\n> \n> s/v4l2/V4L2/\n> \n> > respectively when the device supports them.\n> > \n> > If not supported, they are not registered.\n> > \n> > Signed-off-by: Cláudio Paulo <claudio.paulo@makewise.pt>\n> > ---\n> > Fixed more issues raised in review.\n> > \n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 97 ++++++++++++++++++--\n> >  1 file changed, 91 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 6b32fa18..401d3052 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -304,13 +304,26 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> >  \t\tcid = V4L2_CID_EXPOSURE_ABSOLUTE;\n> >  \telse if (id == controls::AnalogueGain)\n> >  \t\tcid = V4L2_CID_GAIN;\n> > +\telse if (id == controls::LensPosition)\n> > +\t\tcid = V4L2_CID_FOCUS_ABSOLUTE;\n> > +\telse if (id == controls::AfMode)\n> > +\t\tcid = V4L2_CID_FOCUS_AUTO;\n> >  \telse\n> >  \t\treturn -EINVAL;\n> >  \n> > +\t/* Check if the device supports this control. */\n> > +\tif (controls->idMap()->find(cid) == controls->idMap()->end())\n> > +\t\treturn -EINVAL;\n> > +\n> >  \tconst ControlInfo &v4l2Info = controls->infoMap()->at(cid);\n> > -\tint32_t min = v4l2Info.min().get<int32_t>();\n> > -\tint32_t def = v4l2Info.def().get<int32_t>();\n> > -\tint32_t max = v4l2Info.max().get<int32_t>();\n> > +\n> > +\tint32_t min = -1, def = -1, max = -1;\n> > +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> > +\t\tmin = v4l2Info.min().get<int32_t>();\n> > +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> > +\t\tdef = v4l2Info.def().get<int32_t>();\n> > +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> > +\t\tmax = v4l2Info.max().get<int32_t>();\n> >  \n> >  \t/*\n> >  \t * See UVCCameraData::addControl() for explanations of the different\n> > @@ -358,6 +371,21 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> >  \t\tbreak;\n> >  \t}\n> >  \n> > +\tcase V4L2_CID_FOCUS_ABSOLUTE: {\n> > +\t\tfloat focusedAt50Cm = 0.15f * (max - min);\n> > +\t\tfloat scale = focusedAt50Cm / 2.0f;\n> > +\n> > +\t\tfloat fvalue = value.get<float>() * scale + min;\n> > +\t\tcontrols->set(cid, static_cast<int32_t>(lroundf(fvalue)));\n> \n> You should use std::lround() here. Please remember to replace <math.h>\n> with <cmath> at the top of the file.\n> \n> > +\t\tbreak;\n> > +\t}\n> > +\n> > +\tcase V4L2_CID_FOCUS_AUTO: {\n> > +\t\tint32_t ivalue = value.get<int32_t>() != controls::AfModeManual;\n> > +\t\tcontrols->set(cid, ivalue);\n> > +\t\tbreak;\n> > +\t}\n> > +\n> >  \tdefault: {\n> >  \t\tint32_t ivalue = value.get<int32_t>();\n> >  \t\tcontrols->set(cid, ivalue);\n> > @@ -655,14 +683,32 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> >  \tcase V4L2_CID_GAIN:\n> >  \t\tid = &controls::AnalogueGain;\n> >  \t\tbreak;\n> > +\tcase V4L2_CID_FOCUS_ABSOLUTE:\n> > +\t\tid = &controls::LensPosition;\n> > +\t\tbreak;\n> > +\tcase V4L2_CID_FOCUS_AUTO:\n> > +\t\tid = &controls::AfMode;\n> > +\t\tbreak;\n> >  \tdefault:\n> >  \t\treturn;\n> >  \t}\n> >  \n> >  \t/* Map the control info. */\n> > -\tint32_t min = v4l2Info.min().get<int32_t>();\n> > -\tint32_t max = v4l2Info.max().get<int32_t>();\n> > -\tint32_t def = v4l2Info.def().get<int32_t>();\n> > +\tint32_t min = -1, def = -1, max = -1;\n> > +\tif (v4l2Info.min().type() == ControlTypeInteger32)\n> > +\t\tmin = v4l2Info.min().get<int32_t>();\n> > +\telse if (v4l2Info.min().type() == ControlTypeBool)\n> > +\t\tmin = v4l2Info.min().get<bool>();\n> > +\n> > +\tif (v4l2Info.def().type() == ControlTypeInteger32)\n> > +\t\tdef = v4l2Info.def().get<int32_t>();\n> > +\telse if (v4l2Info.def().type() == ControlTypeBool)\n> > +\t\tdef = v4l2Info.def().get<bool>();\n> > +\n> > +\tif (v4l2Info.max().type() == ControlTypeInteger32)\n> > +\t\tmax = v4l2Info.max().get<int32_t>();\n> > +\telse if (v4l2Info.max().type() == ControlTypeBool)\n> > +\t\tmax = v4l2Info.max().get<bool>();\n> \n> As the min, max and def values all have the same type, you can write\n> \n> \tif (v4l2Info.min().type() == ControlTypeInteger32) {\n> \t\tmin = v4l2Info.min().get<int32_t>();\n> \t\tdef = v4l2Info.def().get<int32_t>();\n> \t\tmax = v4l2Info.max().get<int32_t>();\n> \t} else if (v4l2Info.min().type() == ControlTypeBool) {\n> \t\tmin = v4l2Info.min().get<bool>();\n> \t\tdef = v4l2Info.def().get<bool>();\n> \t\tmax = v4l2Info.max().get<bool>();\n> \t}\n> \n> >  \n> >  \tswitch (cid) {\n> >  \tcase V4L2_CID_BRIGHTNESS: {\n> > @@ -738,6 +784,45 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> >  \t\t};\n> >  \t\tbreak;\n> >  \t}\n> \n> Add a blank line here.\n> \n> > +\tcase V4L2_CID_FOCUS_ABSOLUTE: {\n> > +\t\t/*\n> > +\t\t * LensPosition expects values to be in the range of\n> > +\t\t * [0.0f, maxDioptres], while a value of 0 means focused to\n> > +\t\t * infinity, 0.5 means focused at 2m, 2 means focused at 50cm\n> > +\t\t * and maxDioptres will be the closest possible focus which\n> > +\t\t * will equate to a focus distance of (1 / maxDioptres) metres.\n> > +\t\t *\n> > +\t\t * \\todo These values will definitely vary for each different\n> > +\t\t * hardware, so this should be tunable somehow. In this case\n> > +\t\t * 0.15f (~= 150 / (1023 - 1)) was a value read from\n> > +\t\t * V4L2_CID_FOCUS_ABSOLUTE control when using a random camera\n> > +\t\t * and having it autofocus at a distance of about 50cm.\n> > +\t\t * This means the minimum focus distance of this camera should\n> > +\t\t * be 75mm (0.15 / 2) which equals a maxDioptres value of ~=\n> > +\t\t * 13.3333 (2 / 0.15). Also, the values might not scale\n> > +\t\t * linearly, but this implementation assumes they do.\n> > +\t\t */\n> > +\t\tfloat focusedAt50Cm = 0.15f * (max - min);\n> > +\t\tfloat scale = 2.0f / focusedAt50Cm;\n> > +\n> > +\t\tinfo = ControlInfo{\n> > +\t\t\t{ 0.0f },\n> > +\t\t\t{ (max - min) * scale },\n> > +\t\t\t{ (def - min) * scale }\n> > +\t\t};\n> > +\t\tbreak;\n> > +\t}\n> \n> And here too.\n> \n> > +\tcase V4L2_CID_FOCUS_AUTO: {\n> > +\t\tint32_t manual = controls::AfModeManual;\n> > +\t\tint32_t continuous = controls::AfModeContinuous;\n> > +\n> > +\t\tinfo = ControlInfo{\n> > +\t\t\t{ manual },\n> > +\t\t\t{ continuous },\n> > +\t\t\t{ def ? continuous : manual },\n> \n> How about hardcoding the default to continuous, to make it less\n> device-specific ? Then you could write\n> \n> \t\tinfo = ControlInfo{\n> \t\t\t{ controls::AfModeManual },\n> \t\t\t{ controls::AfModeContinuous },\n> \t\t\t{ controls::AfModeContinuous },\n> \t\t};\n> \n> But for enumerated controls, you should use the ControlInfo constructor\n> that takes a list of supported values:\n> \n> \t\tinfo = ControlInfo{\n> \t\t\tstd::array<ControlValue, 2>{\n> \t\t\t\tstatic_cast<int32_t>(controls::AfModeManual),\n> \t\t\t\tstatic_cast<int32_t>(controls::AfModeContinuous),\n> \t\t\t},\n> \t\t\t{ static_cast<int32_t>(controls::AfModeContinuous) },\n> \t\t};\n> \n> This is a bit complicated. I've posted a patch (\"\") that let's you\n\nThe patch is named \"[PATCH] libcamera: controls: Handle enum values\nwithout a cast\" and I've CC'ed you.\n\n> simplify the construct:\n> \n> \t\tinfo = ControlInfo{\n> \t\t\tstd::array<ControlValue, 2>{\n> \t\t\t\tcontrols::AfModeManual,\n> \t\t\t\tcontrols::AfModeContinuous,\n> \t\t\t},\n> \t\t\t{ controls::AfModeContinuous },\n> \t\t};\n> \n> This being said, I think you should also check what the minimum and\n> maximum values are for the V4L2 control. It's conceivable that a UVC\n> device would implement the V4L2_CID_FOCUS_AUTO control but only supports\n> manual focus, or auto-focus. I think the following should work.\n> \n> \t\tstd::vector<ControlValue> values;\n> \n> \t\tif (!min || !max)\n> \t\t\tvalues.push_back(controls::AfModeManual);\n> \t\tif (min ||max)\n> \t\t\tvalues.push_back(controls::AfModeContinuous);\n> \n> \t\tinfo = ControlInfo{ values, values.back() };\n> \t\tbreak;\n> \n> With these changes (please test them, as I may have made a mistake), I\n> think we can merge the next version.\n> \n> > +\t\t};\n> > +\t\tbreak;\n> > +\t}\n> >  \n> >  \tdefault:\n> >  \t\tinfo = v4l2Info;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 97603C3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Sep 2024 00:25:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4960363511;\n\tThu, 26 Sep 2024 02:25:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2716C63500\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Sep 2024 02:25:17 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 48C8F169;\n\tThu, 26 Sep 2024 02:23:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ALxawGSy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727310229;\n\tbh=V0rElE1toYvXsBXqwmlHQe1qDbwYYdBVYr5JQBfpv+M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ALxawGSyPGyVy98fDkc6VkpAIHaFZcOo11r6u1NU7TvWACaC7VFKgnpR4FCrMnFRj\n\tp4jRRBVBX9fMrxutKKQyVM5RQ4j+abMcwDObbmdmfYZDLwNVfVwzb1qWpvasFIMr5u\n\tpvIUZgWLPpInK3bL/tKOHRb5OnHeeiP0A5dORqn0=","Date":"Thu, 26 Sep 2024 03:25:15 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Cl=C3=A1udio?= Paulo <claudio.paulo@makewise.pt>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3] libcamera: pipeline: uvcvideo: Map focus controls","Message-ID":"<20240926002515.GA27736@pendragon.ideasonboard.com>","References":"<01020191f6184753-fa7f549e-0b7c-4f7a-9bcf-c9389c9f6fd1-000000@eu-west-1.amazonses.com>\n\t<20240925235929.GA10330@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240925235929.GA10330@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]