[{"id":33680,"web_url":"https://patchwork.libcamera.org/comment/33680/","msgid":"<5qbrkunzblepxs3beviz4xqqvfo4ndch4g4a347w4owjlxp6yp@ucifzswmtclo>","date":"2025-03-21T14:17:37","subject":"Re: [PATCH v3 4/4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","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 Fri, Mar 14, 2025 at 06:42:48PM +0100, Barnabás Pőcze wrote:\n> The documentation of `ExposureTime` states that its value is ignored if\n> `ExposureTimeMode` is not manual. This is currently not handled, and if\n> `ExposureTimeMode` is automatic and `ExposureTime` is set, then the\n> controls will be rejected, as expected.\n>\n> Only try to set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode\n> is manual. To be able to handle requests that set both `ExposureTime{,Mode}`,\n> process `ExposureTimeMode` first directly in `processControls()`, and store\n> this new mode in a temporary location. Then have `processControl()` act on\n> this temporary state, and only persist the temporary state if `setControls()`\n> on the video device succeeds.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=242\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 52 ++++++++++++--------\n> 1 file changed, 31 insertions(+), 21 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 7d882ebe1..22d286e49 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -108,7 +108,7 @@ public:\n> \tbool match(DeviceEnumerator *enumerator) override;\n>\n> private:\n> -\tint processControl(const UVCCameraData *data, ControlList *controls,\n> +\tint processControl(const UVCCameraData::State &state, ControlList *controls,\n> \t\t\t unsigned int id, const ControlValue &value);\n> \tint processControls(UVCCameraData *data, Request *request);\n>\n> @@ -333,7 +333,7 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n> \tdata->state_.reset();\n> }\n>\n> -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n> +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n> \t\t\t\t unsigned int id, const ControlValue &value)\n> {\n> \tuint32_t cid;\n> @@ -378,26 +378,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> \t}\n>\n> \tcase V4L2_CID_EXPOSURE_AUTO: {\n> -\t\tstd::optional mode;\n> -\n> -\t\tswitch (value.get()) {\n> -\t\tcase controls::ExposureTimeModeAuto:\n> -\t\t\tmode = data->autoExposureMode_;\n> -\t\t\tbreak;\n> -\t\tcase controls::ExposureTimeModeManual:\n> -\t\t\tmode = data->manualExposureMode_;\n> -\t\t\tbreak;\n> -\t\t}\n> -\n> -\t\tif (!mode)\n> -\t\t\treturn -EINVAL;\n> -\n> -\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> +\t\t/* Handled directly in `processControls()`. */\n> \t\tbreak;\n> \t}\n>\n> \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> -\t\tcontrols->set(cid, value.get() / 100);\n> +\t\tif (state.exp == controls::ExposureTimeModeManual)\n> +\t\t\tcontrols->set(cid, value.get() / 100);\n> \t\tbreak;\n>\n> \tcase V4L2_CID_CONTRAST:\n> @@ -428,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> {\n> \tControlList controls(data->video_->controls());\n> +\tconst auto &reqControls = request->controls();\n> +\tauto newState = data->state_;\n> +\n> +\tif (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n> +\t\tstd::optional mode;\n>\n> -\tfor (const auto &[id, value] : request->controls())\n> -\t\tprocessControl(data, &controls, id, value);\n> +\t\tswitch (*exp) {\n> +\t\tcase controls::ExposureTimeModeAuto:\n> +\t\t\tmode = data->autoExposureMode_;\n> +\t\t\tbreak;\n> +\t\tcase controls::ExposureTimeModeManual:\n> +\t\t\tmode = data->manualExposureMode_;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (!mode)\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tcontrols.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> +\t\tnewState.exp = static_cast(*exp);\n\nCan't you just update data->state before calling processControl to\navoid passing newState to that function ?\n\n> +\t}\n> +\n> +\tfor (const auto &[id, value] : reqControls)\n> +\t\tprocessControl(newState, &controls, id, value);\n>\n> \tfor (const auto &ctrl : controls)\n> \t\tLOG(UVC, Debug)\n> @@ -443,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> \t\treturn ret < 0 ? ret : -EINVAL;\n> \t}\n>\n> -\treturn ret;\n> +\tdata->state_ = newState;\n> +\n> +\treturn 0;\n> }\n>\n> int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\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 8F8F4C3272\n\tfor ;\n\tFri, 21 Mar 2025 14:17:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B630568951;\n\tFri, 21 Mar 2025 15:17:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 85E3268947\n\tfor ;\n\tFri, 21 Mar 2025 15:17:41 +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 19EF1465;\n\tFri, 21 Mar 2025 15:15:57 +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=\"UfjW2ZEO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742566557;\n\tbh=amw8b74rt8OD1vdekg2OoUDIosq0RIBjiWnU7jIPc3Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UfjW2ZEOYZxAep44L94KTSExti2tQgHxCHW6DLxRKGCG2yhoCP8lMsVyLz/433iru\n\tvL5ue/bclyMOG0Y6CfGr9cxsBjnlbN6zyJwOA9mkob7ObM1oNUINhWP9POns7ZyXbe\n\txJmsyIEN+on2t7AI8gVlH74u2uwtX7YhWZKM7lUI=","Date":"Fri, 21 Mar 2025 15:17:37 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 4/4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","Message-ID":"<5qbrkunzblepxs3beviz4xqqvfo4ndch4g4a347w4owjlxp6yp@ucifzswmtclo>","References":"<20250314174248.1015718-1-barnabas.pocze@ideasonboard.com>\n\t<20250314174248.1015718-5-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":"<20250314174248.1015718-5-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\" "}},{"id":33681,"web_url":"https://patchwork.libcamera.org/comment/33681/","msgid":"","date":"2025-03-21T14:33:59","subject":"Re: [PATCH v3 4/4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 03. 21. 15:17 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Fri, Mar 14, 2025 at 06:42:48PM +0100, Barnabás Pőcze wrote:\n>> The documentation of `ExposureTime` states that its value is ignored if\n>> `ExposureTimeMode` is not manual. This is currently not handled, and if\n>> `ExposureTimeMode` is automatic and `ExposureTime` is set, then the\n>> controls will be rejected, as expected.\n>>\n>> Only try to set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode\n>> is manual. To be able to handle requests that set both `ExposureTime{,Mode}`,\n>> process `ExposureTimeMode` first directly in `processControls()`, and store\n>> this new mode in a temporary location. Then have `processControl()` act on\n>> this temporary state, and only persist the temporary state if `setControls()`\n>> on the video device succeeds.\n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=242\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 52 ++++++++++++--------\n>> 1 file changed, 31 insertions(+), 21 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> index 7d882ebe1..22d286e49 100644\n>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> @@ -108,7 +108,7 @@ public:\n>> \tbool match(DeviceEnumerator *enumerator) override;\n>>\n>> private:\n>> -\tint processControl(const UVCCameraData *data, ControlList *controls,\n>> +\tint processControl(const UVCCameraData::State &state, ControlList *controls,\n>> \t\t\t unsigned int id, const ControlValue &value);\n>> \tint processControls(UVCCameraData *data, Request *request);\n>>\n>> @@ -333,7 +333,7 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n>> \tdata->state_.reset();\n>> }\n>>\n>> -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n>> +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n>> \t\t\t\t unsigned int id, const ControlValue &value)\n>> {\n>> \tuint32_t cid;\n>> @@ -378,26 +378,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n>> \t}\n>>\n>> \tcase V4L2_CID_EXPOSURE_AUTO: {\n>> -\t\tstd::optional mode;\n>> -\n>> -\t\tswitch (value.get()) {\n>> -\t\tcase controls::ExposureTimeModeAuto:\n>> -\t\t\tmode = data->autoExposureMode_;\n>> -\t\t\tbreak;\n>> -\t\tcase controls::ExposureTimeModeManual:\n>> -\t\t\tmode = data->manualExposureMode_;\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\n>> -\t\tif (!mode)\n>> -\t\t\treturn -EINVAL;\n>> -\n>> -\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n>> +\t\t/* Handled directly in `processControls()`. */\n>> \t\tbreak;\n>> \t}\n>>\n>> \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n>> -\t\tcontrols->set(cid, value.get() / 100);\n>> +\t\tif (state.exp == controls::ExposureTimeModeManual)\n>> +\t\t\tcontrols->set(cid, value.get() / 100);\n>> \t\tbreak;\n>>\n>> \tcase V4L2_CID_CONTRAST:\n>> @@ -428,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n>> int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>> {\n>> \tControlList controls(data->video_->controls());\n>> +\tconst auto &reqControls = request->controls();\n>> +\tauto newState = data->state_;\n>> +\n>> +\tif (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n>> +\t\tstd::optional mode;\n>>\n>> -\tfor (const auto &[id, value] : request->controls())\n>> -\t\tprocessControl(data, &controls, id, value);\n>> +\t\tswitch (*exp) {\n>> +\t\tcase controls::ExposureTimeModeAuto:\n>> +\t\t\tmode = data->autoExposureMode_;\n>> +\t\t\tbreak;\n>> +\t\tcase controls::ExposureTimeModeManual:\n>> +\t\t\tmode = data->manualExposureMode_;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\n>> +\t\tif (!mode)\n>> +\t\t\treturn -EINVAL;\n>> +\n>> +\t\tcontrols.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n>> +\t\tnewState.exp = static_cast(*exp);\n> \n> Can't you just update data->state before calling processControl to\n> avoid passing newState to that function ?\n\nIf the controls cannot be set, then I think the state should not be changed, this\nis why it operates on a copy, which is only persisted after `setControls()` succeeds.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> +\t}\n>> +\n>> +\tfor (const auto &[id, value] : reqControls)\n>> +\t\tprocessControl(newState, &controls, id, value);\n>>\n>> \tfor (const auto &ctrl : controls)\n>> \t\tLOG(UVC, Debug)\n>> @@ -443,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>> \t\treturn ret < 0 ? ret : -EINVAL;\n>> \t}\n>>\n>> -\treturn ret;\n>> +\tdata->state_ = newState;\n>> +\n>> +\treturn 0;\n>> }\n>>\n>> int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\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 9C411C3274\n\tfor ;\n\tFri, 21 Mar 2025 14:34:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7A2E16894B;\n\tFri, 21 Mar 2025 15:34:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18D8668947\n\tfor ;\n\tFri, 21 Mar 2025 15:34:03 +0100 (CET)","from [192.168.33.18] (185.221.143.221.nat.pool.zt.hu\n\t[185.221.143.221])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 78E8E465;\n\tFri, 21 Mar 2025 15:32:18 +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=\"TmG1PWdv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742567538;\n\tbh=F8n36ehE/rJqp6BOLyeSvuL2UEZ2e+4Uva6lqrdajR8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TmG1PWdvpvycnyFjzYXRBwOUAXTFc1LAz+W3AmL5xttpVkpMBjWTCYskUPoaGNfJm\n\twV7DGsevGGBER6ySbniIKlcGMQwMy8aIEgONnn/gYuIcSL5xAPR+LRv6PU/Fdx8iZw\n\tXvZBOK9nVGeUvWA2egrBeRA9XTDVo+o5aEJy8p1E=","Message-ID":"","Date":"Fri, 21 Mar 2025 15:33:59 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 4/4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250314174248.1015718-1-barnabas.pocze@ideasonboard.com>\n\t<20250314174248.1015718-5-barnabas.pocze@ideasonboard.com>\n\t<5qbrkunzblepxs3beviz4xqqvfo4ndch4g4a347w4owjlxp6yp@ucifzswmtclo>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<5qbrkunzblepxs3beviz4xqqvfo4ndch4g4a347w4owjlxp6yp@ucifzswmtclo>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":33683,"web_url":"https://patchwork.libcamera.org/comment/33683/","msgid":"","date":"2025-03-21T14:41:45","subject":"Re: [PATCH v3 4/4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Fri, Mar 21, 2025 at 03:33:59PM +0100, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 03. 21. 15:17 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Fri, Mar 14, 2025 at 06:42:48PM +0100, Barnabás Pőcze wrote:\n> > > The documentation of `ExposureTime` states that its value is ignored if\n> > > `ExposureTimeMode` is not manual. This is currently not handled, and if\n> > > `ExposureTimeMode` is automatic and `ExposureTime` is set, then the\n> > > controls will be rejected, as expected.\n> > >\n> > > Only try to set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode\n> > > is manual. To be able to handle requests that set both `ExposureTime{,Mode}`,\n> > > process `ExposureTimeMode` first directly in `processControls()`, and store\n> > > this new mode in a temporary location. Then have `processControl()` act on\n> > > this temporary state, and only persist the temporary state if `setControls()`\n> > > on the video device succeeds.\n> > >\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=242\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 52 ++++++++++++--------\n> > > 1 file changed, 31 insertions(+), 21 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 7d882ebe1..22d286e49 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -108,7 +108,7 @@ public:\n> > > \tbool match(DeviceEnumerator *enumerator) override;\n> > >\n> > > private:\n> > > -\tint processControl(const UVCCameraData *data, ControlList *controls,\n> > > +\tint processControl(const UVCCameraData::State &state, ControlList *controls,\n> > > \t\t\t unsigned int id, const ControlValue &value);\n> > > \tint processControls(UVCCameraData *data, Request *request);\n> > >\n> > > @@ -333,7 +333,7 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n> > > \tdata->state_.reset();\n> > > }\n> > >\n> > > -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n> > > +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n> > > \t\t\t\t unsigned int id, const ControlValue &value)\n> > > {\n> > > \tuint32_t cid;\n> > > @@ -378,26 +378,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> > > \t}\n> > >\n> > > \tcase V4L2_CID_EXPOSURE_AUTO: {\n> > > -\t\tstd::optional mode;\n> > > -\n> > > -\t\tswitch (value.get()) {\n> > > -\t\tcase controls::ExposureTimeModeAuto:\n> > > -\t\t\tmode = data->autoExposureMode_;\n> > > -\t\t\tbreak;\n> > > -\t\tcase controls::ExposureTimeModeManual:\n> > > -\t\t\tmode = data->manualExposureMode_;\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\n> > > -\t\tif (!mode)\n> > > -\t\t\treturn -EINVAL;\n> > > -\n> > > -\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> > > +\t\t/* Handled directly in `processControls()`. */\n> > > \t\tbreak;\n> > > \t}\n> > >\n> > > \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> > > -\t\tcontrols->set(cid, value.get() / 100);\n> > > +\t\tif (state.exp == controls::ExposureTimeModeManual)\n> > > +\t\t\tcontrols->set(cid, value.get() / 100);\n> > > \t\tbreak;\n> > >\n> > > \tcase V4L2_CID_CONTRAST:\n> > > @@ -428,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> > > int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > > {\n> > > \tControlList controls(data->video_->controls());\n> > > +\tconst auto &reqControls = request->controls();\n> > > +\tauto newState = data->state_;\n> > > +\n> > > +\tif (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n> > > +\t\tstd::optional mode;\n> > >\n> > > -\tfor (const auto &[id, value] : request->controls())\n> > > -\t\tprocessControl(data, &controls, id, value);\n> > > +\t\tswitch (*exp) {\n> > > +\t\tcase controls::ExposureTimeModeAuto:\n> > > +\t\t\tmode = data->autoExposureMode_;\n> > > +\t\t\tbreak;\n> > > +\t\tcase controls::ExposureTimeModeManual:\n> > > +\t\t\tmode = data->manualExposureMode_;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (!mode)\n> > > +\t\t\treturn -EINVAL;\n> > > +\n> > > +\t\tcontrols.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> > > +\t\tnewState.exp = static_cast(*exp);\n> >\n> > Can't you just update data->state before calling processControl to\n> > avoid passing newState to that function ?\n>\n> If the controls cannot be set, then I think the state should not be changed, this\n> is why it operates on a copy, which is only persisted after `setControls()` succeeds.\n\nWhich control ? Even if ABSOLUTE cannot be set (because we're in AUTO)\nif the mode is changed, it should anyway be\n\nAh you mean to protect against a failure in\n\n\tint ret = data->video_->setControls(&controls);\n\n?\n\nThen yes, maybe keep the state update after this makes sense\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n> >\n> > > +\t}\n> > > +\n> > > +\tfor (const auto &[id, value] : reqControls)\n> > > +\t\tprocessControl(newState, &controls, id, value);\n> > >\n> > > \tfor (const auto &ctrl : controls)\n> > > \t\tLOG(UVC, Debug)\n> > > @@ -443,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > > \t\treturn ret < 0 ? ret : -EINVAL;\n> > > \t}\n> > >\n> > > -\treturn ret;\n> > > +\tdata->state_ = newState;\n> > > +\n> > > +\treturn 0;\n> > > }\n> > >\n> > > int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\n> > > --\n> > > 2.48.1\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 E0541C3274\n\tfor ;\n\tFri, 21 Mar 2025 14:42:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0EFB668951;\n\tFri, 21 Mar 2025 15:42:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04D8A68947\n\tfor ;\n\tFri, 21 Mar 2025 15:42:06 +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 A9E62465;\n\tFri, 21 Mar 2025 15:40:17 +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=\"hGsBL/P3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742568021;\n\tbh=EsrqX5pBlvtCCOOYhj16LvaKND98YQ8apVoDB1cmKYs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hGsBL/P3dq21tESqX0O87OrNDQCcSlu+Zb10wH0bnj10lcWXAW4l8N/IZwAHrVU8g\n\tmOTvpI8l/5lUj09rq/jymt+vCV6EXV4SyBcCr5erqzg9x+5d1L16QaT2yvMaKjVCBv\n\tOrcTEsQTcK49ckpoUGko30Z/eCF5Jn/nJkAoV39I=","Date":"Fri, 21 Mar 2025 15:41:45 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 4/4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","Message-ID":"","References":"<20250314174248.1015718-1-barnabas.pocze@ideasonboard.com>\n\t<20250314174248.1015718-5-barnabas.pocze@ideasonboard.com>\n\t<5qbrkunzblepxs3beviz4xqqvfo4ndch4g4a347w4owjlxp6yp@ucifzswmtclo>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}}]