From patchwork Tue Jan 15 15:18:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 233 Return-Path: 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 19E2660C78 for ; Tue, 15 Jan 2019 16:18:53 +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 84BDA55C for ; Tue, 15 Jan 2019 16:18:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547565532; bh=kdznshboMzFNLLFQJ+OaNhbXtZBIq03ilNd5xxWeZUk=; h=From:To:Subject:Date:In-Reply-To:References:From; b=AiX1vnmpEwKCvJthZ+TTHUC7/g8dQD0bzW2FHynqAAPDSXsROIkuSK58blox3hgul 1eYgBNSwcQJ/corHu6A4mJ1RkWX8vv+J1CP6SXiTCSbwh/zI3E54dzicHBF/xqshpY W3nyIkM6nhYP/Cpr+ZZQCmotFfO3C/6xtdz/0MJc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 15 Jan 2019 17:18:42 +0200 Message-Id: <20190115151849.1547-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190115151849.1547-1-laurent.pinchart@ideasonboard.com> References: <20190115151849.1547-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/8] 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: Tue, 15 Jan 2019 15:18:53 -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 Reviewed-by: Niklas Söderlund --- Changes since v1: - ASSERT() factory name uniqueness --- src/libcamera/camera_manager.cpp | 22 +++--- src/libcamera/include/pipeline_handler.h | 29 ++++---- src/libcamera/pipeline_handler.cpp | 95 ++++++++---------------- 3 files changed, 57 insertions(+), 89 deletions(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index be327f5d5638..91ef6753f405 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -74,20 +74,24 @@ 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; + } + + LOG(Debug) << "Pipeline handler \"" << factory->name() + << "\" matched"; + 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 1daada8653e2..08e3291e741a 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,78 +112,29 @@ 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; - } - - LOG(Debug) << "Registered pipeline handler \"" << name << "\""; - 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) +void PipelineHandlerFactory::registerType(PipelineHandlerFactory *factory) { - 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(); + std::vector &factories = handlers(); - if (pipe->match(enumerator)) { - LOG(Debug) << "Pipeline handler \"" << name << "\" matched"; - 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() -{ - std::map &factories = registry(); - std::vector handlers; + for (PipelineHandlerFactory *f : factories) + ASSERT(factory->name() != f->name()); - for (auto const &handler : factories) - handlers.push_back(handler.first); + factories.push_back(factory); - return handlers; + LOG(Debug) << "Registered pipeline handler \"" << factory->name() << "\""; } /** @@ -180,9 +145,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; }