Patch Detail
Show a patch.
GET /api/patches/18611/?format=api
{ "id": 18611, "url": "https://patchwork.libcamera.org/api/patches/18611/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18611/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20230509221750.1116531-1-pobrn@protonmail.com>", "date": "2023-05-09T23:07:57", "name": "[libcamera-devel,RFC,v3,1/2] libcamera: camera: Take span of StreamRole instead of vector", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "9bb6894731bcf6c4101404e716c3137659d46e84", "submitter": { "id": 133, "url": "https://patchwork.libcamera.org/api/people/133/?format=api", "name": "Pőcze Barnabás", "email": "pobrn@protonmail.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18611/mbox/", "series": [ { "id": 3871, "url": "https://patchwork.libcamera.org/api/series/3871/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3871", "date": "2023-05-09T23:07:57", "name": "[libcamera-devel,RFC,v3,1/2] libcamera: camera: Take span of StreamRole instead of vector", "version": 3, "mbox": "https://patchwork.libcamera.org/series/3871/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18611/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18611/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 20E3FBD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 9 May 2023 23:08:03 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 976CF633B4;\n\tWed, 10 May 2023 01:08:02 +0200 (CEST)", "from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96E326053A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 May 2023 01:08:01 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683673682;\n\tbh=y4gdikqAQXqT9tbF4elr9Dkg8CS6F+IIsBjlQB9Xfsc=;\n\th=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:From;\n\tb=JsSv0kiFtMV20imYpYKkbW9GBV3nkUsRGTICwLxmL8Fiae0WP3Z4G0/pYJPUJ/0rR\n\tgN8wcgo8pBp+SlX7I0QjmD96Bio8CGKqzA6TEkqyW+5zM3qoqsiHyrtXzyaHjXKmxZ\n\tjzsXajLY2OmMyv2I/hWOxDjGSEjidzu9qnGeqdyl5dM2L3L2ekB85bIHsnNKJYl1vJ\n\tmlz907dQm761sH44rThotbn0wic1Q/KvSvR5o4xxsEkmzxVLHEPzY5jA7Zvo6Qq8Ym\n\tYWoqpRgrITsFz8NKwi+Nv+4nbC5ljmN6gW1m0hVZA5ud/s2bsPYmJMxXRb9a1bXWX5\n\tFAqqnA87T5CVA==", "v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1683673680; x=1683932880;\n\tbh=bqIfTx04M2uRIjtb7Dz5brDHuZe/evsnekI4o3C956w=;\n\th=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date:\n\tSubject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector;\n\tb=vZ6pcOgPN1tV/UzgoRXe/KDKhVlDyF1gVfdKJIJ0uwFG+XeisvoY2oehDbZsYRe+M\n\tT/wcMNzkL2VnW8uhziMBG3jqMxN37wZGYkUn4ba6buzlI0dLxoWwSWzBJcsVJEZO6Y\n\tPzvKhBVmigD3bgRdo+H53z5i3eapCq9SP3RVPhCfrXgizPcIEnEr8JmG80Qnt1vxqG\n\tETqyWC0rOVWTvnO1YDyAsIEOWTfICrpYIQroTuUJUl5+9RasFVKywZMV1tfnssGtgT\n\t/9Wk1uxAtW/TYVl2Xj5b0xoAtFDRjFkCfJoln8rtg94/P+5JmKs5qwxFrXn/dktbhh\n\tQ+cTkWFTaZKYA==" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"vZ6pcOgP\"; dkim-atps=neutral", "Date": "Tue, 09 May 2023 23:07:57 +0000", "To": "libcamera-devel@lists.libcamera.org", "Message-ID": "<20230509221750.1116531-1-pobrn@protonmail.com>", "Feedback-ID": "20568564:user:proton", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=utf-8", "Content-Transfer-Encoding": "quoted-printable", "Subject": "[libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take span\n\tof StreamRole instead of vector", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Change the parameter type of `generateConfiguration()` from `const std::vector&`\nto `libcamera::Span`. A span is almost always preferable to a const vector ref\nbecause it does not force dynamic allocation when none are needed, and it allows\nany contiguous container to be used.\n\nA new overload is added that accepts an initializer list so that\n\n cam->generateConfiguration({ ... })\n\nkeeps working.\n\nThere is no API break since a span can be constructed from a vector\nand the initializer list overload takes care of the initializer lists,\nbut this change causes an ABI break.\n\nSigned-off-by: Barnabás Pőcze <pobrn@protonmail.com>\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(-)", "diff": "diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\nindex 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<const StreamRole> 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<const StreamRole> roles)\n {\n return nullptr;\n }\ndiff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex 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 <initializer_list>\n #include <memory>\n #include <set>\n #include <stdint.h>\n@@ -105,7 +106,16 @@ public:\n \tconst ControlList &properties() const;\n \n \tconst std::set<Stream *> &streams() const;\n-\tstd::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles = {});\n+\n+\tstd::unique_ptr<CameraConfiguration>\n+\tgenerateConfiguration(Span<const StreamRole> roles = {});\n+\n+\tstd::unique_ptr<CameraConfiguration>\n+\tgenerateConfiguration(std::initializer_list<StreamRole> roles)\n+\t{\n+\t\treturn generateConfiguration(Span(roles.begin(), roles.end()));\n+\t}\n+\n \tint configure(CameraConfiguration *config);\n \n \tstd::unique_ptr<Request> createRequest(uint64_t cookie = 0);\ndiff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\nindex 4c4dfe62..aaeb3a9e 100644\n--- a/include/libcamera/internal/pipeline_handler.h\n+++ b/include/libcamera/internal/pipeline_handler.h\n@@ -49,7 +49,7 @@ public:\n \tvoid release(Camera *camera);\n \n \tvirtual std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n-\t\tconst StreamRoles &roles) = 0;\n+\t\tSpan<const StreamRole> roles) = 0;\n \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n \n \tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 99683e49..cf9ca01e 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -938,7 +938,7 @@ const std::set<Stream *> &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<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)\n+std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(Span<const StreamRole> roles)\n {\n \tPrivate *const d = _d();\n \n@@ -971,6 +971,12 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR\n \treturn config;\n }\n \n+/**\n+ * \\fn std::unique_ptr<CameraConfiguration> \\\n+ * Camera::generateConfiguration(std::initializer_list<StreamRole> roles)\n+ * \\overload\n+ */\n+\n /**\n * \\brief Configure the camera prior to capture\n * \\param[in] config The camera configurations to setup\ndiff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\nindex 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 \tbool match(DeviceEnumerator *enumerator) override;\n \n \tstd::unique_ptr<CameraConfiguration>\n-\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n+\tgenerateConfiguration(Camera *camera, Span<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n \n std::unique_ptr<CameraConfiguration>\n PipelineHandlerISI::generateConfiguration(Camera *camera,\n-\t\t\t\t\t const StreamRoles &roles)\n+\t\t\t\t\t Span<const StreamRole> roles)\n {\n \tISICameraData *data = cameraData(camera);\n \tstd::unique_ptr<ISICameraConfiguration> config =\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 355cb0cb..ada8c272 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -135,7 +135,7 @@ public:\n \tPipelineHandlerIPU3(CameraManager *manager);\n \n \tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n-\t\tconst StreamRoles &roles) override;\n+\t\tSpan<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -390,7 +390,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n }\n \n std::unique_ptr<CameraConfiguration>\n-PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)\n+PipelineHandlerIPU3::generateConfiguration(Camera *camera, Span<const StreamRole> roles)\n {\n \tIPU3CameraData *data = cameraData(camera);\n \tstd::unique_ptr<IPU3CameraConfiguration> config =\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 8a30fe06..1fdfde7b 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -148,7 +148,7 @@ public:\n \tPipelineHandlerRkISP1(CameraManager *manager);\n \n \tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n-\t\tconst StreamRoles &roles) override;\n+\t\tSpan<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n \n std::unique_ptr<CameraConfiguration>\n PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n-\tconst StreamRoles &roles)\n+\tSpan<const StreamRole> roles)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n \ndiff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\nindex 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<CameraConfiguration>\n-PipelineHandlerBase::generateConfiguration(Camera *camera, const StreamRoles &roles)\n+PipelineHandlerBase::generateConfiguration(Camera *camera, Span<const StreamRole> roles)\n {\n \tCameraData *data = cameraData(camera);\n \tstd::unique_ptr<CameraConfiguration> config =\ndiff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\nindex 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 \t\t\t\t\t\t BayerFormat::Packing packingReq);\n \n \tstd::unique_ptr<CameraConfiguration>\n-\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n+\tgenerateConfiguration(Camera *camera, Span<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, libcamera::Stream *stream,\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 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 \tSimplePipelineHandler(CameraManager *manager);\n \n \tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n-\t\tconst StreamRoles &roles) override;\n+\t\tSpan<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -1044,7 +1044,7 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)\n }\n \n std::unique_ptr<CameraConfiguration>\n-SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)\n+SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRole> roles)\n {\n \tSimpleCameraData *data = cameraData(camera);\n \tstd::unique_ptr<CameraConfiguration> config =\ndiff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\nindex 277465b7..03935876 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -75,7 +75,7 @@ public:\n \tPipelineHandlerUVC(CameraManager *manager);\n \n \tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n-\t\tconst StreamRoles &roles) override;\n+\t\tSpan<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n \n std::unique_ptr<CameraConfiguration>\n PipelineHandlerUVC::generateConfiguration(Camera *camera,\n-\tconst StreamRoles &roles)\n+\tSpan<const StreamRole> roles)\n {\n \tUVCCameraData *data = cameraData(camera);\n \tstd::unique_ptr<CameraConfiguration> config =\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex 204f5ad7..49ee949f 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -85,7 +85,7 @@ public:\n \tPipelineHandlerVimc(CameraManager *manager);\n \n \tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n-\t\tconst StreamRoles &roles) override;\n+\t\tSpan<const StreamRole> roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n \n std::unique_ptr<CameraConfiguration>\n PipelineHandlerVimc::generateConfiguration(Camera *camera,\n-\tconst StreamRoles &roles)\n+\tSpan<const StreamRole> roles)\n {\n \tVimcCameraData *data = cameraData(camera);\n \tstd::unique_ptr<CameraConfiguration> config =\ndiff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\nindex d14e18e2..3f04871f 100644\n--- a/src/py/libcamera/py_main.cpp\n+++ b/src/py/libcamera/py_main.cpp\n@@ -156,7 +156,10 @@ PYBIND11_MODULE(_libcamera, m)\n \t\t})\n \n \t\t/* Keep the camera alive, as StreamConfiguration contains a Stream* */\n-\t\t.def(\"generate_configuration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n+\t\t.def(\"generate_configuration\", [](Camera &self, const std::vector<StreamRole> &roles) {\n+\t\t\treturn self.generateConfiguration(roles);\n+\t\t}, py::keep_alive<0, 1>())\n+\n \t\t.def(\"configure\", &Camera::configure)\n \n \t\t.def(\"create_request\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n", "prefixes": [ "libcamera-devel", "RFC", "v3", "1/2" ] }