[{"id":33604,"web_url":"https://patchwork.libcamera.org/comment/33604/","msgid":"<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>","date":"2025-03-17T17:33:06","subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:\n> `Camera::Private::properties_` is a default constructed `ControlList`,\n> therefore it does not have an associated `ControlIdMap`. `controlInfo_`\n> is in a similar situation.\n>\n> Extend the `Camera::Private` constructor to initialize the control id map\n> of both properly.\n>\n> Multiple pipeline handlers copy the sensor's property list and\n> set that as camera properties, and since the `CameraSensor{Legacy,Raw}`\n> classes set the proper id map, the camera properties will have it too.\n>\n> However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,\n> and thus there will be no id map set. To fix this, extend the\n> `Camera::Private` constructor to set `properties::properties`.\n>\n> As for `controlInfo_`, all pipeline handlers overwrite it during\n> camera initialization (and thus it will have the correct id map),\n> but still initialize the id map so that it is set at all times.\n>\n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\nMakes very much sense, thanks\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\n> ---\n>  src/libcamera/camera.cpp | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 56c585199..c180a5fdd 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -21,6 +21,7 @@\n>  #include <libcamera/color_space.h>\n>  #include <libcamera/control_ids.h>\n>  #include <libcamera/framebuffer_allocator.h>\n> +#include <libcamera/property_ids.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>\n> @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n>   * \\param[in] pipe The pipeline handler responsible for the camera device\n>   */\n>  Camera::Private::Private(PipelineHandler *pipe)\n> -\t: requestSequence_(0), pipe_(pipe->shared_from_this()),\n> +\t: controlInfo_({}, controls::controls), properties_(properties::properties),\n> +\t  requestSequence_(0), pipe_(pipe->shared_from_this()),\n>  \t  disconnected_(false), state_(CameraAvailable)\n>  {\n>  }\n> --\n> 2.48.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5AD9FC32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Mar 2025 17:33:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77DAF68826;\n\tMon, 17 Mar 2025 18:33:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 86E29617F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Mar 2025 18:33:10 +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 C2544965;\n\tMon, 17 Mar 2025 18:31:28 +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=\"N56PHA9h\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742232688;\n\tbh=Cq2gdFC3wtOQo/WSTwRODhbZjfg6W3M9X4At+apWhzA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N56PHA9h0aoTqNzs5j6jE8vZg+AO1dy0UAtRS8cQKhPFi1dHu6Tto7OnErEI8noOB\n\tce4PMz7T0M0COXTbzxbLLxx1xnJtbbINTN/P0ugKQrmTb0HHz0UOZrzBVc55UE9Emc\n\tTyHp9nl/DHFqX6iGlm0qTVYu4rxHFP/1kV4QuEwM=","Date":"Mon, 17 Mar 2025 18:33:06 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","Message-ID":"<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>","References":"<20250310170246.185147-1-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":"<20250310170246.185147-1-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33605,"web_url":"https://patchwork.libcamera.org/comment/33605/","msgid":"<174223387170.3727128.13347239722253876818@ping.linuxembedded.co.uk>","date":"2025-03-17T17:51:11","subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2025-03-17 17:33:06)\n> Hi Barnabás\n> \n> On Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:\n> > `Camera::Private::properties_` is a default constructed `ControlList`,\n> > therefore it does not have an associated `ControlIdMap`. `controlInfo_`\n> > is in a similar situation.\n> >\n> > Extend the `Camera::Private` constructor to initialize the control id map\n> > of both properly.\n> >\n> > Multiple pipeline handlers copy the sensor's property list and\n> > set that as camera properties, and since the `CameraSensor{Legacy,Raw}`\n> > classes set the proper id map, the camera properties will have it too.\n> >\n> > However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,\n> > and thus there will be no id map set. To fix this, extend the\n> > `Camera::Private` constructor to set `properties::properties`.\n> >\n> > As for `controlInfo_`, all pipeline handlers overwrite it during\n> > camera initialization (and thus it will have the correct id map),\n> > but still initialize the id map so that it is set at all times.\n> >\n> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> \n> Makes very much sense, thanks\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nYes, I like the sound of this too.\n\nDo we have correct uses of ControlList using the default constructor ?\nMaybe we should try to remove or deprecate it so that ControlLists\nshould always construct with a correct type ?\n\nBut I fear we already use ControlList independently so that wouldn't be\npossible or feasible so as this 'fixes' pipeline handlers in the default\ncase ...\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> > ---\n> >  src/libcamera/camera.cpp | 4 +++-\n> >  1 file changed, 3 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 56c585199..c180a5fdd 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -21,6 +21,7 @@\n> >  #include <libcamera/color_space.h>\n> >  #include <libcamera/control_ids.h>\n> >  #include <libcamera/framebuffer_allocator.h>\n> > +#include <libcamera/property_ids.h>\n> >  #include <libcamera/request.h>\n> >  #include <libcamera/stream.h>\n> >\n> > @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> >   * \\param[in] pipe The pipeline handler responsible for the camera device\n> >   */\n> >  Camera::Private::Private(PipelineHandler *pipe)\n> > -     : requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > +     : controlInfo_({}, controls::controls), properties_(properties::properties),\n> > +       requestSequence_(0), pipe_(pipe->shared_from_this()),\n> >         disconnected_(false), state_(CameraAvailable)\n> >  {\n> >  }\n> > --\n> > 2.48.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 3D1A5C32FA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Mar 2025 17:51:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5EEB368942;\n\tMon, 17 Mar 2025 18:51:16 +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 ACF39617F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Mar 2025 18:51:14 +0100 (CET)","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 E9B878DB;\n\tMon, 17 Mar 2025 18:49:32 +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=\"GF4mJE/5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742233773;\n\tbh=pfM8Tlf3iFXqHNWNTK1eJvoPj4SGzqc7/OT5XVakssM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GF4mJE/5cBk0dfpQMu8EKZc+XgMCoZ2iHNBzinMMlgNUsb+1f/A0Q5Zi1bJifd8or\n\t+8vddKT0wUhNdZ/gL8XYmOBidbh6dz43BK6eicSfLOyChDOxtQfZCH3sUKHGYt5k2q\n\tCT+hqGoyJDDeGKX4XZQhkl5UTiqzz0izavI6T+I8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>","References":"<20250310170246.185147-1-barnabas.pocze@ideasonboard.com>\n\t<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>","Subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Mon, 17 Mar 2025 17:51:11 +0000","Message-ID":"<174223387170.3727128.13347239722253876818@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33614,"web_url":"https://patchwork.libcamera.org/comment/33614/","msgid":"<35j4zwmlhwzmnudby2yb2qmcekublmwxirsoqx2mroiczhlzbm@ivmz75wh4b3r>","date":"2025-03-17T18:50:06","subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Mar 17, 2025 at 05:51:11PM +0000, Kieran Bingham wrote:\n> Quoting Jacopo Mondi (2025-03-17 17:33:06)\n> > Hi Barnabás\n> >\n> > On Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:\n> > > `Camera::Private::properties_` is a default constructed `ControlList`,\n> > > therefore it does not have an associated `ControlIdMap`. `controlInfo_`\n> > > is in a similar situation.\n> > >\n> > > Extend the `Camera::Private` constructor to initialize the control id map\n> > > of both properly.\n> > >\n> > > Multiple pipeline handlers copy the sensor's property list and\n> > > set that as camera properties, and since the `CameraSensor{Legacy,Raw}`\n> > > classes set the proper id map, the camera properties will have it too.\n> > >\n> > > However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,\n> > > and thus there will be no id map set. To fix this, extend the\n> > > `Camera::Private` constructor to set `properties::properties`.\n> > >\n> > > As for `controlInfo_`, all pipeline handlers overwrite it during\n> > > camera initialization (and thus it will have the correct id map),\n> > > but still initialize the id map so that it is set at all times.\n> > >\n> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> >\n> > Makes very much sense, thanks\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>\n> Yes, I like the sound of this too.\n>\n> Do we have correct uses of ControlList using the default constructor ?\n> Maybe we should try to remove or deprecate it so that ControlLists\n> should always construct with a correct type ?\n\nWe don't have a ControlIdMap for V4L2 controls unfortunately...\n\n>\n> But I fear we already use ControlList independently so that wouldn't be\n> possible or feasible so as this 'fixes' pipeline handlers in the default\n> case ...\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> > > ---\n> > >  src/libcamera/camera.cpp | 4 +++-\n> > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 56c585199..c180a5fdd 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -21,6 +21,7 @@\n> > >  #include <libcamera/color_space.h>\n> > >  #include <libcamera/control_ids.h>\n> > >  #include <libcamera/framebuffer_allocator.h>\n> > > +#include <libcamera/property_ids.h>\n> > >  #include <libcamera/request.h>\n> > >  #include <libcamera/stream.h>\n> > >\n> > > @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> > >   * \\param[in] pipe The pipeline handler responsible for the camera device\n> > >   */\n> > >  Camera::Private::Private(PipelineHandler *pipe)\n> > > -     : requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > +     : controlInfo_({}, controls::controls), properties_(properties::properties),\n> > > +       requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > >         disconnected_(false), state_(CameraAvailable)\n> > >  {\n> > >  }\n> > > --\n> > > 2.48.1\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E9A6EC32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Mar 2025 18:50:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FE44617F8;\n\tMon, 17 Mar 2025 19:50:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BF8E8617F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Mar 2025 19:50:09 +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 D31B5352;\n\tMon, 17 Mar 2025 19:48:27 +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=\"hzp0/zDB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742237307;\n\tbh=JLqN8xh1xU9qvwGH2jijMUQtMDmg+XxHtxvUkihXqS0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hzp0/zDBbXtzwyodAR/uJmM7TB1C8867HjQsUDfLsAjqIYn00j2d18TykPLnQ8jyo\n\tafOEKmnADSaZwKNlV8flFI6OGec8Irmw/B4bFMFwXIJ0B2FdeU+0w6WxcHzgd/7uXp\n\tN12Dkirj3ttoEUHiWmiUUNPwf7SiP1XlO77VLI1Y=","Date":"Mon, 17 Mar 2025 19:50:06 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","Message-ID":"<35j4zwmlhwzmnudby2yb2qmcekublmwxirsoqx2mroiczhlzbm@ivmz75wh4b3r>","References":"<20250310170246.185147-1-barnabas.pocze@ideasonboard.com>\n\t<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>\n\t<174223387170.3727128.13347239722253876818@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":"<174223387170.3727128.13347239722253876818@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33622,"web_url":"https://patchwork.libcamera.org/comment/33622/","msgid":"<174224823043.110831.15369215913597599467@ping.linuxembedded.co.uk>","date":"2025-03-17T21:50:30","subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2025-03-17 18:50:06)\n> Hi Kieran\n> \n> On Mon, Mar 17, 2025 at 05:51:11PM +0000, Kieran Bingham wrote:\n> > Quoting Jacopo Mondi (2025-03-17 17:33:06)\n> > > Hi Barnabás\n> > >\n> > > On Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:\n> > > > `Camera::Private::properties_` is a default constructed `ControlList`,\n> > > > therefore it does not have an associated `ControlIdMap`. `controlInfo_`\n> > > > is in a similar situation.\n> > > >\n> > > > Extend the `Camera::Private` constructor to initialize the control id map\n> > > > of both properly.\n> > > >\n> > > > Multiple pipeline handlers copy the sensor's property list and\n> > > > set that as camera properties, and since the `CameraSensor{Legacy,Raw}`\n> > > > classes set the proper id map, the camera properties will have it too.\n> > > >\n> > > > However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,\n> > > > and thus there will be no id map set. To fix this, extend the\n> > > > `Camera::Private` constructor to set `properties::properties`.\n> > > >\n> > > > As for `controlInfo_`, all pipeline handlers overwrite it during\n> > > > camera initialization (and thus it will have the correct id map),\n> > > > but still initialize the id map so that it is set at all times.\n> > > >\n> > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > >\n> > > Makes very much sense, thanks\n> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> >\n> > Yes, I like the sound of this too.\n> >\n> > Do we have correct uses of ControlList using the default constructor ?\n> > Maybe we should try to remove or deprecate it so that ControlLists\n> > should always construct with a correct type ?\n> \n> We don't have a ControlIdMap for V4L2 controls unfortunately...\n\nI have a low priority 'todo' somewhere in my notes to investigate what\ncould happen if we set up a 'vendor namespace' of V4L2 controls that map\ndirectly to V4L2 controls ... I 'almost' think that's possible!?\n\nBased on include/uapi/linux/v4l2-controls.h we could 'vendor' the V4L2\ncontrol classes ... and then have a direct mapping of V4L2 controls as\nlibcamera::V4L2::* with a bit of python to automate the mapping.\n\nAll of the V4L2 classes are high up in the int32 address space:\n\n/* Control classes */\n#define V4L2_CTRL_CLASS_USER\t\t0x00980000\t/* Old-style 'user' controls */\n#define V4L2_CTRL_CLASS_CODEC\t\t0x00990000\t/* Stateful codec controls */\n#define V4L2_CTRL_CLASS_CAMERA\t\t0x009a0000\t/* Camera class controls */\n#define V4L2_CTRL_CLASS_FM_TX\t\t0x009b0000\t/* FM Modulator controls */\n#define V4L2_CTRL_CLASS_FLASH\t\t0x009c0000\t/* Camera flash controls */\n#define V4L2_CTRL_CLASS_JPEG\t\t0x009d0000\t/* JPEG-compression controls */\n#define V4L2_CTRL_CLASS_IMAGE_SOURCE\t0x009e0000\t/* Image source controls */\n#define V4L2_CTRL_CLASS_IMAGE_PROC\t0x009f0000\t/* Image processing controls */\n#define V4L2_CTRL_CLASS_DV\t\t0x00a00000\t/* Digital Video controls */\n#define V4L2_CTRL_CLASS_FM_RX\t\t0x00a10000\t/* FM Receiver controls */\n#define V4L2_CTRL_CLASS_RF_TUNER\t0x00a20000\t/* RF tuner controls */\n#define V4L2_CTRL_CLASS_DETECT\t\t0x00a30000\t/* Detection controls */\n#define V4L2_CTRL_CLASS_CODEC_STATELESS 0x00a40000\t/* Stateless codecs controls */\n#define V4L2_CTRL_CLASS_COLORIMETRY\t0x00a50000\t/* Colorimetry controls */\n\n\nwhich is incredibly convenient ...\n\n\n\n> > But I fear we already use ControlList independently so that wouldn't be\n> > possible or feasible so as this 'fixes' pipeline handlers in the default\n> > case ...\n> >\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >\n> > > > ---\n> > > >  src/libcamera/camera.cpp | 4 +++-\n> > > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index 56c585199..c180a5fdd 100644\n> > > > --- a/src/libcamera/camera.cpp\n> > > > +++ b/src/libcamera/camera.cpp\n> > > > @@ -21,6 +21,7 @@\n> > > >  #include <libcamera/color_space.h>\n> > > >  #include <libcamera/control_ids.h>\n> > > >  #include <libcamera/framebuffer_allocator.h>\n> > > > +#include <libcamera/property_ids.h>\n> > > >  #include <libcamera/request.h>\n> > > >  #include <libcamera/stream.h>\n> > > >\n> > > > @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> > > >   * \\param[in] pipe The pipeline handler responsible for the camera device\n> > > >   */\n> > > >  Camera::Private::Private(PipelineHandler *pipe)\n> > > > -     : requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > > +     : controlInfo_({}, controls::controls), properties_(properties::properties),\n> > > > +       requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > >         disconnected_(false), state_(CameraAvailable)\n> > > >  {\n> > > >  }\n> > > > --\n> > > > 2.48.1\n> > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 71131C32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Mar 2025 21:50:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4EDB68947;\n\tMon, 17 Mar 2025 22:50:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C95A68826\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Mar 2025 22:50:33 +0100 (CET)","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 4E800352;\n\tMon, 17 Mar 2025 22:48:51 +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=\"lVghz82j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742248131;\n\tbh=1jZHvds+7/tv6H086O8UAvGY0NTNIaOXXXAOVbfmGHw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=lVghz82jswDw1/HMwHHKaMJzDeEBYdb62l/JX42VIAnUyLmD2VNvgQYOC9MlqDSHn\n\tAPCAaQqQk2UUlgiDhWv8esBDCGFDUN3BsrA8SqxvSkF935hoylABhVp/EUhvoHK/Ww\n\tppCWnNdQoS1R923Xl4u8LxeqP4eEwyTDJiidsibQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<35j4zwmlhwzmnudby2yb2qmcekublmwxirsoqx2mroiczhlzbm@ivmz75wh4b3r>","References":"<20250310170246.185147-1-barnabas.pocze@ideasonboard.com>\n\t<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>\n\t<174223387170.3727128.13347239722253876818@ping.linuxembedded.co.uk>\n\t<35j4zwmlhwzmnudby2yb2qmcekublmwxirsoqx2mroiczhlzbm@ivmz75wh4b3r>","Subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Mon, 17 Mar 2025 21:50:30 +0000","Message-ID":"<174224823043.110831.15369215913597599467@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33627,"web_url":"https://patchwork.libcamera.org/comment/33627/","msgid":"<h6qz7fp737cywyl5x6pajgdazhkitwoftdbey3ot4w3dgriwum@piq34l26e3hc>","date":"2025-03-18T07:45:12","subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Mar 17, 2025 at 09:50:30PM +0000, Kieran Bingham wrote:\n> Quoting Jacopo Mondi (2025-03-17 18:50:06)\n> > Hi Kieran\n> >\n> > On Mon, Mar 17, 2025 at 05:51:11PM +0000, Kieran Bingham wrote:\n> > > Quoting Jacopo Mondi (2025-03-17 17:33:06)\n> > > > Hi Barnabás\n> > > >\n> > > > On Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:\n> > > > > `Camera::Private::properties_` is a default constructed `ControlList`,\n> > > > > therefore it does not have an associated `ControlIdMap`. `controlInfo_`\n> > > > > is in a similar situation.\n> > > > >\n> > > > > Extend the `Camera::Private` constructor to initialize the control id map\n> > > > > of both properly.\n> > > > >\n> > > > > Multiple pipeline handlers copy the sensor's property list and\n> > > > > set that as camera properties, and since the `CameraSensor{Legacy,Raw}`\n> > > > > classes set the proper id map, the camera properties will have it too.\n> > > > >\n> > > > > However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,\n> > > > > and thus there will be no id map set. To fix this, extend the\n> > > > > `Camera::Private` constructor to set `properties::properties`.\n> > > > >\n> > > > > As for `controlInfo_`, all pipeline handlers overwrite it during\n> > > > > camera initialization (and thus it will have the correct id map),\n> > > > > but still initialize the id map so that it is set at all times.\n> > > > >\n> > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > > >\n> > > > Makes very much sense, thanks\n> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > >\n> > > Yes, I like the sound of this too.\n> > >\n> > > Do we have correct uses of ControlList using the default constructor ?\n> > > Maybe we should try to remove or deprecate it so that ControlLists\n> > > should always construct with a correct type ?\n> >\n> > We don't have a ControlIdMap for V4L2 controls unfortunately...\n>\n> I have a low priority 'todo' somewhere in my notes to investigate what\n> could happen if we set up a 'vendor namespace' of V4L2 controls that map\n> directly to V4L2 controls ... I 'almost' think that's possible!?\n>\n> Based on include/uapi/linux/v4l2-controls.h we could 'vendor' the V4L2\n> control classes ... and then have a direct mapping of V4L2 controls as\n> libcamera::V4L2::* with a bit of python to automate the mapping.\n>\n> All of the V4L2 classes are high up in the int32 address space:\n>\n> /* Control classes */\n> #define V4L2_CTRL_CLASS_USER\t\t0x00980000\t/* Old-style 'user' controls */\n> #define V4L2_CTRL_CLASS_CODEC\t\t0x00990000\t/* Stateful codec controls */\n> #define V4L2_CTRL_CLASS_CAMERA\t\t0x009a0000\t/* Camera class controls */\n> #define V4L2_CTRL_CLASS_FM_TX\t\t0x009b0000\t/* FM Modulator controls */\n> #define V4L2_CTRL_CLASS_FLASH\t\t0x009c0000\t/* Camera flash controls */\n> #define V4L2_CTRL_CLASS_JPEG\t\t0x009d0000\t/* JPEG-compression controls */\n> #define V4L2_CTRL_CLASS_IMAGE_SOURCE\t0x009e0000\t/* Image source controls */\n> #define V4L2_CTRL_CLASS_IMAGE_PROC\t0x009f0000\t/* Image processing controls */\n> #define V4L2_CTRL_CLASS_DV\t\t0x00a00000\t/* Digital Video controls */\n> #define V4L2_CTRL_CLASS_FM_RX\t\t0x00a10000\t/* FM Receiver controls */\n> #define V4L2_CTRL_CLASS_RF_TUNER\t0x00a20000\t/* RF tuner controls */\n> #define V4L2_CTRL_CLASS_DETECT\t\t0x00a30000\t/* Detection controls */\n> #define V4L2_CTRL_CLASS_CODEC_STATELESS 0x00a40000\t/* Stateless codecs controls */\n> #define V4L2_CTRL_CLASS_COLORIMETRY\t0x00a50000\t/* Colorimetry controls */\n>\n>\n> which is incredibly convenient ...\n\nYes, that's convenient. And we discussed about mapping v4l2 controls\nto libcamera controls many times years ago.\n\nHowever the enumeration of v4l2 controls is not under our control, and\nI can imagine that bad things happen if the enumeration of v4l2\ncontrols on the device that runs libcamera differ from the one we have\nin our headers file copy.\n\nNewly introduced controls in mainline should only be added to the\nexisting ones, so it should be safe ?\n\nHowever, and I'm not sure it's something we should really be concerned\nwith or not, this becomes a problem with controls introduced\ndownstream in vendor's BSP, and I presume quite some people runs\nlibcamera on some sort of BSP, specifically for some mature platforms\nwhere the mainline kernel drivers have been absorbed in the BSP\nversions. In this case, to generate a libcamera control for you vendor\ncontrol, you will have to recompile.\n\nDo we care ?\n\n>\n>\n>\n> > > But I fear we already use ControlList independently so that wouldn't be\n> > > possible or feasible so as this 'fixes' pipeline handlers in the default\n> > > case ...\n> > >\n> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > >\n> > > > > ---\n> > > > >  src/libcamera/camera.cpp | 4 +++-\n> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > > index 56c585199..c180a5fdd 100644\n> > > > > --- a/src/libcamera/camera.cpp\n> > > > > +++ b/src/libcamera/camera.cpp\n> > > > > @@ -21,6 +21,7 @@\n> > > > >  #include <libcamera/color_space.h>\n> > > > >  #include <libcamera/control_ids.h>\n> > > > >  #include <libcamera/framebuffer_allocator.h>\n> > > > > +#include <libcamera/property_ids.h>\n> > > > >  #include <libcamera/request.h>\n> > > > >  #include <libcamera/stream.h>\n> > > > >\n> > > > > @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> > > > >   * \\param[in] pipe The pipeline handler responsible for the camera device\n> > > > >   */\n> > > > >  Camera::Private::Private(PipelineHandler *pipe)\n> > > > > -     : requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > > > +     : controlInfo_({}, controls::controls), properties_(properties::properties),\n> > > > > +       requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > > >         disconnected_(false), state_(CameraAvailable)\n> > > > >  {\n> > > > >  }\n> > > > > --\n> > > > > 2.48.1\n> > > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 170E7C32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 Mar 2025 07:45:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E2EC68928;\n\tTue, 18 Mar 2025 08:45:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 967D9617F6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Mar 2025 08:45:16 +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 5E606778;\n\tTue, 18 Mar 2025 08:43:34 +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=\"ghFWttfE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742283814;\n\tbh=8gtDuh/SvtcWOsOg3AwHFl3fKcgYsYajccXWpY92sU8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ghFWttfEFKYkhw8FVPqNf7Dj+ZbRddr5QEMD6Yh0akagy84ysnkv2dncYoaEW/duz\n\tzjKUQ2xkqmzZDt15yTeP2PkaL+p0aE/RYYJA2N+wHc7X6T0YeGnbPvWd1TSf44wXvu\n\tJPOZhZ4U3bQCtJFOp8wD6Il3JD112m+jXhrjyBWw=","Date":"Tue, 18 Mar 2025 08:45:12 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, =?utf-8?b?QmFybmFiw6Fz?=\n\t=?utf-8?q?_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","Message-ID":"<h6qz7fp737cywyl5x6pajgdazhkitwoftdbey3ot4w3dgriwum@piq34l26e3hc>","References":"<20250310170246.185147-1-barnabas.pocze@ideasonboard.com>\n\t<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>\n\t<174223387170.3727128.13347239722253876818@ping.linuxembedded.co.uk>\n\t<35j4zwmlhwzmnudby2yb2qmcekublmwxirsoqx2mroiczhlzbm@ivmz75wh4b3r>\n\t<174224823043.110831.15369215913597599467@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":"<174224823043.110831.15369215913597599467@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33633,"web_url":"https://patchwork.libcamera.org/comment/33633/","msgid":"<174229693884.1463678.5650343929901880137@ping.linuxembedded.co.uk>","date":"2025-03-18T11:22:18","subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2025-03-18 07:45:12)\n> Hi Kieran\n> \n> On Mon, Mar 17, 2025 at 09:50:30PM +0000, Kieran Bingham wrote:\n> > Quoting Jacopo Mondi (2025-03-17 18:50:06)\n> > > Hi Kieran\n> > >\n> > > On Mon, Mar 17, 2025 at 05:51:11PM +0000, Kieran Bingham wrote:\n> > > > Quoting Jacopo Mondi (2025-03-17 17:33:06)\n> > > > > Hi Barnabás\n> > > > >\n> > > > > On Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:\n> > > > > > `Camera::Private::properties_` is a default constructed `ControlList`,\n> > > > > > therefore it does not have an associated `ControlIdMap`. `controlInfo_`\n> > > > > > is in a similar situation.\n> > > > > >\n> > > > > > Extend the `Camera::Private` constructor to initialize the control id map\n> > > > > > of both properly.\n> > > > > >\n> > > > > > Multiple pipeline handlers copy the sensor's property list and\n> > > > > > set that as camera properties, and since the `CameraSensor{Legacy,Raw}`\n> > > > > > classes set the proper id map, the camera properties will have it too.\n> > > > > >\n> > > > > > However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,\n> > > > > > and thus there will be no id map set. To fix this, extend the\n> > > > > > `Camera::Private` constructor to set `properties::properties`.\n> > > > > >\n> > > > > > As for `controlInfo_`, all pipeline handlers overwrite it during\n> > > > > > camera initialization (and thus it will have the correct id map),\n> > > > > > but still initialize the id map so that it is set at all times.\n> > > > > >\n> > > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > > > >\n> > > > > Makes very much sense, thanks\n> > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > >\n> > > > Yes, I like the sound of this too.\n> > > >\n> > > > Do we have correct uses of ControlList using the default constructor ?\n> > > > Maybe we should try to remove or deprecate it so that ControlLists\n> > > > should always construct with a correct type ?\n> > >\n> > > We don't have a ControlIdMap for V4L2 controls unfortunately...\n> >\n> > I have a low priority 'todo' somewhere in my notes to investigate what\n> > could happen if we set up a 'vendor namespace' of V4L2 controls that map\n> > directly to V4L2 controls ... I 'almost' think that's possible!?\n> >\n> > Based on include/uapi/linux/v4l2-controls.h we could 'vendor' the V4L2\n> > control classes ... and then have a direct mapping of V4L2 controls as\n> > libcamera::V4L2::* with a bit of python to automate the mapping.\n> >\n> > All of the V4L2 classes are high up in the int32 address space:\n> >\n> > /* Control classes */\n> > #define V4L2_CTRL_CLASS_USER          0x00980000      /* Old-style 'user' controls */\n> > #define V4L2_CTRL_CLASS_CODEC         0x00990000      /* Stateful codec controls */\n> > #define V4L2_CTRL_CLASS_CAMERA                0x009a0000      /* Camera class controls */\n> > #define V4L2_CTRL_CLASS_FM_TX         0x009b0000      /* FM Modulator controls */\n> > #define V4L2_CTRL_CLASS_FLASH         0x009c0000      /* Camera flash controls */\n> > #define V4L2_CTRL_CLASS_JPEG          0x009d0000      /* JPEG-compression controls */\n> > #define V4L2_CTRL_CLASS_IMAGE_SOURCE  0x009e0000      /* Image source controls */\n> > #define V4L2_CTRL_CLASS_IMAGE_PROC    0x009f0000      /* Image processing controls */\n> > #define V4L2_CTRL_CLASS_DV            0x00a00000      /* Digital Video controls */\n> > #define V4L2_CTRL_CLASS_FM_RX         0x00a10000      /* FM Receiver controls */\n> > #define V4L2_CTRL_CLASS_RF_TUNER      0x00a20000      /* RF tuner controls */\n> > #define V4L2_CTRL_CLASS_DETECT                0x00a30000      /* Detection controls */\n> > #define V4L2_CTRL_CLASS_CODEC_STATELESS 0x00a40000    /* Stateless codecs controls */\n> > #define V4L2_CTRL_CLASS_COLORIMETRY   0x00a50000      /* Colorimetry controls */\n> >\n> >\n> > which is incredibly convenient ...\n> \n> Yes, that's convenient. And we discussed about mapping v4l2 controls\n> to libcamera controls many times years ago.\n\nBack then we didn't have control namespaces though ...\n\n> However the enumeration of v4l2 controls is not under our control, and\n> I can imagine that bad things happen if the enumeration of v4l2\n> controls on the device that runs libcamera differ from the one we have\n> in our headers file copy.\n\nIndeed, but at least mapping the ones we use directly seems better to\nalign things.\n\nAnd as they'd be generated - they should be able to be constructed from\nknown defintions.\n\n\n> Newly introduced controls in mainline should only be added to the\n> existing ones, so it should be safe ?\n\nAbsolutely - include/uapi/linux/v4l2-controls.h is a Linux ABI.\n\n> However, and I'm not sure it's something we should really be concerned\n> with or not, this becomes a problem with controls introduced\n> downstream in vendor's BSP, and I presume quite some people runs\n> libcamera on some sort of BSP, specifically for some mature platforms\n> where the mainline kernel drivers have been absorbed in the BSP\n> versions. In this case, to generate a libcamera control for you vendor\n> control, you will have to recompile.\n\nIf they're going to add new controls, they'll have to recompile anyway.\nTo me - putting V4L2 controls into a namespace is more about clarifying\nthe usage of V4L2 ids' and also ensuring that those IDs are 'reserved'\nso could never conflict with libcamera controls.\n\nThey could still be referenced by the V4L2 macros for the ID. But the\nID would be clearly defined.\n\n> Do we care ?\n> \n\nAbout which bit? Vendor BSPs needing recompilation? Not me ;-)\n\n--\nKieran\n\n> >\n> >\n> >\n> > > > But I fear we already use ControlList independently so that wouldn't be\n> > > > possible or feasible so as this 'fixes' pipeline handlers in the default\n> > > > case ...\n> > > >\n> > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > >\n> > > > > > ---\n> > > > > >  src/libcamera/camera.cpp | 4 +++-\n> > > > > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > > > index 56c585199..c180a5fdd 100644\n> > > > > > --- a/src/libcamera/camera.cpp\n> > > > > > +++ b/src/libcamera/camera.cpp\n> > > > > > @@ -21,6 +21,7 @@\n> > > > > >  #include <libcamera/color_space.h>\n> > > > > >  #include <libcamera/control_ids.h>\n> > > > > >  #include <libcamera/framebuffer_allocator.h>\n> > > > > > +#include <libcamera/property_ids.h>\n> > > > > >  #include <libcamera/request.h>\n> > > > > >  #include <libcamera/stream.h>\n> > > > > >\n> > > > > > @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> > > > > >   * \\param[in] pipe The pipeline handler responsible for the camera device\n> > > > > >   */\n> > > > > >  Camera::Private::Private(PipelineHandler *pipe)\n> > > > > > -     : requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > > > > +     : controlInfo_({}, controls::controls), properties_(properties::properties),\n> > > > > > +       requestSequence_(0), pipe_(pipe->shared_from_this()),\n> > > > > >         disconnected_(false), state_(CameraAvailable)\n> > > > > >  {\n> > > > > >  }\n> > > > > > --\n> > > > > > 2.48.1\n> > > > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 CDD18C32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 Mar 2025 11:22:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DD99068942;\n\tTue, 18 Mar 2025 12:22:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5AD5F68940\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Mar 2025 12:22:22 +0100 (CET)","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 C9E2513DE;\n\tTue, 18 Mar 2025 12:20:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pyUdXJ2g\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742296839;\n\tbh=gPMzvfITr8YVh6+f1GTX0pgM+bZyMKCRBo0rsHjMVuA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pyUdXJ2gd5cham8/1QCYhZBSQzfdN/Lj7pDXiBayTfCF77noRHAW0nB/s8tBeTFe7\n\tyQ5BGXKFlD5uKyflGh0fVYAkC3HeHdtvpOuZb9VFhrYXw6cBzt3gHIOA/s5A52STdE\n\tJOHpuldHMORc47embePe2jUJ2Ky+LpwKEIqeGZOc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<h6qz7fp737cywyl5x6pajgdazhkitwoftdbey3ot4w3dgriwum@piq34l26e3hc>","References":"<20250310170246.185147-1-barnabas.pocze@ideasonboard.com>\n\t<7b3zctlhgibhb5paejr3yge5wpmm4g64zdw4d3a3367nrr57zb@kx5fel527xnl>\n\t<174223387170.3727128.13347239722253876818@ping.linuxembedded.co.uk>\n\t<35j4zwmlhwzmnudby2yb2qmcekublmwxirsoqx2mroiczhlzbm@ivmz75wh4b3r>\n\t<174224823043.110831.15369215913597599467@ping.linuxembedded.co.uk>\n\t<h6qz7fp737cywyl5x6pajgdazhkitwoftdbey3ot4w3dgriwum@piq34l26e3hc>","Subject":"Re: [PATCH v1] libcamera: camera: Ensure correct id maps are always\n\tset","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, =?utf-8?q?Barnab=C3=A1s_?=\n\t=?utf-8?b?UMWRY3pl?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Tue, 18 Mar 2025 11:22:18 +0000","Message-ID":"<174229693884.1463678.5650343929901880137@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]