[{"id":33561,"web_url":"https://patchwork.libcamera.org/comment/33561/","msgid":"","date":"2025-03-04T09:49:08","subject":"Re: [PATCH v3 1/2] libcamera: pipeline: uvcvideo: Fix\n\t`ExposureTimeMode` control setup","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Mar 03, 2025 at 02:42:33PM +0100, Barnabás Pőcze wrote:\n> `ControlInfo(Span{...})` calls the incorrect constructor\n> of `ControlInfo`. The intended constructor to be called is\n> `ControlInfo(Span, ...)` however that is not called\n> because a span of `const int32_t` is passed. Instead, the constructor\n> `ControlInfo(const ControlValue &min, const ControlValue &max, ...)`\n> will be called.\n>\n> Furthermore, since `values.back()` is used, only the last element of\n> the array is actually set.\n>\n> To fix this, convert the array to contain `ControlValue` objects and use\n> a separate variable to keep track of which element to set next.\n>\n> For each of `ExposureTimeMode{Auto,Manual}` save the V4L2 control value\n> that is to be used when the libcamera control is set.\n>\n> Fixes: bad8d591f8acfa (\"libcamera: uvcvideo: Register ExposureTimeMode control\")\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 61 ++++++++++++++------\n> 1 file changed, 42 insertions(+), 19 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 7470b5627..a6cc37366 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -6,6 +6,7 @@\n> */\n>\n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -56,6 +57,9 @@ public:\n> \tStream stream_;\n> \tstd::map> formats_;\n>\n> +\tstd::optional autoExposureMode_;\n> +\tstd::optional manualExposureMode_;\n> +\n> private:\n> \tbool generateId();\n>\n> @@ -723,25 +727,44 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> \t\t * ExposureTimeModeManual = { V4L2_EXPOSURE_MANUAL,\n> \t\t *\t\t\t V4L2_EXPOSURE_SHUTTER_PRIORITY }\n> \t\t */\n> -\t\tstd::array values{};\n> -\n> -\t\tauto it = std::find_if(v4l2Values.begin(), v4l2Values.end(),\n> -\t\t\t[&](const ControlValue &val) {\n> -\t\t\t\treturn (val.get() == V4L2_EXPOSURE_APERTURE_PRIORITY ||\n> -\t\t\t\t\tval.get() == V4L2_EXPOSURE_AUTO) ? true : false;\n> -\t\t\t});\n> -\t\tif (it != v4l2Values.end())\n> -\t\t\tvalues.back() = static_cast(controls::ExposureTimeModeAuto);\n> -\n> -\t\tit = std::find_if(v4l2Values.begin(), v4l2Values.end(),\n> -\t\t\t[&](const ControlValue &val) {\n> -\t\t\t\treturn (val.get() == V4L2_EXPOSURE_SHUTTER_PRIORITY ||\n> -\t\t\t\t\tval.get() == V4L2_EXPOSURE_MANUAL) ? true : false;\n> -\t\t\t});\n> -\t\tif (it != v4l2Values.end())\n> -\t\t\tvalues.back() = static_cast(controls::ExposureTimeModeManual);\n> -\n> -\t\tinfo = ControlInfo{Span{values}, values[0]};\n> +\n> +\t\tstd::bitset<\n> +\t\t\tstd::max(V4L2_EXPOSURE_AUTO,\n> +\t\t\tstd::max(V4L2_EXPOSURE_APERTURE_PRIORITY,\n> +\t\t\tstd::max(V4L2_EXPOSURE_MANUAL,\n> +\t\t\t\t V4L2_EXPOSURE_SHUTTER_PRIORITY))) + 1\n> +\t\t> exposureModes;\n> +\n> +\t\tfor (const ControlValue &value : v4l2Values) {\n> +\t\t\tauto x = value.get();\n> +\n> +\t\t\tif (0 <= x && static_cast(x) < exposureModes.size())\n> +\t\t\t\texposureModes[x] = true;\n> +\t\t}\n> +\n> +\t\tif (exposureModes[V4L2_EXPOSURE_AUTO])\n> +\t\t\tautoExposureMode_ = V4L2_EXPOSURE_AUTO;\n> +\t\telse if (exposureModes[V4L2_EXPOSURE_APERTURE_PRIORITY])\n> +\t\t\tautoExposureMode_ = V4L2_EXPOSURE_APERTURE_PRIORITY;\n\nAs we control exposure time and analogue gain modes separately, to\nactually implement V4L2_EXPOSURE_APERTURE_PRIORITY (or\nV4L2_EXPOSURE_SHUTTER_PRIORITY) one would have to match the right\nExposureTimeMode and AnalogueGainMode combinations (auto exp + manual\ngain = APERTURE_PRIORITY, manual exp + auto gain = SHUTTER_PRIORITY).\n\nI'm not sure how many uvc devices support this advanced combinations,\nand this is not worse than before for sure.\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\n> +\t\tif (exposureModes[V4L2_EXPOSURE_MANUAL])\n> +\t\t\tmanualExposureMode_ = V4L2_EXPOSURE_MANUAL;\n> +\t\telse if (exposureModes[V4L2_EXPOSURE_SHUTTER_PRIORITY])\n> +\t\t\tmanualExposureMode_ = V4L2_EXPOSURE_SHUTTER_PRIORITY;\n> +\n> +\t\tstd::array values;\n> +\t\tstd::size_t count = 0;\n> +\n> +\t\tif (autoExposureMode_)\n> +\t\t\tvalues[count++] = controls::ExposureTimeModeAuto;\n> +\n> +\t\tif (manualExposureMode_)\n> +\t\t\tvalues[count++] = controls::ExposureTimeModeManual;\n> +\n> +\t\tif (count == 0)\n> +\t\t\treturn;\n> +\n> +\t\tinfo = ControlInfo{ Span{ values.data(), count }, values[0] };\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> \t\tbreak;\n> \t}\n> \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> --\n> 2.48.1\n>","headers":{"Return-Path":"","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 C7AEEC32DC\n\tfor ;\n\tTue, 4 Mar 2025 09:49:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C86F168777;\n\tTue, 4 Mar 2025 10:49:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7AD368754\n\tfor ;\n\tTue, 4 Mar 2025 10:49:11 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A381E11D9;\n\tTue, 4 Mar 2025 10:47:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bEq7OkHU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1741081659;\n\tbh=0j0EeMvrUgdTVPJyPHxdp0rCoacuWB9C/1uE3eN1T54=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bEq7OkHUVUjJJPSpHtrkUG+KeLHBwuh01IqP2QQ+1shnUVA91U9gVqIfG/2l7aCGM\n\tGh3i5Kx+/QzPsAggIL+1OL37387uDXAmksXC3TxtL/faB6pHzrjGpkw4WtIlqM/QsK\n\tLJBcc0y79UhOfmCtgeFd+NZDCp2k1AtnamFKdxng=","Date":"Tue, 4 Mar 2025 10:49:08 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 1/2] libcamera: pipeline: uvcvideo: Fix\n\t`ExposureTimeMode` control setup","Message-ID":"","References":"<20250303134234.699293-1-barnabas.pocze@ideasonboard.com>\n\t<20250303134234.699293-2-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250303134234.699293-2-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]