[libcamera-devel,v4,3/6] libcamera: camera_sensor: Introduce CameraSensorFactory

Message ID 20200326145927.324919-4-jacopo@jmondi.org
State RFC
Delegated to: Jacopo Mondi
Headers show
Series
  • Camera properties and camera sensor factory
Related show

Commit Message

Jacopo Mondi March 26, 2020, 2:59 p.m. UTC
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.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/camera_sensor.cpp               | 132 ++++++++++++++++++
 src/libcamera/include/camera_sensor.h         |  32 +++++
 src/libcamera/pipeline/ipu3/ipu3.cpp          |   9 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  18 +--
 src/libcamera/pipeline/vimc/vimc.cpp          |   6 +-
 test/camera-sensor.cpp                        |   9 +-
 .../v4l2_videodevice_test.cpp                 |   3 +-
 test/v4l2_videodevice/v4l2_videodevice_test.h |   6 +-
 8 files changed, 183 insertions(+), 32 deletions(-)

Patch

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index fc24b3bd547d..c1a29e0b3cfc 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,134 @@  std::string CameraSensor::logPrefix() const
 	return "'" + subdev_->entity()->name() + "'";
 }
 
+/**
+ * \class CameraSensorFactory
+ * \brief Factory of camera sensors
+ *
+ * The class provides camera sensor with 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 subclasses use the REGISTER_CAMERA_SENSOR() macro to register
+ * themselves to the camera sensor factory. Users of the factory create camera
+ * sensor instances for a media entity by using the static createInstance()
+ * method, which returns an instance of the CameraSensor sub-class matching the
+ * entity, 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 CameraSensor instances
+ * to support camera sensors represented by the media entity with \a name.
+ *
+ * Creating an instance of the factory registers it with the global list of
+ * 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 ensure 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
+ * camera 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 responsible for deleting the instance.
+ *
+ * Ownership of the created camera sensor instance is passed to the caller as
+ * an unique pointer.
+ *
+ * \return A unique pointer to the 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.
+		 *
+		 * \todo Delegate matching to the CameraSensor sub-class
+		 * to support more complex matching criteria.
+		 */
+		if (name.find(key) == std::string::npos)
+			continue;
+
+		LOG(CameraSensor, Debug) << "Create camera sensor for '"
+					 << name << "'";
+
+		CameraSensorFactory *factory = it.second;
+		return std::unique_ptr<CameraSensor>(factory->create(entity));
+	}
+
+	LOG(CameraSensor, Debug) << "No specific support for sensor '"
+				 << entity->name()
+				 << "': using generic implementation";
+	return std::make_unique<CameraSensor>(entity);
+}
+
+/**
+ * \def REGISTER_CAMERA_SENSOR(sensor, sensorName)
+ * \brief Register camera sensor \a name with the sensor factory
+ * \param[in] sensor The name of the camera sensor class to register
+ * \param[in] sensorName The name of the image sensor supported by the factory
+ *
+ * Register camera sensor subclass \a sensor in the global list of factories.
+ * \a sensorName corresponds to the sensor entity name without the bus
+ * information, for instance "ov5670" for a sensor entity named "ov5645 4-003c".
+ */
+
 } /* namespace libcamera */
diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
index 99cff98128dc..8ffc787e740f 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, sensorName)			\
+class sensor##Factory final : public CameraSensorFactory		\
+{									\
+public:									\
+	sensor##Factory() : CameraSensorFactory(sensorName) {}		\
+									\
+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 1b44460e4d88..df2ab17cd5d6 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_;
@@ -292,7 +291,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())
@@ -1313,7 +1312,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 2f909cef7c75..a0e6490909ed 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -113,20 +113,14 @@  class RkISP1CameraData : public CameraData
 {
 public:
 	RkISP1CameraData(PipelineHandler *pipe)
-		: CameraData(pipe), sensor_(nullptr), frame_(0),
-		  frameInfo_(pipe)
+		: 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_;
@@ -386,7 +380,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;
 	}
@@ -441,7 +435,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())
@@ -554,7 +548,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;
 
 	/*
@@ -889,7 +883,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/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index b04a9726efa5..805bac0a7377 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -8,6 +8,7 @@ 
 #include <algorithm>
 #include <array>
 #include <iomanip>
+#include <memory>
 #include <tuple>
 
 #include <linux/media-bus-format.h>
@@ -46,7 +47,6 @@  public:
 
 	~VimcCameraData()
 	{
-		delete sensor_;
 		delete debayer_;
 		delete scaler_;
 		delete video_;
@@ -56,7 +56,7 @@  public:
 	int init(MediaDevice *media);
 	void bufferReady(FrameBuffer *buffer);
 
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	V4L2Subdevice *debayer_;
 	V4L2Subdevice *scaler_;
 	V4L2VideoDevice *video_;
@@ -398,7 +398,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 93b9e72da5b4..e1c8a886f2db 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_;