[{"id":34992,"web_url":"https://patchwork.libcamera.org/comment/34992/","msgid":"<175312444966.50296.8634540821801476411@ping.linuxembedded.co.uk>","date":"2025-07-21T19:00:49","subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-05-06 16:53:14)\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> To fix that, keep track of the current value of `ExposureTimeMode`, and\n> only set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode is\n> manual.\n> \n> However, the current state of `V4L2_CID_EXPOSURE_AUTO` is not retrieved\n> when streaming starts, thus if the user does not set `ExposureTimeMode`\n> then it will remain in an \"unset\" state and all attempts to set\n> `ExposureTime` will be ignored. This is done for two reasons:\n> \n> * the current values of controls cannot be retrieved with libcamera,\n> therefore this unset state cannot be observed by the user;\n> * the user will have to set `ExposureTimeMode` either way if they\n> want to guarantee that setting `ExposureTime` will have any effect.\n> \n> If `ExposureTimeMode` is not exposed by the camera, but `ExposeTime` is\n> supported, then the exposure mode is assumed to be manual to allow\n> changing `V4L2_CID_EXPOSURE_ABSOLUTE`.\n> \n> To be able to handle requests that set both `ExposureTime{,Mode}`, process\n> `ExposureTimeMode` first directly in `processControls()`, and store this\n> new mode in a temporary location. Then have `processControl()` act on this\n> 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> changes in v4:\n> * first two patches of v3 already merged\n> * last two patches of v3 combined into this single patch\n> * do not retrieve V4L2_CID_EXPOSURE_AUTO when streaming starts\n> \n> changes in v3:\n> * fix default value: set it based on the v4l2 default\n> * store current value of `ExposureTimeMode`\n> * fix https://bugs.libcamera.org/show_bug.cgi?id=242 as well\n> \n> changes in v2:\n> * split into two patches\n> * make it compile with old libstdc++ when _GLIBCXX_DEBUG is set\n> \n> v3: https://patchwork.libcamera.org/patch/22959/\n> https://patchwork.libcamera.org/patch/22958/\n> v2: https://patchwork.libcamera.org/project/libcamera/list/?series=5006\n> v1: https://patchwork.libcamera.org/project/libcamera/list/?series=4974\n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 67 ++++++++++++++------\n> 1 file changed, 46 insertions(+), 21 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 586e932d2..af0c8dedd 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -53,6 +53,8 @@ public:\n> \n> const std::string &id() const { return id_; }\n> \n> + bool supportsExposureTimeMode() const { return autoExposureMode_ || manualExposureMode_; }\n> +\n> Mutex openLock_;\n> std::unique_ptr video_;\n> Stream stream_;\n> @@ -61,6 +63,15 @@ public:\n> std::optional autoExposureMode_;\n> std::optional manualExposureMode_;\n> \n> + struct State {\n> + std::optional exp;\n> +\n> + void reset()\n> + {\n> + exp.reset();\n> + }\n> + } state_ = {};\n\nDoesn't std::optional already support .reset() [0] ? So I can't see benefit\nto adding this extra State wrapper ?\n\n[0] https://en.cppreference.com/w/cpp/utility/optional/reset.html\n\nOr is it that you anticipate extra modes to be tracked in this state ?\n\n> +\n> private:\n> bool generateId();\n> \n> @@ -98,7 +109,7 @@ public:\n> bool match(DeviceEnumerator *enumerator) override;\n> \n> private:\n> - int processControl(const UVCCameraData *data, ControlList *controls,\n> + int processControl(const UVCCameraData::State &state, ControlList *controls,\n\nOh! I see - passing State makes much more sense.\n\nIgnore the above then ;-)\n\nDoes this interact ok with the AeEnable handling that occurs in\nsrc/libcamera/camera.cpp?\n\n> unsigned int id, const ControlValue &value);\n> int processControls(UVCCameraData *data, Request *request);\n> \n> @@ -302,6 +313,9 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n> return ret;\n> }\n> \n> + if (!data->supportsExposureTimeMode())\n> + data->state_.exp = controls::ExposureTimeModeManual;\n> +\n> return 0;\n> }\n> \n> @@ -310,9 +324,10 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n> UVCCameraData *data = cameraData(camera);\n> data->video_->streamOff();\n> data->video_->releaseBuffers();\n> + data->state_.reset();\n> }\n> \n> -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n> +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n> unsigned int id, const ControlValue &value)\n> {\n> uint32_t cid;\n> @@ -359,26 +374,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> }\n> \n> case V4L2_CID_EXPOSURE_AUTO: {\n> - std::optional mode;\n> -\n> - switch (value.get()) {\n> - case controls::ExposureTimeModeAuto:\n> - mode = data->autoExposureMode_;\n> - break;\n> - case controls::ExposureTimeModeManual:\n> - mode = data->manualExposureMode_;\n> - break;\n> - }\n> -\n> - if (!mode)\n> - return -EINVAL;\n> -\n> - controls->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> + /* Handled directly in `processControls()`. */\n> break;\n> }\n> \n> case V4L2_CID_EXPOSURE_ABSOLUTE:\n> - controls->set(cid, value.get() / 100);\n> + if (state.exp == controls::ExposureTimeModeManual)\n> + controls->set(cid, value.get() / 100);\n> break;\n> \n> case V4L2_CID_CONTRAST:\n> @@ -413,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> {\n> ControlList controls(data->video_->controls());\n> + const auto &reqControls = request->controls();\n> + auto newState = data->state_;\n> \n> - for (const auto &[id, value] : request->controls())\n> - processControl(data, &controls, id, value);\n> + if (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n> + std::optional mode;\n> +\n> + switch (*exp) {\n> + case controls::ExposureTimeModeAuto:\n> + mode = data->autoExposureMode_;\n> + break;\n> + case controls::ExposureTimeModeManual:\n> + mode = data->manualExposureMode_;\n> + break;\n> + }\n> +\n> + if (!mode)\n> + return -EINVAL;\n> +\n> + controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> + newState.exp = static_cast(*exp);\n> + }\n> +\n> + for (const auto &[id, value] : reqControls)\n> + processControl(newState, &controls, id, value);\n> \n> for (const auto &ctrl : controls)\n> LOG(UVC, Debug)\n> @@ -428,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> return ret < 0 ? ret : -EINVAL;\n> }\n> \n> - return ret;\n> + data->state_ = newState;\n> +\n> + return 0;\n> }\n> \n> int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\n> --\n> 2.49.0","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 E4E90BDCC1\n\tfor ;\n\tMon, 21 Jul 2025 19:00:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 236146900E;\n\tMon, 21 Jul 2025 21:00:54 +0200 (CEST)","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 BA39668FB1\n\tfor ;\n\tMon, 21 Jul 2025 21:00:52 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 916917917;\n\tMon, 21 Jul 2025 21:00:15 +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=\"raQdz8tw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753124415;\n\tbh=axzgQGmrh26ZcLd11iqU2y7hZFAYP4xwMoOxEKBEO9E=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=raQdz8twqbON3SPDCgXi7MBcz+1GLXsbaXD1vPBOaFgOqZQvP7wsqGYcXxFb+OKNL\n\tBe9wVlGUKoUw6QqOup35Aatd/+BaT/BLkU3MT47Vhl5BrD9AolzklrkaK2Ru6c74J+\n\tKwNdXbMQdybbUFaXnz0bVfYcczCTXWIBcfROew/g=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250506155314.1544104-1-barnabas.pocze@ideasonboard.com>","References":"<20250506155314.1544104-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 21 Jul 2025 20:00:49 +0100","Message-ID":"<175312444966.50296.8634540821801476411@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":35099,"web_url":"https://patchwork.libcamera.org/comment/35099/","msgid":"<20250724114512.GJ11202@pendragon.ideasonboard.com>","date":"2025-07-24T11:45:12","subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jul 21, 2025 at 08:00:49PM +0100, Kieran Bingham wrote:\n> Quoting Barnabás Pőcze (2025-05-06 16:53:14)\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> > To fix that, keep track of the current value of `ExposureTimeMode`, and\n> > only set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode is\n> > manual.\n> > \n> > However, the current state of `V4L2_CID_EXPOSURE_AUTO` is not retrieved\n> > when streaming starts, thus if the user does not set `ExposureTimeMode`\n> > then it will remain in an \"unset\" state and all attempts to set\n> > `ExposureTime` will be ignored. This is done for two reasons:\n> > \n> > * the current values of controls cannot be retrieved with libcamera,\n> > therefore this unset state cannot be observed by the user;\n> > * the user will have to set `ExposureTimeMode` either way if they\n> > want to guarantee that setting `ExposureTime` will have any effect.\n\nShouldn't we actually set all controls to their default value when\nstreaming start ?\n\n> > If `ExposureTimeMode` is not exposed by the camera, but `ExposeTime` is\n> > supported, then the exposure mode is assumed to be manual to allow\n> > changing `V4L2_CID_EXPOSURE_ABSOLUTE`.\n> > \n> > To be able to handle requests that set both `ExposureTime{,Mode}`, process\n> > `ExposureTimeMode` first directly in `processControls()`, and store this\n> > new mode in a temporary location. Then have `processControl()` act on this\n> > 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> > changes in v4:\n> > * first two patches of v3 already merged\n> > * last two patches of v3 combined into this single patch\n> > * do not retrieve V4L2_CID_EXPOSURE_AUTO when streaming starts\n> > \n> > changes in v3:\n> > * fix default value: set it based on the v4l2 default\n> > * store current value of `ExposureTimeMode`\n> > * fix https://bugs.libcamera.org/show_bug.cgi?id=242 as well\n> > \n> > changes in v2:\n> > * split into two patches\n> > * make it compile with old libstdc++ when _GLIBCXX_DEBUG is set\n> > \n> > v3: https://patchwork.libcamera.org/patch/22959/\n> > https://patchwork.libcamera.org/patch/22958/\n> > v2: https://patchwork.libcamera.org/project/libcamera/list/?series=5006\n> > v1: https://patchwork.libcamera.org/project/libcamera/list/?series=4974\n> > ---\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 67 ++++++++++++++------\n> > 1 file changed, 46 insertions(+), 21 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 586e932d2..af0c8dedd 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -53,6 +53,8 @@ public:\n> > \n> > const std::string &id() const { return id_; }\n> > \n> > + bool supportsExposureTimeMode() const { return autoExposureMode_ || manualExposureMode_; }\n> > +\n> > Mutex openLock_;\n> > std::unique_ptr video_;\n> > Stream stream_;\n> > @@ -61,6 +63,15 @@ public:\n> > std::optional autoExposureMode_;\n> > std::optional manualExposureMode_;\n> > \n> > + struct State {\n> > + std::optional exp;\n> > +\n> > + void reset()\n> > + {\n> > + exp.reset();\n> > + }\n> > + } state_ = {};\n> \n> Doesn't std::optional already support .reset() [0] ? So I can't see benefit\n> to adding this extra State wrapper ?\n> \n> [0] https://en.cppreference.com/w/cpp/utility/optional/reset.html\n> \n> Or is it that you anticipate extra modes to be tracked in this state ?\n> \n> > +\n> > private:\n> > bool generateId();\n> > \n> > @@ -98,7 +109,7 @@ public:\n> > bool match(DeviceEnumerator *enumerator) override;\n> > \n> > private:\n> > - int processControl(const UVCCameraData *data, ControlList *controls,\n> > + int processControl(const UVCCameraData::State &state, ControlList *controls,\n> \n> Oh! I see - passing State makes much more sense.\n> \n> Ignore the above then ;-)\n> \n> Does this interact ok with the AeEnable handling that occurs in\n> src/libcamera/camera.cpp?\n> \n> > unsigned int id, const ControlValue &value);\n> > int processControls(UVCCameraData *data, Request *request);\n> > \n> > @@ -302,6 +313,9 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n> > return ret;\n> > }\n> > \n> > + if (!data->supportsExposureTimeMode())\n> > + data->state_.exp = controls::ExposureTimeModeManual;\n> > +\n> > return 0;\n> > }\n> > \n> > @@ -310,9 +324,10 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n> > UVCCameraData *data = cameraData(camera);\n> > data->video_->streamOff();\n> > data->video_->releaseBuffers();\n> > + data->state_.reset();\n> > }\n> > \n> > -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n> > +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n> > unsigned int id, const ControlValue &value)\n> > {\n> > uint32_t cid;\n> > @@ -359,26 +374,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> > }\n> > \n> > case V4L2_CID_EXPOSURE_AUTO: {\n> > - std::optional mode;\n> > -\n> > - switch (value.get()) {\n> > - case controls::ExposureTimeModeAuto:\n> > - mode = data->autoExposureMode_;\n> > - break;\n> > - case controls::ExposureTimeModeManual:\n> > - mode = data->manualExposureMode_;\n> > - break;\n> > - }\n> > -\n> > - if (!mode)\n> > - return -EINVAL;\n> > -\n> > - controls->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> > + /* Handled directly in `processControls()`. */\n> > break;\n> > }\n> > \n> > case V4L2_CID_EXPOSURE_ABSOLUTE:\n> > - controls->set(cid, value.get() / 100);\n> > + if (state.exp == controls::ExposureTimeModeManual)\n> > + controls->set(cid, value.get() / 100);\n> > break;\n> > \n> > case V4L2_CID_CONTRAST:\n> > @@ -413,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> > int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > {\n> > ControlList controls(data->video_->controls());\n> > + const auto &reqControls = request->controls();\n> > + auto newState = data->state_;\n\nPlease use explicit types when possible to increase readability.\n\n> > \n> > - for (const auto &[id, value] : request->controls())\n> > - processControl(data, &controls, id, value);\n\nA comment here to explain why this happens outside of\nPipelineHandlerUVC::processControl() would be useful.\n\n> > + if (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n\nAnd avoid abbreviating too much (including in the State structure).\n\n> > + std::optional mode;\n> > +\n> > + switch (*exp) {\n> > + case controls::ExposureTimeModeAuto:\n> > + mode = data->autoExposureMode_;\n> > + break;\n> > + case controls::ExposureTimeModeManual:\n> > + mode = data->manualExposureMode_;\n> > + break;\n> > + }\n> > +\n> > + if (!mode)\n> > + return -EINVAL;\n> > +\n> > + controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> > + newState.exp = static_cast(*exp);\n> > + }\n> > +\n> > + for (const auto &[id, value] : reqControls)\n> > + processControl(newState, &controls, id, value);\n> > \n> > for (const auto &ctrl : controls)\n> > LOG(UVC, Debug)\n> > @@ -428,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > return ret < 0 ? ret : -EINVAL;\n> > }\n> > \n> > - return ret;\n> > + data->state_ = newState;\n> > +\n> > + return 0;\n> > }\n> > \n> > int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)","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 51A19C3237\n\tfor ;\n\tThu, 24 Jul 2025 11:45:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6DA3D690F6;\n\tThu, 24 Jul 2025 13:45: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 38027690E2\n\tfor ;\n\tThu, 24 Jul 2025 13:45:16 +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 UTF8SMTPSA id DFF56EAE;\n\tThu, 24 Jul 2025 13:44:36 +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=\"odn/1511\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753357477;\n\tbh=nFKO92Tnr3/YkoGUGHFysvuGlrZEjfxv2ZcvgBE01WA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=odn/1511u9F5VROSn4hjfpjA+XmjeQsmGZDxaB9gCUQo+AknTuoNVyk87fNDGTWaS\n\tLTWUWS3LdmBaWOX2GqOQyeDIshkfmxHqb+nYUj6AUEqZi8tZ/1zTZdL9mjuE0yW0l0\n\tlsku3mMKmwzjUthHgqdaWJVTr4Ls3hkTBy+Aw+cw=","Date":"Thu, 24 Jul 2025 14:45:12 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","Message-ID":"<20250724114512.GJ11202@pendragon.ideasonboard.com>","References":"<20250506155314.1544104-1-barnabas.pocze@ideasonboard.com>\n\t<175312444966.50296.8634540821801476411@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<175312444966.50296.8634540821801476411@ping.linuxembedded.co.uk>","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":35103,"web_url":"https://patchwork.libcamera.org/comment/35103/","msgid":"<957cb90c-a9f2-4372-9050-fc59ef13198c@ideasonboard.com>","date":"2025-07-24T12:50:57","subject":"Re: [PATCH v4] 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. 07. 24. 13:45 keltezéssel, Laurent Pinchart írta:\n> On Mon, Jul 21, 2025 at 08:00:49PM +0100, Kieran Bingham wrote:\n>> Quoting Barnabás Pőcze (2025-05-06 16:53:14)\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>>> To fix that, keep track of the current value of `ExposureTimeMode`, and\n>>> only set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode is\n>>> manual.\n>>>\n>>> However, the current state of `V4L2_CID_EXPOSURE_AUTO` is not retrieved\n>>> when streaming starts, thus if the user does not set `ExposureTimeMode`\n>>> then it will remain in an \"unset\" state and all attempts to set\n>>> `ExposureTime` will be ignored. This is done for two reasons:\n>>>\n>>> * the current values of controls cannot be retrieved with libcamera,\n>>> therefore this unset state cannot be observed by the user;\n>>> * the user will have to set `ExposureTimeMode` either way if they\n>>> want to guarantee that setting `ExposureTime` will have any effect.\n> \n> Shouldn't we actually set all controls to their default value when\n> streaming start ?\n\nSee https://bugs.libcamera.org/show_bug.cgi?id=271. It's not clear to me\nwhat the behaviour should be. Setting the default values are some point\nis probably desired.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>>> If `ExposureTimeMode` is not exposed by the camera, but `ExposeTime` is\n>>> supported, then the exposure mode is assumed to be manual to allow\n>>> changing `V4L2_CID_EXPOSURE_ABSOLUTE`.\n>>>\n>>> To be able to handle requests that set both `ExposureTime{,Mode}`, process\n>>> `ExposureTimeMode` first directly in `processControls()`, and store this\n>>> new mode in a temporary location. Then have `processControl()` act on this\n>>> 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>>> changes in v4:\n>>> * first two patches of v3 already merged\n>>> * last two patches of v3 combined into this single patch\n>>> * do not retrieve V4L2_CID_EXPOSURE_AUTO when streaming starts\n>>>\n>>> changes in v3:\n>>> * fix default value: set it based on the v4l2 default\n>>> * store current value of `ExposureTimeMode`\n>>> * fix https://bugs.libcamera.org/show_bug.cgi?id=242 as well\n>>>\n>>> changes in v2:\n>>> * split into two patches\n>>> * make it compile with old libstdc++ when _GLIBCXX_DEBUG is set\n>>>\n>>> v3: https://patchwork.libcamera.org/patch/22959/\n>>> https://patchwork.libcamera.org/patch/22958/\n>>> v2: https://patchwork.libcamera.org/project/libcamera/list/?series=5006\n>>> v1: https://patchwork.libcamera.org/project/libcamera/list/?series=4974\n>>> ---\n>>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 67 ++++++++++++++------\n>>> 1 file changed, 46 insertions(+), 21 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>>> index 586e932d2..af0c8dedd 100644\n>>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>>> @@ -53,6 +53,8 @@ public:\n>>>\n>>> const std::string &id() const { return id_; }\n>>>\n>>> + bool supportsExposureTimeMode() const { return autoExposureMode_ || manualExposureMode_; }\n>>> +\n>>> Mutex openLock_;\n>>> std::unique_ptr video_;\n>>> Stream stream_;\n>>> @@ -61,6 +63,15 @@ public:\n>>> std::optional autoExposureMode_;\n>>> std::optional manualExposureMode_;\n>>>\n>>> + struct State {\n>>> + std::optional exp;\n>>> +\n>>> + void reset()\n>>> + {\n>>> + exp.reset();\n>>> + }\n>>> + } state_ = {};\n>>\n>> Doesn't std::optional already support .reset() [0] ? So I can't see benefit\n>> to adding this extra State wrapper ?\n>>\n>> [0] https://en.cppreference.com/w/cpp/utility/optional/reset.html\n>>\n>> Or is it that you anticipate extra modes to be tracked in this state ?\n>>\n>>> +\n>>> private:\n>>> bool generateId();\n>>>\n>>> @@ -98,7 +109,7 @@ public:\n>>> bool match(DeviceEnumerator *enumerator) override;\n>>>\n>>> private:\n>>> - int processControl(const UVCCameraData *data, ControlList *controls,\n>>> + int processControl(const UVCCameraData::State &state, ControlList *controls,\n>>\n>> Oh! I see - passing State makes much more sense.\n>>\n>> Ignore the above then ;-)\n>>\n>> Does this interact ok with the AeEnable handling that occurs in\n>> src/libcamera/camera.cpp?\n>>\n>>> unsigned int id, const ControlValue &value);\n>>> int processControls(UVCCameraData *data, Request *request);\n>>>\n>>> @@ -302,6 +313,9 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n>>> return ret;\n>>> }\n>>>\n>>> + if (!data->supportsExposureTimeMode())\n>>> + data->state_.exp = controls::ExposureTimeModeManual;\n>>> +\n>>> return 0;\n>>> }\n>>>\n>>> @@ -310,9 +324,10 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n>>> UVCCameraData *data = cameraData(camera);\n>>> data->video_->streamOff();\n>>> data->video_->releaseBuffers();\n>>> + data->state_.reset();\n>>> }\n>>>\n>>> -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n>>> +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n>>> unsigned int id, const ControlValue &value)\n>>> {\n>>> uint32_t cid;\n>>> @@ -359,26 +374,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n>>> }\n>>>\n>>> case V4L2_CID_EXPOSURE_AUTO: {\n>>> - std::optional mode;\n>>> -\n>>> - switch (value.get()) {\n>>> - case controls::ExposureTimeModeAuto:\n>>> - mode = data->autoExposureMode_;\n>>> - break;\n>>> - case controls::ExposureTimeModeManual:\n>>> - mode = data->manualExposureMode_;\n>>> - break;\n>>> - }\n>>> -\n>>> - if (!mode)\n>>> - return -EINVAL;\n>>> -\n>>> - controls->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n>>> + /* Handled directly in `processControls()`. */\n>>> break;\n>>> }\n>>>\n>>> case V4L2_CID_EXPOSURE_ABSOLUTE:\n>>> - controls->set(cid, value.get() / 100);\n>>> + if (state.exp == controls::ExposureTimeModeManual)\n>>> + controls->set(cid, value.get() / 100);\n>>> break;\n>>>\n>>> case V4L2_CID_CONTRAST:\n>>> @@ -413,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n>>> int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>>> {\n>>> ControlList controls(data->video_->controls());\n>>> + const auto &reqControls = request->controls();\n>>> + auto newState = data->state_;\n> \n> Please use explicit types when possible to increase readability.\n> \n>>>\n>>> - for (const auto &[id, value] : request->controls())\n>>> - processControl(data, &controls, id, value);\n> \n> A comment here to explain why this happens outside of\n> PipelineHandlerUVC::processControl() would be useful.\n> \n>>> + if (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n> \n> And avoid abbreviating too much (including in the State structure).\n> \n>>> + std::optional mode;\n>>> +\n>>> + switch (*exp) {\n>>> + case controls::ExposureTimeModeAuto:\n>>> + mode = data->autoExposureMode_;\n>>> + break;\n>>> + case controls::ExposureTimeModeManual:\n>>> + mode = data->manualExposureMode_;\n>>> + break;\n>>> + }\n>>> +\n>>> + if (!mode)\n>>> + return -EINVAL;\n>>> +\n>>> + controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n>>> + newState.exp = static_cast(*exp);\n>>> + }\n>>> +\n>>> + for (const auto &[id, value] : reqControls)\n>>> + processControl(newState, &controls, id, value);\n>>>\n>>> for (const auto &ctrl : controls)\n>>> LOG(UVC, Debug)\n>>> @@ -428,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>>> return ret < 0 ? ret : -EINVAL;\n>>> }\n>>>\n>>> - return ret;\n>>> + data->state_ = newState;\n>>> +\n>>> + return 0;\n>>> }\n>>>\n>>> int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)","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 A90D4C3237\n\tfor ;\n\tThu, 24 Jul 2025 12:51:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ADCF1690F6;\n\tThu, 24 Jul 2025 14:51:04 +0200 (CEST)","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 5DDCB690E2\n\tfor ;\n\tThu, 24 Jul 2025 14:51:02 +0200 (CEST)","from [192.168.33.11] (185.221.140.39.nat.pool.zt.hu\n\t[185.221.140.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF276C78;\n\tThu, 24 Jul 2025 14:50:22 +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=\"ocedtQo3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753361423;\n\tbh=EWXgk3nKxFn8DDU58g+pEoY5suI+lQrgNw8JI+ZmBKQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=ocedtQo3+JMLUsEamRT5Zy4dGlNLtb5cl99sley1vaDMsBtG3jvF0pVaTitZg90rH\n\thLBJn5rwMYBOp8LO0JX7WYqBFqTKbBQdvoaL7m8/S2U2WAuzzG/SOzms0Y5cR97PR8\n\tqETL/r2YkM1yG7jjMrHgI0LzmNUOIyEJGpZwzYGo=","Message-ID":"<957cb90c-a9f2-4372-9050-fc59ef13198c@ideasonboard.com>","Date":"Thu, 24 Jul 2025 14:50:57 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","To":"Laurent Pinchart ,\n\tKieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250506155314.1544104-1-barnabas.pocze@ideasonboard.com>\n\t<175312444966.50296.8634540821801476411@ping.linuxembedded.co.uk>\n\t<20250724114512.GJ11202@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250724114512.GJ11202@pendragon.ideasonboard.com>","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":35717,"web_url":"https://patchwork.libcamera.org/comment/35717/","msgid":"<20250908094543.GA26062@pendragon.ideasonboard.com>","date":"2025-09-08T09:45:43","subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jul 24, 2025 at 02:50:57PM +0200, Barnabás Pőcze wrote:\n> 2025. 07. 24. 13:45 keltezéssel, Laurent Pinchart írta:\n> > On Mon, Jul 21, 2025 at 08:00:49PM +0100, Kieran Bingham wrote:\n> >> Quoting Barnabás Pőcze (2025-05-06 16:53:14)\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> >>> To fix that, keep track of the current value of `ExposureTimeMode`, and\n> >>> only set `V4L2_CID_EXPOSURE_ABSOLUTE` if the current exposure mode is\n> >>> manual.\n> >>>\n> >>> However, the current state of `V4L2_CID_EXPOSURE_AUTO` is not retrieved\n> >>> when streaming starts, thus if the user does not set `ExposureTimeMode`\n> >>> then it will remain in an \"unset\" state and all attempts to set\n> >>> `ExposureTime` will be ignored. This is done for two reasons:\n> >>>\n> >>> * the current values of controls cannot be retrieved with libcamera,\n> >>> therefore this unset state cannot be observed by the user;\n> >>> * the user will have to set `ExposureTimeMode` either way if they\n> >>> want to guarantee that setting `ExposureTime` will have any effect.\n> > \n> > Shouldn't we actually set all controls to their default value when\n> > streaming start ?\n> \n> See https://bugs.libcamera.org/show_bug.cgi?id=271. It's not clear to me\n> what the behaviour should be. Setting the default values are some point\n> is probably desired.\n\nI've replied to the bug.\n\n> >>> If `ExposureTimeMode` is not exposed by the camera, but `ExposeTime` is\n> >>> supported, then the exposure mode is assumed to be manual to allow\n> >>> changing `V4L2_CID_EXPOSURE_ABSOLUTE`.\n> >>>\n> >>> To be able to handle requests that set both `ExposureTime{,Mode}`, process\n> >>> `ExposureTimeMode` first directly in `processControls()`, and store this\n> >>> new mode in a temporary location. Then have `processControl()` act on this\n> >>> 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> >>> changes in v4:\n> >>> * first two patches of v3 already merged\n> >>> * last two patches of v3 combined into this single patch\n> >>> * do not retrieve V4L2_CID_EXPOSURE_AUTO when streaming starts\n> >>>\n> >>> changes in v3:\n> >>> * fix default value: set it based on the v4l2 default\n> >>> * store current value of `ExposureTimeMode`\n> >>> * fix https://bugs.libcamera.org/show_bug.cgi?id=242 as well\n> >>>\n> >>> changes in v2:\n> >>> * split into two patches\n> >>> * make it compile with old libstdc++ when _GLIBCXX_DEBUG is set\n> >>>\n> >>> v3: https://patchwork.libcamera.org/patch/22959/\n> >>> https://patchwork.libcamera.org/patch/22958/\n> >>> v2: https://patchwork.libcamera.org/project/libcamera/list/?series=5006\n> >>> v1: https://patchwork.libcamera.org/project/libcamera/list/?series=4974\n> >>> ---\n> >>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 67 ++++++++++++++------\n> >>> 1 file changed, 46 insertions(+), 21 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> >>> index 586e932d2..af0c8dedd 100644\n> >>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> >>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> >>> @@ -53,6 +53,8 @@ public:\n> >>>\n> >>> const std::string &id() const { return id_; }\n> >>>\n> >>> + bool supportsExposureTimeMode() const { return autoExposureMode_ || manualExposureMode_; }\n> >>> +\n> >>> Mutex openLock_;\n> >>> std::unique_ptr video_;\n> >>> Stream stream_;\n> >>> @@ -61,6 +63,15 @@ public:\n> >>> std::optional autoExposureMode_;\n> >>> std::optional manualExposureMode_;\n> >>>\n> >>> + struct State {\n> >>> + std::optional exp;\n> >>> +\n> >>> + void reset()\n> >>> + {\n> >>> + exp.reset();\n> >>> + }\n> >>> + } state_ = {};\n> >>\n> >> Doesn't std::optional already support .reset() [0] ? So I can't see benefit\n> >> to adding this extra State wrapper ?\n> >>\n> >> [0] https://en.cppreference.com/w/cpp/utility/optional/reset.html\n> >>\n> >> Or is it that you anticipate extra modes to be tracked in this state ?\n> >>\n> >>> +\n> >>> private:\n> >>> bool generateId();\n> >>>\n> >>> @@ -98,7 +109,7 @@ public:\n> >>> bool match(DeviceEnumerator *enumerator) override;\n> >>>\n> >>> private:\n> >>> - int processControl(const UVCCameraData *data, ControlList *controls,\n> >>> + int processControl(const UVCCameraData::State &state, ControlList *controls,\n> >>\n> >> Oh! I see - passing State makes much more sense.\n> >>\n> >> Ignore the above then ;-)\n> >>\n> >> Does this interact ok with the AeEnable handling that occurs in\n> >> src/libcamera/camera.cpp?\n> >>\n> >>> unsigned int id, const ControlValue &value);\n> >>> int processControls(UVCCameraData *data, Request *request);\n> >>>\n> >>> @@ -302,6 +313,9 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n> >>> return ret;\n> >>> }\n> >>>\n> >>> + if (!data->supportsExposureTimeMode())\n> >>> + data->state_.exp = controls::ExposureTimeModeManual;\n> >>> +\n> >>> return 0;\n> >>> }\n> >>>\n> >>> @@ -310,9 +324,10 @@ void PipelineHandlerUVC::stopDevice(Camera *camera)\n> >>> UVCCameraData *data = cameraData(camera);\n> >>> data->video_->streamOff();\n> >>> data->video_->releaseBuffers();\n> >>> + data->state_.reset();\n> >>> }\n> >>>\n> >>> -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls,\n> >>> +int PipelineHandlerUVC::processControl(const UVCCameraData::State &state, ControlList *controls,\n> >>> unsigned int id, const ControlValue &value)\n> >>> {\n> >>> uint32_t cid;\n> >>> @@ -359,26 +374,13 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> >>> }\n> >>>\n> >>> case V4L2_CID_EXPOSURE_AUTO: {\n> >>> - std::optional mode;\n> >>> -\n> >>> - switch (value.get()) {\n> >>> - case controls::ExposureTimeModeAuto:\n> >>> - mode = data->autoExposureMode_;\n> >>> - break;\n> >>> - case controls::ExposureTimeModeManual:\n> >>> - mode = data->manualExposureMode_;\n> >>> - break;\n> >>> - }\n> >>> -\n> >>> - if (!mode)\n> >>> - return -EINVAL;\n> >>> -\n> >>> - controls->set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> >>> + /* Handled directly in `processControls()`. */\n> >>> break;\n> >>> }\n> >>>\n> >>> case V4L2_CID_EXPOSURE_ABSOLUTE:\n> >>> - controls->set(cid, value.get() / 100);\n> >>> + if (state.exp == controls::ExposureTimeModeManual)\n> >>> + controls->set(cid, value.get() / 100);\n> >>> break;\n> >>>\n> >>> case V4L2_CID_CONTRAST:\n> >>> @@ -413,9 +415,30 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> >>> int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> >>> {\n> >>> ControlList controls(data->video_->controls());\n> >>> + const auto &reqControls = request->controls();\n> >>> + auto newState = data->state_;\n> > \n> > Please use explicit types when possible to increase readability.\n> > \n> >>>\n> >>> - for (const auto &[id, value] : request->controls())\n> >>> - processControl(data, &controls, id, value);\n> > \n> > A comment here to explain why this happens outside of\n> > PipelineHandlerUVC::processControl() would be useful.\n> > \n> >>> + if (const auto exp = reqControls.get(controls::ExposureTimeMode)) {\n> > \n> > And avoid abbreviating too much (including in the State structure).\n> > \n> >>> + std::optional mode;\n> >>> +\n> >>> + switch (*exp) {\n> >>> + case controls::ExposureTimeModeAuto:\n> >>> + mode = data->autoExposureMode_;\n> >>> + break;\n> >>> + case controls::ExposureTimeModeManual:\n> >>> + mode = data->manualExposureMode_;\n> >>> + break;\n> >>> + }\n> >>> +\n> >>> + if (!mode)\n> >>> + return -EINVAL;\n> >>> +\n> >>> + controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(*mode));\n> >>> + newState.exp = static_cast(*exp);\n> >>> + }\n> >>> +\n> >>> + for (const auto &[id, value] : reqControls)\n> >>> + processControl(newState, &controls, id, value);\n> >>>\n> >>> for (const auto &ctrl : controls)\n> >>> LOG(UVC, Debug)\n> >>> @@ -428,7 +451,9 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> >>> return ret < 0 ? ret : -EINVAL;\n> >>> }\n> >>>\n> >>> - return ret;\n> >>> + data->state_ = newState;\n> >>> +\n> >>> + return 0;\n> >>> }\n> >>>\n> >>> int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)","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 80C01BDB13\n\tfor ;\n\tMon, 8 Sep 2025 09:46:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 32D2369356;\n\tMon, 8 Sep 2025 11:46:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0AF0613A4\n\tfor ;\n\tMon, 8 Sep 2025 11:46:04 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(230.215-178-91.adsl-dyn.isp.belgacom.be [91.178.215.230])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 81FAD725;\n\tMon, 8 Sep 2025 11:44:52 +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=\"OxXRztIq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757324692;\n\tbh=nx6k36pYjqEfnRrZyZDLHNDYbPXmZD57aaNFpnDUVtY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OxXRztIqoC3dX006d2T47ODMF1txH5QBgNVwnYp9b3Lh2E4/tm/U7S41I68mCPj+c\n\tPW2Kq94pn3CjoUlYvsWuyh9tOBshrOWY3srCJIboywDGXxwlJoSCm90RWBobupZEx8\n\tkKE60zdyBcDPTxJgAPoObR2MDlknlkyz7YOBgyzc=","Date":"Mon, 8 Sep 2025 11:45:43 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4] libcamera: pipeline: uvcvideo: Fix `ExposureTime`\n\tcontrol handling","Message-ID":"<20250908094543.GA26062@pendragon.ideasonboard.com>","References":"<20250506155314.1544104-1-barnabas.pocze@ideasonboard.com>\n\t<175312444966.50296.8634540821801476411@ping.linuxembedded.co.uk>\n\t<20250724114512.GJ11202@pendragon.ideasonboard.com>\n\t<957cb90c-a9f2-4372-9050-fc59ef13198c@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<957cb90c-a9f2-4372-9050-fc59ef13198c@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\" "}}]