[libcamera-devel] [PATCH v3 3/7] libcamera: camera_sensor: Introduce CameraSensorFactory

Jacopo Mondi jacopo at jmondi.org
Tue Feb 18 12:27:48 CET 2020


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

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 src/libcamera/camera_sensor.cpp               | 133 ++++++++++++++++++
 src/libcamera/include/camera_sensor.h         |  32 +++++
 src/libcamera/pipeline/ipu3/ipu3.cpp          |   9 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  17 +--
 src/libcamera/pipeline/vimc.cpp               |   6 +-
 test/camera-sensor.cpp                        |   9 +-
 .../v4l2_videodevice_test.cpp                 |   3 +-
 test/v4l2_videodevice/v4l2_videodevice_test.h |   6 +-
 8 files changed, 184 insertions(+), 31 deletions(-)

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 2219a4307436..fa577c62b97a 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -12,6 +12,8 @@
 #include <iomanip>
 #include <limits.h>
 #include <math.h>
+#include <memory>
+#include <string.h>
 
 #include <libcamera/property_ids.h>
 
@@ -366,4 +368,135 @@ std::string CameraSensor::logPrefix() const
 	return "'" + subdev_->entity()->name() + "'";
 }
 
+/**
+ * \class CameraSensorFactory
+ * \brief Factory of camera sensors
+ *
+ * The class provides to camera sensor the ability to register themselves to the
+ * factory to allow the creation of their instances by matching the name of the
+ * media entity which represents the sensor.
+ *
+ * Camera sensor handlers use the REGISTER_CAMERA_SENSOR() macro to
+ * add themselves to the camera sensor factory. Users of the factory
+ * creates camera sensor handler instances by using the static
+ * CameraSensorFactory::createInstance() method, which returns a pointer
+ * to the opportune CameraSensor sub-class if any, or a newly created instance
+ * of the generic CameraSensor class.
+ */
+
+/**
+ * \brief Construct a camera sensor factory
+ * \param[in] name The name of the media entity representing the sensor
+ *
+ * Register a new camera sensor factory which creates sensor handler instances
+ * that support camera sensors represented by the media entity with \a name.
+ *
+ * Creating an instance of the factory registers is with the global list o
+ * factories, accessible through the factories() function.
+ */
+CameraSensorFactory::CameraSensorFactory(const char *name)
+{
+	registerFactory(name, this);
+}
+
+/**
+ * \brief Retrieve the list of registered camera sensor factories
+ *
+ * The static factories map is defined inside the function to ensures it gets
+ * initialized on first use, without any dependency on link order.
+ *
+ * \return The static list of registered camera sensor factories
+ */
+CameraSensorFactory::FactoriesMap &CameraSensorFactory::factories()
+{
+	static FactoriesMap factories;
+	return factories;
+}
+
+/**
+ * \brief Register a camera sensor factory
+ * \param[in] name The name of the media entity that represents the sensor
+ * \param[in] factory The factory instance to register
+ *
+ * The camera sensor \a factory is registered associated with an entity \a name.
+ * When a camera sensor is created for a media entity with createSensor(), the
+ * name of the media entity is used to select the corresponding factory.
+ *
+ * The caller is responsible for guaranteeing the uniqueness of the
+ * amera sensor entity name.
+ */
+void CameraSensorFactory::registerFactory(const char *name,
+					  CameraSensorFactory *factory)
+{
+	FactoriesMap &map = factories();
+
+	if (map.find(name) != map.end()) {
+		LOG(CameraSensor, Error)
+			<< "Unable to register camera sensor factory '"
+			<< name << "': already registered";
+		return;
+	}
+
+	map[name] = factory;
+}
+
+/**
+ * \brief Create a camera sensor corresponding to \a entity
+ * \param[in] entity The media entity that represents the camera sensor
+ *
+ * Create a new instance of a CameraSensor subclass for the sensor represented
+ * by \a entity using one of the registered factories. If no specific class is
+ * available for the sensor, an instance of the generic CameraSensor class is
+ * created and returned.
+ *
+ * Ownership of the created camera sensor instance is passed to the caller
+ * which is reponsible for deleting the instance.
+ * FIXME: is it worth using a smart pointer here ?
+ *
+ * \return A newly created camera sensor instance
+ */
+std::unique_ptr<CameraSensor> CameraSensorFactory::createSensor(const MediaEntity *entity)
+{
+	const std::string &name = entity->name();
+	for (const auto &it : factories()) {
+		const std::string &key = it.first;
+
+		/*
+		 * Search for the key, provided by the CameraSensor sub-class at
+		 * factory registration time, with the sensor entity name, to
+		 * support both canonical sensor entity names (in the "devname
+		 * i2c-adapt:i2c-addr" format) and non-canonical sensor names
+		 * such as VIMC's "Sensor B" ones.
+		 *
+		 * \todo Delegate matching to the CameraSensor sub-class
+		 * to support more complex matching criteria.
+		 */
+		if (name.find(key) == std::string::npos)
+			continue;
+
+		LOG(CameraSensor, Info) << "Create camera sensor for '"
+					<< name << "'";
+
+		CameraSensorFactory *factory = it.second;
+		return std::unique_ptr<CameraSensor>(factory->create(entity));
+	}
+
+	LOG(CameraSensor, Info) << "Unsupported sensor '" << entity->name()
+				<< "': using generic camera sensor";
+	return std::make_unique<CameraSensor>(entity);
+}
+
+/**
+ * \def REGISTER_CAMERA_SENSOR(sensor, entityName)
+ * \brief Register camera sensor \a name with the sensor factory
+ * \param[in] sensor The name of the camera sensor class to register
+ * \param[in] entityName The name of the image sensor supported by the factory
+ *
+ * Register camera sensor factory \a sensor with the global list of factories.
+ * The camera sensor factory supports image sensors identified by
+ * \a entityName. The sensor name, as reported by the media entity that
+ * identifies it, is usually composed by the here provided \a entityName and
+ * the I2C bus and device ids.
+ */
+
 } /* namespace libcamera */
diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
index 99cff98128dc..633955591b36 100644
--- a/src/libcamera/include/camera_sensor.h
+++ b/src/libcamera/include/camera_sensor.h
@@ -7,6 +7,8 @@
 #ifndef __LIBCAMERA_CAMERA_SENSOR_H__
 #define __LIBCAMERA_CAMERA_SENSOR_H__
 
+#include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -61,6 +63,36 @@ private:
 	ControlList properties_;
 };
 
+class CameraSensorFactory
+{
+public:
+	CameraSensorFactory(const char *name);
+	virtual ~CameraSensorFactory() {}
+
+	static std::unique_ptr<CameraSensor> createSensor(const MediaEntity *entity);
+
+private:
+	using FactoriesMap = std::map<const std::string, CameraSensorFactory *>;
+	static FactoriesMap &factories();
+	static void registerFactory(const char *name,
+				    CameraSensorFactory *factory);
+	virtual CameraSensor *create(const MediaEntity *entity) = 0;
+};
+
+#define REGISTER_CAMERA_SENSOR(sensor, entityName)			\
+class sensor##Factory final : public CameraSensorFactory		\
+{									\
+public:									\
+	sensor##Factory() : CameraSensorFactory(entityName) {}		\
+									\
+private:								\
+	CameraSensor *create(const MediaEntity *entity)			\
+	{								\
+		return new sensor(entity);				\
+	}								\
+};									\
+static sensor##Factory global_##sensor##Factory
+
 } /* namespace libcamera */
 
 #endif /* __LIBCAMERA_CAMERA_SENSOR_H__ */
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 387bb070b505..adef6c6703b6 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -100,7 +100,7 @@ public:
 	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
 
 	CIO2Device()
-		: output_(nullptr), csi2_(nullptr), sensor_(nullptr)
+		: output_(nullptr), csi2_(nullptr)
 	{
 	}
 
@@ -108,7 +108,6 @@ public:
 	{
 		delete output_;
 		delete csi2_;
-		delete sensor_;
 	}
 
 	int init(const MediaDevice *media, unsigned int index);
@@ -125,7 +124,7 @@ public:
 
 	V4L2VideoDevice *output_;
 	V4L2Subdevice *csi2_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 
 private:
 	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
