[RFC PATCH v1] libcamera: camera: Take span of StreamRole instead of vector

Barnabás Pőcze pobrn at protonmail.com
Wed Feb 15 18:48:52 CET 2023


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 <pobrn at protonmail.com>
---

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(-)

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<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..ccf0d24c 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -105,7 +105,14 @@ 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 = {});
+
+	template<std::size_t N>
+	std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRole (&roles)[N])
+	{
+		return generateConfiguration(Span<const StreamRole>(roles));
+	}
+
 	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 0da167a7..0f68a511 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 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<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,
@@ -571,7 +571,7 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)

 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 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<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,
@@ -661,7 +661,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 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<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,
@@ -1038,7 +1038,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)
--
2.39.1



More information about the libcamera-devel mailing list