Patch Detail
Show a patch.
GET /api/1.1/patches/18285/?format=api
{ "id": 18285, "url": "https://patchwork.libcamera.org/api/1.1/patches/18285/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18285/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<mailman.41.1676483347.775.libcamera-devel@lists.libcamera.org>", "date": "2023-02-15T17:48:52", "name": "[libcamera-devel,RFC,v1] libcamera: camera: Take span of StreamRole instead of vector", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "eb782c526f18af106373dddd8932e5d4bdb100d2", "submitter": { "id": 133, "url": "https://patchwork.libcamera.org/api/1.1/people/133/?format=api", "name": "Pőcze Barnabás", "email": "pobrn@protonmail.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18285/mbox/", "series": [ { "id": 3756, "url": "https://patchwork.libcamera.org/api/1.1/series/3756/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3756", "date": "2023-02-15T17:48:52", "name": "[libcamera-devel,RFC,v1] libcamera: camera: Take span of StreamRole instead of vector", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3756/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18285/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18285/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 30907BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 15 Feb 2023 17:49:09 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BBC2625E6;\n\tWed, 15 Feb 2023 18:49:08 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1676483348;\n\tbh=UuinjeVZwV3/KWO71eNlFeO7uYqwPPcdKb9w2ddHYRY=;\n\th=Date:To:List-Id:List-Post:From:List-Subscribe:List-Unsubscribe:\n\tList-Archive:Reply-To:List-Help:Subject:From;\n\tb=LBJtTVRvfJZ/Ag9x7U2Qgpn51iCLN7hdjX0L4O7k9Dk1eQfwfrljeFtPoFf7hFirT\n\tlEuUWaabTGMNJuGUKImLfm5AgzrT8LBctJWpJBAu1d5KgyJYkGmqViCtInU3zfD64s\n\tvMSmT8RiFr9hbmJXRJNCz2O3zOsNKHBMtIdt+jzoHVRWEzXLBhNu+TOabX8ThJJmqw\n\tchWcwhnnFVWgK2mGO+ADEbNTjUxrDWY7w/B8DkXvlouWzAuml/Vpvnw4wKGpeA+ONV\n\tV3rVtD5lzDI/vJOd4JTiYDxbKRYiQVrE5cekl0kAx7V3nUb0NcQ0/jzylc2JM7fsqU\n\ta131X5dPXkF9g==", "Date": "Wed, 15 Feb 2023 17:48:52 +0000", "To": "libcamera-devel@lists.libcamera.org", "MIME-Version": "1.0", "Message-ID": "<mailman.41.1676483347.775.libcamera-devel@lists.libcamera.org>", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t<libcamera-devel@lists.libcamera.org>", "Precedence": "list", "X-Mailman-Version": "2.1.29", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "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/>", "Reply-To": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "Subject": "[libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span of\n\tStreamRole instead of vector", "Content-Type": "message/rfc822", "Content-Disposition": "inline", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Change the parameter type of `generateConfiguration()`\nfrom an std::vector to libcamera::Span. There is no need\nto require a dynamic allocation. A span is preferable\nto a const vector ref.\n\nA new overload is added that accepts a C-style array so that\n\n cam->generateConfiguration({ ... })\n\nkeeps working.\n\nThere is no API break since a span can be constructed from\na vector and the array 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\nI think the change is reasonable, but there are two questions:\n\n * what to do with `StreamRoles`? should it be removed?\n * is the array overload needed? if an API break is acceptable\n it can be removed and users could write e.g.\n generateConfiguration(std::array { ... })\n\n---\n Documentation/guides/pipeline-handler.rst | 4 ++--\n include/libcamera/camera.h | 9 ++++++++-\n include/libcamera/internal/pipeline_handler.h | 2 +-\n src/libcamera/camera.cpp | 2 +-\n src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n src/libcamera/pipeline/simple/simple.cpp | 4 ++--\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--\n src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--\n src/py/libcamera/py_main.cpp | 5 ++++-\n 12 files changed, 30 insertions(+), 20 deletions(-)\n\n--\n2.39.1", "diff": "diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\nindex e1930fdf..92d6d043 100644\n--- a/Documentation/guides/pipeline-handler.rst\n+++ b/Documentation/guides/pipeline-handler.rst\n@@ -203,7 +203,7 @@ implementations for the overridden class members.\n PipelineHandlerVivid(CameraManager *manager);\n\n CameraConfiguration *generateConfiguration(Camera *camera,\n- const StreamRoles &roles) override;\n+ Span<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..ccf0d24c 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -105,7 +105,14 @@ public:\n \tconst ControlList &properties() const;\n\n \tconst std::set<Stream *> &streams() const;\n-\tstd::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles = {});\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Span<const StreamRole> roles = {});\n+\n+\ttemplate<std::size_t N>\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRole (&roles)[N])\n+\t{\n+\t\treturn generateConfiguration(Span<const StreamRole>(roles));\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 0da167a7..0f68a511 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\ndiff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\nindex 0c67e35d..df5919ed 100644\n--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n@@ -112,7 +112,7 @@ public:\n \tbool match(DeviceEnumerator *enumerator) override;\n\n \tstd::unique_ptr<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@@ -571,7 +571,7 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\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/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 84120954..f62570cc 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -361,7 +361,7 @@ public:\n \tPipelineHandlerRPi(CameraManager *manager);\n\n \tstd::unique_ptr<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@@ -661,7 +661,7 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n }\n\n std::unique_ptr<CameraConfiguration>\n-PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)\n+PipelineHandlerRPi::generateConfiguration(Camera *camera, Span<const StreamRole> roles)\n {\n \tRPiCameraData *data = cameraData(camera);\n \tstd::unique_ptr<CameraConfiguration> 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/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 24ded4db..9d71a369 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -312,7 +312,7 @@ public:\n \tSimplePipelineHandler(CameraManager *manager);\n\n \tstd::unique_ptr<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@@ -1038,7 +1038,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", "v1" ] }