[{"id":27100,"web_url":"https://patchwork.libcamera.org/comment/27100/","msgid":"<168419329164.4131343.11781476214754804501@Monstersaurus>","date":"2023-05-15T23:28:11","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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 via libcamera-devel (2023-05-10 00:07:57)\n> Change the parameter type of `generateConfiguration()` from `const std::vector&`\n> to `libcamera::Span`. A span is almost always preferable to a const vector ref\n> because it does not force dynamic allocation when none are needed, and it allows\n> any contiguous container to be used.\n\nSounds like a reasonable move to me.\n\n> \n> A new overload is added that accepts an initializer list so that\n> \n> cam->generateConfiguration({ ... })\n> \n> keeps working.\n> \n> There is no API break since a span can be constructed from a vector\n> and the initializer list overload takes care of the initializer lists,\n> but this change causes an ABI break.\n\nThat's fine too. I'd be interested how the abi-compliance-checker\nreports this:\n\n Creating compatibility report ...\n Binary compatibility: 100%\n Source compatibility: 99.9%\n Total binary compatibility problems: 0, warnings: 0\n Total source compatibility problems: 2, warnings: 0\n Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n\nWell - at least it noticed something ;-)\n\n\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> Documentation/guides/pipeline-handler.rst | 4 ++--\n> include/libcamera/camera.h | 12 +++++++++++-\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> src/libcamera/camera.cpp | 8 +++++++-\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n> src/libcamera/pipeline/rpi/common/pipeline_base.h | 2 +-\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> 13 files changed, 39 insertions(+), 20 deletions(-)\n> \n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index 57644534..10b9c75c 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..004bc894 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -7,6 +7,7 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -105,7 +106,16 @@ public:\n> const ControlList &properties() const;\n> \n> const std::set &streams() const;\n> - std::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> +\n> + std::unique_ptr\n> + generateConfiguration(Span roles = {});\n> +\n> + std::unique_ptr\n> + generateConfiguration(std::initializer_list roles)\n> + {\n> + return generateConfiguration(Span(roles.begin(), roles.end()));\n> + }\n> +\n> int configure(CameraConfiguration *config);\n> \n> std::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> void release(Camera *camera);\n> \n> virtual std::unique_ptr generateConfiguration(Camera *camera,\n> - const StreamRoles &roles) = 0;\n> + Span roles) = 0;\n> virtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> \n> virtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 99683e49..cf9ca01e 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> Private *const d = _d();\n> \n> @@ -971,6 +971,12 @@ std::unique_ptr Camera::generateConfiguration(const StreamR\n> return config;\n> }\n> \n> +/**\n> + * \\fn std::unique_ptr \\\n> + * Camera::generateConfiguration(std::initializer_list roles)\n> + * \\overload\n> + */\n> +\n> /**\n> * \\brief Configure the camera prior to capture\n> * \\param[in] config The camera configurations to setup\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 449d9012..9bdfff0b 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -104,7 +104,7 @@ public:\n> bool match(DeviceEnumerator *enumerator) override;\n> \n> std::unique_ptr\n> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> + generateConfiguration(Camera *camera, Span roles) override;\n> int configure(Camera *camera, CameraConfiguration *config) override;\n> \n> int exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> \n> std::unique_ptr\n> PipelineHandlerISI::generateConfiguration(Camera *camera,\n> - const StreamRoles &roles)\n> + Span roles)\n> {\n> ISICameraData *data = cameraData(camera);\n> std::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> PipelineHandlerIPU3(CameraManager *manager);\n> \n> std::unique_ptr generateConfiguration(Camera *camera,\n> - const StreamRoles &roles) override;\n> + Span roles) override;\n\nCheckstyle is quite noisy on the alignments of these. But you haven't\nchanged the alignments...\n\nI might be tempted to say we should align to checkstyle when applying\njust to keep the style differences down.\n\n> int configure(Camera *camera, CameraConfiguration *config) override;\n> \n> int 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> IPU3CameraData *data = cameraData(camera);\n> std::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> PipelineHandlerRkISP1(CameraManager *manager);\n> \n> std::unique_ptr 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> @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> \n> std::unique_ptr\n> PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> - const StreamRoles &roles)\n> + Span roles)\n> {\n> RkISP1CameraData *data = cameraData(camera);\n> \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index ba1797bc..d7f1d547 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -381,7 +381,7 @@ V4L2DeviceFormat PipelineHandlerBase::toV4L2DeviceFormat(const V4L2VideoDevice *\n> }\n> \n> std::unique_ptr\n> -PipelineHandlerBase::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> +PipelineHandlerBase::generateConfiguration(Camera *camera, Span roles)\n> {\n> CameraData *data = cameraData(camera);\n> std::unique_ptr config =\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index 6b19b56c..f648e810 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -214,7 +214,7 @@ public:\n> BayerFormat::Packing packingReq);\n> \n> std::unique_ptr\n> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> + generateConfiguration(Camera *camera, Span roles) override;\n> int configure(Camera *camera, CameraConfiguration *config) override;\n> \n> int exportFrameBuffers(Camera *camera, libcamera::Stream *stream,\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 050285fd..b9858353 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -316,7 +316,7 @@ public:\n> SimplePipelineHandler(CameraManager *manager);\n> \n> std::unique_ptr 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> @@ -1044,7 +1044,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> SimpleCameraData *data = cameraData(camera);\n> std::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> PipelineHandlerUVC(CameraManager *manager);\n> \n> std::unique_ptr 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> @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> \n> std::unique_ptr\n> PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> - const StreamRoles &roles)\n> + Span roles)\n> {\n> UVCCameraData *data = cameraData(camera);\n> std::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> PipelineHandlerVimc(CameraManager *manager);\n> \n> std::unique_ptr 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> @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> \n> std::unique_ptr\n> PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> - const StreamRoles &roles)\n> + Span roles)\n> {\n> VimcCameraData *data = cameraData(camera);\n> std::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> })\n> \n> /* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> - .def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> + .def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> + return self.generateConfiguration(roles);\n> + }, py::keep_alive<0, 1>())\n> +\n\nThat's the only bit that I'm not sure how to fully parse the\nneed/change... but it doesn't look out of place - It would be nice to\nsee this tested or reviewed by someone handling more of the python parts\nbut other wise:\n\n\nReviewed-by: Kieran Bingham \n\n> .def(\"configure\", &Camera::configure)\n> \n> .def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> -- \n> 2.40.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 31B91C3284\n\tfor ;\n\tMon, 15 May 2023 23:28:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94C8F60496;\n\tTue, 16 May 2023 01:28:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B0CD60493\n\tfor ;\n\tTue, 16 May 2023 01:28:15 +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 2113C8D;\n\tTue, 16 May 2023 01:28:04 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1684193297;\n\tbh=ynpVtycZn0MW7FxEijKpTh6DtVtHBq04aJOb2E18jn0=;\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:\n\tFrom;\n\tb=pqU2v2jqQTiyvng8dSr9wWFoVUR+FzdTV64VlckeKGTWa42z0xRhFPCxSDnkH7wAl\n\tp8YvRByF7RijKcBZdpUeZRFrFZZtfLHFzMVsl1r4eCjpNtRfZglyCw9EGm2OK9S9t2\n\tvRRb3FqkHuUq9EIBWAt9BMotpXfdqCYQWBgZ+Hm18FtYjFa78nk110+1Th/q66HwT8\n\tFtTVe5SUkV3bKS7u0TJj7xMwvXGQqBETSkNmJuM9PX+KNlW3FOuQ1NUfOxn78nkEO1\n\teBIO98eff72o0KZw+6DdrCJe+jhndNX2vDADg8BdT6AyRlUma3hIzEN7Von38InPZ6\n\tyCKvjpzZtJD2A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1684193284;\n\tbh=ynpVtycZn0MW7FxEijKpTh6DtVtHBq04aJOb2E18jn0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=By3AfOtRvGMBG9LKwuYR+F86CBYg1aHt4VADN1V/Ma8Ggt17AQvcgcGtEFwYS9ueS\n\tClERaiK0EnOG4fUQ3m1kYppegR2+KF0ypHYCV5ear4QoLJx2erRni3gMaeRc6MV+xH\n\tmuoVyid/XJDdmkcd/h5keoSlfKTe1YHQyQVy9UIA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"By3AfOtR\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230509221750.1116531-1-pobrn@protonmail.com>","References":"<20230509221750.1116531-1-pobrn@protonmail.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 16 May 2023 00:28:11 +0100","Message-ID":"<168419329164.4131343.11781476214754804501@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27113,"web_url":"https://patchwork.libcamera.org/comment/27113/","msgid":"","date":"2023-05-18T15:17:41","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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. május 16., kedd 1:28 keltezéssel, Kieran Bingham írta:\n\n> Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)\n> > Change the parameter type of `generateConfiguration()` from `const std::vector&`\n> > to `libcamera::Span`. A span is almost always preferable to a const vector ref\n> > because it does not force dynamic allocation when none are needed, and it allows\n> > any contiguous container to be used.\n> \n> Sounds like a reasonable move to me.\n> \n> >\n> > A new overload is added that accepts an initializer list so that\n> >\n> > cam->generateConfiguration({ ... })\n> >\n> > keeps working.\n> >\n> > There is no API break since a span can be constructed from a vector\n> > and the initializer list overload takes care of the initializer lists,\n> > but this change causes an ABI break.\n> \n> That's fine too. I'd be interested how the abi-compliance-checker\n> reports this:\n> \n> Creating compatibility report ...\n> Binary compatibility: 100%\n> Source compatibility: 99.9%\n> Total binary compatibility problems: 0, warnings: 0\n> Total source compatibility problems: 2, warnings: 0\n> Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n> \n> Well - at least it noticed something ;-)\n\nUnfortunately I think that is incorrect. This is definitely an ABI break.\n(And as far as I can tell not an API break.)\n\n\n> \n> \n> >\n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > Documentation/guides/pipeline-handler.rst | 4 ++--\n> > include/libcamera/camera.h | 12 +++++++++++-\n> > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > src/libcamera/camera.cpp | 8 +++++++-\n> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n> > src/libcamera/pipeline/rpi/common/pipeline_base.h | 2 +-\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> > 13 files changed, 39 insertions(+), 20 deletions(-)\n> >\n> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > index 57644534..10b9c75c 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..004bc894 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -7,6 +7,7 @@\n> >\n> > #pragma once\n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -105,7 +106,16 @@ public:\n> > const ControlList &properties() const;\n> >\n> > const std::set &streams() const;\n> > - std::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> > +\n> > + std::unique_ptr\n> > + generateConfiguration(Span roles = {});\n> > +\n> > + std::unique_ptr\n> > + generateConfiguration(std::initializer_list roles)\n> > + {\n> > + return generateConfiguration(Span(roles.begin(), roles.end()));\n> > + }\n> > +\n> > int configure(CameraConfiguration *config);\n> >\n> > std::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> > void release(Camera *camera);\n> >\n> > virtual std::unique_ptr generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles) = 0;\n> > + Span roles) = 0;\n> > virtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> >\n> > virtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 99683e49..cf9ca01e 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> > Private *const d = _d();\n> >\n> > @@ -971,6 +971,12 @@ std::unique_ptr Camera::generateConfiguration(const StreamR\n> > return config;\n> > }\n> >\n> > +/**\n> > + * \\fn std::unique_ptr \\\n> > + * Camera::generateConfiguration(std::initializer_list roles)\n> > + * \\overload\n> > + */\n> > +\n> > /**\n> > * \\brief Configure the camera prior to capture\n> > * \\param[in] config The camera configurations to setup\n> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > index 449d9012..9bdfff0b 100644\n> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > @@ -104,7 +104,7 @@ public:\n> > bool match(DeviceEnumerator *enumerator) override;\n> >\n> > std::unique_ptr\n> > - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > + generateConfiguration(Camera *camera, Span roles) override;\n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > int exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> >\n> > std::unique_ptr\n> > PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > ISICameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerIPU3(CameraManager *manager);\n> >\n> > std::unique_ptr generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles) override;\n> > + Span roles) override;\n> \n> Checkstyle is quite noisy on the alignments of these. But you haven't\n> changed the alignments...\n> \n> I might be tempted to say we should align to checkstyle when applying\n> just to keep the style differences down.\n\nSo what should I do here?\n\n\n\n> \n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > int 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> > IPU3CameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerRkISP1(CameraManager *manager);\n> >\n> > std::unique_ptr 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> > @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > RkISP1CameraData *data = cameraData(camera);\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index ba1797bc..d7f1d547 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -381,7 +381,7 @@ V4L2DeviceFormat PipelineHandlerBase::toV4L2DeviceFormat(const V4L2VideoDevice *\n> > }\n> >\n> > std::unique_ptr\n> > -PipelineHandlerBase::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > +PipelineHandlerBase::generateConfiguration(Camera *camera, Span roles)\n> > {\n> > CameraData *data = cameraData(camera);\n> > std::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index 6b19b56c..f648e810 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -214,7 +214,7 @@ public:\n> > BayerFormat::Packing packingReq);\n> >\n> > std::unique_ptr\n> > - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > + generateConfiguration(Camera *camera, Span roles) override;\n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> >\n> > int exportFrameBuffers(Camera *camera, libcamera::Stream *stream,\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 050285fd..b9858353 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -316,7 +316,7 @@ public:\n> > SimplePipelineHandler(CameraManager *manager);\n> >\n> > std::unique_ptr 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> > @@ -1044,7 +1044,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> > SimpleCameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerUVC(CameraManager *manager);\n> >\n> > std::unique_ptr 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> > @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > UVCCameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerVimc(CameraManager *manager);\n> >\n> > std::unique_ptr 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> > @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> >\n> > std::unique_ptr\n> > PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > VimcCameraData *data = cameraData(camera);\n> > std::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> > })\n> >\n> > /* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> > - .def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> > + .def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> > + return self.generateConfiguration(roles);\n> > + }, py::keep_alive<0, 1>())\n> > +\n> \n> That's the only bit that I'm not sure how to fully parse the\n> need/change... but it doesn't look out of place - It would be nice to\n> see this tested or reviewed by someone handling more of the python parts\n> but other wise:\n\nAs far as I understand, pybind11 provides (de)serializers for some standard STL\ntypes, std::vector is one of them, but it does not work with libcamera::Span\nout of the box of course.\n(https://pybind11.readthedocs.io/en/latest/advanced/cast/stl.html)\n\n\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > .def(\"configure\", &Camera::configure)\n> >\n> > .def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > --\n> > 2.40.1\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 E43AEC3284\n\tfor ;\n\tThu, 18 May 2023 15:18:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 555EE627D4;\n\tThu, 18 May 2023 17:18:03 +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 2ED8D627D4\n\tfor ;\n\tThu, 18 May 2023 17:18:01 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1684423083;\n\tbh=nliwzInD9UPWhxlyN7jlkqHny4aL9zUj3wkDo1h/89A=;\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=38Qy0hK3NltdEvdeLr0IYgaM1JRnp0ESSKgxv/ITmyYnCaBE4e/8OMhBfbcgyHpO5\n\txNhbnaudwjb+ifbFH27yf1CZWOBck5hWn63XkIdWpQpi/dvqWcOplb7RgM699VLCDD\n\tqtjWk9fLqQoGXp/ayO2apC3FwgXDgFhCqrSLjMlA6pMDmfqqbfWk/OKb6UCMs821JO\n\t0g/JXIUIUDwNLBedEUUApB82qu6oEVMOYuWTtxDbuQrr21qZgQ7l0pBabCZBrdBC3V\n\tlApKj8UuzvgfUQ8sy3+A9pYJ+LLHOt4ZkLFCHCgskpWEXPXvZe6D606ZknhuNK7U8t\n\t6zds4uxFXymVQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1684423080; x=1684682280;\n\tbh=z7Lq6nWv9yJGZ2rqVJyVT/opiStxp08VZGckcpzRaYw=;\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=kxy45XG4eDM5V2QyhcsnohJaPjKIBen/2EcIm1powLRAr7ogWDmqxFzyheZD2JFE0\n\tUl/s1btGBc9CUwZrdajsF2X82TARwSz2Egwm5EhdA7G6lUB072EhkSmxy1qOuGARgG\n\tEsw141KcM45xi9S6w1FxcarbtNXrnOTmnqAiI7MT2Aj+iSkgceaJ1cO/gG8jg3gWZn\n\t0s174uMOZlj79Qe1CVkKkslicYHv16wo54+VfGC5d5B6ICmYn+8uw7Jop8bipy4Z8y\n\thdV0MpXlgcL/+wXeo/8mBDCBq9Sgi3RLbdbq/SzaTH8SL2kVhIf/CboNIUzRPmjrSC\n\t/ZsF1B4VdKVmQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"kxy45XG4\"; dkim-atps=neutral","Date":"Thu, 18 May 2023 15:17:41 +0000","To":"Kieran Bingham ","Message-ID":"","In-Reply-To":"<168419329164.4131343.11781476214754804501@Monstersaurus>","References":"<20230509221750.1116531-1-pobrn@protonmail.com>\n\t<168419329164.4131343.11781476214754804501@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 v3 1/2] libcamera: camera: Take\n\tspan of 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":27116,"web_url":"https://patchwork.libcamera.org/comment/27116/","msgid":"<168442657637.2429330.3349880893030595487@Monstersaurus>","date":"2023-05-18T16:16:16","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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-05-18 16:17:41)\n> Hi\n> \n> \n> 2023. május 16., kedd 1:28 keltezéssel, Kieran Bingham írta:\n> \n> > Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)\n> > > Change the parameter type of `generateConfiguration()` from `const std::vector&`\n> > > to `libcamera::Span`. A span is almost always preferable to a const vector ref\n> > > because it does not force dynamic allocation when none are needed, and it allows\n> > > any contiguous container to be used.\n> > \n> > Sounds like a reasonable move to me.\n> > \n> > >\n> > > A new overload is added that accepts an initializer list so that\n> > >\n> > > cam->generateConfiguration({ ... })\n> > >\n> > > keeps working.\n> > >\n> > > There is no API break since a span can be constructed from a vector\n> > > and the initializer list overload takes care of the initializer lists,\n> > > but this change causes an ABI break.\n> > \n> > That's fine too. I'd be interested how the abi-compliance-checker\n> > reports this:\n> > \n> > Creating compatibility report ...\n> > Binary compatibility: 100%\n> > Source compatibility: 99.9%\n> > Total binary compatibility problems: 0, warnings: 0\n> > Total source compatibility problems: 2, warnings: 0\n> > Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n> > \n> > Well - at least it noticed something ;-)\n> \n> Unfortunately I think that is incorrect. This is definitely an ABI break.\n> (And as far as I can tell not an API break.)\n> \n\nHrm... a little worrying for the abi-compliance-checker then...\n\n\n\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > Documentation/guides/pipeline-handler.rst | 4 ++--\n> > > include/libcamera/camera.h | 12 +++++++++++-\n> > > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > > src/libcamera/camera.cpp | 8 +++++++-\n> > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> > > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n> > > src/libcamera/pipeline/rpi/common/pipeline_base.h | 2 +-\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> > > 13 files changed, 39 insertions(+), 20 deletions(-)\n> > >\n> > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > > index 57644534..10b9c75c 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..004bc894 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -7,6 +7,7 @@\n> > >\n> > > #pragma once\n> > >\n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -105,7 +106,16 @@ public:\n> > > const ControlList &properties() const;\n> > >\n> > > const std::set &streams() const;\n> > > - std::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> > > +\n> > > + std::unique_ptr\n> > > + generateConfiguration(Span roles = {});\n> > > +\n> > > + std::unique_ptr\n> > > + generateConfiguration(std::initializer_list roles)\n> > > + {\n> > > + return generateConfiguration(Span(roles.begin(), roles.end()));\n> > > + }\n> > > +\n> > > int configure(CameraConfiguration *config);\n> > >\n> > > std::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> > > void release(Camera *camera);\n> > >\n> > > virtual std::unique_ptr generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles) = 0;\n> > > + Span roles) = 0;\n> > > virtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> > >\n> > > virtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 99683e49..cf9ca01e 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> > > Private *const d = _d();\n> > >\n> > > @@ -971,6 +971,12 @@ std::unique_ptr Camera::generateConfiguration(const StreamR\n> > > return config;\n> > > }\n> > >\n> > > +/**\n> > > + * \\fn std::unique_ptr \\\n> > > + * Camera::generateConfiguration(std::initializer_list roles)\n> > > + * \\overload\n> > > + */\n> > > +\n> > > /**\n> > > * \\brief Configure the camera prior to capture\n> > > * \\param[in] config The camera configurations to setup\n> > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > index 449d9012..9bdfff0b 100644\n> > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > @@ -104,7 +104,7 @@ public:\n> > > bool match(DeviceEnumerator *enumerator) override;\n> > >\n> > > std::unique_ptr\n> > > - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > > + generateConfiguration(Camera *camera, Span roles) override;\n> > > int configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > int exportFrameBuffers(Camera *camera, Stream *stream,\n> > > @@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles)\n> > > + Span roles)\n> > > {\n> > > ISICameraData *data = cameraData(camera);\n> > > std::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> > > PipelineHandlerIPU3(CameraManager *manager);\n> > >\n> > > std::unique_ptr generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles) override;\n> > > + Span roles) override;\n> > \n> > Checkstyle is quite noisy on the alignments of these. But you haven't\n> > changed the alignments...\n> > \n> > I might be tempted to say we should align to checkstyle when applying\n> > just to keep the style differences down.\n> \n> So what should I do here?\n\nUp to you. You can fix the alignment to match checkstyle's desires, or\nwe can do it after or when applying the patch.\n\nDepends if we actually need a second revision of this. I haven't seen\nanything needing a change yet?\n\n \n> > > int configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > int 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> > > IPU3CameraData *data = cameraData(camera);\n> > > std::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> > > PipelineHandlerRkISP1(CameraManager *manager);\n> > >\n> > > std::unique_ptr 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> > > @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles)\n> > > + Span roles)\n> > > {\n> > > RkISP1CameraData *data = cameraData(camera);\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index ba1797bc..d7f1d547 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -381,7 +381,7 @@ V4L2DeviceFormat PipelineHandlerBase::toV4L2DeviceFormat(const V4L2VideoDevice *\n> > > }\n> > >\n> > > std::unique_ptr\n> > > -PipelineHandlerBase::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > > +PipelineHandlerBase::generateConfiguration(Camera *camera, Span roles)\n> > > {\n> > > CameraData *data = cameraData(camera);\n> > > std::unique_ptr config =\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > > index 6b19b56c..f648e810 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > > @@ -214,7 +214,7 @@ public:\n> > > BayerFormat::Packing packingReq);\n> > >\n> > > std::unique_ptr\n> > > - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > > + generateConfiguration(Camera *camera, Span roles) override;\n> > > int configure(Camera *camera, CameraConfiguration *config) override;\n> > >\n> > > int exportFrameBuffers(Camera *camera, libcamera::Stream *stream,\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 050285fd..b9858353 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -316,7 +316,7 @@ public:\n> > > SimplePipelineHandler(CameraManager *manager);\n> > >\n> > > std::unique_ptr 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> > > @@ -1044,7 +1044,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> > > SimpleCameraData *data = cameraData(camera);\n> > > std::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> > > PipelineHandlerUVC(CameraManager *manager);\n> > >\n> > > std::unique_ptr 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> > > @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles)\n> > > + Span roles)\n> > > {\n> > > UVCCameraData *data = cameraData(camera);\n> > > std::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> > > PipelineHandlerVimc(CameraManager *manager);\n> > >\n> > > std::unique_ptr 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> > > @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> > >\n> > > std::unique_ptr\n> > > PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> > > - const StreamRoles &roles)\n> > > + Span roles)\n> > > {\n> > > VimcCameraData *data = cameraData(camera);\n> > > std::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> > > })\n> > >\n> > > /* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> > > - .def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> > > + .def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> > > + return self.generateConfiguration(roles);\n> > > + }, py::keep_alive<0, 1>())\n> > > +\n> > \n> > That's the only bit that I'm not sure how to fully parse the\n> > need/change... but it doesn't look out of place - It would be nice to\n> > see this tested or reviewed by someone handling more of the python parts\n> > but other wise:\n> \n> As far as I understand, pybind11 provides (de)serializers for some standard STL\n> types, std::vector is one of them, but it does not work with libcamera::Span\n> out of the box of course.\n> (https://pybind11.readthedocs.io/en/latest/advanced/cast/stl.html)\n> \n> \n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > > .def(\"configure\", &Camera::configure)\n> > >\n> > > .def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > > --\n> > > 2.40.1\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 8BBFAC3284\n\tfor ;\n\tThu, 18 May 2023 16:16:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EEB7C6285F;\n\tThu, 18 May 2023 18:16:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4475627D4\n\tfor ;\n\tThu, 18 May 2023 18:16:19 +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 B54146D6;\n\tThu, 18 May 2023 18:16:06 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1684426581;\n\tbh=ulLRvrgvOUAeWOq69pz9Dh4LgiI+0rTpfUUt1IIeqes=;\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=fCiB4t9/dklgSoflcAoXcV4ST0DKjoh87paIJpok7KCgW0e3oGGk/QMwPc4pulZJE\n\tgTfK0gjEruU/b5hBY7Grr6ANyYiaxsWqdb3yAZZJKN09l9o1QbM/JDTY5K12Kcdwqu\n\tm+Gt2XKocnzXqyehvAEnNRWxmIb2S9vE8azltCjJwhOqzPyMKIff71OWg1Cd/ofsSv\n\tNCTEoz4bnS7ii13dAQu3TuqiAV6yeUsKd0ec5ZI48iQL6xDvRssubJeG2j73NcEPKT\n\trHfnC8vB/iBMLxu+ENn9ltDk/YXZ0+GxrE3RGGC5ORRuTZZ6bzlruK8xtmnjTGxLr4\n\tpfrw7VKB2kCQw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1684426566;\n\tbh=ulLRvrgvOUAeWOq69pz9Dh4LgiI+0rTpfUUt1IIeqes=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=QDA2gaClyJCF+aZcCoDY5kj52+6HNeM/+f0LhXU0cwTrjeI3iRkSjViK9QE6Ccm3s\n\t3hRwbdT2onJrBFbBr7QI0m66ptN9B2EoypYPo/JwcaJyj6kOseu7kFZ/MT9Y3W7nG2\n\tu+Sd8ZWdCnZ8aAeW8z9kqte/k0fat4DAqAWw5wqc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QDA2gaCl\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20230509221750.1116531-1-pobrn@protonmail.com>\n\t<168419329164.4131343.11781476214754804501@Monstersaurus>\n\t","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Date":"Thu, 18 May 2023 17:16:16 +0100","Message-ID":"<168442657637.2429330.3349880893030595487@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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":27143,"web_url":"https://patchwork.libcamera.org/comment/27143/","msgid":"","date":"2023-05-25T22:41:23","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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. május 18., csütörtök 18:16 keltezéssel, Kieran Bingham írta:\n\n> Quoting Barnabás Pőcze (2023-05-18 16:17:41)\n> > Hi\n> >\n> >\n> > 2023. május 16., kedd 1:28 keltezéssel, Kieran Bingham írta:\n> >\n> > > Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)\n> > > > Change the parameter type of `generateConfiguration()` from `const std::vector&`\n> > > > to `libcamera::Span`. A span is almost always preferable to a const vector ref\n> > > > because it does not force dynamic allocation when none are needed, and it allows\n> > > > any contiguous container to be used.\n> > >\n> > > Sounds like a reasonable move to me.\n> > >\n> > > >\n> > > > A new overload is added that accepts an initializer list so that\n> > > >\n> > > > cam->generateConfiguration({ ... })\n> > > >\n> > > > keeps working.\n> > > >\n> > > > There is no API break since a span can be constructed from a vector\n> > > > and the initializer list overload takes care of the initializer lists,\n> > > > but this change causes an ABI break.\n> > >\n> > > That's fine too. I'd be interested how the abi-compliance-checker\n> > > reports this:\n> > >\n> > > Creating compatibility report ...\n> > > Binary compatibility: 100%\n> > > Source compatibility: 99.9%\n> > > Total binary compatibility problems: 0, warnings: 0\n> > > Total source compatibility problems: 2, warnings: 0\n> > > Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n> > >\n> > > Well - at least it noticed something ;-)\n> >\n> > Unfortunately I think that is incorrect. This is definitely an ABI break.\n> > (And as far as I can tell not an API break.)\n> >\n> \n> Hrm... a little worrying for the abi-compliance-checker then...\n\nWell, to be fair, it *is* an API break as well, but very unlikely to be\nnoticed by anyone. I don't know why it does not notice the ABI break,\nthe mangled names are completely different...\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 589A6C3284\n\tfor ;\n\tThu, 25 May 2023 22:41:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3B9E626FA;\n\tFri, 26 May 2023 00:41:28 +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 75D3E6039D\n\tfor ;\n\tFri, 26 May 2023 00:41:27 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685054488;\n\tbh=XhZy+7BnDbv6xb3WZ9wfid87KRCVZmKWuj8RPe6gasQ=;\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=WC4wmgz7Rw8IkKh1etjDFoc+f15/BIgMWa5FjvPEqb76/SVA2G0JZ85JQJ5iR0atf\n\teIYHUJb7iy/dHIqG8YHNbvqCH/IJ1vDL90mNSrKD0QFpchUYuUr9Q2F1Lh/XluTNPh\n\tt6ippUj6CnDQKx5HA6Fog2E1R3aIyJxZjL9x5YCOc45G66idVpAAhsHVXqZpMwCoHM\n\tL+wUOmJgOlzLOJhG0bktX6H6EmwVRgBIyXtZSDG8+WY6dcU6b3nIVtS57LkUs4NggI\n\teasF4NAcfBmr2xLwH1IrGr270dsnbZ1jykvO86lwRf0fody2yGJd7Y/x2l6qHK8Xzz\n\t8yRTdhHZdPCOQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1685054486; x=1685313686;\n\tbh=3fmt6Hq8n0j3PqW5zmousIurHmYkwU1F0OSh9KEr6To=;\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=kS5/3YPfrqTGE9L9/M8lKRhE9189y9UkqmOi8rRYoXFa3ujMNUqxDu/QHnGkD4+7X\n\tmycV1xlSL1RNppu38mts52OGNJQ+Yktk4TpAyG5xypZDRa3WqWlIe7AdTqWdnemJSa\n\tsnOwl1HbdGW8paKUgGaRIbTX9kVHkEE8KV+VM5lPvEuAloJhdrBoKrl6Hnw7jziT+6\n\tDG1CA0IE0Io/Kg6su7ysUaMIWIpoEfvDMZsnwjsA9LWDNXDY2STrDTW7259aZOj3fp\n\tcih4QxSg3b2cPF6StZ0+cPDhfUN/mo7paZkLh/HGklAOiv3JUmuOEGJQ6hS+FBUR5G\n\tXHhLc+irGERMw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"kS5/3YPf\"; dkim-atps=neutral","Date":"Thu, 25 May 2023 22:41:23 +0000","To":"Kieran Bingham ","Message-ID":"","In-Reply-To":"<168442657637.2429330.3349880893030595487@Monstersaurus>","References":"<20230509221750.1116531-1-pobrn@protonmail.com>\n\t<168419329164.4131343.11781476214754804501@Monstersaurus>\n\t\n\t<168442657637.2429330.3349880893030595487@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 v3 1/2] libcamera: camera: Take\n\tspan of 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":27180,"web_url":"https://patchwork.libcamera.org/comment/27180/","msgid":"<6678824876badbd44cf5e17688155bad4672e8ca.camel@ndufresne.ca>","date":"2023-05-30T14:44:46","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of StreamRole instead of vector","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi,\n\nLe jeudi 25 mai 2023 à 22:41 +0000, Barnabás Pőcze via libcamera-devel a écrit :\n> Hi\n> \n> \n> 2023. május 18., csütörtök 18:16 keltezéssel, Kieran Bingham írta:\n> \n> > Quoting Barnabás Pőcze (2023-05-18 16:17:41)\n> > > Hi\n> > > \n> > > \n> > > 2023. május 16., kedd 1:28 keltezéssel, Kieran Bingham írta:\n> > > \n> > > > Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)\n> > > > > Change the parameter type of `generateConfiguration()` from `const std::vector&`\n> > > > > to `libcamera::Span`. A span is almost always preferable to a const vector ref\n> > > > > because it does not force dynamic allocation when none are needed, and it allows\n> > > > > any contiguous container to be used.\n> > > > \n> > > > Sounds like a reasonable move to me.\n> > > > \n> > > > > \n> > > > > A new overload is added that accepts an initializer list so that\n> > > > > \n> > > > > cam->generateConfiguration({ ... })\n> > > > > \n> > > > > keeps working.\n> > > > > \n> > > > > There is no API break since a span can be constructed from a vector\n> > > > > and the initializer list overload takes care of the initializer lists,\n> > > > > but this change causes an ABI break.\n> > > > \n> > > > That's fine too. I'd be interested how the abi-compliance-checker\n> > > > reports this:\n> > > > \n> > > > Creating compatibility report ...\n> > > > Binary compatibility: 100%\n> > > > Source compatibility: 99.9%\n> > > > Total binary compatibility problems: 0, warnings: 0\n> > > > Total source compatibility problems: 2, warnings: 0\n> > > > Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n> > > > \n> > > > Well - at least it noticed something ;-)\n> > > \n> > > Unfortunately I think that is incorrect. This is definitely an ABI break.\n> > > (And as far as I can tell not an API break.)\n> > > \n> > \n> > Hrm... a little worrying for the abi-compliance-checker then...\n> \n> Well, to be fair, it *is* an API break as well, but very unlikely to be\n> noticed by anyone. I don't know why it does not notice the ABI break,\n> the mangled names are completely different...\n\nRemoving symbols is an API break. Changing the size of a structure in a way that\nboth the application and the lib may endup with different size interpretation is\nan ABI break.\n\nI think what you are are saying is that this is build compatible with the old\nversion, avoiding the pain of porting, but a rebuilt is needed in order to use\nthe new symbols (which in C++ are generated, but they remains API in the context\nof generic checkers).\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 BB14FC3213\n\tfor ;\n\tTue, 30 May 2023 14:44:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2A05C626F8;\n\tTue, 30 May 2023 16:44:50 +0200 (CEST)","from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com\n\t[IPv6:2607:f8b0:4864:20::72f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1A8F60595\n\tfor ;\n\tTue, 30 May 2023 16:44:48 +0200 (CEST)","by mail-qk1-x72f.google.com with SMTP id\n\taf79cd13be357-75b17298108so279479485a.0\n\tfor ;\n\tTue, 30 May 2023 07:44:48 -0700 (PDT)","from nicolas-tpx395.localdomain ([2606:6d00:17:6c0::7a9])\n\tby smtp.gmail.com with ESMTPSA id\n\tp26-20020a05620a133a00b0075b11b2a227sm4121372qkj.34.2023.05.30.07.44.46\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 30 May 2023 07:44:47 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685457890;\n\tbh=eXP/UE1dSc+O1Y0AELvWuv7/Jf2q3kTmbxyHwdITm00=;\n\th=To:Date: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=sZ3nYCnSU/HGCoox06d4CDTE3EamIj9hawizjcxomdJdOhgZy/p1xBtKLlDQkF1zH\n\tib1fotq7KSZZ6QoDX7wQHlhHyDXI1IsSEtRxQr/RqejHFBrTk8OZXEaZk9GdTAtbJp\n\t27Px8sxeErwZX9uDiPG3kw2J1Q8xj9q0bHdSSEKoVXySjxwBB5/JJIiDYsCcsBOZj/\n\tNLzae63qTbqhKeKPD9A4B4xZlVzVM4pG3BfeloHqb82dHh/wra4LsRtICyETTYwCO2\n\tV9VmgMtev4VDM93xpiZuLFeeOeWUPrGten0sx75h7x6uJdRQUBy7xFud7QK/F3TjBe\n\tPU8nYjcFfealA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20221208.gappssmtp.com; s=20221208; t=1685457887;\n\tx=1688049887; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=f9gD9mgwbiuMm2Nt9PrhKkBUWhVdq0X58/kxjpwsYLc=;\n\tb=HBzjAH6E8PJCdhcrsY86ctet4S0ltkQfD/i4JmdfXTRgChS4FeJoUvDenBwGMiIR5q\n\t4BDlIb4fGOp/7K3JfbUgMu6jrp7BhTBx8SABHyFZa8nTET2H11Yqqia7t1byVXsFjUqe\n\tClz1JqSjOc21CnftsoMjUurdBZsqXIRAU6Kh+EpvbR6ys+rMPUZyUr7Dkvs+3XycbfBj\n\tb3YlDn1OeNyupRZSxg2U73ovOKnylETDppEDoC7zmHuWau+EwzDdfPYHN+eGxXezk4zi\n\tSQcS5NF4+OM8cT5JPqXEO6tVscFO6h3rXXAPJzP/wUsOMza2mBtZsVrVdOJrgqlRsaNq\n\tpSKQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20221208.gappssmtp.com\n\theader.i=@ndufresne-ca.20221208.gappssmtp.com header.b=\"HBzjAH6E\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685457887; x=1688049887;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=f9gD9mgwbiuMm2Nt9PrhKkBUWhVdq0X58/kxjpwsYLc=;\n\tb=U1l7Cj3WBhxNDfnCohZ00xIcJ+vxBZSiCb16qUIfuqWs01Jp5L11TaaC0KZ1gvbFWk\n\to1SbeztSC7KGWAXz+ZeHLTmjlJWGOiSGbfo7f1M4Cz1EcMRkZ6COydfSU05kRhP2WEf/\n\tgQuBEU3ZgazYhsXXiTnkem5ub7IFo5ucmNQDVVFluIHoTcE37zgQzkDH4tQgQ2YVxfHA\n\ti5mr5kYlHxfJOSy9/0wQN9AALnupAlmqPjkWo44g575csoUfB5qTkNaywuw7hqZzsmEl\n\tg21GndJbH0Mx4CqCbZ1Yl1hbtoGxl2mEd8Duv0dX7Iax0cXtJHCtvBppdnvvDMXLDlPQ\n\tVmXg==","X-Gm-Message-State":"AC+VfDyFVy+f4G4yBDsPA96KgeaSRtc+3vnLMPbqDXJu0Sg86W+Lnjv2\n\tS5mIbWPdhSoipLAcQ0rz9qeWYJvf9VeXLocx9PM=","X-Google-Smtp-Source":"ACHHUZ6JXE5tFkJOEy5/ZkS1LrLvBnLDeYr5+Q7o/p7q6F8oIezBcBiVpuuUeLEFSQTfH5O5/rYOCw==","X-Received":"by 2002:a05:620a:8fc3:b0:75b:23a1:40e with SMTP id\n\trj3-20020a05620a8fc300b0075b23a1040emr1964955qkn.36.1685457887655; \n\tTue, 30 May 2023 07:44:47 -0700 (PDT)","Message-ID":"<6678824876badbd44cf5e17688155bad4672e8ca.camel@ndufresne.ca>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= , Kieran\n\tBingham ","Date":"Tue, 30 May 2023 10:44:46 -0400","In-Reply-To":"","References":"<20230509221750.1116531-1-pobrn@protonmail.com>\n\t<168419329164.4131343.11781476214754804501@Monstersaurus>\n\t\n\t<168442657637.2429330.3349880893030595487@Monstersaurus>\n\t","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.1 (3.48.1-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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":"Nicolas Dufresne via libcamera-devel\n\t","Reply-To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27184,"web_url":"https://patchwork.libcamera.org/comment/27184/","msgid":"<20230530170341.GD22516@pendragon.ideasonboard.com>","date":"2023-05-30T17:03:41","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of StreamRole instead of vector","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\n(CC'ing Tomi)\n\nOn Tue, May 16, 2023 at 12:28:11AM +0100, Kieran Bingham via libcamera-devel wrote:\n> Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)\n> > Change the parameter type of `generateConfiguration()` from `const std::vector&`\n> > to `libcamera::Span`. A span is almost always preferable to a const vector ref\n> > because it does not force dynamic allocation when none are needed, and it allows\n> > any contiguous container to be used.\n> \n> Sounds like a reasonable move to me.\n> \n> > A new overload is added that accepts an initializer list so that\n> > \n> > cam->generateConfiguration({ ... })\n> > \n> > keeps working.\n> > \n> > There is no API break since a span can be constructed from a vector\n> > and the initializer list overload takes care of the initializer lists,\n> > but this change causes an ABI break.\n> \n> That's fine too. I'd be interested how the abi-compliance-checker\n> reports this:\n> \n> Creating compatibility report ...\n> Binary compatibility: 100%\n> Source compatibility: 99.9%\n> Total binary compatibility problems: 0, warnings: 0\n> Total source compatibility problems: 2, warnings: 0\n> Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n> \n> Well - at least it noticed something ;-)\n> \n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > Documentation/guides/pipeline-handler.rst | 4 ++--\n> > include/libcamera/camera.h | 12 +++++++++++-\n> > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > src/libcamera/camera.cpp | 8 +++++++-\n> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n> > src/libcamera/pipeline/rpi/common/pipeline_base.h | 2 +-\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> > 13 files changed, 39 insertions(+), 20 deletions(-)\n> > \n> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > index 57644534..10b9c75c 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..004bc894 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -7,6 +7,7 @@\n> > \n> > #pragma once\n> > \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -105,7 +106,16 @@ public:\n> > const ControlList &properties() const;\n> > \n> > const std::set &streams() const;\n> > - std::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n> > +\n> > + std::unique_ptr\n> > + generateConfiguration(Span roles = {});\n> > +\n> > + std::unique_ptr\n> > + generateConfiguration(std::initializer_list roles)\n> > + {\n> > + return generateConfiguration(Span(roles.begin(), roles.end()));\n> > + }\n> > +\n> > int configure(CameraConfiguration *config);\n> > \n> > std::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> > void release(Camera *camera);\n> > \n> > virtual std::unique_ptr generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles) = 0;\n> > + Span roles) = 0;\n> > virtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> > \n> > virtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 99683e49..cf9ca01e 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> > Private *const d = _d();\n> > \n> > @@ -971,6 +971,12 @@ std::unique_ptr Camera::generateConfiguration(const StreamR\n> > return config;\n> > }\n> > \n> > +/**\n> > + * \\fn std::unique_ptr \\\n> > + * Camera::generateConfiguration(std::initializer_list roles)\n> > + * \\overload\n> > + */\n> > +\n> > /**\n> > * \\brief Configure the camera prior to capture\n> > * \\param[in] config The camera configurations to setup\n> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > index 449d9012..9bdfff0b 100644\n> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > @@ -104,7 +104,7 @@ public:\n> > bool match(DeviceEnumerator *enumerator) override;\n> > \n> > std::unique_ptr\n> > - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > + generateConfiguration(Camera *camera, Span roles) override;\n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> > \n> > int exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> > \n> > std::unique_ptr\n> > PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > ISICameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerIPU3(CameraManager *manager);\n> > \n> > std::unique_ptr generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles) override;\n> > + Span roles) override;\n> \n> Checkstyle is quite noisy on the alignments of these. But you haven't\n> changed the alignments...\n> \n> I might be tempted to say we should align to checkstyle when applying\n> just to keep the style differences down.\n> \n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> > \n> > int 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> > IPU3CameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerRkISP1(CameraManager *manager);\n> > \n> > std::unique_ptr 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> > @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> > \n> > std::unique_ptr\n> > PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > RkISP1CameraData *data = cameraData(camera);\n> > \n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index ba1797bc..d7f1d547 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -381,7 +381,7 @@ V4L2DeviceFormat PipelineHandlerBase::toV4L2DeviceFormat(const V4L2VideoDevice *\n> > }\n> > \n> > std::unique_ptr\n> > -PipelineHandlerBase::generateConfiguration(Camera *camera, const StreamRoles &roles)\n> > +PipelineHandlerBase::generateConfiguration(Camera *camera, Span roles)\n> > {\n> > CameraData *data = cameraData(camera);\n> > std::unique_ptr config =\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index 6b19b56c..f648e810 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -214,7 +214,7 @@ public:\n> > BayerFormat::Packing packingReq);\n> > \n> > std::unique_ptr\n> > - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> > + generateConfiguration(Camera *camera, Span roles) override;\n> > int configure(Camera *camera, CameraConfiguration *config) override;\n> > \n> > int exportFrameBuffers(Camera *camera, libcamera::Stream *stream,\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 050285fd..b9858353 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -316,7 +316,7 @@ public:\n> > SimplePipelineHandler(CameraManager *manager);\n> > \n> > std::unique_ptr 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> > @@ -1044,7 +1044,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> > SimpleCameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerUVC(CameraManager *manager);\n> > \n> > std::unique_ptr 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> > @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> > \n> > std::unique_ptr\n> > PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > UVCCameraData *data = cameraData(camera);\n> > std::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> > PipelineHandlerVimc(CameraManager *manager);\n> > \n> > std::unique_ptr 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> > @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> > \n> > std::unique_ptr\n> > PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> > - const StreamRoles &roles)\n> > + Span roles)\n> > {\n> > VimcCameraData *data = cameraData(camera);\n> > std::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> > })\n> > \n> > /* Keep the camera alive, as StreamConfiguration contains a Stream* */\n> > - .def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> > + .def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n> > + return self.generateConfiguration(roles);\n> > + }, py::keep_alive<0, 1>())\n> > +\n> \n> That's the only bit that I'm not sure how to fully parse the\n> need/change... but it doesn't look out of place - It would be nice to\n> see this tested or reviewed by someone handling more of the python parts\n> but other wise:\n\nTomi, could you review this part ?\n\n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> > .def(\"configure\", &Camera::configure)\n> > \n> > .def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B4631C3213\n\tfor ;\n\tTue, 30 May 2023 17:03:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1ACD9626F8;\n\tTue, 30 May 2023 19:03:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A50560595\n\tfor ;\n\tTue, 30 May 2023 19:03:44 +0200 (CEST)","from pendragon.ideasonboard.com (om126205198071.34.openmobile.ne.jp\n\t[126.205.198.71])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C27A4A9;\n\tTue, 30 May 2023 19:03:21 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685466226;\n\tbh=dpVK+pJ6UjSFd57sKTsZ0EEZhbnSuChZIDmMUyynMLg=;\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=TEVvTxmANzYI8jvNg+iRqVhlzIVmgzjoL66MhtW5nI6rn2S8WaRuV5hRtPA2vYNQs\n\tKppTU/4MugjKbHCaw5hNt/8xtjpHd7hbfKWzAcK7cZIFB+OEWEk8ctCl5N1sXVIlV6\n\tM4CJkDB4wUuOqGBDl+WeKU7UTjhJVddmfXgemZimGq2Ss77Pcq6mcrWx7xi1xj/qr1\n\t3FYd61QsQ4JwRy60xUSoiJ7A3w0B+dTiD+gln0gHQg0Yz9YygPMJiB+yQ208xgZamg\n\tPk8X34YuEzvamXC7iMG0gp/VjwZyFe6pNHeAGQziiw0ca6dA3IV8hWzjNIZWKNo5JM\n\tkEFqT7pfbAtVw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685466203;\n\tbh=dpVK+pJ6UjSFd57sKTsZ0EEZhbnSuChZIDmMUyynMLg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=F/nEsi8azcVSzTlvroRSkhEV/h/WM+UHxP/xAIi1WMTzGuA5UfgGOYfVaYcCkhHTx\n\t/u3749Z/6c+jAzBuZat2FHFB/55neHT7xChhrM5xz5PqpD44WoqgZitHGie3rKlQtn\n\tRqQ4rVkiKHrHy+T7bpbS/IodoSnVQS2wV9xdolRc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"F/nEsi8a\"; dkim-atps=neutral","Date":"Tue, 30 May 2023 20:03:41 +0300","To":"Kieran Bingham ","Message-ID":"<20230530170341.GD22516@pendragon.ideasonboard.com>","References":"<20230509221750.1116531-1-pobrn@protonmail.com>\n\t<168419329164.4131343.11781476214754804501@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<168419329164.4131343.11781476214754804501@Monstersaurus>","Subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27187,"web_url":"https://patchwork.libcamera.org/comment/27187/","msgid":"<62a89327-426d-877a-3295-e1b955b86bcf@ideasonboard.com>","date":"2023-05-30T17:42:55","subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of StreamRole instead of vector","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 30/05/2023 20:03, Laurent Pinchart wrote:\n> Hello,\n> \n> (CC'ing Tomi)\n> \n> On Tue, May 16, 2023 at 12:28:11AM +0100, Kieran Bingham via libcamera-devel wrote:\n>> Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)\n>>> Change the parameter type of `generateConfiguration()` from `const std::vector&`\n>>> to `libcamera::Span`. A span is almost always preferable to a const vector ref\n>>> because it does not force dynamic allocation when none are needed, and it allows\n>>> any contiguous container to be used.\n>>\n>> Sounds like a reasonable move to me.\n>>\n>>> A new overload is added that accepts an initializer list so that\n>>>\n>>> cam->generateConfiguration({ ... })\n>>>\n>>> keeps working.\n>>>\n>>> There is no API break since a span can be constructed from a vector\n>>> and the initializer list overload takes care of the initializer lists,\n>>> but this change causes an ABI break.\n>>\n>> That's fine too. I'd be interested how the abi-compliance-checker\n>> reports this:\n>>\n>> Creating compatibility report ...\n>> Binary compatibility: 100%\n>> Source compatibility: 99.9%\n>> Total binary compatibility problems: 0, warnings: 0\n>> Total source compatibility problems: 2, warnings: 0\n>> Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html\n>>\n>> Well - at least it noticed something ;-)\n>>\n>>> Signed-off-by: Barnabás Pőcze \n>>> ---\n>>> Documentation/guides/pipeline-handler.rst | 4 ++--\n>>> include/libcamera/camera.h | 12 +++++++++++-\n>>> include/libcamera/internal/pipeline_handler.h | 2 +-\n>>> src/libcamera/camera.cpp | 8 +++++++-\n>>> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n>>> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n>>> src/libcamera/pipeline/rpi/common/pipeline_base.h | 2 +-\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>>> 13 files changed, 39 insertions(+), 20 deletions(-)\n>>>\n>>> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n>>> index 57644534..10b9c75c 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..004bc894 100644\n>>> --- a/include/libcamera/camera.h\n>>> +++ b/include/libcamera/camera.h\n>>> @@ -7,6 +7,7 @@\n>>> \n>>> #pragma once\n>>> \n>>> +#include \n>>> #include \n>>> #include \n>>> #include \n>>> @@ -105,7 +106,16 @@ public:\n>>> const ControlList &properties() const;\n>>> \n>>> const std::set &streams() const;\n>>> - std::unique_ptr generateConfiguration(const StreamRoles &roles = {});\n>>> +\n>>> + std::unique_ptr\n>>> + generateConfiguration(Span roles = {});\n>>> +\n>>> + std::unique_ptr\n>>> + generateConfiguration(std::initializer_list roles)\n>>> + {\n>>> + return generateConfiguration(Span(roles.begin(), roles.end()));\n>>> + }\n>>> +\n>>> int configure(CameraConfiguration *config);\n>>> \n>>> std::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>>> void release(Camera *camera);\n>>> \n>>> virtual std::unique_ptr generateConfiguration(Camera *camera,\n>>> - const StreamRoles &roles) = 0;\n>>> + Span roles) = 0;\n>>> virtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n>>> \n>>> virtual int exportFrameBuffers(Camera *camera, Stream *stream,\n>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>> index 99683e49..cf9ca01e 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>>> Private *const d = _d();\n>>> \n>>> @@ -971,6 +971,12 @@ std::unique_ptr Camera::generateConfiguration(const StreamR\n>>> return config;\n>>> }\n>>> \n>>> +/**\n>>> + * \\fn std::unique_ptr \\\n>>> + * Camera::generateConfiguration(std::initializer_list roles)\n>>> + * \\overload\n>>> + */\n>>> +\n>>> /**\n>>> * \\brief Configure the camera prior to capture\n>>> * \\param[in] config The camera configurations to setup\n>>> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>>> index 449d9012..9bdfff0b 100644\n>>> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>>> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>>> @@ -104,7 +104,7 @@ public:\n>>> bool match(DeviceEnumerator *enumerator) override;\n>>> \n>>> std::unique_ptr\n>>> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n>>> + generateConfiguration(Camera *camera, Span roles) override;\n>>> int configure(Camera *camera, CameraConfiguration *config) override;\n>>> \n>>> int exportFrameBuffers(Camera *camera, Stream *stream,\n>>> @@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n>>> \n>>> std::unique_ptr\n>>> PipelineHandlerISI::generateConfiguration(Camera *camera,\n>>> - const StreamRoles &roles)\n>>> + Span roles)\n>>> {\n>>> ISICameraData *data = cameraData(camera);\n>>> std::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>>> PipelineHandlerIPU3(CameraManager *manager);\n>>> \n>>> std::unique_ptr generateConfiguration(Camera *camera,\n>>> - const StreamRoles &roles) override;\n>>> + Span roles) override;\n>>\n>> Checkstyle is quite noisy on the alignments of these. But you haven't\n>> changed the alignments...\n>>\n>> I might be tempted to say we should align to checkstyle when applying\n>> just to keep the style differences down.\n>>\n>>> int configure(Camera *camera, CameraConfiguration *config) override;\n>>> \n>>> int 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>>> IPU3CameraData *data = cameraData(camera);\n>>> std::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>>> PipelineHandlerRkISP1(CameraManager *manager);\n>>> \n>>> std::unique_ptr 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>>> @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n>>> \n>>> std::unique_ptr\n>>> PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n>>> - const StreamRoles &roles)\n>>> + Span roles)\n>>> {\n>>> RkISP1CameraData *data = cameraData(camera);\n>>> \n>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>> index ba1797bc..d7f1d547 100644\n>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>> @@ -381,7 +381,7 @@ V4L2DeviceFormat PipelineHandlerBase::toV4L2DeviceFormat(const V4L2VideoDevice *\n>>> }\n>>> \n>>> std::unique_ptr\n>>> -PipelineHandlerBase::generateConfiguration(Camera *camera, const StreamRoles &roles)\n>>> +PipelineHandlerBase::generateConfiguration(Camera *camera, Span roles)\n>>> {\n>>> CameraData *data = cameraData(camera);\n>>> std::unique_ptr config =\n>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n>>> index 6b19b56c..f648e810 100644\n>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n>>> @@ -214,7 +214,7 @@ public:\n>>> BayerFormat::Packing packingReq);\n>>> \n>>> std::unique_ptr\n>>> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n>>> + generateConfiguration(Camera *camera, Span roles) override;\n>>> int configure(Camera *camera, CameraConfiguration *config) override;\n>>> \n>>> int exportFrameBuffers(Camera *camera, libcamera::Stream *stream,\n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 050285fd..b9858353 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -316,7 +316,7 @@ public:\n>>> SimplePipelineHandler(CameraManager *manager);\n>>> \n>>> std::unique_ptr 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>>> @@ -1044,7 +1044,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>>> SimpleCameraData *data = cameraData(camera);\n>>> std::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>>> PipelineHandlerUVC(CameraManager *manager);\n>>> \n>>> std::unique_ptr 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>>> @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n>>> \n>>> std::unique_ptr\n>>> PipelineHandlerUVC::generateConfiguration(Camera *camera,\n>>> - const StreamRoles &roles)\n>>> + Span roles)\n>>> {\n>>> UVCCameraData *data = cameraData(camera);\n>>> std::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>>> PipelineHandlerVimc(CameraManager *manager);\n>>> \n>>> std::unique_ptr 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>>> @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n>>> \n>>> std::unique_ptr\n>>> PipelineHandlerVimc::generateConfiguration(Camera *camera,\n>>> - const StreamRoles &roles)\n>>> + Span roles)\n>>> {\n>>> VimcCameraData *data = cameraData(camera);\n>>> std::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>>> })\n>>> \n>>> /* Keep the camera alive, as StreamConfiguration contains a Stream* */\n>>> - .def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n>>> + .def(\"generate_configuration\", [](Camera &self, const std::vector &roles) {\n>>> + return self.generateConfiguration(roles);\n>>> + }, py::keep_alive<0, 1>())\n>>> +\n>>\n>> That's the only bit that I'm not sure how to fully parse the\n>> need/change... but it doesn't look out of place - It would be nice to\n>> see this tested or reviewed by someone handling more of the python parts\n>> but other wise:\n> \n> Tomi, could you review this part ?\n\nLooks fine to me.\n\n Tomi","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 6F5C0C31E9\n\tfor ;\n\tTue, 30 May 2023 17:43:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3C12626FA;\n\tTue, 30 May 2023 19:43:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 14DB460599\n\tfor ;\n\tTue, 30 May 2023 19:43:00 +0200 (CEST)","from [192.168.88.20] (91-154-35-171.elisa-laajakaista.fi\n\t[91.154.35.171])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DC0DE4A9;\n\tTue, 30 May 2023 19:42:38 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685468581;\n\tbh=cMr0nGD/wO4UbMpxgf/GMTJ1OIhexYBnvDSWqKxQxwk=;\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=XzwAw3QuJtYbVIi4WTJclmzafiVY11JJYahwp8bTGx0hzTy56AinPxh3c1p/6sGRN\n\thY67JZiq0nPYuIjpeB56Ra6Vx+oE9AoalY9Mr8FSAXno1/28aKtG4xmNARw4htdUE6\n\tOMsY9npm++qD3Ak5Frmnz4fYKxl6x80QbJltmT/8mooBW7ehfPKee5JxzbmZWGwbFE\n\t8L8rpFNVtLkTRuLmA3Ke9NAaJFb/UqcY0MMKv0PSyR3yPoB5h1c1Ed7O7ymq+Zjmwh\n\t8TjFC8O4w9ekuhTRxgMn9BsMclw+oUe0aHohKduM4aKgxDde32T69gexoEc1FAffX4\n\tkkw8S2hyt0AXw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685468559;\n\tbh=cMr0nGD/wO4UbMpxgf/GMTJ1OIhexYBnvDSWqKxQxwk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=cK8e2eob0JDVjN4v6yxVyE8Q29+fedlH3+0idGPBtregTKMOfLxHld/epXKF8bs8o\n\tV4F9eQ5H//gB3aYb6xX+oFGFKCOMUjqdcW2h4xTQDLcADj/O3PQvX6XesP1kQf6XeQ\n\t9lOkqDeNGUfm7MpZArFOpchDO7tMAKYW7SXLEh4Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cK8e2eob\"; dkim-atps=neutral","Message-ID":"<62a89327-426d-877a-3295-e1b955b86bcf@ideasonboard.com>","Date":"Tue, 30 May 2023 20:42:55 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.11.0","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tKieran Bingham ","References":"<20230509221750.1116531-1-pobrn@protonmail.com>\n\t<168419329164.4131343.11781476214754804501@Monstersaurus>\n\t<20230530170341.GD22516@pendragon.ideasonboard.com>","In-Reply-To":"<20230530170341.GD22516@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take\n\tspan of 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":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]