From patchwork Sat Jan 12 14:46:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 217 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E573C60C69 for ; Sat, 12 Jan 2019 15:46:28 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B9B053E for ; Sat, 12 Jan 2019 15:46:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547304388; bh=EQgaFPrIbBk6n01Rpv0UqLSnRMpYKUAv1clnUFUxKp4=; h=From:To:Subject:Date:In-Reply-To:References:From; b=vuTdTBWVNARvg6HtT+74xV+k/27l15JCZdZxQ/9U38Vfu6mREdI36k/oTL0jxteYI V1xIvjTu4LasyECg73CK4ntfU7I+vsLwwQC4xD+sWsqXpiU60wj9lzK2YJbaQ2rHRk 61ycH/Zz6tFxFiWPtVpIe/XFk+MVgVHd9JXJSuMI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Jan 2019 16:46:17 +0200 Message-Id: <20190112144619.5816-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190112144619.5816-1-laurent.pinchart@ideasonboard.com> References: <20190112144619.5816-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/3] libcamera: pipeline_handler: Don't index factories by name X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Jan 2019 14:46:29 -0000 Pipeline handler factories are register in a map indexed by their name, and the list of names is used to expose the factories and look them up. This is unnecessary cumbersome, we can instead store factories in a vector and expose it directly. The pipeline factory users will still have access to the factory names through the factory name() function. The PipelineHandlerFactory::create() method becomes so simple that it can be inlined in its single caller, removing the unneeded usage of the DeviceEnumerator in the factory. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/libcamera/camera_manager.cpp | 20 +++--- src/libcamera/include/pipeline_handler.h | 29 ++++---- src/libcamera/pipeline_handler.cpp | 91 +++++++----------------- 3 files changed, 52 insertions(+), 88 deletions(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index be327f5d5638..e4072529fdc4 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -74,20 +74,22 @@ int CameraManager::start() * file and only fallback on all handlers if there is no * configuration file. */ - std::vector handlers = PipelineHandlerFactory::handlers(); - - for (std::string const &handler : handlers) { - PipelineHandler *pipe; + std::vector &handlers = PipelineHandlerFactory::handlers(); + for (PipelineHandlerFactory *factory : handlers) { /* * Try each pipeline handler until it exhaust * all pipelines it can provide. */ - do { - pipe = PipelineHandlerFactory::create(handler, enumerator_); - if (pipe) - pipes_.push_back(pipe); - } while (pipe); + while (1) { + PipelineHandler *pipe = factory->create(); + if (!pipe->match(enumerator_)) { + delete pipe; + break; + } + + pipes_.push_back(pipe); + } } /* TODO: register hot-plug callback here */ diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index fdf8b8db2e0a..764dde9ccc65 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -31,29 +31,28 @@ public: class PipelineHandlerFactory { public: + PipelineHandlerFactory(const char *name); virtual ~PipelineHandlerFactory() { }; virtual PipelineHandler *create() = 0; - static void registerType(const std::string &name, PipelineHandlerFactory *factory); - static PipelineHandler *create(const std::string &name, DeviceEnumerator *enumerator); - static std::vector handlers(); + const std::string &name() const { return name_; } + + static void registerType(PipelineHandlerFactory *factory); + static std::vector &handlers(); private: - static std::map ®istry(); + std::string name_; }; -#define REGISTER_PIPELINE_HANDLER(handler) \ -class handler##Factory : public PipelineHandlerFactory { \ -public: \ - handler##Factory() \ - { \ - PipelineHandlerFactory::registerType(#handler, this); \ - } \ - virtual PipelineHandler *create() { \ - return new handler(); \ - } \ -}; \ +#define REGISTER_PIPELINE_HANDLER(handler) \ +class handler##Factory : public PipelineHandlerFactory { \ +public: \ + handler##Factory() : PipelineHandlerFactory(#handler) { } \ + PipelineHandler *create() final { \ + return new handler(); \ + } \ +}; \ static handler##Factory global_##handler##Factory; } /* namespace libcamera */ diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index ee7694879848..005a7619927f 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -5,7 +5,6 @@ * pipeline_handler.cpp - Pipeline handler infrastructure */ -#include "device_enumerator.h" #include "log.h" #include "pipeline_handler.h" @@ -40,7 +39,7 @@ namespace libcamera { * system * * This function is the main entry point of the pipeline handler. It is called - * by the device enumerator with the \a enumerator passed as an argument. It + * by the camera manager with the \a enumerator passed as an argument. It * shall acquire from the \a enumerator all the media devices it needs for a * single pipeline and create one or multiple Camera instances. * @@ -88,6 +87,21 @@ namespace libcamera { * static list of factories. */ +/** + * \brief Construct a pipeline handler factory + * \param[in] name Name of the pipeline handler class + * + * Creating an instance of the factory registers is with the global list of + * factories, accessible through the handlers() function. + * + * The factory \a name is used for debug purpose and shall be unique. + */ +PipelineHandlerFactory::PipelineHandlerFactory(const char *name) + : name_(name) +{ + registerType(this); +} + /** * \fn PipelineHandlerFactory::create() * \brief Create an instance of the PipelineHandler corresponding to the factory @@ -98,75 +112,24 @@ namespace libcamera { * subclass corresponding to the factory */ +/** + * \fn PipelineHandlerFactory::name() + * \brief Retrieve the factory name + * \return The factory name + */ + /** * \brief Add a pipeline handler class to the registry - * \param[in] name Name of the pipeline handler class * \param[in] factory Factory to use to construct the pipeline handler * * The caller is responsible to guarantee the uniqueness of the pipeline handler * name. */ -void PipelineHandlerFactory::registerType(const std::string &name, - PipelineHandlerFactory *factory) -{ - std::map &factories = registry(); - - if (factories.count(name)) { - LOG(Error) << "Registering '" << name << "' pipeline twice"; - return; - } - - factories[name] = factory; -} - -/** - * \brief Create an instance of a pipeline handler if it matches media devices - * present in the system - * \param[in] name Name of the pipeline handler to instantiate - * \param[in] enumerator Device enumerator to search for a match for the handler - * - * This function matches the media devices required by pipeline \a name against - * the devices enumerated by \a enumerator. - * - * \return the newly created pipeline handler instance if a match was found, or - * nullptr otherwise - */ -PipelineHandler *PipelineHandlerFactory::create(const std::string &name, - DeviceEnumerator *enumerator) -{ - std::map &factories = registry(); - - auto it = factories.find(name); - if (it == factories.end()) { - LOG(Error) << "Trying to create non-existing pipeline handler " - << name; - return nullptr; - } - - PipelineHandler *pipe = it->second->create(); - - if (pipe->match(enumerator)) - return pipe; - - delete pipe; - return nullptr; -} - -/** - * \brief Retrieve the names of all pipeline handlers registered with the - * factory - * - * \return a list of all registered pipeline handler names - */ -std::vector PipelineHandlerFactory::handlers() +void PipelineHandlerFactory::registerType(PipelineHandlerFactory *factory) { - std::map &factories = registry(); - std::vector handlers; - - for (auto const &handler : factories) - handlers.push_back(handler.first); + std::vector &factories = handlers(); - return handlers; + factories.push_back(factory); } /** @@ -177,9 +140,9 @@ std::vector PipelineHandlerFactory::handlers() * * \return the list of pipeline handler factories */ -std::map &PipelineHandlerFactory::registry() +std::vector &PipelineHandlerFactory::handlers() { - static std::map factories; + static std::vector factories; return factories; } From patchwork Sat Jan 12 14:46:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 218 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 80B4860B2E for ; Sat, 12 Jan 2019 15:46:29 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C1676513 for ; Sat, 12 Jan 2019 15:46:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547304388; bh=w4pW37E5+lvaZ7774b5bFyHQMtHNPdOufp57opoRR7U=; h=From:To:Subject:Date:In-Reply-To:References:From; b=SLAOUhwN1Lck7o1+y69MCW2O03qrhQkBuN2Y4KCV4+0P6wECnsLR3d7EuIKFzyD5Z EBbFDIWKtH5NlJ6ZCbxJwnwtDL4eeWhE79Vs3vE9M1bhauWxtDyk8ZMihyKQXy69Zc N9kKY5zUUlTavl034U8d2Qr23uG80yuu/m4kKX5o= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Jan 2019 16:46:18 +0200 Message-Id: <20190112144619.5816-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190112144619.5816-1-laurent.pinchart@ideasonboard.com> References: <20190112144619.5816-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/3] libcamera: camera_manager: Improve class documentation X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Jan 2019 14:46:29 -0000 Move documentation from the \file directive to the CameraManager class, as it documents the class, not the file. Improve the documentation to provide a brief overview of how the camera manager operates, and fix a few typos and inconsistencies. The documentation mentions hotplug even though it isn't implement yet, as this is a planned feature. More improvements are needed for the documentation of the CameraManager member functions, and will be added as part of the API improvements in the near future. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/libcamera/camera_manager.cpp | 56 +++++++++++++++++++------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index e4072529fdc4..d91ca72a3802 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -15,30 +15,42 @@ /** * \file camera_manager.h - * \brief Manage all cameras handled by libcamera - * - * The responsibility of the camera manager is to control the lifetime - * management of objects provided by libcamera. - * - * When a user wish to interact with libcamera it creates and starts a - * CameraManager object. Once confirmed the camera manager is running - * the application can list all cameras detected by the library, get - * one or more of the cameras and interact with them. - * - * When the user is done with the camera it should be returned to the - * camera manager. Once all cameras are returned to the camera manager - * the user is free to stop the manager. - * - * \todo Add ability to add and remove media devices based on - * hot-(un)plug events coming from the device enumerator. - * - * \todo Add interface to register a notification callback to the user - * to be able to inform it new cameras have been hot-plugged or - * cameras have been removed due to hot-unplug. + * \brief The camera manager */ namespace libcamera { +/** + * \class CameraManager + * \brief Provide access and manage all cameras in the system + * + * The camera manager is the entry point to libcamera. Ii enumerates devices, + * associates them with pipeline managers, and provides access to the cameras + * in the system to applications. The manager owns all Camera objects and + * handles hot-plugging and hot-unplugging to manage the lifetime of cameras. + * + * To interact with libcamera, an application retrieves the camera manager + * instance with CameraManager::instance(). The manager is initially stopped, + * and shall be configured before being started. In particular a custom event + * dispatcher shall be installed if needed with + * CameraManager::setEventDispatcher(). + * + * Once the camera manager is configured, it shall be started with start(). + * This will enumerate all the cameras present in the system, which can then be + * listed with list() and retrieved with get(). + * + * Cameras are reference-counted, and shall be returned to the camera manager + * with Camera::put() after being used. Once all cameras have been returned to + * the manager, it can be stopped with stop(). + * + * \todo Add ability to add and remove media devices based on hot-(un)plug + * events coming from the device enumerator. + * + * \todo Add interface to register a notification callback to the user to be + * able to inform it new cameras have been hot-plugged or cameras have been + * removed due to hot-unplug. + */ + CameraManager::CameraManager() : enumerator_(nullptr), dispatcher_(nullptr) { @@ -57,7 +69,7 @@ CameraManager::~CameraManager() * interact with cameras in the system until either the camera manager * is stopped or the camera is unplugged from the system. * - * \return true on successful start false otherwise + * \return 0 on successful start, or a negative error code otherwise */ int CameraManager::start() { @@ -100,7 +112,7 @@ int CameraManager::start() /** * \brief Stop the camera manager * - * Before stopping the camera manger the caller is responsible for making + * Before stopping the camera manager the caller is responsible for making * sure all cameras provided by the manager are returned to the manager. * * After the manager has been stopped no resource provided by the camera From patchwork Sat Jan 12 14:46:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 219 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D46E360B2E for ; Sat, 12 Jan 2019 15:46:29 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 77A5953E for ; Sat, 12 Jan 2019 15:46:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547304389; bh=5R9epvHQ83x3CW2ZQqwXMDsM7EqXMPlUbjl7DXCxCPo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=eOQyNdeifP2NMpegIl8M/qDfW9vdwXgH2zaQo1OHYgytXoJnnSIPamRcZXGoq05aC pf7Fn9RP14IsPbaDde96+nLEPSOk0hq7eNnaYVnxq/gqhXmGSSNrsVQP354aL8Rbat Yk8i8+r/lf+jGh1IQIxHEyA2MQE9uBKvVB6ExtcE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Jan 2019 16:46:19 +0200 Message-Id: <20190112144619.5816-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190112144619.5816-1-laurent.pinchart@ideasonboard.com> References: <20190112144619.5816-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/3] Documentation: Exclude pipeline handlers directory X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Jan 2019 14:46:30 -0000 The pipeline handlers don't define APIs, neither public not internal. There is thus no need to generate Doxygen documentation from thoses classes. Add them to the EXCLUDE files pattern. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- Documentation/Doxyfile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in index 5c0d87baac9c..dd74b7295d50 100644 --- a/Documentation/Doxyfile.in +++ b/Documentation/Doxyfile.in @@ -833,7 +833,7 @@ RECURSIVE = YES # Note that relative paths are relative to the directory from which doxygen is # run. -EXCLUDE = +EXCLUDE = ../src/libcamera/pipeline/ # The EXCLUDE_SYMLINKS tag can be used to select whether or not files or # directories that are symbolic links (a Unix file system feature) are excluded