From patchwork Tue Feb 18 11:27:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 2852 X-Patchwork-Delegate: jacopo@jmondi.org Return-Path: Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B00D761EF2 for ; Tue, 18 Feb 2020 12:25:16 +0100 (CET) X-Originating-IP: 93.34.114.233 Received: from uno.lan (93-34-114-233.ip49.fastwebnet.it [93.34.114.233]) (Authenticated sender: jacopo@jmondi.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 501B360008; Tue, 18 Feb 2020 11:25:16 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Tue, 18 Feb 2020 12:27:48 +0100 Message-Id: <20200218112752.3910410-4-jacopo@jmondi.org> X-Mailer: git-send-email 2.25.0 In-Reply-To: <20200218112752.3910410-1-jacopo@jmondi.org> References: <20200218112752.3910410-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 3/7] 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: Tue, 18 Feb 2020 11:25:17 -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. Signed-off-by: Jacopo Mondi --- 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 #include #include +#include +#include #include @@ -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 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(factory->create(entity)); + } + + LOG(CameraSensor, Info) << "Unsupported sensor '" << entity->name() + << "': using generic camera sensor"; + return std::make_unique(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 +#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, 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 sensor_; private: std::vector> 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 sensor_; unsigned int frame_; std::vector 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(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(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 #include #include +#include #include #include @@ -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 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 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 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 enumerator_; std::shared_ptr media_; - CameraSensor *sensor_; + std::unique_ptr sensor_; V4L2Subdevice *debayer_; V4L2VideoDevice *capture_; std::vector> buffers_;