From patchwork Tue Oct 22 14:53:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 21737 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 C665EBD160 for ; Tue, 22 Oct 2024 14:53:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 73EFB6539A; Tue, 22 Oct 2024 16:53:32 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="K5f/QOsm"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BB5365388 for ; Tue, 22 Oct 2024 16:53:30 +0200 (CEST) Received: from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E36AA47; Tue, 22 Oct 2024 16:51:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1729608703; bh=5N91qP8czPcDe1wMoL+UhnEI7D2pyPXPm4wtarLxn0A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=K5f/QOsmmXF3JpgXX3qgiIwR66SGh1sfO6BiiKTIbsc3vh+zt4NYSP9Un/7W+cf+z mjEgc/cF5CwTl95V6Et8fm+IeygRyqlCCXnDkjQnzp+czcoqEnLCOxXFvvh/PJv3o9 UJwScPRW+XRlWYs/kSIChPGUXmFFtEQr2zyHPEQw= From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Cc: Jacopo Mondi , Jacopo Mondi , Stefan Klug Subject: [PATCH 1/3] libcamera: camera_sensor: Introduce CameraSensorFactory Date: Tue, 22 Oct 2024 16:53:13 +0200 Message-ID: <20241022145321.25923-2-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.47.0 In-Reply-To: <20241022145321.25923-1-jacopo.mondi@ideasonboard.com> References: <20241022145321.25923-1-jacopo.mondi@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: , 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 Reviewed-by: Kieran Bingham --- 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/mali-c55/mali-c55.cpp | 5 +- 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 +- 12 files changed, 233 insertions(+), 40 deletions(-) diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index a42c15fa37ce..2d6cb697a870 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -38,7 +38,6 @@ enum class Orientation; class CameraSensor : protected Loggable { public: - explicit CameraSensor(const MediaEntity *entity); ~CameraSensor(); int init(); @@ -81,6 +80,7 @@ public: int setTestPatternMode(controls::draft::TestPatternModeEnum mode); protected: + explicit CameraSensor(const MediaEntity *entity); std::string logPrefix() const override; private: @@ -122,4 +122,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 72aa6c75608a..4e66b3368d5a 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; @@ -1057,7 +1056,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 74a5d93f88ae..aa544d7b0303 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/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 45c71c1dd619..4b71e0dad15e 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -141,9 +141,8 @@ int MaliC55CameraData::init() * Register a CameraSensor if we connect to a sensor and create * an entity for the connected CSI-2 receiver. */ - sensor_ = std::make_unique(entity_); - ret = sensor_->init(); - if (ret) + sensor_ = CameraSensorFactoryBase::create(entity_); + if (!sensor_) return ret; const MediaPad *sourcePad = entity_->getPadByIndex(0); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index d8d1d65fbbf9..95c25e976015 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1122,10 +1122,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 3041fd1ed9fd..4f56bd33df05 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -774,13 +774,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 3ddce71d3757..67f583b8a22c 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -388,8 +388,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 @@ -480,12 +478,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 2165bae890cb..2e474133439c 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -532,10 +532,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 1b224f1989fe..025c9eefdd12 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -1202,4 +1203,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 1d402c43355b..869c788965da 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 b5871ce69e18..7c9003ec4c4d 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_;