[{"id":33145,"web_url":"https://patchwork.libcamera.org/comment/33145/","msgid":"<1qps3iEd3cgRnZe20EC79ySlBA755G2UFqZuG3gzBZu9jzB6zA3-MRHvPqQ_W1rUdbpH5UfWJLwcb3w78TT82RRmPsyC9tTrKAJCH_h5KHw=@protonmail.com>","date":"2025-01-23T15:57:40","subject":"Re: [PATCH v9 03/12] libcamera: uvcvideo: Register ExposureTimeMode\n\tcontrol","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2025. január 20., hétfő 21:44 keltezéssel, Laurent Pinchart írta:\n\n> From: Jacopo Mondi \n> \n> Port the UVC pipeline handler to use the new ExposureTimeMode control\n> when processing Camera controls in place of the AeEnable control.\n> \n> The V4L2_CID_EXPOSURE_AUTO control allows 4 possible values, which\n> map to ExposureTimeModeAuto and ExposureTimeModeManual.\n> \n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Paul Elder \n> Reviewed-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 53 ++++++++++++++++++--\n> 1 file changed, 48 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 8c2c6baf3575..7470b56270f6 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -298,7 +298,7 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> \t\tcid = V4L2_CID_CONTRAST;\n> \telse if (id == controls::Saturation)\n> \t\tcid = V4L2_CID_SATURATION;\n> -\telse if (id == controls::AeEnable)\n> +\telse if (id == controls::ExposureTimeMode)\n> \t\tcid = V4L2_CID_EXPOSURE_AUTO;\n> \telse if (id == controls::ExposureTime)\n> \t\tcid = V4L2_CID_EXPOSURE_ABSOLUTE;\n> @@ -647,7 +647,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> \t\tid = &controls::Saturation;\n> \t\tbreak;\n> \tcase V4L2_CID_EXPOSURE_AUTO:\n> -\t\tid = &controls::AeEnable;\n> +\t\tid = &controls::ExposureTimeMode;\n> \t\tbreak;\n> \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> \t\tid = &controls::ExposureTime;\n> @@ -660,6 +660,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> \t}\n> \n> \t/* Map the control info. */\n> +\tconst std::vector &v4l2Values = v4l2Info.values();\n> \tint32_t min = v4l2Info.min().get();\n> \tint32_t max = v4l2Info.max().get();\n> \tint32_t def = v4l2Info.def().get();\n> @@ -697,10 +698,52 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> \t\t};\n> \t\tbreak;\n> \n> -\tcase V4L2_CID_EXPOSURE_AUTO:\n> -\t\tinfo = ControlInfo{ false, true, true };\n> -\t\tbreak;\n> +\tcase V4L2_CID_EXPOSURE_AUTO: {\n> +\t\t/*\n> +\t\t * From the V4L2_CID_EXPOSURE_AUTO documentation:\n> +\t\t *\n> +\t\t * ------------------------------------------------------------\n> +\t\t * V4L2_EXPOSURE_AUTO:\n> +\t\t * Automatic exposure time, automatic iris aperture.\n> +\t\t *\n> +\t\t * V4L2_EXPOSURE_MANUAL:\n> +\t\t * Manual exposure time, manual iris.\n> +\t\t *\n> +\t\t * V4L2_EXPOSURE_SHUTTER_PRIORITY:\n> +\t\t * Manual exposure time, auto iris.\n> +\t\t *\n> +\t\t * V4L2_EXPOSURE_APERTURE_PRIORITY:\n> +\t\t * Auto exposure time, manual iris.\n> +\t\t *-------------------------------------------------------------\n> +\t\t *\n> +\t\t * ExposureTimeModeAuto = { V4L2_EXPOSURE_AUTO,\n> +\t\t * \t\t\t V4L2_EXPOSURE_APERTURE_PRIORITY }\n> +\t\t *\n> +\t\t *\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\nI think this does not call the intended constructor. Shouldn't it be something like this?\n\n@@ -725,7 +724,8 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n * ExposureTimeModeManual = { V4L2_EXPOSURE_MANUAL,\n * V4L2_EXPOSURE_SHUTTER_PRIORITY }\n */\n- std::array values{};\n+ std::array values{};\n+ std::size_t i = 0;\n \n auto it = std::find_if(v4l2Values.begin(), v4l2Values.end(),\n [&](const ControlValue &val) {\n@@ -733,7 +733,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n val.get() == V4L2_EXPOSURE_AUTO) ? true : false;\n });\n if (it != v4l2Values.end())\n- values.back() = static_cast(controls::ExposureTimeModeAuto);\n+ values[i++] = static_cast(controls::ExposureTimeModeAuto);\n \n it = std::find_if(v4l2Values.begin(), v4l2Values.end(),\n [&](const ControlValue &val) {\n@@ -741,9 +741,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n val.get() == V4L2_EXPOSURE_MANUAL) ? true : false;\n });\n if (it != v4l2Values.end())\n- values.back() = static_cast(controls::ExposureTimeModeManual);\n+ values[i++] = static_cast(controls::ExposureTimeModeManual);\n \n- info = ControlInfo{Span{values}, values[0]};\n+ info = ControlInfo{Span{values.data(), i}, values[0]};\n break;\n }\n case V4L2_CID_EXPOSURE_ABSOLUTE:\n\nI think `PipelineHandlerUVC::processControl()` is also problematic:\n\n\tcase V4L2_CID_EXPOSURE_AUTO: {\n\t\tint32_t ivalue = value.get()\n\t\t\t ? V4L2_EXPOSURE_APERTURE_PRIORITY\n\t\t\t : V4L2_EXPOSURE_MANUAL;\n\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, ivalue);\n\t\tbreak;\n\t}\n\nThe control's type is no longer boolean, no?\n\n\nRegards,\nBarnabás Pőcze\n\n> +\t\tbreak;\n> +\t}\n> \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> \t\t/*\n> \t\t * ExposureTime is in units of 1 µs, and UVC expects\n> --\n> Regards,\n> \n> Laurent Pinchart\n> \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 C9D6CBDE6B\n\tfor ;\n\tThu, 23 Jan 2025 15:57:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 064CF6855B;\n\tThu, 23 Jan 2025 16:57:48 +0100 (CET)","from mail-10628.protonmail.ch (mail-10628.protonmail.ch\n\t[79.135.106.28])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BDF2361878\n\tfor ;\n\tThu, 23 Jan 2025 16:57:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"lLUjM/Hn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1737647865; x=1737907065;\n\tbh=rVhbgXH0fvBZvY234x4FJQD+WLWLpi2u7FfWbALbmQM=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=lLUjM/Hn33D8t3EycNs6wptGPfPtrfVyVNDZiuhJCzkZXfWNZXhXyspVT2zryzgkp\n\tPieG7p2+fFDPBbvDtSpKsAxkH5vGnmcrcZQoX+ixUt1CczFaa7Hz8+XYKnL+OzrR+F\n\tbvoLOxnFpSfGouivCoYhsgcoLBGbPY2rpEbR6xAULDILvcxeRY1vFR/YDKvfz1vxCV\n\tEGjio+DhobtGhZ8/BNQvFVN0iiXkf6eSX9M0xx5aQPtuf47hte7wLU9vdyxP2MgjLC\n\tKRSMKpRjRucbyDMfxUoJeladERMX82swq99Wb5R5vBcyrQDBrDe1aJSkMoyFxPlAFm\n\t5wqnQZZivf4OQ==","Date":"Thu, 23 Jan 2025 15:57:40 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNaushir Patuck , \n\tDavid Plowman ,\n\tPaul Elder ,\n\tJacopo Mondi ","Subject":"Re: [PATCH v9 03/12] libcamera: uvcvideo: Register ExposureTimeMode\n\tcontrol","Message-ID":"<1qps3iEd3cgRnZe20EC79ySlBA755G2UFqZuG3gzBZu9jzB6zA3-MRHvPqQ_W1rUdbpH5UfWJLwcb3w78TT82RRmPsyC9tTrKAJCH_h5KHw=@protonmail.com>","In-Reply-To":"<20250120204515.24096-4-laurent.pinchart@ideasonboard.com>","References":"<20250120204515.24096-1-laurent.pinchart@ideasonboard.com>\n\t<20250120204515.24096-4-laurent.pinchart@ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"bf6fde1c3432c369ad4c44ec5d9c9295d396a0de","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}}]