[libcamera-devel] [PATCH v3 02/14] libcamera: camera: Pass Private pointer to Camera constructor

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Aug 12 01:26:13 CEST 2021


In order to allow subclassing Camera::Private in pipeline handlers, pass
the pointer to the private data to the Camera constructor, and to the
Camera::createCamera() function.

The Camera::Private id_ and streams_ members now need to be initialized
by the Camera constructor instead of the Camera::Private constructor, to
allow storage of the streams in a pipeline handler-specific subclass of
Camera::Private.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
 include/libcamera/camera.h                    |  4 +--
 include/libcamera/internal/camera.h           |  3 +--
 src/libcamera/camera.cpp                      | 26 ++++++++++++-------
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++-
 .../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  |  5 +++-
 src/libcamera/pipeline/vimc/vimc.cpp          |  4 ++-
 9 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index b081907e0cb1..05cdab724b10 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -78,7 +78,7 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>,
 	LIBCAMERA_DECLARE_PRIVATE()
 
 public:
-	static std::shared_ptr<Camera> create(PipelineHandler *pipe,
+	static std::shared_ptr<Camera> create(std::unique_ptr<Private> d,
 					      const std::string &id,
 					      const std::set<Stream *> &streams);
 
@@ -107,7 +107,7 @@ public:
 private:
 	LIBCAMERA_DISABLE_COPY(Camera)
 
-	Camera(PipelineHandler *pipe, const std::string &id,
+	Camera(std::unique_ptr<Private> d, const std::string &id,
 	       const std::set<Stream *> &streams);
 	~Camera();
 
diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
index b60ed140356a..9ec8321a9a21 100644
--- a/include/libcamera/internal/camera.h
+++ b/include/libcamera/internal/camera.h
@@ -26,8 +26,7 @@ class Camera::Private : public Extensible::Private
 	LIBCAMERA_DECLARE_PUBLIC(Camera)
 
 public:
-	Private(PipelineHandler *pipe, const std::string &id,
-		const std::set<Stream *> &streams);
+	Private(PipelineHandler *pipe);
 	~Private();
 
 private:
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 8e99e2a96eaf..d9f6b784e0dc 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -332,11 +332,13 @@ std::size_t CameraConfiguration::size() const
  * \brief The vector of stream configurations
  */
 
-Camera::Private::Private(PipelineHandler *pipe,
-			 const std::string &id,
-			 const std::set<Stream *> &streams)
-	: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),
-	  disconnected_(false), state_(CameraAvailable)
+/**
+ * \brief Construct a Camera::Private instance
+ * \param[in] pipe The pipeline handler responsible for the camera device
+ */
+Camera::Private::Private(PipelineHandler *pipe)
+	: pipe_(pipe->shared_from_this()), disconnected_(false),
+	  state_(CameraAvailable)
 {
 }
 
@@ -513,7 +515,7 @@ void Camera::Private::setState(State state)
 
 /**
  * \brief Create a camera instance
- * \param[in] pipe The pipeline handler responsible for the camera device
+ * \param[in] d Camera private data
  * \param[in] id The ID of the camera device
  * \param[in] streams Array of streams the camera provides
  *
@@ -527,10 +529,12 @@ void Camera::Private::setState(State state)
  *
  * \return A shared pointer to the newly created camera object
  */
-std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
+std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
 				       const std::string &id,
 				       const std::set<Stream *> &streams)
 {
+	ASSERT(d);
+
 	struct Deleter : std::default_delete<Camera> {
 		void operator()(Camera *camera)
 		{
@@ -541,7 +545,7 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
 		}
 	};
 
-	Camera *camera = new Camera(pipe, id, streams);
+	Camera *camera = new Camera(std::move(d), id, streams);
 
 	return std::shared_ptr<Camera>(camera, Deleter());
 }
@@ -594,10 +598,12 @@ const std::string &Camera::id() const
  * application API calls by returning errors immediately.
  */
 
-Camera::Camera(PipelineHandler *pipe, const std::string &id,
+Camera::Camera(std::unique_ptr<Private> d, const std::string &id,
 	       const std::set<Stream *> &streams)
-	: Extensible(std::make_unique<Private>(pipe, id, streams))
+	: Extensible(std::move(d))
 {
+	_d()->id_ = id;
+	_d()->streams_ = streams;
 }
 
 Camera::~Camera()
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 19cb5c4ec9c3..b9c0941c5ea8 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -23,6 +23,7 @@
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
@@ -1199,7 +1200,8 @@ int PipelineHandlerIPU3::registerCameras()
 		/* Create and register the Camera instance. */
 		std::string cameraId = cio2->sensor()->id();
 		std::shared_ptr<Camera> camera =
-			Camera::create(this, cameraId, streams);
+			Camera::create(std::make_unique<Camera::Private>(this),
+				       cameraId, streams);
 
 		registerCamera(std::move(camera), std::move(data));
 
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 2e8774ae360d..91e906ef14ce 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -29,6 +29,7 @@
 #include <linux/videodev2.h>
 
 #include "libcamera/internal/bayer_format.h"
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
@@ -1105,7 +1106,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
 
 	/* Create and register the camera. */
 	std::shared_ptr<Camera> camera =
-		Camera::create(this, data->sensor_->id(), streams);
+		Camera::create(std::make_unique<Camera::Private>(this),
+			       data->sensor_->id(), streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	return true;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 42911a8fdfdb..c2872289337b 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -27,6 +27,7 @@
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
@@ -970,7 +971,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 		&data->selfPathStream_,
 	};
 	std::shared_ptr<Camera> camera =
-		Camera::create(this, data->sensor_->id(), streams);
+		Camera::create(std::make_unique<Camera::Private>(this),
+			       data->sensor_->id(), streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	return 0;
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index b29fff9820e5..ad581969fca1 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -28,6 +28,7 @@
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/media_device.h"
@@ -1046,7 +1047,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 			       [](Stream &stream) { return &stream; });
 
 		std::shared_ptr<Camera> camera =
-			Camera::create(this, data->sensor_->id(), streams);
+			Camera::create(std::make_unique<Camera::Private>(this),
+				       data->sensor_->id(), streams);
 		registerCamera(std::move(camera), std::move(data));
 		registered = true;
 	}
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 0f634b8da609..a9b6af4b5bdf 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -22,6 +22,7 @@
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
@@ -470,7 +471,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
 	}
 
 	std::set<Stream *> streams{ &data->stream_ };
-	std::shared_ptr<Camera> camera = Camera::create(this, id, streams);
+	std::shared_ptr<Camera> camera =
+		Camera::create(std::make_unique<Camera::Private>(this), id,
+			       streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	/* Enable hot-unplug notifications. */
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 12f7517fd0ae..999d975716fa 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -29,6 +29,7 @@
 #include <libcamera/ipa/vimc_ipa_interface.h>
 #include <libcamera/ipa/vimc_ipa_proxy.h>
 
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/ipa_manager.h"
@@ -438,7 +439,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 	/* Create and register the camera. */
 	std::set<Stream *> streams{ &data->stream_ };
 	std::shared_ptr<Camera> camera =
-		Camera::create(this, data->sensor_->id(), streams);
+		Camera::create(std::make_unique<Camera::Private>(this),
+			       data->sensor_->id(), streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	return true;
-- 
Regards,

Laurent Pinchart



More information about the libcamera-devel mailing list