[libcamera-devel] [RFC PATCH v2 1/2] libcamera: camera: Take span of StreamRole instead of vector
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue May 9 09:13:56 CEST 2023
Hi Barnabás,
Thank you for the patch.
On Wed, May 03, 2023 at 12:47:28PM +0000, Barnabás Pőcze via libcamera-devel wrote:
> Change the parameter type of `generateConfiguration()` from
> `const std::vector&` to `libcamera::Span`. A span is almost
> always preferable to a const vector ref because it does not
> force dynamic allocation when none are needed, and it allows
> any contiguous container to be used.
There's no need to wrap at 60 columns, you can use the normal 72 columns
wrapping for commit messages.
> A new overload is added that accepts an initializer list so that
>
> cam->generateConfiguration({ ... })
>
> keeps working.
>
> There is no API break since a span can be constructed from a vector
> and the initializer list overload takes care of the initializer lists,
> but this change causes an ABI break.
>
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
> Documentation/guides/pipeline-handler.rst | 4 ++--
> include/libcamera/camera.h | 12 +++++++++++-
> 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, 33 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 1acd1812..e08bb72f 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<const StreamRole> 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<const StreamRole> roles)
> {
> return nullptr;
> }
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 5bb06584..425fe51b 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -7,6 +7,7 @@
>
> #pragma once
>
> +#include <initializer_list>
> #include <memory>
> #include <set>
> #include <stdint.h>
> @@ -105,7 +106,16 @@ public:
> const ControlList &properties() const;
>
> const std::set<Stream *> &streams() const;
> - std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles = {});
> + std::unique_ptr<CameraConfiguration> generateConfiguration(Span<const StreamRole> roles = {});
> +
> + /**
> + * \overload
> + */
Documentation belongs to the .cpp file.
> + std::unique_ptr<CameraConfiguration> generateConfiguration(std::initializer_list<StreamRole> roles)
I'd wrap this to
std::unique_ptr<CameraConfiguration>
generateConfiguration(std::initializer_list<StreamRole> roles)
as it's getting long. Maybe above as well.
> + {
> + return generateConfiguration(Span(roles.begin(), roles.end()));
> + }
> +
> int configure(CameraConfiguration *config);
>
> std::unique_ptr<Request> 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<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) = 0;
> + Span<const StreamRole> 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 99683e49..f53fcdfd 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -938,7 +938,7 @@ const std::set<Stream *> &Camera::streams() const
> * \return A CameraConfiguration if the requested roles can be satisfied, or a
> * null pointer otherwise.
> */
> -std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)
> +std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(Span<const StreamRole> 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 449d9012..9bdfff0b 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -104,7 +104,7 @@ public:
> bool match(DeviceEnumerator *enumerator) override;
>
> std::unique_ptr<CameraConfiguration>
> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;
> + generateConfiguration(Camera *camera, Span<const StreamRole> roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> int exportFrameBuffers(Camera *camera, Stream *stream,
> @@ -739,7 +739,7 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)
>
> std::unique_ptr<CameraConfiguration>
> PipelineHandlerISI::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> + Span<const StreamRole> roles)
> {
> ISICameraData *data = cameraData(camera);
> std::unique_ptr<ISICameraConfiguration> 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<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) override;
> + Span<const StreamRole> 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<CameraConfiguration>
> -PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)
> +PipelineHandlerIPU3::generateConfiguration(Camera *camera, Span<const StreamRole> roles)
> {
> IPU3CameraData *data = cameraData(camera);
> std::unique_ptr<IPU3CameraConfiguration> config =
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 00600441..6421eda5 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -367,7 +367,7 @@ public:
> PipelineHandlerRPi(CameraManager *manager);
>
> std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) override;
> + Span<const StreamRole> roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> int exportFrameBuffers(Camera *camera, Stream *stream,
> @@ -667,7 +667,7 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)
> }
>
> std::unique_ptr<CameraConfiguration>
> -PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)
> +PipelineHandlerRPi::generateConfiguration(Camera *camera, Span<const StreamRole> roles)
> {
> RPiCameraData *data = cameraData(camera);
> std::unique_ptr<CameraConfiguration> 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<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) override;
> + Span<const StreamRole> 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<CameraConfiguration>
> PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> + Span<const StreamRole> roles)
> {
> RkISP1CameraData *data = cameraData(camera);
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index e1f8b989..d6e0758e 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -315,7 +315,7 @@ public:
> SimplePipelineHandler(CameraManager *manager);
>
> std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) override;
> + Span<const StreamRole> roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> int exportFrameBuffers(Camera *camera, Stream *stream,
> @@ -1043,7 +1043,7 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)
> }
>
> std::unique_ptr<CameraConfiguration>
> -SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)
> +SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRole> roles)
> {
> SimpleCameraData *data = cameraData(camera);
> std::unique_ptr<CameraConfiguration> 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<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) override;
> + Span<const StreamRole> 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<CameraConfiguration>
> PipelineHandlerUVC::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> + Span<const StreamRole> roles)
> {
> UVCCameraData *data = cameraData(camera);
> std::unique_ptr<CameraConfiguration> 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<CameraConfiguration> generateConfiguration(Camera *camera,
> - const StreamRoles &roles) override;
> + Span<const StreamRole> 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<CameraConfiguration>
> PipelineHandlerVimc::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> + Span<const StreamRole> roles)
> {
> VimcCameraData *data = cameraData(camera);
> std::unique_ptr<CameraConfiguration> 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<StreamRole> &roles) {
> + return self.generateConfiguration(roles);
> + }, py::keep_alive<0, 1>())
> +
> .def("configure", &Camera::configure)
>
> .def("create_request", &Camera::createRequest, py::arg("cookie") = 0)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list