From patchwork Fri Mar 1 21:21:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19610 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 83811BD160 for ; Fri, 1 Mar 2024 21:21:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 44CB762C87; Fri, 1 Mar 2024 22:21:49 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NH99IXGD"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C23C62964 for ; Fri, 1 Mar 2024 22:21:47 +0100 (CET) Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CC13FD04; Fri, 1 Mar 2024 22:21:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1709328093; bh=GCqgbrGku0bAvnN7Dr08jbBMDRPurnEkme7R64HAgyQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NH99IXGD+e084Y9r6IDk7g1DPuHmzIyGPYoL5uF5QQ1Aq/f58bCr3tP21H6Slbv1E koC9HMoFQcdiu/4MokX3ZLCFlhaYQmH61s2rOp2ugax+0fBTCBGr0XaWifpemDRWPW X6dAPt3vjvFFiTSoIqqyqH1aklQ0G67S2mjKfhdY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH/RFC 19/32] libcamera: camera_sensor: Introduce CameraSensorFactory Date: Fri, 1 Mar 2024 23:21:08 +0200 Message-ID: <20240301212121.9072-20-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240301212121.9072-1-laurent.pinchart@ideasonboard.com> References: <20240301212121.9072-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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: , Cc: Sakari Ailus Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Jacopo Mondi 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 Signed-off-by: Laurent Pinchart Reviewed-by: Stefan Klug --- 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/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 +- 11 files changed, 231 insertions(+), 37 deletions(-) diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index d05f48ebeebe..577af779cd6e 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -39,7 +39,6 @@ enum class Orientation; class CameraSensor : protected Loggable { public: - explicit CameraSensor(const MediaEntity *entity); ~CameraSensor(); int init(); @@ -82,6 +81,7 @@ public: int setTestPatternMode(controls::draft::TestPatternModeEnum mode); protected: + explicit CameraSensor(const MediaEntity *entity); std::string logPrefix() const override; private: @@ -123,4 +123,50 @@ private: std::unique_ptr focusLens_; }; +class CameraSensorFactoryBase +{ +public: + CameraSensorFactoryBase(); + virtual ~CameraSensorFactoryBase() = default; + + static std::unique_ptr create(MediaEntity *entity); + +private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorFactoryBase) + + static std::vector &factories(); + + static void registerFactory(CameraSensorFactoryBase *factory); + + virtual bool match(const MediaEntity *entity) const = 0; + + virtual std::unique_ptr + createInstance(MediaEntity *entity) const = 0; +}; + +template +class CameraSensorFactory final : public CameraSensorFactoryBase +{ +public: + CameraSensorFactory() + : CameraSensorFactoryBase() + { + } + +private: + bool match(const MediaEntity *entity) const override + { + return _CameraSensor::match(entity); + } + + std::unique_ptr + createInstance(MediaEntity *entity) const override + { + return _CameraSensor::create(entity); + } +}; + +#define REGISTER_CAMERA_SENSOR(sensor) \ + static CameraSensorFactory 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 a3782aea2ba9..a00567c6873c 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; @@ -1063,7 +1062,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) std::unique_ptr data = std::make_unique(this); - data->sensor_ = std::make_unique(sensor); + data->sensor_ = CameraSensorFactoryBase::create(sensor); data->csis_ = std::make_unique(csi); data->xbarSink_ = sink; diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 43c816baf6ef..e09583ea418f 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(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/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index abb21968413a..1a3e7938fa91 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1109,10 +1109,9 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::make_unique(this, &mainPath_, hasSelfPath_ ? &selfPath_ : nullptr); - data->sensor_ = std::make_unique(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 7e420b3f90a4..d662c8f12145 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -772,13 +772,10 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr &camera CameraData *data = cameraData.get(); int ret; - data->sensor_ = std::make_unique(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 01f2a97798ba..0bd021388c8f 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -371,8 +371,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 @@ -463,12 +461,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, } /* Finally also remember the sensor. */ - sensor_ = std::make_unique(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 5e66ee1d26c1..ae0ca21907ec 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -510,10 +510,9 @@ int VimcCameraData::init() return ret; /* Create and open the camera sensor, debayer, scaler and video device. */ - sensor_ = std::make_unique(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 5c4f35324055..a35683eb4b58 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -1204,4 +1205,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 create(MediaEntity *entity) + { + std::unique_ptr sensor = + std::make_unique(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 CameraSensorFactoryBase::create(MediaEntity *entity) +{ + const std::vector &factories = + CameraSensorFactoryBase::factories(); + + for (const CameraSensorFactoryBase *factory : factories) { + if (!factory->match(entity)) + continue; + + std::unique_ptr 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::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 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 &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 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 9503d7753fb9..acd3a1241ae1 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 enumerator_; std::shared_ptr media_; - CameraSensor *sensor_; + std::unique_ptr 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 d2de1a6de29f..f06db72a9df5 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 enumerator_; std::shared_ptr media_; - libcamera::CameraSensor *sensor_; + std::unique_ptr sensor_; libcamera::V4L2Subdevice *debayer_; libcamera::V4L2VideoDevice *capture_; std::vector> buffers_;