[{"id":34672,"web_url":"https://patchwork.libcamera.org/comment/34672/","msgid":"<175100506752.2836253.8155788842577174175@neptunite.rasen.tech>","date":"2025-06-27T06:17:47","subject":"Re: [PATCH v1] libcamera: pipeline: uvcvideo: Handle controls during\n\tstartup","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-06-11 23:04:51)\n> Process the control list passed to `Camera::start()`, and set\n> the V4L2 controls accordingly.\n> \n> Signed-off-by: Barnabás Pőcze \n\nLooks good to me.\n\nReviewed-by: Paul Elder \n\n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 ++++++++++++++------\n> 1 file changed, 19 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index d52b88042..202420ea3 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -100,7 +100,7 @@ public:\n> private:\n> int processControl(const UVCCameraData *data, ControlList *controls,\n> unsigned int id, const ControlValue &value);\n> - int processControls(UVCCameraData *data, Request *request);\n> + int processControls(UVCCameraData *data, const ControlList &reqControls);\n> \n> bool acquireDevice(Camera *camera) override;\n> void releaseDevice(Camera *camera) override;\n> @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> return data->video_->exportBuffers(count, buffers);\n> }\n> \n> -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls)\n> {\n> UVCCameraData *data = cameraData(camera);\n> unsigned int count = data->stream_.configuration().bufferCount;\n> @@ -297,12 +297,23 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n> return ret;\n> \n> ret = data->video_->streamOn();\n> - if (ret < 0) {\n> - data->video_->releaseBuffers();\n> - return ret;\n> + if (ret < 0)\n> + goto err_release_buffers;\n> +\n> + if (controls) {\n> + ret = processControls(data, *controls);\n> + if (ret < 0)\n> + goto err_stream_off;\n> }\n> \n> return 0;\n> +\n> +err_stream_off:\n> + data->video_->streamOff();\n> +err_release_buffers:\n> + data->video_->releaseBuffers();\n> +\n> + return ret;\n> }\n> \n> void PipelineHandlerUVC::stopDevice(Camera *camera)\n> @@ -412,11 +423,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> return 0;\n> }\n> \n> -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls)\n> {\n> ControlList controls(data->video_->controls());\n> \n> - for (const auto &[id, value] : request->controls())\n> + for (const auto &[id, value] : reqControls)\n> processControl(data, &controls, id, value);\n> \n> for (const auto &ctrl : controls)\n> @@ -444,7 +455,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\n> return -ENOENT;\n> }\n> \n> - int ret = processControls(data, request);\n> + int ret = processControls(data, request->controls());\n> if (ret < 0)\n> return ret;\n> \n> -- \n> 2.49.0\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 117A6BDCBF\n\tfor ;\n\tFri, 27 Jun 2025 06:17:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF44068DF7;\n\tFri, 27 Jun 2025 08:17:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27BD761528\n\tfor ;\n\tFri, 27 Jun 2025 08:17:54 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:e17c:62b2:2597:dc1b])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 489A96A6;\n\tFri, 27 Jun 2025 08:17:34 +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=\"dCM4BsAO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751005054;\n\tbh=oy+SLgtxzXg20IvEJfKMQ4tAQsCf1jx4onIZjVIMGnQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=dCM4BsAO2P0L1X2mSvLpsNTxsx51r+YUv1lBlGSOej+N8XvGRNepVIPWuWoCAqz4H\n\tMkP1jEQGiivXbvS4OJPmh7JPKNv7FmAT1pF8feYPHqNchFS84HKM45s15PRP4xR8jk\n\txiMIdL0dGrUfG4MikTOBHPjvfEdStLlEbvZRRpOs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250611140451.592358-1-barnabas.pocze@ideasonboard.com>","References":"<20250611140451.592358-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v1] libcamera: pipeline: uvcvideo: Handle controls during\n\tstartup","From":"Paul Elder ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 27 Jun 2025 15:17:47 +0900","Message-ID":"<175100506752.2836253.8155788842577174175@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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":34675,"web_url":"https://patchwork.libcamera.org/comment/34675/","msgid":"<175101069035.3281735.15291090352660835569@ping.linuxembedded.co.uk>","date":"2025-06-27T07:51:30","subject":"Re: [PATCH v1] libcamera: pipeline: uvcvideo: Handle controls during\n\tstartup","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2025-06-27 07:17:47)\n> Quoting Barnabás Pőcze (2025-06-11 23:04:51)\n> > Process the control list passed to `Camera::start()`, and set\n> > the V4L2 controls accordingly.\n> > \n> > Signed-off-by: Barnabás Pőcze \n> \n> Looks good to me.\n> \n> Reviewed-by: Paul Elder \n> \n> > ---\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 ++++++++++++++------\n> > 1 file changed, 19 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index d52b88042..202420ea3 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -100,7 +100,7 @@ public:\n> > private:\n> > int processControl(const UVCCameraData *data, ControlList *controls,\n> > unsigned int id, const ControlValue &value);\n> > - int processControls(UVCCameraData *data, Request *request);\n> > + int processControls(UVCCameraData *data, const ControlList &reqControls);\n> > \n> > bool acquireDevice(Camera *camera) override;\n> > void releaseDevice(Camera *camera) override;\n> > @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> > return data->video_->exportBuffers(count, buffers);\n> > }\n> > \n> > -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> > +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls)\n> > {\n> > UVCCameraData *data = cameraData(camera);\n> > unsigned int count = data->stream_.configuration().bufferCount;\n> > @@ -297,12 +297,23 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n> > return ret;\n> > \n> > ret = data->video_->streamOn();\n> > - if (ret < 0) {\n> > - data->video_->releaseBuffers();\n> > - return ret;\n> > + if (ret < 0)\n> > + goto err_release_buffers;\n> > +\n\nDoes anyone know if there's a preferred order on startup here? I feel\nlike we should set the controls before the first frame ... will that\nensure they all take effect? or are there any issues setting controls\nbefore calling streamOn ?\n\nI expect the controls should persist correctly in V4L2 so that they are\nprepared when we call stream on then.\n\n> > + if (controls) {\n> > + ret = processControls(data, *controls);\n> > + if (ret < 0)\n> > + goto err_stream_off;\n> > }\n> > \n> > return 0;\n> > +\n> > +err_stream_off:\n> > + data->video_->streamOff();\n> > +err_release_buffers:\n> > + data->video_->releaseBuffers();\n> > +\n> > + return ret;\n> > }\n> > \n> > void PipelineHandlerUVC::stopDevice(Camera *camera)\n> > @@ -412,11 +423,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> > return 0;\n> > }\n> > \n> > -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls)\n> > {\n> > ControlList controls(data->video_->controls());\n> > \n> > - for (const auto &[id, value] : request->controls())\n> > + for (const auto &[id, value] : reqControls)\n> > processControl(data, &controls, id, value);\n> > \n> > for (const auto &ctrl : controls)\n> > @@ -444,7 +455,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\n> > return -ENOENT;\n> > }\n> > \n> > - int ret = processControls(data, request);\n> > + int ret = processControls(data, request->controls());\n> > if (ret < 0)\n> > return ret;\n> > \n> > -- \n> > 2.49.0\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 280C4BDCBF\n\tfor ;\n\tFri, 27 Jun 2025 07:51:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0DB8368DF7;\n\tFri, 27 Jun 2025 09:51:36 +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 B033262C43\n\tfor ;\n\tFri, 27 Jun 2025 09:51:33 +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 D0D0573B;\n\tFri, 27 Jun 2025 09:51:13 +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=\"e+kdB7GG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751010673;\n\tbh=hLLgizCmKMSW7JiSV+5Af2fBTwrlfHkCMwTZp8w6ebQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=e+kdB7GGMmIPvRBPCJVL22NtG79RwloQSd/0VAMruuzQYgLTY1hOjdaduPHpYZpxR\n\tKb+Y0DGDQyeSmrmgBh0W5by1xH3RKKyjEK5qfLDP/Yvj/BUj7RFW6rwKyQBIyaENbs\n\tWBYIwYaby7TQmUx8lVYsUF6RMFWfN7kXiJMaiALc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175100506752.2836253.8155788842577174175@neptunite.rasen.tech>","References":"<20250611140451.592358-1-barnabas.pocze@ideasonboard.com>\n\t<175100506752.2836253.8155788842577174175@neptunite.rasen.tech>","Subject":"Re: [PATCH v1] libcamera: pipeline: uvcvideo: Handle controls during\n\tstartup","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tPaul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 27 Jun 2025 08:51:30 +0100","Message-ID":"<175101069035.3281735.15291090352660835569@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":34749,"web_url":"https://patchwork.libcamera.org/comment/34749/","msgid":"<20250630231322.GH15184@pendragon.ideasonboard.com>","date":"2025-06-30T23:13:22","subject":"Re: [PATCH v1] libcamera: pipeline: uvcvideo: Handle controls during\n\tstartup","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jun 27, 2025 at 08:51:30AM +0100, Kieran Bingham wrote:\n> Quoting Paul Elder (2025-06-27 07:17:47)\n> > Quoting Barnabás Pőcze (2025-06-11 23:04:51)\n> > > Process the control list passed to `Camera::start()`, and set\n> > > the V4L2 controls accordingly.\n> > > \n> > > Signed-off-by: Barnabás Pőcze \n> > \n> > Looks good to me.\n> > \n> > Reviewed-by: Paul Elder \n> > \n> > > ---\n> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 ++++++++++++++------\n> > > 1 file changed, 19 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index d52b88042..202420ea3 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -100,7 +100,7 @@ public:\n> > > private:\n> > > int processControl(const UVCCameraData *data, ControlList *controls,\n> > > unsigned int id, const ControlValue &value);\n> > > - int processControls(UVCCameraData *data, Request *request);\n> > > + int processControls(UVCCameraData *data, const ControlList &reqControls);\n> > > \n> > > bool acquireDevice(Camera *camera) override;\n> > > void releaseDevice(Camera *camera) override;\n> > > @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> > > return data->video_->exportBuffers(count, buffers);\n> > > }\n> > > \n> > > -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> > > +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls)\n> > > {\n> > > UVCCameraData *data = cameraData(camera);\n> > > unsigned int count = data->stream_.configuration().bufferCount;\n> > > @@ -297,12 +297,23 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList\n> > > return ret;\n> > > \n> > > ret = data->video_->streamOn();\n> > > - if (ret < 0) {\n> > > - data->video_->releaseBuffers();\n> > > - return ret;\n> > > + if (ret < 0)\n> > > + goto err_release_buffers;\n> > > +\n> \n> Does anyone know if there's a preferred order on startup here? I feel\n> like we should set the controls before the first frame ... will that\n> ensure they all take effect? or are there any issues setting controls\n> before calling streamOn ?\n> \n> I expect the controls should persist correctly in V4L2 so that they are\n> prepared when we call stream on then.\n\nSetting them before starting streaming should ensure they take effect\nfrom the very first frame, yes. I think that would be better.\n\n> > > + if (controls) {\n> > > + ret = processControls(data, *controls);\n> > > + if (ret < 0)\n> > > + goto err_stream_off;\n> > > }\n> > > \n> > > return 0;\n> > > +\n> > > +err_stream_off:\n> > > + data->video_->streamOff();\n> > > +err_release_buffers:\n> > > + data->video_->releaseBuffers();\n> > > +\n> > > + return ret;\n> > > }\n> > > \n> > > void PipelineHandlerUVC::stopDevice(Camera *camera)\n> > > @@ -412,11 +423,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c\n> > > return 0;\n> > > }\n> > > \n> > > -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> > > +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls)\n> > > {\n> > > ControlList controls(data->video_->controls());\n> > > \n> > > - for (const auto &[id, value] : request->controls())\n> > > + for (const auto &[id, value] : reqControls)\n> > > processControl(data, &controls, id, value);\n> > > \n> > > for (const auto &ctrl : controls)\n> > > @@ -444,7 +455,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\n> > > return -ENOENT;\n> > > }\n> > > \n> > > - int ret = processControls(data, request);\n> > > + int ret = processControls(data, request->controls());\n> > > if (ret < 0)\n> > > return ret;\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 8B4A0BDCBF\n\tfor ;\n\tMon, 30 Jun 2025 23:13:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 863EC68E0E;\n\tTue, 1 Jul 2025 01:13:50 +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 3A39761524\n\tfor ;\n\tTue, 1 Jul 2025 01:13:48 +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 C9CC67E4;\n\tTue, 1 Jul 2025 01:13:25 +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=\"GPE/wKpQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751325206;\n\tbh=OfA7g8fSP0oW1bycjSATpzrARX/eHUvoft+uiDngft8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GPE/wKpQV7NQxezB+7j6wmXi7/ek80QArLJKnSn5IQi0FeTTbauf5YPInkmQ16NQY\n\trBQ/j7yycSUor9lrvzjllO2Rjw9Hq5WT+JXRjq2rpt+jYxbtvphfsEQSSdXWIs214D\n\t9BI7EYEVLtw9mcZ40lCrpmVIfvM4QVK7JEdHiOzs=","Date":"Tue, 1 Jul 2025 02:13:22 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tPaul Elder , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: pipeline: uvcvideo: Handle controls during\n\tstartup","Message-ID":"<20250630231322.GH15184@pendragon.ideasonboard.com>","References":"<20250611140451.592358-1-barnabas.pocze@ideasonboard.com>\n\t<175100506752.2836253.8155788842577174175@neptunite.rasen.tech>\n\t<175101069035.3281735.15291090352660835569@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":"<175101069035.3281735.15291090352660835569@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\" "}}]