@@ -294,7 +293,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
 
 CameraConfiguration::Status IPU3CameraConfiguration::validate()
 {
-	const CameraSensor *sensor = data_->cio2_.sensor_;
+	const CameraSensor *sensor = data_->cio2_.sensor_.get();
 	Status status = Valid;
 
 	if (config_.empty())
@@ -1322,7 +1321,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
 
 	MediaLink *link = links[0];
 	MediaEntity *sensorEntity = link->source()->entity();
-	sensor_ = new CameraSensor(sensorEntity);
+	sensor_ = CameraSensorFactory::createSensor(sensorEntity);
 	ret = sensor_->init();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 13433b216747..3e3b22018430 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -114,20 +114,15 @@ class RkISP1CameraData : public CameraData
 {
 public:
 	RkISP1CameraData(PipelineHandler *pipe)
-		: CameraData(pipe), sensor_(nullptr), frame_(0),
+		: CameraData(pipe), frame_(0),
 		  frameInfo_(pipe)
 	{
 	}
 
-	~RkISP1CameraData()
-	{
-		delete sensor_;
-	}
-
 	int loadIPA();
 
 	Stream stream_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	unsigned int frame_;
 	std::vector<IPABuffer> ipaBuffers_;
 	RkISP1Frames frameInfo_;
@@ -389,7 +384,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
 	case RKISP1_IPA_ACTION_V4L2_SET: {
 		const ControlList &controls = action.controls[0];
 		timeline_.scheduleAction(std::make_unique<RkISP1ActionSetSensor>(frame,
-										 sensor_,
+										 sensor_.get(),
 										 controls));
 		break;
 	}
@@ -444,7 +439,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 		/* \todo Add support for 8-bit greyscale to DRM formats */
 	};
 
-	const CameraSensor *sensor = data_->sensor_;
+	const CameraSensor *sensor = data_->sensor_.get();
 	Status status = Valid;
 
 	if (config_.empty())
@@ -557,7 +552,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 		static_cast<RkISP1CameraConfiguration *>(c);
 	RkISP1CameraData *data = cameraData(camera);
 	StreamConfiguration &cfg = config->at(0);
-	CameraSensor *sensor = data->sensor_;
+	CameraSensor *sensor = data->sensor_.get();
 	int ret;
 
 	/*
@@ -907,7 +902,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 
 	data->controlInfo_ = std::move(ctrls);
 
-	data->sensor_ = new CameraSensor(sensor);
+	data->sensor_ = CameraSensorFactory::createSensor(sensor);
 	ret = data->sensor_->init();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index fd4df0b03c26..a51d67b2f4f6 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -8,6 +8,7 @@
 #include <algorithm>
 #include <array>
 #include <iomanip>
+#include <memory>
 #include <tuple>
 
 #include <linux/drm_fourcc.h>
@@ -47,7 +48,6 @@ public:
 
 	~VimcCameraData()
 	{
-		delete sensor_;
 		delete debayer_;
 		delete scaler_;
 		delete video_;
@@ -57,7 +57,7 @@ public:
 	int init(MediaDevice *media);
 	void bufferReady(FrameBuffer *buffer);
 
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	V4L2Subdevice *debayer_;
 	V4L2Subdevice *scaler_;
 	V4L2VideoDevice *video_;
@@ -403,7 +403,7 @@ int VimcCameraData::init(MediaDevice *media)
 		return ret;
 
 	/* Create and open the camera sensor, debayer, scaler and video device. */
-	sensor_ = new CameraSensor(media->getEntityByName("Sensor B"));
+	sensor_ = CameraSensorFactory::createSensor(media->getEntityByName("Sensor B"));
 	ret = sensor_->init();
 	if (ret)
 		return ret;
diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
index 27c190fe7ace..8709e481dc7b 100644
--- a/test/camera-sensor.cpp
+++ b/test/camera-sensor.cpp
@@ -50,7 +50,7 @@ protected:
 			return TestFail;
 		}
 
-		sensor_ = new CameraSensor(entity);
+		sensor_ = CameraSensorFactory::createSensor(entity);
 		if (sensor_->init() < 0) {
 			cerr << "Unable to initialise camera sensor" << endl;
 			return TestFail;
@@ -100,15 +100,10 @@ protected:
 		return TestPass;
 	}
 
-	void cleanup()
-	{
-		delete sensor_;
-	}
-
 private:
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 	std::shared_ptr<MediaDevice> media_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 };
 
 TEST_REGISTER(CameraSensorTest)
diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
index 577da4cb601c..ae038de74864 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
@@ -61,7 +61,7 @@ int V4L2VideoDeviceTest::init()
 		return TestFail;
 
 	if (driver_ == "vimc") {
-		sensor_ = new CameraSensor(media_->getEntityByName("Sensor A"));
+		sensor_ = CameraSensorFactory::createSensor(media_->getEntityByName("Sensor A"));
 		if (sensor_->init())
 			return TestFail;
 
@@ -97,6 +97,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 9acaceb84fe0..f635e0929efe 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.h
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.h
@@ -25,8 +25,8 @@ class V4L2VideoDeviceTest : public Test
 {
 public:
 	V4L2VideoDeviceTest(const char *driver, const char *entity)
-		: driver_(driver), entity_(entity), sensor_(nullptr),
-		  debayer_(nullptr), capture_(nullptr)
+		: driver_(driver), entity_(entity), debayer_(nullptr),
+		  capture_(nullptr)
 	{
 	}
 
@@ -38,7 +38,7 @@ protected:
 	std::string entity_;
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 	std::shared_ptr<MediaDevice> media_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	V4L2Subdevice *debayer_;
 	V4L2VideoDevice *capture_;
 	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
-- 
2.25.0



More information about the libcamera-devel mailing list