From patchwork Thu Mar 26 14:59:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 3331 X-Patchwork-Delegate: jacopo@jmondi.org Return-Path: Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AA3F762B90 for ; Thu, 26 Mar 2020 15:56:34 +0100 (CET) X-Originating-IP: 2.224.242.101 Received: from localhost.localdomain (2-224-242-101.ip172.fastwebnet.it [2.224.242.101]) (Authenticated sender: jacopo@jmondi.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 615F020009 for ; Thu, 26 Mar 2020 14:56:33 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Mar 2020 15:59:24 +0100 Message-Id: <20200326145927.324919-4-jacopo@jmondi.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200326145927.324919-1-jacopo@jmondi.org> References: <20200326145927.324919-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/6] libcamera: camera_sensor: Introduce CameraSensorFactory X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Mar 2020 14:56:36 -0000 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 Reviewed-by: Laurent Pinchart Signed-off-by: Jacopo Mondi --- 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(-) 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 #include #include +#include +#include #include @@ -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 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(factory->create(entity)); + } + + LOG(CameraSensor, Debug) << "No specific support for sensor '" + << entity->name() + << "': using generic implementation"; + return std::make_unique(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 +#include #include #include @@ -61,6 +63,36 @@ private: ControlList properties_; }; +class CameraSensorFactory +{ +public: + CameraSensorFactory(const char *name); + virtual ~CameraSensorFactory() {} + + static std::unique_ptr createSensor(const MediaEntity *entity); + +private: + using FactoriesMap = std::map; + 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 sensor_; private: std::vector> 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 sensor_; unsigned int frame_; std::vector 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(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(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 #include #include +#include #include #include @@ -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 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 enumerator_; std::shared_ptr media_; - CameraSensor *sensor_; + std::unique_ptr 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 enumerator_; std::shared_ptr media_; - CameraSensor *sensor_; + std::unique_ptr sensor_; V4L2Subdevice *debayer_; V4L2VideoDevice *capture_; std::vector> buffers_;