[PATCH 1/3] libcamera: camera_sensor: Introduce CameraSensorFactory

Jacopo Mondi jacopo.mondi at ideasonboard.com
Tue Oct 22 16:53:13 CEST 2024


From: Jacopo Mondi <jacopo at jmondi.org>

Introduce a factory to create CameraSensor derived classes instances by
inspecting the sensor media entity name and provide a convenience macro
to register specialized sensor handlers.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
---
 include/libcamera/internal/camera_sensor.h    |  48 +++++-
 src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |   9 +-
 src/libcamera/pipeline/ipu3/cio2.cpp          |   7 +-
 src/libcamera/pipeline/mali-c55/mali-c55.cpp  |   5 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   7 +-
 .../pipeline/rpi/common/pipeline_base.cpp     |   5 +-
 src/libcamera/pipeline/simple/simple.cpp      |   9 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |   7 +-
 src/libcamera/sensor/camera_sensor.cpp        | 162 ++++++++++++++++++
 test/camera-sensor.cpp                        |   7 +-
 .../v4l2_videodevice_test.cpp                 |   5 +-
 test/v4l2_videodevice/v4l2_videodevice_test.h |   2 +-
 12 files changed, 233 insertions(+), 40 deletions(-)

diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
index a42c15fa37ce..2d6cb697a870 100644
--- a/include/libcamera/internal/camera_sensor.h
+++ b/include/libcamera/internal/camera_sensor.h
@@ -38,7 +38,6 @@ enum class Orientation;
 class CameraSensor : protected Loggable
 {
 public:
-	explicit CameraSensor(const MediaEntity *entity);
 	~CameraSensor();
 
 	int init();
@@ -81,6 +80,7 @@ public:
 	int setTestPatternMode(controls::draft::TestPatternModeEnum mode);
 
 protected:
+	explicit CameraSensor(const MediaEntity *entity);
 	std::string logPrefix() const override;
 
 private:
@@ -122,4 +122,50 @@ private:
 	std::unique_ptr<CameraLens> focusLens_;
 };
 
+class CameraSensorFactoryBase
+{
+public:
+	CameraSensorFactoryBase();
+	virtual ~CameraSensorFactoryBase() = default;
+
+	static std::unique_ptr<CameraSensor> create(MediaEntity *entity);
+
+private:
+	LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorFactoryBase)
+
+	static std::vector<CameraSensorFactoryBase *> &factories();
+
+	static void registerFactory(CameraSensorFactoryBase *factory);
+
+	virtual bool match(const MediaEntity *entity) const = 0;
+
+	virtual std::unique_ptr<CameraSensor>
+	createInstance(MediaEntity *entity) const = 0;
+};
+
+template<typename _CameraSensor>
+class CameraSensorFactory final : public CameraSensorFactoryBase
+{
+public:
+	CameraSensorFactory()
+		: CameraSensorFactoryBase()
+	{
+	}
+
+private:
+	bool match(const MediaEntity *entity) const override
+	{
+		return _CameraSensor::match(entity);
+	}
+
+	std::unique_ptr<CameraSensor>
+	createInstance(MediaEntity *entity) const override
+	{
+		return _CameraSensor::create(entity);
+	}
+};
+
+#define REGISTER_CAMERA_SENSOR(sensor) \
+static CameraSensorFactory<sensor> global_##sensor##Factory{};
+
 } /* namespace libcamera */
diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
index 72aa6c75608a..4e66b3368d5a 100644
--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
@@ -157,11 +157,10 @@ PipelineHandlerISI *ISICameraData::pipe()
 /* Open and initialize pipe components. */
 int ISICameraData::init()
 {
-	int ret = sensor_->init();
-	if (ret)
-		return ret;
+	if (!sensor_)
+		return -ENODEV;
 
-	ret = csis_->open();
+	int ret = csis_->open();
 	if (ret)
 		return ret;
 
@@ -1057,7 +1056,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
 		std::unique_ptr<ISICameraData> data =
 			std::make_unique<ISICameraData>(this);
 
-		data->sensor_ = std::make_unique<CameraSensor>(sensor);
+		data->sensor_ = CameraSensorFactoryBase::create(sensor);
 		data->csis_ = std::make_unique<V4L2Subdevice>(csi);
 		data->xbarSink_ = sink;
 
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 74a5d93f88ae..aa544d7b0303 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -134,10 +134,9 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
 
 	MediaLink *link = links[0];
 	MediaEntity *sensorEntity = link->source()->entity();
-	sensor_ = std::make_unique<CameraSensor>(sensorEntity);
-	ret = sensor_->init();
-	if (ret)
-		return ret;
+	sensor_ = CameraSensorFactoryBase::create(sensorEntity);
+	if (!sensor_)
+		return -ENODEV;
 
 	ret = link->setEnabled(true);
 	if (ret)
diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
index 45c71c1dd619..4b71e0dad15e 100644
--- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
+++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
@@ -141,9 +141,8 @@ int MaliC55CameraData::init()
 	 * Register a CameraSensor if we connect to a sensor and create
 	 * an entity for the connected CSI-2 receiver.
 	 */
-	sensor_ = std::make_unique<CameraSensor>(entity_);
-	ret = sensor_->init();
-	if (ret)
+	sensor_ = CameraSensorFactoryBase::create(entity_);
+	if (!sensor_)
 		return ret;
 
 	const MediaPad *sourcePad = entity_->getPadByIndex(0);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index d8d1d65fbbf9..95c25e976015 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1122,10 +1122,9 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 		std::make_unique<RkISP1CameraData>(this, &mainPath_,
 						   hasSelfPath_ ? &selfPath_ : nullptr);
 
-	data->sensor_ = std::make_unique<CameraSensor>(sensor);
-	ret = data->sensor_->init();
-	if (ret)
-		return ret;
+	data->sensor_ = CameraSensorFactoryBase::create(sensor);
+	if (!data->sensor_)
+		return -ENODEV;
 
 	/* Initialize the camera properties. */
 	data->properties_ = data->sensor_->properties();
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index 3041fd1ed9fd..4f56bd33df05 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -774,13 +774,10 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
 	CameraData *data = cameraData.get();
 	int ret;
 
-	data->sensor_ = std::make_unique<CameraSensor>(sensorEntity);
+	data->sensor_ = CameraSensorFactoryBase::create(sensorEntity);
 	if (!data->sensor_)
 		return -EINVAL;
 
-	if (data->sensor_->init())
-		return -EINVAL;
-
 	/* Populate the map of sensor supported formats and sizes. */
 	for (auto const mbusCode : data->sensor_->mbusCodes())
 		data->sensorFormats_.emplace(mbusCode,
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 3ddce71d3757..67f583b8a22c 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -388,8 +388,6 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
 				   MediaEntity *sensor)
 	: Camera::Private(pipe), streams_(numStreams)
 {
-	int ret;
-
 	/*
 	 * Find the shortest path from the camera sensor to a video capture
 	 * device using the breadth-first search algorithm. This heuristic will
@@ -480,12 +478,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
 	}
 
 	/* Finally also remember the sensor. */
-	sensor_ = std::make_unique<CameraSensor>(sensor);
-	ret = sensor_->init();
-	if (ret) {
-		sensor_.reset();
+	sensor_ = CameraSensorFactoryBase::create(sensor);
+	if (!sensor_)
 		return;
-	}
 
 	LOG(SimplePipeline, Debug)
 		<< "Found pipeline: "
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 2165bae890cb..2e474133439c 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -532,10 +532,9 @@ int VimcCameraData::init()
 		return ret;
 
 	/* Create and open the camera sensor, debayer, scaler and video device. */
-	sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B"));
-	ret = sensor_->init();
-	if (ret)
-		return ret;
+	sensor_ = CameraSensorFactoryBase::create(media_->getEntityByName("Sensor B"));
+	if (!sensor_)
+		return -ENODEV;
 
 	debayer_ = V4L2Subdevice::fromEntityName(media_, "Debayer B");
 	if (debayer_->open())
diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
index 1b224f1989fe..025c9eefdd12 100644
--- a/src/libcamera/sensor/camera_sensor.cpp
+++ b/src/libcamera/sensor/camera_sensor.cpp
@@ -11,6 +11,7 @@
 #include <cmath>
 #include <float.h>
 #include <limits.h>
+#include <map>
 #include <string.h>
 
 #include <libcamera/base/utils.h>
@@ -1202,4 +1203,165 @@ std::string CameraSensor::logPrefix() const
 	return "'" + entity_->name() + "'";
 }
 
+namespace {
+
+/* Transitory default camera sensor implementation */
+class CameraSensorDefault : public CameraSensor
+{
+public:
+	CameraSensorDefault(MediaEntity *entity)
+		: CameraSensor(entity)
+	{
+	}
+
+	static bool match([[maybe_unused]] const MediaEntity *entity)
+	{
+		return true;
+	}
+
+	static std::unique_ptr<CameraSensorDefault> create(MediaEntity *entity)
+	{
+		std::unique_ptr<CameraSensorDefault> sensor =
+			std::make_unique<CameraSensorDefault>(entity);
+
+		if (sensor->init())
+			return nullptr;
+
+		return sensor;
+	}
+};
+
+REGISTER_CAMERA_SENSOR(CameraSensorDefault)
+
+}; /* namespace */
+
+/**
+ * \class CameraSensorFactoryBase
+ * \brief Base class for camera sensor factories
+ *
+ * The CameraSensorFactoryBase class is the base of all specializations of
+ * the CameraSensorFactory class template. It implements the factory
+ * registration, maintains a registry of factories, and provides access to the
+ * registered factories.
+ */
+
+/**
+ * \brief Construct a camera sensor factory base
+ *
+ * Creating an instance of the factory base registers it with the global list of
+ * factories, accessible through the factories() function.
+ */
+CameraSensorFactoryBase::CameraSensorFactoryBase()
+{
+	registerFactory(this);
+}
+
+/**
+ * \brief Create an instance of the CameraSensor corresponding to a media entity
+ * \param[in] entity The media entity on the source end of the sensor
+ *
+ * \return A unique pointer to a new instance of the CameraSensor subclass
+ * matching the entity, or a null pointer if no such factory exists
+ */
+std::unique_ptr<CameraSensor> CameraSensorFactoryBase::create(MediaEntity *entity)
+{
+	const std::vector<CameraSensorFactoryBase *> &factories =
+		CameraSensorFactoryBase::factories();
+
+	for (const CameraSensorFactoryBase *factory : factories) {
+		if (!factory->match(entity))
+			continue;
+
+		std::unique_ptr<CameraSensor> sensor = factory->createInstance(entity);
+		if (!sensor) {
+			LOG(CameraSensor, Error)
+				<< "Failed to create sensor for '"
+				<< entity->name();
+			return nullptr;
+		}
+
+		return sensor;
+	}
+
+	return nullptr;
+}
+
+/**
+ * \brief Retrieve the list of all camera sensor factories
+ * \return The list of camera sensor factories
+ */
+std::vector<CameraSensorFactoryBase *> &CameraSensorFactoryBase::factories()
+{
+	/*
+	 * The static factories map is defined inside the function to ensure
+	 * it gets initialized on first use, without any dependency on link
+	 * order.
+	 */
+	static std::vector<CameraSensorFactoryBase *> factories;
+	return factories;
+}
+
+/**
+ * \brief Add a camera sensor class to the registry
+ * \param[in] factory Factory to use to construct the camera sensor
+ */
+void CameraSensorFactoryBase::registerFactory(CameraSensorFactoryBase *factory)
+{
+	std::vector<CameraSensorFactoryBase *> &factories =
+		CameraSensorFactoryBase::factories();
+
+	factories.push_back(factory);
+}
+
+/**
+ * \class CameraSensorFactory
+ * \brief Registration of CameraSensorFactory classes and creation of instances
+ * \tparam _CameraSensor The camera sensor class type for this factory
+ *
+ * To facilitate discovery and instantiation of CameraSensor classes, the
+ * CameraSensorFactory class implements auto-registration of camera sensors.
+ * Each CameraSensor subclass shall register itself using the
+ * REGISTER_CAMERA_SENSOR() macro, which will create a corresponding instance
+ * of a CameraSensorFactory subclass and register it with the static list of
+ * factories.
+ */
+
+/**
+ * \fn CameraSensorFactory::CameraSensorFactory()
+ * \brief Construct a camera sensor factory
+ *
+ * Creating an instance of the factory registers it with the global list of
+ * factories, accessible through the CameraSensorFactoryBase::factories()
+ * function.
+ */
+
+/**
+ * \fn CameraSensorFactory::createInstance() const
+ * \brief Create an instance of the CameraSensor corresponding to the factory
+ *
+ * \return A unique pointer to a newly constructed instance of the CameraSensor
+ * subclass corresponding to the factory
+ */
+
+/**
+ * \def REGISTER_CAMERA_SENSOR(sensor)
+ * \brief Register a camera sensor type to the sensor factory
+ * \param[in] sensor Class name of the CameraSensor derived class to register
+ *
+ * Register a CameraSensor subclass with the factory and make it available to
+ * try and match sensors. The subclass needs to implement two static functions:
+ *
+ * \code{.cpp}
+ * static bool match(const MediaEntity *entity);
+ * static std::unique_ptr<sensor> create(MediaEntity *entity);
+ * \endcode
+ *
+ * The match() function tests if the sensor class supports the camera sensor
+ * identified by a MediaEntity.
+ *
+ * The create() function creates a new instance of the sensor class. It may
+ * return a null pointer if initialization of the instance fails. It will only
+ * be called if the match() function has returned true for the given entity.
+ */
+
 } /* namespace libcamera */
diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
index 1d402c43355b..869c788965da 100644
--- a/test/camera-sensor.cpp
+++ b/test/camera-sensor.cpp
@@ -52,8 +52,8 @@ protected:
 			return TestFail;
 		}
 
-		sensor_ = new CameraSensor(entity);
-		if (sensor_->init() < 0) {
+		sensor_ = CameraSensorFactoryBase::create(entity);
+		if (!sensor_) {
 			cerr << "Unable to initialise camera sensor" << endl;
 			return TestFail;
 		}
@@ -118,13 +118,12 @@ protected:
 
 	void cleanup()
 	{
-		delete sensor_;
 	}
 
 private:
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 	std::shared_ptr<MediaDevice> media_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	CameraLens *lens_;
 };
 
diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
index 1113cf5bf8cf..9fbd24cc91ea 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
@@ -64,8 +64,8 @@ int V4L2VideoDeviceTest::init()
 	format.size.height = 480;
 
 	if (driver_ == "vimc") {
-		sensor_ = new CameraSensor(media_->getEntityByName("Sensor A"));
-		if (sensor_->init())
+		sensor_ = CameraSensorFactoryBase::create(media_->getEntityByName("Sensor A"));
+		if (!sensor_)
 			return TestFail;
 
 		debayer_ = new V4L2Subdevice(media_->getEntityByName("Debayer A"));
@@ -98,6 +98,5 @@ void V4L2VideoDeviceTest::cleanup()
 	capture_->close();
 
 	delete debayer_;
-	delete sensor_;
 	delete capture_;
 }
diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h
index b5871ce69e18..7c9003ec4c4d 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.h
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.h
@@ -36,7 +36,7 @@ protected:
 	std::string entity_;
 	std::unique_ptr<libcamera::DeviceEnumerator> enumerator_;
 	std::shared_ptr<libcamera::MediaDevice> media_;
-	libcamera::CameraSensor *sensor_;
+	std::unique_ptr<libcamera::CameraSensor> sensor_;
 	libcamera::V4L2Subdevice *debayer_;
 	libcamera::V4L2VideoDevice *capture_;
 	std::vector<std::unique_ptr<libcamera::FrameBuffer>> buffers_;
-- 
2.47.0



More information about the libcamera-devel mailing list