From patchwork Wed Feb 15 17:48:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 18285 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 30907BE080 for ; Wed, 15 Feb 2023 17:49:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8BBC2625E6; Wed, 15 Feb 2023 18:49:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1676483348; bh=UuinjeVZwV3/KWO71eNlFeO7uYqwPPcdKb9w2ddHYRY=; h=Date:To:List-Id:List-Post:From:List-Subscribe:List-Unsubscribe: List-Archive:Reply-To:List-Help:Subject:From; b=LBJtTVRvfJZ/Ag9x7U2Qgpn51iCLN7hdjX0L4O7k9Dk1eQfwfrljeFtPoFf7hFirT lEuUWaabTGMNJuGUKImLfm5AgzrT8LBctJWpJBAu1d5KgyJYkGmqViCtInU3zfD64s vMSmT8RiFr9hbmJXRJNCz2O3zOsNKHBMtIdt+jzoHVRWEzXLBhNu+TOabX8ThJJmqw chWcwhnnFVWgK2mGO+ADEbNTjUxrDWY7w/B8DkXvlouWzAuml/Vpvnw4wKGpeA+ONV V3rVtD5lzDI/vJOd4JTiYDxbKRYiQVrE5cekl0kAx7V3nUb0NcQ0/jzylc2JM7fsqU a131X5dPXkF9g== Date: Wed, 15 Feb 2023 17:48:52 +0000 To: libcamera-devel@lists.libcamera.org MIME-Version: 1.0 Message-ID: List-Id: List-Post: X-Patchwork-Original-From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Precedence: list X-Mailman-Version: 2.1.29 X-BeenThere: libcamera-devel@lists.libcamera.org List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= List-Help: Subject: [libcamera-devel] [RFC PATCH v1] libcamera: camera: Take span of StreamRole instead of vector Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Change the parameter type of `generateConfiguration()` from an std::vector to libcamera::Span. There is no need to require a dynamic allocation. A span is preferable to a const vector ref. A new overload is added that accepts a C-style array so that cam->generateConfiguration({ ... }) keeps working. There is no API break since a span can be constructed from a vector and the array overload takes care of the initializer lists, but this change causes an ABI break. Signed-off-by: Barnabás Pőcze --- I think the change is reasonable, but there are two questions: * what to do with `StreamRoles`? should it be removed? * is the array overload needed? if an API break is acceptable it can be removed and users could write e.g. generateConfiguration(std::array { ... }) --- Documentation/guides/pipeline-handler.rst | 4 ++-- include/libcamera/camera.h | 9 ++++++++- include/libcamera/internal/pipeline_handler.h | 2 +- src/libcamera/camera.cpp | 2 +- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++-- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/simple/simple.cpp | 4 ++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++-- src/libcamera/pipeline/vimc/vimc.cpp | 4 ++-- src/py/libcamera/py_main.cpp | 5 ++++- 12 files changed, 30 insertions(+), 20 deletions(-) -- 2.39.1 diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index e1930fdf..92d6d043 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -203,7 +203,7 @@ implementations for the overridden class members. PipelineHandlerVivid(CameraManager *manager); CameraConfiguration *generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -223,7 +223,7 @@ implementations for the overridden class members. } CameraConfiguration *PipelineHandlerVivid::generateConfiguration(Camera *camera, - const StreamRoles &roles) + Span roles) { return nullptr; } diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 5bb06584..ccf0d24c 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -105,7 +105,14 @@ public: const ControlList &properties() const; const std::set &streams() const; - std::unique_ptr generateConfiguration(const StreamRoles &roles = {}); + std::unique_ptr generateConfiguration(Span roles = {}); + + template + std::unique_ptr generateConfiguration(const StreamRole (&roles)[N]) + { + return generateConfiguration(Span(roles)); + } + int configure(CameraConfiguration *config); std::unique_ptr createRequest(uint64_t cookie = 0); diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 4c4dfe62..aaeb3a9e 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -49,7 +49,7 @@ public: void release(Camera *camera); virtual std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) = 0; + Span roles) = 0; virtual int configure(Camera *camera, CameraConfiguration *config) = 0; virtual int exportFrameBuffers(Camera *camera, Stream *stream, diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 0da167a7..0f68a511 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -938,7 +938,7 @@ const std::set &Camera::streams() const * \return A CameraConfiguration if the requested roles can be satisfied, or a * null pointer otherwise. */ -std::unique_ptr Camera::generateConfiguration(const StreamRoles &roles) +std::unique_ptr Camera::generateConfiguration(Span roles) { Private *const d = _d(); diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 0c67e35d..df5919ed 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -112,7 +112,7 @@ public: bool match(DeviceEnumerator *enumerator) override; std::unique_ptr - generateConfiguration(Camera *camera, const StreamRoles &roles) override; + generateConfiguration(Camera *camera, Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -571,7 +571,7 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager) std::unique_ptr PipelineHandlerISI::generateConfiguration(Camera *camera, - const StreamRoles &roles) + Span roles) { ISICameraData *data = cameraData(camera); std::unique_ptr config = diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 355cb0cb..ada8c272 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -135,7 +135,7 @@ public: PipelineHandlerIPU3(CameraManager *manager); std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -390,7 +390,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) } std::unique_ptr -PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) +PipelineHandlerIPU3::generateConfiguration(Camera *camera, Span roles) { IPU3CameraData *data = cameraData(camera); std::unique_ptr config = diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 84120954..f62570cc 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -361,7 +361,7 @@ public: PipelineHandlerRPi(CameraManager *manager); std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -661,7 +661,7 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager) } std::unique_ptr -PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles) +PipelineHandlerRPi::generateConfiguration(Camera *camera, Span roles) { RPiCameraData *data = cameraData(camera); std::unique_ptr config = diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 8a30fe06..1fdfde7b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -148,7 +148,7 @@ public: PipelineHandlerRkISP1(CameraManager *manager); std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -609,7 +609,7 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) std::unique_ptr PipelineHandlerRkISP1::generateConfiguration(Camera *camera, - const StreamRoles &roles) + Span roles) { RkISP1CameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 24ded4db..9d71a369 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -312,7 +312,7 @@ public: SimplePipelineHandler(CameraManager *manager); std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -1038,7 +1038,7 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) } std::unique_ptr -SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles) +SimplePipelineHandler::generateConfiguration(Camera *camera, Span roles) { SimpleCameraData *data = cameraData(camera); std::unique_ptr config = diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 277465b7..03935876 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -75,7 +75,7 @@ public: PipelineHandlerUVC(CameraManager *manager); std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -180,7 +180,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) std::unique_ptr PipelineHandlerUVC::generateConfiguration(Camera *camera, - const StreamRoles &roles) + Span roles) { UVCCameraData *data = cameraData(camera); std::unique_ptr config = diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 204f5ad7..49ee949f 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -85,7 +85,7 @@ public: PipelineHandlerVimc(CameraManager *manager); std::unique_ptr generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -191,7 +191,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) std::unique_ptr PipelineHandlerVimc::generateConfiguration(Camera *camera, - const StreamRoles &roles) + Span roles) { VimcCameraData *data = cameraData(camera); std::unique_ptr config = diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index d14e18e2..3f04871f 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -156,7 +156,10 @@ PYBIND11_MODULE(_libcamera, m) }) /* Keep the camera alive, as StreamConfiguration contains a Stream* */ - .def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>()) + .def("generate_configuration", [](Camera &self, const std::vector &roles) { + return self.generateConfiguration(roles); + }, py::keep_alive<0, 1>()) + .def("configure", &Camera::configure) .def("create_request", &Camera::createRequest, py::arg("cookie") = 0)