[{"id":26463,"web_url":"https://patchwork.libcamera.org/comment/26463/","msgid":"<20230220082308.e5mup7spjkxqffxr@uno.localdomain>","date":"2023-02-20T08:23:08","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hello Barnabás\n\nOn Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> Date: Wed, 15 Feb 2023 17:48:52 +0000\n> From: Barnabás Pőcze \n> To: libcamera-devel@lists.libcamera.org\n> Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> of vector\n>\n> Change the parameter type of `generateConfiguration()`\n> from an std::vector to libcamera::Span. There is no need\n> to require a dynamic allocation. A span is preferable\n> to a const vector ref.\n\nI might be missing what the benefit would be here... I understand that, being\na Span nothing but a lightweight container for a pointer to memory and a size,\nthis allows to use multiple STL containers to pass roles in, but I\nwonder if there any real benefit.\n\nI do expect application to either parse user provided options and\nthus need to construct a dynamically grown list of roles, or either\npass in an initializers list\n\n> A new overload is added that accepts a C-style array so that\n>\n> cam->generateConfiguration({ ... })\n>\n> keeps working.\n\nFor which you need an overload.\n\nCan you expand a bit more on the intended use case ?\n\nThanks\n j\n\n>\n> There is no API break since a span can be constructed from\n> a vector and the array overload takes care of the initializer lists,\n> but this change causes an ABI break.\n>\n> Signed-off-by: Barnabás Pőcze \n> ---\n>\n> I think the change is reasonable, but there are two questions:\n>\n> * what to do with `StreamRoles`? should it be removed?\n> * is the array overload needed? if an API break is acceptable\n> it can be removed and users could write e.g.\n> generateConfiguration(std::array { ... })\n>\n> ---\n> Documentation/guides/pipeline-handler.rst | 4 ++--\n> include/libcamera/camera.h | 9 ++++++++-\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> src/libcamera/camera.cpp | 2 +-\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> src/libcamera/pipeline/simple/simple.cpp | 4 ++--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--\n> src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--\n> src/py/libcamera/py_main.cpp | 5 ++++-\n> 12 files changed, 30 insertions(+), 20 deletions(-)\n>\n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index e1930fdf..92d6d043 100644\n> --- a/Documentation/guides/pipeline-handler.rst\n> +++ b/Documentation/guides/pipeline-handler.rst\n> @@ -203,7 +203,7 @@ implementations for the overridden class members.\n> PipelineHandlerVivid(CameraManager *manager);\n>\n> CameraConfiguration *generateConfiguration(Camera *camera,\n> - const StreamRoles &roles) override;\n> + Span roles) override;\n> int configure(Camera *camera, CameraConfiguration *config) override;\n>\n> int exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -223,7 +223,7 @@ implementations for the overridden class members.\n> }\n>\n> CameraConfiguration *PipelineHandlerVivid::generateConfiguration(Camera *camera,\n> - const StreamRoles &roles)\n> + Span roles)\n> {\n> return nullptr;\n> }\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 5bb06584..ccf0d24c 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -105,7 +105,14 @@ public:\n> \tconst ControlList &properties() const;\n>\n> \tconst std::set &streams() const;\n> -\tstd::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> +\tstd::unique_ptr generateConfiguration(Span roles = {});\n> +\n> +\ttemplate\n> +\tstd::unique_ptr generateConfiguration(const StreamRole (&roles)[N])\n> +\t{\n> +\t\treturn generateConfiguration(Span(roles));\n> +\t}\n> +\n> \tint configure(CameraConfiguration *config);\n>\n> \tstd::unique_ptr createRequest(uint64_t cookie = 0);\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 4c4dfe62..aaeb3a9e 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -49,7 +49,7 @@ public:\n> \tvoid release(Camera *camera);\n>\n> \tvirtual std::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) = 0;\n> +\t\tSpan roles) = 0;\n> \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n>\n> \tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 0da167a7..0f68a511 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -938,7 +938,7 @@ const std::set &Camera::streams() const\n> * \\return A CameraConfiguration if the requested roles can be satisfied, or a\n> * null pointer otherwise.\n> */\n> -std::unique_ptr Camera::generateConfiguration(const StreamRoles &roles)\n> +std::unique_ptr Camera::generateConfiguration(Span roles)\n> {\n> \tPrivate *const d = _d();\n>\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 0c67e35d..df5919ed 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -112,7 +112,7 @@ public:\n> \tbool match(DeviceEnumerator *enumerator) override;\n>\n> \tstd::unique_ptr\n> -\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> +\tgenerateConfiguration(Camera *camera, Span roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -571,7 +571,7 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n>\n> std::unique_ptr\n> PipelineHandlerISI::generateConfiguration(Camera *camera,\n> -\t\t\t\t\t const StreamRoles &roles)\n> +\t\t\t\t\t Span roles)\n> {\n> \tISICameraData *data = cameraData(camera);\n> \tstd::unique_ptr config =\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 355cb0cb..ada8c272 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -135,7 +135,7 @@ public:\n> \tPipelineHandlerIPU3(CameraManager *manager);\n>\n> \tstd::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) override;\n> +\t\tSpan roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -390,7 +390,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> }\n>\n> std::unique_ptr\n> -PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> +PipelineHandlerIPU3::generateConfiguration(Camera *camera, Span roles)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tstd::unique_ptr config =\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 84120954..f62570cc 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -361,7 +361,7 @@ public:\n> \tPipelineHandlerRPi(CameraManager *manager);\n>\n> \tstd::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) override;\n> +\t\tSpan roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -661,7 +661,7 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n> }\n>\n> std::unique_ptr\n> -PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> +PipelineHandlerRPi::generateConfiguration(Camera *camera, Span roles)\n> {\n> \tRPiCameraData *data = cameraData(camera);\n> \tstd::unique_ptr config =\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 8a30fe06..1fdfde7b 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -148,7 +148,7 @@ public:\n> \tPipelineHandlerRkISP1(CameraManager *manager);\n>\n> \tstd::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) override;\n> +\t\tSpan roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n>\n> std::unique_ptr\n> PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> -\tconst StreamRoles &roles)\n> +\tSpan roles)\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 24ded4db..9d71a369 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -312,7 +312,7 @@ public:\n> \tSimplePipelineHandler(CameraManager *manager);\n>\n> \tstd::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) override;\n> +\t\tSpan roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -1038,7 +1038,7 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)\n> }\n>\n> std::unique_ptr\n> -SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> +SimplePipelineHandler::generateConfiguration(Camera *camera, Span roles)\n> {\n> \tSimpleCameraData *data = cameraData(camera);\n> \tstd::unique_ptr config =\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 277465b7..03935876 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -75,7 +75,7 @@ public:\n> \tPipelineHandlerUVC(CameraManager *manager);\n>\n> \tstd::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) override;\n> +\t\tSpan roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n>\n> std::unique_ptr\n> PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> -\tconst StreamRoles &roles)\n> +\tSpan roles)\n> {\n> \tUVCCameraData *data = cameraData(camera);\n> \tstd::unique_ptr config =\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 204f5ad7..49ee949f 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -85,7 +85,7 @@ public:\n> \tPipelineHandlerVimc(CameraManager *manager);\n>\n> \tstd::unique_ptr generateConfiguration(Camera *camera,\n> -\t\tconst StreamRoles &roles) override;\n> +\t\tSpan roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n>\n> \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n>\n> std::unique_ptr\n> PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> -\tconst StreamRoles &roles)\n> +\tSpan roles)\n> {\n> \tVimcCameraData *data = cameraData(camera);\n> \tstd::unique_ptr config =\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index d14e18e2..3f04871f 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -156,7 +156,10 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t})\n>\n> \t\t/* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> -\t\t.def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> +\t\t.def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> +\t\t\treturn self.generateConfiguration(roles);\n> +\t\t}, py::keep_alive<0, 1>())\n> +\n> \t\t.def(\"configure\", &Camera::configure)\n>\n> \t\t.def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> --\n> 2.39.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B53EABDCBF\n\tfor ;\n\tMon, 20 Feb 2023 08:23:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2F2162606;\n\tMon, 20 Feb 2023 09:23:13 +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 C96D261EE5\n\tfor ;\n\tMon, 20 Feb 2023 09:23:12 +0100 (CET)","from ideasonboard.com (host-95-252-227-22.retail.telecomitalia.it\n\t[95.252.227.22])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C73A1A49;\n\tMon, 20 Feb 2023 09:23:11 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1676881394;\n\tbh=FP8gLSByoor7Kb4yhsob9Z5T3D3E3V/NirARueYZXYA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=W5RNeqN+3b8++jv0ShChHu/5ipfY1eJkJ7a0SXiicGEf+Ct2Re7B/F6WZxQt8RN8p\n\tva0Pci+GkeVCC+dhBBzdbzF+6BfST3K67UzJrrHrKmDdMrogUn18EMuFICdwjsrAjg\n\tJZZ0ej6Fdb2QZQkjzYqbSyHgGbSWxDu4FK0rf99VC24asr7WsZwi+dVSF8m3DBlmZK\n\tlQBtv8hRSuxmfdL9VJWMo3dMbJyqMllRUXE5pnEYnp2vx7cTtawMlKgjxP1LMj1vB7\n\tQgA5QUPyVBuY3FyygXnvwzm4HD9UTc4jwRFFYE1OwMmMgeRxNsA6TR1xZFoPD3OwKS\n\tKKn7HodSZbD5Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1676881392;\n\tbh=FP8gLSByoor7Kb4yhsob9Z5T3D3E3V/NirARueYZXYA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RVeF/rswO7BLtw4c/BhrJCXSxILKV3n3X2ydh6DSsGwoJvQ2+COAQ/a0bR+y4OSOh\n\tQoNh53i6gVqQwdCmtd1XgtfSMHDTjtkJJd76o81Yks0UzRrZ5kXzDa38AWi8RNxzWJ\n\tCo8arwTSiPdN9BS4VK+RT6iwqc5P2hNyY5jOltyI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RVeF/rsw\"; dkim-atps=neutral","Date":"Mon, 20 Feb 2023 09:23:08 +0100","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Message-ID":"<20230220082308.e5mup7spjkxqffxr@uno.localdomain>","References":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26467,"web_url":"https://patchwork.libcamera.org/comment/26467/","msgid":"","date":"2023-02-20T12:58:54","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. február 20., hétfő 9:23 keltezéssel, Jacopo Mondi írta:\n\n> Hello Barnabás\n> \n> On Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > Date: Wed, 15 Feb 2023 17:48:52 +0000\n> > From: Barnabás Pőcze \n> > To: libcamera-devel@lists.libcamera.org\n> > Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> > of vector\n> >\n> > Change the parameter type of `generateConfiguration()`\n> > from an std::vector to libcamera::Span. There is no need\n> > to require a dynamic allocation. A span is preferable\n> > to a const vector ref.\n> \n> I might be missing what the benefit would be here... I understand that, being\n> a Span nothing but a lightweight container for a pointer to memory and a size,\n> this allows to use multiple STL containers to pass roles in, but I\n> wonder if there any real benefit.\n> \n> I do expect application to either parse user provided options and\n> thus need to construct a dynamically grown list of roles, or either\n> pass in an initializers list\n> \n> > A new overload is added that accepts a C-style array so that\n> >\n> > cam->generateConfiguration({ ... })\n> >\n> > keeps working.\n> \n> For which you need an overload.\n> \n> Can you expand a bit more on the intended use case ?\n\nThe way I see it, there are two benefits:\n * any container that places elements in contiguous memory should work\n (e.g. std::array and std::vector with a custom allocator, which were\n previously not supported)\n * the cost of constructing an std::vector can be avoided in some cases\n (e.g. when a fixed list of roles is used)\n\nBut of course I understand if it is considered a micro optimization\nand rejected on that basis.\n\n\nBest regards,\nBarnabás Pőcze\n\n> \n> Thanks\n> j\n> \n> >\n> > There is no API break since a span can be constructed from\n> > a vector and the array overload takes care of the initializer lists,\n> > but this change causes an ABI break.\n> >\n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> >\n> > I think the change is reasonable, but there are two questions:\n> >\n> > * what to do with `StreamRoles`? should it be removed?\n> > * is the array overload needed? if an API break is acceptable\n> > it can be removed and users could write e.g.\n> > generateConfiguration(std::array { ... })\n> >\n> > ---\n> > Documentation/guides/pipeline-handler.rst | 4 ++--\n> > include/libcamera/camera.h | 9 ++++++++-\n> > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > src/libcamera/camera.cpp | 2 +-\n> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> > src/libcamera/pipeline/simple/simple.cpp | 4 ++--\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--\n> > src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--\n> > src/py/libcamera/py_main.cpp | 5 ++++-\n> > 12 files changed, 30 insertions(+), 20 deletions(-)\n> >\n> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > index e1930fdf..92d6d043 100644\n> > --- a/Documentation/guides/pipeline-handler.rst\n> > +++ b/Documentation/guides/pipeline-handler.rst\n> > @@ -203,7 +203,7 @@ implementations for the overridden class members.\n> > PipelineHandlerVivid(CameraManager *manager);\n> >\n> > CameraConfiguration *generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles) override;\n> > + Span roles) override;\n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > int exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -223,7 +223,7 @@ implementations for the overridden class members.\n> > }\n> >\n> > CameraConfiguration *PipelineHandlerVivid::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > return nullptr;\n> > }\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 5bb06584..ccf0d24c 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -105,7 +105,14 @@ public:\n> > \tconst ControlList &properties() const;\n> >\n> > \tconst std::set &streams() const;\n> > -\tstd::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> > +\tstd::unique_ptr generateConfiguration(Span roles = {});\n> > +\n> > +\ttemplate\n> > +\tstd::unique_ptr generateConfiguration(const StreamRole (&roles)[N])\n> > +\t{\n> > +\t\treturn generateConfiguration(Span(roles));\n> > +\t}\n> > +\n> > \tint configure(CameraConfiguration *config);\n> >\n> > \tstd::unique_ptr createRequest(uint64_t cookie = 0);\n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index 4c4dfe62..aaeb3a9e 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -49,7 +49,7 @@ public:\n> > \tvoid release(Camera *camera);\n> >\n> > \tvirtual std::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) = 0;\n> > +\t\tSpan roles) = 0;\n> > \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> >\n> > \tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 0da167a7..0f68a511 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -938,7 +938,7 @@ const std::set &Camera::streams() const\n> > * \\return A CameraConfiguration if the requested roles can be satisfied, or a\n> > * null pointer otherwise.\n> > */\n> > -std::unique_ptr Camera::generateConfiguration(const StreamRoles &roles)\n> > +std::unique_ptr Camera::generateConfiguration(Span roles)\n> > {\n> > \tPrivate *const d = _d();\n> >\n> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > index 0c67e35d..df5919ed 100644\n> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > @@ -112,7 +112,7 @@ public:\n> > \tbool match(DeviceEnumerator *enumerator) override;\n> >\n> > \tstd::unique_ptr\n> > -\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > +\tgenerateConfiguration(Camera *camera, Span roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -571,7 +571,7 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > -\t\t\t\t\t const StreamRoles &roles)\n> > +\t\t\t\t\t Span roles)\n> > {\n> > \tISICameraData *data = cameraData(camera);\n> > \tstd::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 355cb0cb..ada8c272 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -135,7 +135,7 @@ public:\n> > \tPipelineHandlerIPU3(CameraManager *manager);\n> >\n> > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) override;\n> > +\t\tSpan roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -390,7 +390,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> > }\n> >\n> > std::unique_ptr\n> > -PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > +PipelineHandlerIPU3::generateConfiguration(Camera *camera, Span roles)\n> > {\n> > \tIPU3CameraData *data = cameraData(camera);\n> > \tstd::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 84120954..f62570cc 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -361,7 +361,7 @@ public:\n> > \tPipelineHandlerRPi(CameraManager *manager);\n> >\n> > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) override;\n> > +\t\tSpan roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -661,7 +661,7 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n> > }\n> >\n> > std::unique_ptr\n> > -PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > +PipelineHandlerRPi::generateConfiguration(Camera *camera, Span roles)\n> > {\n> > \tRPiCameraData *data = cameraData(camera);\n> > \tstd::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 8a30fe06..1fdfde7b 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -148,7 +148,7 @@ public:\n> > \tPipelineHandlerRkISP1(CameraManager *manager);\n> >\n> > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) override;\n> > +\t\tSpan roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > -\tconst StreamRoles &roles)\n> > +\tSpan roles)\n> > {\n> > \tRkISP1CameraData *data = cameraData(camera);\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 24ded4db..9d71a369 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -312,7 +312,7 @@ public:\n> > \tSimplePipelineHandler(CameraManager *manager);\n> >\n> > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) override;\n> > +\t\tSpan roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -1038,7 +1038,7 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)\n> > }\n> >\n> > std::unique_ptr\n> > -SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > +SimplePipelineHandler::generateConfiguration(Camera *camera, Span roles)\n> > {\n> > \tSimpleCameraData *data = cameraData(camera);\n> > \tstd::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 277465b7..03935876 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -75,7 +75,7 @@ public:\n> > \tPipelineHandlerUVC(CameraManager *manager);\n> >\n> > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) override;\n> > +\t\tSpan roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> > -\tconst StreamRoles &roles)\n> > +\tSpan roles)\n> > {\n> > \tUVCCameraData *data = cameraData(camera);\n> > \tstd::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 204f5ad7..49ee949f 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -85,7 +85,7 @@ public:\n> > \tPipelineHandlerVimc(CameraManager *manager);\n> >\n> > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > -\t\tconst StreamRoles &roles) override;\n> > +\t\tSpan roles) override;\n> > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> > -\tconst StreamRoles &roles)\n> > +\tSpan roles)\n> > {\n> > \tVimcCameraData *data = cameraData(camera);\n> > \tstd::unique_ptr config =\n> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > index d14e18e2..3f04871f 100644\n> > --- a/src/py/libcamera/py_main.cpp\n> > +++ b/src/py/libcamera/py_main.cpp\n> > @@ -156,7 +156,10 @@ PYBIND11_MODULE(_libcamera, m)\n> > \t\t})\n> >\n> > \t\t/* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> > -\t\t.def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> > +\t\t.def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> > +\t\t\treturn self.generateConfiguration(roles);\n> > +\t\t}, py::keep_alive<0, 1>())\n> > +\n> > \t\t.def(\"configure\", &Camera::configure)\n> >\n> > \t\t.def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > --\n> > 2.39.1\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 03B2EBDCBF\n\tfor ;\n\tMon, 20 Feb 2023 12:59:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5FBA662600;\n\tMon, 20 Feb 2023 13:59:09 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1676897949;\n\tbh=C73TZoufd1cG9DBOwaeoBwVIb8A++BfQdGj0y0M1B84=;\n\th=Date:To:In-Reply-To:References:List-Id:List-Post:From:Cc:\n\tList-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help:\n\tSubject:From;\n\tb=H0LptQe0dr3E7h8nkhfe+V18vPRGSIiz1Ij0/BJdfsT2EIpo3Bt73AhnU5JKG7f9C\n\trQBxrkt94j6ZbLEI43q6p0leNENePGLgcCxKLdExDsqHGMpZsATaP+lKFsnVFXzHZR\n\tZOAkpo+izUNqHxLAsBvMeEq8Npq8TCJXRw/FnE88O52QoIL20Gb6XO793eDNxNgUpW\n\t/Wdc1zE3UfSusFvN1qg36k8GLc4oZE8RjPynU4XAu3S1X9kF5DbvFz49ZPkUBGEE5a\n\tC0d2KSIF0GGW1rvFpgKTFd/jev/T8nTY8jL621mFY7TKV9AJgj4MgncxFf0USP6s7a\n\tkcZNdsKFU7QLw==","Date":"Mon, 20 Feb 2023 12:58:54 +0000","To":"Jacopo Mondi ","In-Reply-To":"<20230220082308.e5mup7spjkxqffxr@uno.localdomain>","References":"\n\t<20230220082308.e5mup7spjkxqffxr@uno.localdomain>","MIME-Version":"1.0","Message-ID":"","List-Id":"","List-Post":"","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Precedence":"list","Cc":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","X-BeenThere":"libcamera-devel@lists.libcamera.org","List-Subscribe":",\n\t","List-Unsubscribe":",\n\t","List-Archive":"","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","List-Help":"","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","Content-Type":"message/rfc822","Content-Disposition":"inline","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26468,"web_url":"https://patchwork.libcamera.org/comment/26468/","msgid":"<20230220160846.bso4mobcuvjss5u5@uno.localdomain>","date":"2023-02-20T16:08:46","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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, Feb 20, 2023 at 12:58:54PM +0000, Barnabás Pőcze wrote:\n> Hi\n>\n>\n> 2023. február 20., hétfő 9:23 keltezéssel, Jacopo Mondi írta:\n>\n> > Hello Barnabás\n> >\n> > On Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > > Date: Wed, 15 Feb 2023 17:48:52 +0000\n> > > From: Barnabás Pőcze \n> > > To: libcamera-devel@lists.libcamera.org\n> > > Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> > > of vector\n> > >\n> > > Change the parameter type of `generateConfiguration()`\n> > > from an std::vector to libcamera::Span. There is no need\n> > > to require a dynamic allocation. A span is preferable\n> > > to a const vector ref.\n> >\n> > I might be missing what the benefit would be here... I understand that, being\n> > a Span nothing but a lightweight container for a pointer to memory and a size,\n> > this allows to use multiple STL containers to pass roles in, but I\n> > wonder if there any real benefit.\n> >\n> > I do expect application to either parse user provided options and\n> > thus need to construct a dynamically grown list of roles, or either\n> > pass in an initializers list\n> >\n> > > A new overload is added that accepts a C-style array so that\n> > >\n> > > cam->generateConfiguration({ ... })\n> > >\n> > > keeps working.\n> >\n> > For which you need an overload.\n> >\n> > Can you expand a bit more on the intended use case ?\n>\n> The way I see it, there are two benefits:\n> * any container that places elements in contiguous memory should work\n> (e.g. std::array and std::vector with a custom allocator, which were\n> previously not supported)\n\nYou're correct, but considering how applications are expected to\nconstruct the StreamRole vector, I hardly see that being strictly\nnecessary to be done in an array\n\n> * the cost of constructing an std::vector can be avoided in some cases\n> (e.g. when a fixed list of roles is used)\n\nCorrect, but this is not an hard path and the generateConfiguration()\nis expected to be called once per streaming session at most\n\n>\n> But of course I understand if it is considered a micro optimization\n> and rejected on that basis.\n>\n\nIf this wasn't changing the public API it would have be easier indeed\nto accept it.\n\nLet's see what others think\n\n>\n> Best regards,\n> Barnabás Pőcze\n>\n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > There is no API break since a span can be constructed from\n> > > a vector and the array overload takes care of the initializer lists,\n> > > but this change causes an ABI break.\n> > >\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > >\n> > > I think the change is reasonable, but there are two questions:\n> > >\n> > > * what to do with `StreamRoles`? should it be removed?\n> > > * is the array overload needed? if an API break is acceptable\n> > > it can be removed and users could write e.g.\n> > > generateConfiguration(std::array { ... })\n> > >\n> > > ---\n> > > Documentation/guides/pipeline-handler.rst | 4 ++--\n> > > include/libcamera/camera.h | 9 ++++++++-\n> > > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > > src/libcamera/camera.cpp | 2 +-\n> > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> > > src/libcamera/pipeline/simple/simple.cpp | 4 ++--\n> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--\n> > > src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--\n> > > src/py/libcamera/py_main.cpp | 5 ++++-\n> > > 12 files changed, 30 insertions(+), 20 deletions(-)\n> > >\n> > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > > index e1930fdf..92d6d043 100644\n> > > --- a/Documentation/guides/pipeline-handler.rst\n> > > +++ b/Documentation/guides/pipeline-handler.rst\n> > > @@ -203,7 +203,7 @@ implementations for the overridden class members.\n> > > PipelineHandlerVivid(CameraManager *manager);\n> > >\n> > > CameraConfiguration *generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles) override;\n> > > + Span roles) override;\n> > > int configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > int exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -223,7 +223,7 @@ implementations for the overridden class members.\n> > > }\n> > >\n> > > CameraConfiguration *PipelineHandlerVivid::generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles)\n> > > + Span roles)\n> > > {\n> > > return nullptr;\n> > > }\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 5bb06584..ccf0d24c 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -105,7 +105,14 @@ public:\n> > > \tconst ControlList &properties() const;\n> > >\n> > > \tconst std::set &streams() const;\n> > > -\tstd::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> > > +\tstd::unique_ptr generateConfiguration(Span roles = {});\n> > > +\n> > > +\ttemplate\n> > > +\tstd::unique_ptr generateConfiguration(const StreamRole (&roles)[N])\n> > > +\t{\n> > > +\t\treturn generateConfiguration(Span(roles));\n> > > +\t}\n> > > +\n> > > \tint configure(CameraConfiguration *config);\n> > >\n> > > \tstd::unique_ptr createRequest(uint64_t cookie = 0);\n> > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > index 4c4dfe62..aaeb3a9e 100644\n> > > --- a/include/libcamera/internal/pipeline_handler.h\n> > > +++ b/include/libcamera/internal/pipeline_handler.h\n> > > @@ -49,7 +49,7 @@ public:\n> > > \tvoid release(Camera *camera);\n> > >\n> > > \tvirtual std::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) = 0;\n> > > +\t\tSpan roles) = 0;\n> > > \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> > >\n> > > \tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 0da167a7..0f68a511 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -938,7 +938,7 @@ const std::set &Camera::streams() const\n> > > * \\return A CameraConfiguration if the requested roles can be satisfied, or a\n> > > * null pointer otherwise.\n> > > */\n> > > -std::unique_ptr Camera::generateConfiguration(const StreamRoles &roles)\n> > > +std::unique_ptr Camera::generateConfiguration(Span roles)\n> > > {\n> > > \tPrivate *const d = _d();\n> > >\n> > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > index 0c67e35d..df5919ed 100644\n> > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > @@ -112,7 +112,7 @@ public:\n> > > \tbool match(DeviceEnumerator *enumerator) override;\n> > >\n> > > \tstd::unique_ptr\n> > > -\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > > +\tgenerateConfiguration(Camera *camera, Span roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -571,7 +571,7 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > > -\t\t\t\t\t const StreamRoles &roles)\n> > > +\t\t\t\t\t Span roles)\n> > > {\n> > > \tISICameraData *data = cameraData(camera);\n> > > \tstd::unique_ptr config =\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 355cb0cb..ada8c272 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -135,7 +135,7 @@ public:\n> > > \tPipelineHandlerIPU3(CameraManager *manager);\n> > >\n> > > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) override;\n> > > +\t\tSpan roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -390,7 +390,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> > > }\n> > >\n> > > std::unique_ptr\n> > > -PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > > +PipelineHandlerIPU3::generateConfiguration(Camera *camera, Span roles)\n> > > {\n> > > \tIPU3CameraData *data = cameraData(camera);\n> > > \tstd::unique_ptr config =\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 84120954..f62570cc 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -361,7 +361,7 @@ public:\n> > > \tPipelineHandlerRPi(CameraManager *manager);\n> > >\n> > > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) override;\n> > > +\t\tSpan roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -661,7 +661,7 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n> > > }\n> > >\n> > > std::unique_ptr\n> > > -PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > > +PipelineHandlerRPi::generateConfiguration(Camera *camera, Span roles)\n> > > {\n> > > \tRPiCameraData *data = cameraData(camera);\n> > > \tstd::unique_ptr config =\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 8a30fe06..1fdfde7b 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -148,7 +148,7 @@ public:\n> > > \tPipelineHandlerRkISP1(CameraManager *manager);\n> > >\n> > > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) override;\n> > > +\t\tSpan roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > > -\tconst StreamRoles &roles)\n> > > +\tSpan roles)\n> > > {\n> > > \tRkISP1CameraData *data = cameraData(camera);\n> > >\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 24ded4db..9d71a369 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -312,7 +312,7 @@ public:\n> > > \tSimplePipelineHandler(CameraManager *manager);\n> > >\n> > > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) override;\n> > > +\t\tSpan roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -1038,7 +1038,7 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)\n> > > }\n> > >\n> > > std::unique_ptr\n> > > -SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > > +SimplePipelineHandler::generateConfiguration(Camera *camera, Span roles)\n> > > {\n> > > \tSimpleCameraData *data = cameraData(camera);\n> > > \tstd::unique_ptr config =\n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 277465b7..03935876 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -75,7 +75,7 @@ public:\n> > > \tPipelineHandlerUVC(CameraManager *manager);\n> > >\n> > > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) override;\n> > > +\t\tSpan roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> > > -\tconst StreamRoles &roles)\n> > > +\tSpan roles)\n> > > {\n> > > \tUVCCameraData *data = cameraData(camera);\n> > > \tstd::unique_ptr config =\n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index 204f5ad7..49ee949f 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -85,7 +85,7 @@ public:\n> > > \tPipelineHandlerVimc(CameraManager *manager);\n> > >\n> > > \tstd::unique_ptr generateConfiguration(Camera *camera,\n> > > -\t\tconst StreamRoles &roles) override;\n> > > +\t\tSpan roles) override;\n> > > \tint configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > \tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> > > -\tconst StreamRoles &roles)\n> > > +\tSpan roles)\n> > > {\n> > > \tVimcCameraData *data = cameraData(camera);\n> > > \tstd::unique_ptr config =\n> > > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > > index d14e18e2..3f04871f 100644\n> > > --- a/src/py/libcamera/py_main.cpp\n> > > +++ b/src/py/libcamera/py_main.cpp\n> > > @@ -156,7 +156,10 @@ PYBIND11_MODULE(_libcamera, m)\n> > > \t\t})\n> > >\n> > > \t\t/* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> > > -\t\t.def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> > > +\t\t.def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> > > +\t\t\treturn self.generateConfiguration(roles);\n> > > +\t\t}, py::keep_alive<0, 1>())\n> > > +\n> > > \t\t.def(\"configure\", &Camera::configure)\n> > >\n> > > \t\t.def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > > --\n> > > 2.39.1\n> > >\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A4134BE080\n\tfor ;\n\tMon, 20 Feb 2023 16:08:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F09C562600;\n\tMon, 20 Feb 2023 17:08:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE024625FF\n\tfor ;\n\tMon, 20 Feb 2023 17:08:50 +0100 (CET)","from ideasonboard.com (host-95-252-227-22.retail.telecomitalia.it\n\t[95.252.227.22])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BC4E7A49;\n\tMon, 20 Feb 2023 17:08:49 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1676909333;\n\tbh=TuCvWEeL12ttdk0v27THZWiZ0pucjFt66iHfGNvpOIg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ZHGk9V2gCPuD5b6jfITiBNNxj5URKtyM8GvprbgzX6TA7LzKmNFvzAqUSNdkjb7Yx\n\t8D1ReOk75yNq1a63zFDPbId7PHJQYCjaN5D2XsK4DJ6WZSyLoEBDvQCB9ToIVQsM2T\n\tPbK+oqiu4PImSMxUun1FZSlVzxeB7Fpa7rM3JP+dCxcZ7Ao+GpbZIJFr6uEANiHVWT\n\t1Rij3fPj/BIK3/HJNMpXKstVuAU6Ak3jdKEoVujFd2M59ApFq4lpC+lBD5EcuwdEUX\n\t3u0KZgy1OsN6MtDVaFsvtU/KYBQxKcCEbRNaXawQfRyYE2XXUEt4/8TKQfUnjDyf0p\n\tOEPyMPmCcWT8w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1676909330;\n\tbh=TuCvWEeL12ttdk0v27THZWiZ0pucjFt66iHfGNvpOIg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wk/zAyL1Zx1k7czNH6+3CRZzyiqn6yl6/LzzGVZUUOg1chHr21iv1u4S48hrNoDg9\n\tcEiMRonV/FzfE6x/T3UJAA8KUNjZHDE9uE7NJiMKQtveOUy9Q/3cQuTWt4lUq26oUS\n\tlOiqeMFF9j9U0sih8kfzkNi/csJ1w3cVM22WHYOY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"wk/zAyL1\"; dkim-atps=neutral","Date":"Mon, 20 Feb 2023 17:08:46 +0100","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Message-ID":"<20230220160846.bso4mobcuvjss5u5@uno.localdomain>","References":"\n\t<20230220082308.e5mup7spjkxqffxr@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26909,"web_url":"https://patchwork.libcamera.org/comment/26909/","msgid":"","date":"2023-04-19T14:55:39","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. február 20., hétfő 17:08 keltezéssel, Jacopo Mondi írta:\n\n> Hi Barnabás\n> \n> On Mon, Feb 20, 2023 at 12:58:54PM +0000, Barnabás Pőcze wrote:\n> > Hi\n> >\n> >\n> > 2023. február 20., hétfő 9:23 keltezéssel, Jacopo Mondi írta:\n> >\n> > > Hello Barnabás\n> > >\n> > > On Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > > > Date: Wed, 15 Feb 2023 17:48:52 +0000\n> > > > From: Barnabás Pőcze \n> > > > To: libcamera-devel@lists.libcamera.org\n> > > > Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> > > > of vector\n> > > >\n> > > > Change the parameter type of `generateConfiguration()`\n> > > > from an std::vector to libcamera::Span. There is no need\n> > > > to require a dynamic allocation. A span is preferable\n> > > > to a const vector ref.\n> > >\n> > > I might be missing what the benefit would be here... I understand that, being\n> > > a Span nothing but a lightweight container for a pointer to memory and a size,\n> > > this allows to use multiple STL containers to pass roles in, but I\n> > > wonder if there any real benefit.\n> > >\n> > > I do expect application to either parse user provided options and\n> > > thus need to construct a dynamically grown list of roles, or either\n> > > pass in an initializers list\n> > >\n> > > > A new overload is added that accepts a C-style array so that\n> > > >\n> > > > cam->generateConfiguration({ ... })\n> > > >\n> > > > keeps working.\n> > >\n> > > For which you need an overload.\n> > >\n> > > Can you expand a bit more on the intended use case ?\n> >\n> > The way I see it, there are two benefits:\n> > * any container that places elements in contiguous memory should work\n> > (e.g. std::array and std::vector with a custom allocator, which were\n> > previously not supported)\n> \n> You're correct, but considering how applications are expected to\n> construct the StreamRole vector, I hardly see that being strictly\n> necessary to be done in an array\n> \n> > * the cost of constructing an std::vector can be avoided in some cases\n> > (e.g. when a fixed list of roles is used)\n> \n> Correct, but this is not an hard path and the generateConfiguration()\n> is expected to be called once per streaming session at most\n> \n> >\n> > But of course I understand if it is considered a micro optimization\n> > and rejected on that basis.\n> >\n> \n> If this wasn't changing the public API it would have be easier indeed\n> to accept it.\n\nTechnically it changes the API, but I would argue that in such a minimal\nway that it is essentially unnoticeable. And as far as I understand\nlibcamera does not provide stable API/ABI guarantees yet. Am I mistaken?\n\n\n> \n> Let's see what others think\n\nI am wondering if anyone has other thoughts about this? If not, would it be\npossible for me to get a definitive yes/no?\n\n\n> [...]\n\n\nRegards,\nBarnabás Pőcze","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 76BEEBD1F1\n\tfor ;\n\tWed, 19 Apr 2023 14:55:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A668627B1;\n\tWed, 19 Apr 2023 16:55:55 +0200 (CEST)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 51391603A1\n\tfor ;\n\tWed, 19 Apr 2023 16:55:53 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1681916155;\n\tbh=+9Okv+1EHL65H3piPmuZs3CM59U4v4OKNhsLLGyuOk8=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=bKGQRsYbZ6NdyyKH5NpdKCy896pk63jWDyQrcdgiiEWM5DZhDRc6pc5OpSove3ZpS\n\t80AGygm1itZPjuWXdBm/ucn9GvVElyzPLE9avNeFcYat5qwe03yU+WHYYb2szCjQoh\n\tO34TQ07d520TtFyAgLWtX+bDjz2GKY1WJIc9QJWMEOtKBwp4I4hZmTF3Gwg1TWxCRI\n\tzMG6RdxYG6avoBTLVal4KiZtTVL7SeVTvGkvAXBQ/ry+WhPDpfB3Mu9tH6mbahqOs7\n\tOeEN3C6TbI5GZ3G6bGtYne3N4ZpSD7NVE/hgF+Z7AqhlK0LsFM2aMhxcaBHyj5pBi8\n\tz94ZrdvaAKoSQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1681916152; x=1682175352;\n\tbh=pn9ptP4KlZ+iUQYMiV9FuUpsSBZ8tBAQFTa54G9pZVo=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=RLUygTM9ULuZUBQ31TySsVwZKTPga9/3YaN9d1f7soAixvq9XO3zi0JWYwB3THD4l\n\t5t0QqKl8s4poxHW06vSqyfeYwODBEGXuKpVFmcvXKm2Gy6C9tyotmfOBk/qcngyazJ\n\tr+67ZXdep64B7Edb+TNMigDap91c8l5jdoojp0zovPF02fRt7Qc8uzAXt4ejpiPsaK\n\tmLaAkBID6UDFfG5SGjDqpAOwTtTmmjnTHWx3WFQZGTEqXVgZtlWenRK5jHgVPkPu/m\n\tk4YhuHGOPwzLMXhqYiWy6XfHS2SNOcbPGnQyWN2u0xAwJVfstHJ/jHDbG2TaJdiqk9\n\t9RfMX2XSAKMMg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"RLUygTM9\"; dkim-atps=neutral","Date":"Wed, 19 Apr 2023 14:55:39 +0000","To":"Jacopo Mondi ","Message-ID":"","In-Reply-To":"<20230220160846.bso4mobcuvjss5u5@uno.localdomain>","References":"\n\t<20230220082308.e5mup7spjkxqffxr@uno.localdomain>\n\t\n\t<20230220160846.bso4mobcuvjss5u5@uno.localdomain>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26910,"web_url":"https://patchwork.libcamera.org/comment/26910/","msgid":"<168194116921.3199984.2349983735867560066@Monstersaurus>","date":"2023-04-19T21:52:49","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi,\n\nQuoting Barnabás Pőcze via libcamera-devel (2023-04-19 15:55:39)\n> Hi\n> \n> \n> 2023. február 20., hétfő 17:08 keltezéssel, Jacopo Mondi írta:\n> \n> > Hi Barnabás\n> > \n> > On Mon, Feb 20, 2023 at 12:58:54PM +0000, Barnabás Pőcze wrote:\n> > > Hi\n> > >\n> > >\n> > > 2023. február 20., hétfő 9:23 keltezéssel, Jacopo Mondi írta:\n> > >\n> > > > Hello Barnabás\n> > > >\n> > > > On Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > > > > Date: Wed, 15 Feb 2023 17:48:52 +0000\n> > > > > From: Barnabás Pőcze \n> > > > > To: libcamera-devel@lists.libcamera.org\n> > > > > Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> > > > > of vector\n> > > > >\n> > > > > Change the parameter type of `generateConfiguration()`\n> > > > > from an std::vector to libcamera::Span. There is no need\n> > > > > to require a dynamic allocation. A span is preferable\n> > > > > to a const vector ref.\n> > > >\n> > > > I might be missing what the benefit would be here... I understand that, being\n> > > > a Span nothing but a lightweight container for a pointer to memory and a size,\n> > > > this allows to use multiple STL containers to pass roles in, but I\n> > > > wonder if there any real benefit.\n> > > >\n> > > > I do expect application to either parse user provided options and\n> > > > thus need to construct a dynamically grown list of roles, or either\n> > > > pass in an initializers list\n> > > >\n> > > > > A new overload is added that accepts a C-style array so that\n> > > > >\n> > > > > cam->generateConfiguration({ ... })\n> > > > >\n> > > > > keeps working.\n> > > >\n> > > > For which you need an overload.\n> > > >\n> > > > Can you expand a bit more on the intended use case ?\n> > >\n> > > The way I see it, there are two benefits:\n> > > * any container that places elements in contiguous memory should work\n> > > (e.g. std::array and std::vector with a custom allocator, which were\n> > > previously not supported)\n> > \n> > You're correct, but considering how applications are expected to\n> > construct the StreamRole vector, I hardly see that being strictly\n> > necessary to be done in an array\n> > \n> > > * the cost of constructing an std::vector can be avoided in some cases\n> > > (e.g. when a fixed list of roles is used)\n> > \n> > Correct, but this is not an hard path and the generateConfiguration()\n> > is expected to be called once per streaming session at most\n> > \n> > >\n> > > But of course I understand if it is considered a micro optimization\n> > > and rejected on that basis.\n> > >\n> > \n> > If this wasn't changing the public API it would have be easier indeed\n> > to accept it.\n> \n> Technically it changes the API, but I would argue that in such a minimal\n> way that it is essentially unnoticeable. And as far as I understand\n> libcamera does not provide stable API/ABI guarantees yet. Am I mistaken?\n> \n> \n> > \n> > Let's see what others think\n> \n> I am wondering if anyone has other thoughts about this? If not, would it be\n> possible for me to get a definitive yes/no?\n\nNo specific objection to it here, This will also require an update to\nthe vivid pipeline handler, but as that's external we'd have to handle\nthat separately.\n\nLooking at the way StreamRoles is used in applications, it's already\nmostly a dynamic allocation and that is passed by reference into\nlibcamera.\n\nIs there a specific target to this to remove allocations for a given\nuse-case? (I know getting rid of allocations at runtime is highly\ndesireable for real time performance for instance).\n\nThe ABI change here is not an issue, and even though it's an API\n'change' it's still compatible as far as I can tell - so no objection\nhere either.\n\nI think Jacopo asked a question about the existing StreamRoles 'using'\nstatement. Would you see that as something we should keep? or remove\n(or deprecate?) with this change?\n\n--\nKieran\n\n\n\n> \n> \n> > [...]\n> \n> \n> Regards,\n> Barnabás Pőcze","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 ADBDCBE173\n\tfor ;\n\tWed, 19 Apr 2023 21:52:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3302627B8;\n\tWed, 19 Apr 2023 23:52:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 822FF603A1\n\tfor ;\n\tWed, 19 Apr 2023 23:52:52 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64A89CF9;\n\tWed, 19 Apr 2023 23:52:45 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1681941174;\n\tbh=hcolJ9BwTR18bW2ZEL1EdnmRMMvLBbTONrBeX7TZVVQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pUtckgsnacknOuhHgHJrMwtlpSnvtaJZJds5wT0awEWjaUawZP9ZJlWC3nksvKcKT\n\tWiqSiP+M5q1JfGXa1qXl+ANk4AQdkm2VeHVwdYK6NM8Jp1P79kFQPCTv/zFaOt3nhk\n\tdh9ke1kRn451BvSRlahQ0QFqEE/oA339kdGBtDfGJY4LYL4+/bpTwrnJpICo9rj/BY\n\tJIA0u3YQK/Em75bPm/5gi0EAOVv8huuW/+CCNXKFgEbfDVxmFTQ0R2XHIF4LHb3mYR\n\tuH5TBmECFJbJUSgIVmk5X/5yR6lmROVHeNEjR0ZlNyzGxlIRfn0vjoY6IkMY0ltOZL\n\tlRjevJire0RNw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1681941165;\n\tbh=hcolJ9BwTR18bW2ZEL1EdnmRMMvLBbTONrBeX7TZVVQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=CybyxlvyQTc5FTbw6XF3uLRc9PImrORAtY6+VNp/i0gvkmxkpvkUWnc51dMUGt+PZ\n\tHGQEQ8X8OaacRx1j2Fm/EoziWamh7sAXDZXwsO7TYgLpQ2VLao5n95KEYm5+rnJ+ej\n\tQ2pA/A668AapPsUQwRHok6EkbC4mXOu+Y0lxncQY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Cybyxlvy\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"\n\t<20230220082308.e5mup7spjkxqffxr@uno.localdomain>\n\t\n\t<20230220160846.bso4mobcuvjss5u5@uno.localdomain>\n\t","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\t=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= via libcamera-devel\n\t, Jacopo Mondi\n\t","Date":"Wed, 19 Apr 2023 22:52:49 +0100","Message-ID":"<168194116921.3199984.2349983735867560066@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26917,"web_url":"https://patchwork.libcamera.org/comment/26917/","msgid":"<53-XSlPu3-C_NXdGvBFlPFgaHgmelWrLG8eoD8MI0mgDzM5evAdykJZ9K8sP40P7rtTSDDAP-kUmPkbx09lb8pkMkwc9ZzrP5LzrzPFQAbI=@protonmail.com>","date":"2023-04-20T11:35:15","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. április 19., szerda 23:52 keltezéssel, Kieran Bingham írta:\n\n> [...]\n> > > > > On Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > > > > > Date: Wed, 15 Feb 2023 17:48:52 +0000\n> > > > > > From: Barnabás Pőcze \n> > > > > > To: libcamera-devel@lists.libcamera.org\n> > > > > > Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> > > > > > of vector\n> > > > > >\n> > > > > > Change the parameter type of `generateConfiguration()`\n> > > > > > from an std::vector to libcamera::Span. There is no need\n> > > > > > to require a dynamic allocation. A span is preferable\n> > > > > > to a const vector ref.\n> > > > >\n> > > > > I might be missing what the benefit would be here... I understand that, being\n> > > > > a Span nothing but a lightweight container for a pointer to memory and a size,\n> > > > > this allows to use multiple STL containers to pass roles in, but I\n> > > > > wonder if there any real benefit.\n> > > > >\n> > > > > I do expect application to either parse user provided options and\n> > > > > thus need to construct a dynamically grown list of roles, or either\n> > > > > pass in an initializers list\n> > > > >\n> > > > > > A new overload is added that accepts a C-style array so that\n> > > > > >\n> > > > > > cam->generateConfiguration({ ... })\n> > > > > >\n> > > > > > keeps working.\n> > > > >\n> > > > > For which you need an overload.\n> > > > >\n> > > > > Can you expand a bit more on the intended use case ?\n> > > >\n> > > > The way I see it, there are two benefits:\n> > > > * any container that places elements in contiguous memory should work\n> > > > (e.g. std::array and std::vector with a custom allocator, which were\n> > > > previously not supported)\n> > >\n> > > You're correct, but considering how applications are expected to\n> > > construct the StreamRole vector, I hardly see that being strictly\n> > > necessary to be done in an array\n> > >\n> > > > * the cost of constructing an std::vector can be avoided in some cases\n> > > > (e.g. when a fixed list of roles is used)\n> > >\n> > > Correct, but this is not an hard path and the generateConfiguration()\n> > > is expected to be called once per streaming session at most\n> > >\n> > > >\n> > > > But of course I understand if it is considered a micro optimization\n> > > > and rejected on that basis.\n> > > >\n> > >\n> > > If this wasn't changing the public API it would have be easier indeed\n> > > to accept it.\n> >\n> > Technically it changes the API, but I would argue that in such a minimal\n> > way that it is essentially unnoticeable. And as far as I understand\n> > libcamera does not provide stable API/ABI guarantees yet. Am I mistaken?\n> >\n> >\n> > >\n> > > Let's see what others think\n> >\n> > I am wondering if anyone has other thoughts about this? If not, would it be\n> > possible for me to get a definitive yes/no?\n> \n> No specific objection to it here, This will also require an update to\n> the vivid pipeline handler, but as that's external we'd have to handle\n> that separately.\n> \n> Looking at the way StreamRoles is used in applications, it's already\n> mostly a dynamic allocation and that is passed by reference into\n> libcamera.\n> \n> Is there a specific target to this to remove allocations for a given\n> use-case? (I know getting rid of allocations at runtime is highly\n> desireable for real time performance for instance).\n\nThe motivating example is in pipewire[0]. It made me wonder if there is\nactually a reason for using a vector here. And as far as I could tell,\nthere wasn't one, so this patch was born. I would even wager to say that\nin the vast majority of cases (like here), a `const std::vector&` argument\ncan be replaced by `std::span` without significant consequences\nsince what is a `const std::vector&` if not a worse `std::span`\n(there are exceptions, of course)?\n\nSo my motivations for this patch were the following:\n\n - I thought this was a non-intrusive change;\n - it can get rid of the need for constructing the vector in certain cases.\n\nAnd of course I am not arguing that this changes the world, but it is such\na simple change, that I thought \"why not?\". And as I have said I do understand\nif this is considered a micro-optimization and rejected on that basis.\n\n\n> \n> The ABI change here is not an issue, and even though it's an API\n> 'change' it's still compatible as far as I can tell - so no objection\n> here either.\n> \n> I think Jacopo asked a question about the existing StreamRoles 'using'\n> statement. Would you see that as something we should keep? or remove\n> (or deprecate?) with this change?\n\nThere are no API stability guarantees, right? So I would personally remove it, but\nthat might be too harsh of an approach. According to Debian Code Search, it is only\nfound in pipewire and libcamera[1]. (There are other users, I am sure.)\n\n\n> [...]\n\n\nRegards,\nBarnabás Pőcze\n\n\n[0]: https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/83d2e85f490ea97e4ae94b95f20dd06566a14c31/spa/plugins/libcamera/libcamera-utils.cpp#L58\n[1]: https://codesearch.debian.net/search?q=StreamRoles&literal=1","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 3EBCDBD1F1\n\tfor ;\n\tThu, 20 Apr 2023 11:35:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AA15627C1;\n\tThu, 20 Apr 2023 13:35:31 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D1E2603A0\n\tfor ;\n\tThu, 20 Apr 2023 13:35:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1681990531;\n\tbh=bMyy1TN/3CB7t7IijIbgjn8Hc4lDIQRQN29DphA1DYk=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Vt2l9YLJLnvejOF4g3IziBLZ5P0DX7Ql1ikaYu7YWsPD1gLKSa5ZCBLgAZvKwtSTS\n\tn4epOQFVnXtONLf4rOMzVVBNsiFVQdL4lMpU78wMLIw3jautw/t+znaiDhbrks6PDR\n\tJRQrtbNx0jJVQe13uwP327N/mF1rQU/pJML1W4w4cJEK38IKzeqOELzM4nAfa/HGKB\n\taHWDwjgCuCGwUTUvod8TRGs6a852JbYodLk0RtGUpqG6L4SUCWlNR/DxhFunYjtpCY\n\t+Gzlp04Fb1p025KdIXwUIspMh/JEsaBFWLqqfnNSXkQt4icQq3l31uf7vGxX4+uK2o\n\tzAjJIzonTjV9A==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1681990529; x=1682249729;\n\tbh=Fl/Cy9fXEPsZrOmw8lqBUncpDJbO9+dAjtKlOmVFQsM=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=qp0iOBv928kIwhFDMiLWKMJNxZt340wFa0wJpWtyxawBs6fORSWr908qQWpcHysRy\n\tre3lwThocHU4WbkugUH6LaEOJDc/7aF+25vWSc9+Ze8s5pbGeVxSj7LO+1GVp9BpJt\n\tbgJYG2IyxcR6t3rKIwx14OYKYkDFOs4rzlNyEHn/FIqox2qor9k2j4jSIzuEBs3KbC\n\tpjEMyytXfpFummSbhfSs15Q+Wmgws39iF85nzDXXZ/lESX5F1k8m1LRPHpQQrFwwwi\n\t2SHMcXfzBlbOvquU+KBfAt8PgMAwIgBWi4C8scyimHC+auqYVCrXcFWDmDvfIaxV/0\n\tIvjhUdm6+/YfQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"qp0iOBv9\"; dkim-atps=neutral","Date":"Thu, 20 Apr 2023 11:35:15 +0000","To":"Kieran Bingham ","Message-ID":"<53-XSlPu3-C_NXdGvBFlPFgaHgmelWrLG8eoD8MI0mgDzM5evAdykJZ9K8sP40P7rtTSDDAP-kUmPkbx09lb8pkMkwc9ZzrP5LzrzPFQAbI=@protonmail.com>","In-Reply-To":"<168194116921.3199984.2349983735867560066@Monstersaurus>","References":"\n\t<20230220082308.e5mup7spjkxqffxr@uno.localdomain>\n\t\n\t<20230220160846.bso4mobcuvjss5u5@uno.localdomain>\n\t\n\t<168194116921.3199984.2349983735867560066@Monstersaurus>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26919,"web_url":"https://patchwork.libcamera.org/comment/26919/","msgid":"<168199249436.2445904.11987491459306910749@Monstersaurus>","date":"2023-04-20T12:08:14","subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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 (2023-04-20 12:35:15)\n> Hi\n> \n> \n> 2023. április 19., szerda 23:52 keltezéssel, Kieran Bingham írta:\n> \n> > [...]\n> > > > > > On Wed, Feb 15, 2023 at 05:48:52PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > > > > > > Date: Wed, 15 Feb 2023 17:48:52 +0000\n> > > > > > > From: Barnabás Pőcze \n> > > > > > > To: libcamera-devel@lists.libcamera.org\n> > > > > > > Subject: [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead\n> > > > > > > of vector\n> > > > > > >\n> > > > > > > Change the parameter type of `generateConfiguration()`\n> > > > > > > from an std::vector to libcamera::Span. There is no need\n> > > > > > > to require a dynamic allocation. A span is preferable\n> > > > > > > to a const vector ref.\n> > > > > >\n> > > > > > I might be missing what the benefit would be here... I understand that, being\n> > > > > > a Span nothing but a lightweight container for a pointer to memory and a size,\n> > > > > > this allows to use multiple STL containers to pass roles in, but I\n> > > > > > wonder if there any real benefit.\n> > > > > >\n> > > > > > I do expect application to either parse user provided options and\n> > > > > > thus need to construct a dynamically grown list of roles, or either\n> > > > > > pass in an initializers list\n> > > > > >\n> > > > > > > A new overload is added that accepts a C-style array so that\n> > > > > > >\n> > > > > > > cam->generateConfiguration({ ... })\n> > > > > > >\n> > > > > > > keeps working.\n> > > > > >\n> > > > > > For which you need an overload.\n> > > > > >\n> > > > > > Can you expand a bit more on the intended use case ?\n> > > > >\n> > > > > The way I see it, there are two benefits:\n> > > > > * any container that places elements in contiguous memory should work\n> > > > > (e.g. std::array and std::vector with a custom allocator, which were\n> > > > > previously not supported)\n> > > >\n> > > > You're correct, but considering how applications are expected to\n> > > > construct the StreamRole vector, I hardly see that being strictly\n> > > > necessary to be done in an array\n> > > >\n> > > > > * the cost of constructing an std::vector can be avoided in some cases\n> > > > > (e.g. when a fixed list of roles is used)\n> > > >\n> > > > Correct, but this is not an hard path and the generateConfiguration()\n> > > > is expected to be called once per streaming session at most\n> > > >\n> > > > >\n> > > > > But of course I understand if it is considered a micro optimization\n> > > > > and rejected on that basis.\n> > > > >\n> > > >\n> > > > If this wasn't changing the public API it would have be easier indeed\n> > > > to accept it.\n> > >\n> > > Technically it changes the API, but I would argue that in such a minimal\n> > > way that it is essentially unnoticeable. And as far as I understand\n> > > libcamera does not provide stable API/ABI guarantees yet. Am I mistaken?\n> > >\n> > >\n> > > >\n> > > > Let's see what others think\n> > >\n> > > I am wondering if anyone has other thoughts about this? If not, would it be\n> > > possible for me to get a definitive yes/no?\n> > \n> > No specific objection to it here, This will also require an update to\n> > the vivid pipeline handler, but as that's external we'd have to handle\n> > that separately.\n> > \n> > Looking at the way StreamRoles is used in applications, it's already\n> > mostly a dynamic allocation and that is passed by reference into\n> > libcamera.\n> > \n> > Is there a specific target to this to remove allocations for a given\n> > use-case? (I know getting rid of allocations at runtime is highly\n> > desireable for real time performance for instance).\n> \n> The motivating example is in pipewire[0]. It made me wonder if there is\n> actually a reason for using a vector here. And as far as I could tell,\n\n[0] is helpful context thanks. Without that - it's hard to understand\nthe motivation that's all. We can make changes in the API - but without\nunderstanding 'why' it's hard to quantify, or see how it makes\napplications better.\n\n[0] to me looks like a clear case that the user API can be improved with\nthis indeed.\n\n\n> there wasn't one, so this patch was born. I would even wager to say that\n> in the vast majority of cases (like here), a `const std::vector&` argument\n> can be replaced by `std::span` without significant consequences\n> since what is a `const std::vector&` if not a worse `std::span`\n> (there are exceptions, of course)?\n> \n> So my motivations for this patch were the following:\n> \n> - I thought this was a non-intrusive change;\n> - it can get rid of the need for constructing the vector in certain cases.\n> \n> And of course I am not arguing that this changes the world, but it is such\n> a simple change, that I thought \"why not?\". And as I have said I do understand\n> if this is considered a micro-optimization and rejected on that basis.\n\nI think this change sounds reasonable - it's just the original patch\nfelt like it was presented as \"we can do this so why not\" rather than\nshowing \"why do\" instead.\n\nInternally it's easier, but when we change the public API - a\nperspective of how it makes things better for the consumers is helpful.\n\n\n\n> > The ABI change here is not an issue, and even though it's an API\n> > 'change' it's still compatible as far as I can tell - so no objection\n> > here either.\n> > \n> > I think Jacopo asked a question about the existing StreamRoles 'using'\n> > statement. Would you see that as something we should keep? or remove\n> > (or deprecate?) with this change?\n> \n> There are no API stability guarantees, right? So I would personally remove it, but\n> that might be too harsh of an approach. According to Debian Code Search, it is only\n> found in pipewire and libcamera[1]. (There are other users, I am sure.)\n\nNo - no API guarantee right now indeed. For exactly this reason. But\nthat's also why I think this change needs to consider 'what is right'\noverall.\n\nMy worry is if we make the 'StreamRoles' type redundant, then it should\nbe considered or I worry that it will be confusing that some places use\n\"std::span\" while other points use \"StreamRoles\".\n\nOr maybe it's still applicable that we should keep both ?\n\n--\nKieran\n\n\n> \n> > [...]\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> [0]: https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/83d2e85f490ea97e4ae94b95f20dd06566a14c31/spa/plugins/libcamera/libcamera-utils.cpp#L58\n> [1]: https://codesearch.debian.net/search?q=StreamRoles&literal=1","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 9E3C3BD1F1\n\tfor ;\n\tThu, 20 Apr 2023 12:08:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED68B627C1;\n\tThu, 20 Apr 2023 14:08: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 BE633603A0\n\tfor ;\n\tThu, 20 Apr 2023 14:08:17 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5519B9DE;\n\tThu, 20 Apr 2023 14:08:10 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1681992499;\n\tbh=EkRW5YSys2nMjh6UidQzqEyxwjIoPDJBBI5/qvANm9k=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QutvhVJkFx+NxbR9PcM1PW4euLBHFfRVZDUv8Ez7E5FwAVSae+G/mLpMhrX4H08n9\n\tbU9MWm3cehAyhQo67M48w2NN/zUl6sp47IhHXs3yAscByFYqNop16QHZZJ1grCbVBp\n\t6eMdxvJtMtW4cn1lKMlEled3I0vnCGw+MSybT7ZRdFyWztkqiaMKBfmlGxMwNHrNW0\n\t7kr3ru2E+oskkzUo1tKSeGmQVvp7D325R2pk+TBUwzgf+9NuHzFHockeQVWCFxMUqn\n\tbAGirmM7QyOUbhcLSE1Muato+AjvCJD0skdAlXp2pwYocE2bFXWFSkIsmWhjUAaqCn\n\t7/Um9nqSGF6GA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1681992490;\n\tbh=EkRW5YSys2nMjh6UidQzqEyxwjIoPDJBBI5/qvANm9k=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YNw2t3IACEXUT4W3UspYL7ojn9yzCtdqgFH4UaxNeLOgBfb6YUwW6zj5nLVCpVkBD\n\tVHhQ8elAhxwsr1PikkQztRjKaXszREEd8L8EG4TeXq2afA5o4VtrSXksh9ZGJ71uOr\n\t2LXV2naL87VrW3WP+WeRorWkFOuYU4tr456GZJEI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YNw2t3IA\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<53-XSlPu3-C_NXdGvBFlPFgaHgmelWrLG8eoD8MI0mgDzM5evAdykJZ9K8sP40P7rtTSDDAP-kUmPkbx09lb8pkMkwc9ZzrP5LzrzPFQAbI=@protonmail.com>","References":"\n\t<20230220082308.e5mup7spjkxqffxr@uno.localdomain>\n\t\n\t<20230220160846.bso4mobcuvjss5u5@uno.localdomain>\n\t\n\t<168194116921.3199984.2349983735867560066@Monstersaurus>\n\t<53-XSlPu3-C_NXdGvBFlPFgaHgmelWrLG8eoD8MI0mgDzM5evAdykJZ9K8sP40P7rtTSDDAP-kUmPkbx09lb8pkMkwc9ZzrP5LzrzPFQAbI=@protonmail.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Date":"Thu, 20 Apr 2023 13:08:14 +0100","Message-ID":"<168199249436.2445904.11987491459306910749@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span\n\tof StreamRole instead of vector","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]