[{"id":199,"web_url":"https://patchwork.libcamera.org/comment/199/","msgid":"<20190103214224.GK22790@bigcity.dyn.berto.se>","date":"2019-01-03T21:42:24","subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: pipeline_handler:\n\tImprove documentation","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-01-03 03:31:10 +0200, Laurent Pinchart wrote:\n> Miscellaneous documentation improvements for the PipelineHandler and\n> related classes.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/pipeline_handler.cpp | 125 ++++++++++++++++++++---------\n>  1 file changed, 88 insertions(+), 37 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index a737f222b465..093821d4c471 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -11,48 +11,98 @@\n>  \n>  /**\n>   * \\file pipeline_handler.h\n> - * \\brief Create pipelines and cameras from one or more media devices\n> + * \\brief Create pipelines and cameras from a set of media devices\n>   *\n>   * Each pipeline supported by libcamera needs to be backed by a pipeline\n> - * handler implementation which describes the one or many media devices\n> - * needed for a pipeline to function properly.\n> + * handler implementation that operate on a set of media devices. The pipeline\n> + * handler is responsible for matching the media devices it requires with the\n> + * devices present in the system, and once all those devices can be acquired,\n> + * create corresponding Camera instances.\n>   *\n> - * The pipeline handler is responsible for providing a description of the\n> - * media devices it requires to operate. Once all media devices can be\n> - * provided the pipeline handler can acquire them and create camera\n> - * devices that utilize the acquired media devices.\n> - *\n> - * To make it a bit less bit complicated to write pipe line handlers a\n> - * macro REGISTER_PIPELINE_HANDLER() is provided which allows a pipeline\n> - * handler implementation to register itself with the library with ease.\n> + * Every subclass of PipelineHandler shall be registered with libcamera using\n> + * the REGISTER_PIPELINE_HANDLER() macro.\n>   */\n>  \n>  namespace libcamera {\n>  \n>  /**\n>   * \\class PipelineHandler\n> - * \\brief Find a set of media devices and provide cameras\n> + * \\brief Create and manage cameras based on a set of media devices\n>   *\n> - * The responsibility of a PipelineHandler is to describe all media\n> - * devices it would need in order to provide cameras to the system.\n> + * The PipelineHandler matches the media devices provided by a DeviceEnumerator\n> + * with the pipelines it supports and creates corresponding Camera devices.\n> + */\n> +\n> +/**\n> + * \\fn PipelineHandler::match(DeviceEnumerator *enumerator)\n> + * \\brief Match media devices and create camera instances\n> + *\n> + * This function is the main entry point of the pipeline handler. It is called\n> + * by the device enumerator with the enumerator passed as an argument. It shall\n> + * acquire from the enumerator all the media devices it needs for a single\n> + * pipeline and create one or multiple Camera instances.\n> + *\n> + * If all media devices needed by the pipeline handler are found, they must all\n> + * be acquired by a call to MediaDevice::acquire(). This function shall then\n> + * create the corresponding Camera instances, store them internally, and return\n> + * true. Otherwise it shall not acquire any media device (or shall release all\n> + * the media devices is has acquired by calling MediaDevice::release()) and\n> + * return false.\n> + *\n> + * If multiple instances of a pipeline are available in the system, the\n> + * PipelineHandler class will be instanciated once per instance, and its match()\n> + * function called for every instance. Each call shall acquire media devices for\n> + * one pipeline instance, until all compatible media devices are exhausted.\n> + *\n> + * If this function returns true, a new instance of the pipeline handler will\n> + * be created and its match() function called,\n> + *\n> + * \\return true if media devices have been acquired and camera instances\n> + * created, or false otherwise\n> + */\n> +\n> +/**\n> + * \\fn PipelineHandler::count()\n> + * \\brief Retrieve the number of cameras handled by this pipeline handler\n> + * \\return the number of cameras that were created by the match() function\n> + */\n> +\n> +/**\n> + * \\fn PipelineHandler::camera(unsigned int id)\n> + * \\brief Retrieve one of the cameras handled by this pipeline handler\n> + * \\param[in] id the camera index\n> + * \\return a pointer to the Camera identified by \\a id\n>   */\n>  \n>  /**\n>   * \\class PipelineHandlerFactory\n> - * \\brief Keep a registry and create instances of available pipeline handlers\n> - *\n> - * The responsibility of the PipelineHandlerFactory is to keep a list\n> - * of all pipelines in the system. Each pipeline handler should register\n> - * it self with the factory using the REGISTER_PIPELINE_HANDLER() macro.\n> + * \\brief Registration of PipelineHandler classes and creation of instances\n> + *\n> + * To facilitate discovery and instantiation of PipelineHandler classes, the\n> + * PipelineHandlerFactory class maintains a registry of pipeline handler\n> + * classes. Each PipelineHandler subclass shall register itself using the\n> + * REGISTER_PIPELINE_HANDLER() macro, which will create a corresponding\n> + * instance of a PipelineHandlerFactory subclass and register it with the\n> + * static list of factories.\n>   */\n>  \n>  /**\n> - * \\brief Add a pipeline handler to the global list\n> + * \\fn PipelineHandlerFactory::create()\n> + * \\brief Create an instance of the PipelineHandler corresponding to the factory\n>   *\n> - * \\param[in] name Name of the pipeline handler to add\n> - * \\param[in] factory Factory to use to construct the pipeline\n> + * This virtual function is implemented by the REGISTER_PIPELINE_HANDLER() macro.\n> + *\n> + * \\return a pointer to a newly constructed instance of the PipelineHandler\n> + * subclass corresponding to the factory\n> + */\n> +\n> +/**\n> + * \\brief Add a pipeline handler class to the registry\n> + * \\param[in] name Name of the pipeline handler class\n> + * \\param[in] factory Factory to use to construct the pipeline handler\n>   *\n> - * The caller is responsible to guarantee the uniqueness of the pipeline name.\n> + * The caller is responsible to guarantee the uniqueness of the pipeline handler\n> + * name.\n>   */\n>  void PipelineHandlerFactory::registerType(const std::string &name,\n>  \t\t\t\t\t  PipelineHandlerFactory *factory)\n> @@ -68,15 +118,16 @@ void PipelineHandlerFactory::registerType(const std::string &name,\n>  }\n>  \n>  /**\n> - * \\brief Create a new pipeline handler and try to match the media devices it requires\n> - *\n> - * \\param[in] name Name of the pipeline handler to try\n> + * \\brief Create an instance of a pipeline handler if it matches media devices\n> + * present in the system\n> + * \\param[in] name Name of the pipeline handler to instantiate\n>   * \\param[in] enumerator Device enumerator to search for a match for the handler\n>   *\n> - * Try to match the media devices pipeline \\a name requires against the ones\n> - * registered in \\a enumerator.\n> + * This function matches the media devices required by pipeline \\a name against\n> + * the devices enumerated by \\a enumerator.\n>   *\n> - * \\return Pipeline handler if a match was found, nullptr otherwise\n> + * \\return the newly created pipeline handler instance if a match was found, or\n> + * nullptr otherwise\n>   */\n>  PipelineHandler *PipelineHandlerFactory::create(const std::string &name,\n>  \t\t\t\t\t\tDeviceEnumerator *enumerator)\n> @@ -100,9 +151,10 @@ PipelineHandler *PipelineHandlerFactory::create(const std::string &name,\n>  }\n>  \n>  /**\n> - * \\brief List all names of piepline handlers from the global list\n> + * \\brief Retrieve the names of all pipeline handlers registered with the\n> + * factory\n>   *\n> - * \\return List of registerd pipeline handler names\n> + * \\return a list of all registered pipeline handler names\n>   */\n>  std::vector<std::string> PipelineHandlerFactory::handlers()\n>  {\n> @@ -116,12 +168,12 @@ std::vector<std::string> PipelineHandlerFactory::handlers()\n>  }\n>  \n>  /**\n> - * \\brief Static global list of pipeline handlers\n> + * \\brief Retrieve the list of all pipeline handler factories\n>   *\n>   * The static factories map is defined inside the function to ensures it gets\n>   * initialized on first use, without any dependency on link order.\n>   *\n> - * \\return Global list of pipeline handlers\n> + * \\return the list of pipeline handler factories\n>   */\n>  std::map<std::string, PipelineHandlerFactory *> &PipelineHandlerFactory::registry()\n>  {\n> @@ -131,12 +183,11 @@ std::map<std::string, PipelineHandlerFactory *> &PipelineHandlerFactory::registr\n>  \n>  /**\n>   * \\def REGISTER_PIPELINE_HANDLER\n> - * \\brief Register a pipeline handler with the global list\n> - *\n> + * \\brief Register a pipeline handler with the pipeline handler factory\n>   * \\param[in] handler Class name of PipelineHandler derived class to register\n>   *\n> - * Register a specific pipeline handler with the global list and make it\n> - * available to try and match devices.\n> + * Register a PipelineHandler subclass with the factory and make it available to\n> + * try and match devices.\n>   */\n>  \n>  } /* namespace libcamera */\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD27860B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  3 Jan 2019 22:42:26 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id k19-v6so30889060lji.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 03 Jan 2019 13:42:26 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tb81-v6sm11949935ljb.7.2019.01.03.13.42.25\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 03 Jan 2019 13:42:25 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=iteFsa+x92l23VjtHARN0nFjnMLbh6VCP0ecoEFPHRE=;\n\tb=y8TKO9/Zl2G8L49073nTRP7z8LJb3B2J65CAtolz/jRB4Z1uRsdUJEIi8mQmWoELIu\n\tAND/3r3cnHIld6rt23ImhP5DQrbe/WBUllwU3xM0sRr5eMQgwvxH1XyUTFkwOfrD2y8f\n\t+bQO73H7gZYRs2WnNEAuSKm3xbyrjEGhen1/WtVrNvpS5cAWWPLDzMnqfV1O3Wy4mMS8\n\tJan5o0fCcAeeXuBH/+Sbpi+WyupUoXWWsxmnGilYD13z9v36gKPGFR4yXzLpUS1JBqcn\n\tG9qo6bFKXYDEc8I6FuDtGD/+HPK3ZSZU+OIoVffvxxR3e72/a9OTewVVpitnA/eAOlZn\n\th9wA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=iteFsa+x92l23VjtHARN0nFjnMLbh6VCP0ecoEFPHRE=;\n\tb=gshxR/e9awrct6KHHOGXk6HL8lE1m85VJpMonQFp09jYiJoBBjRt8lZmnNGetclwoW\n\tzKU13yfih9lONjLS+nIoMbs0aQxmcAe93ofr2lTtfLrWN5C5PmmOikkC+zGEl9kw72Zp\n\txznrt6ZX3o4/1ogxUH70/zUqRhPodZprMRmiWGvbmirhFp+09CFJT24OH82DxeRe/Df5\n\tL+ax8CQAPxEIBmayNviLOadpE6q//cgwOMzcK6qV2MA22p3ele2Q4T9lKfLYFt1W/1Wz\n\t1xrg3ffiCooC2Kyu/T8xOyIXSk+fnsTjH2ksQyom+aSCtT7Cd4J+STgvWdlL7ISA/+Uu\n\tsUfQ==","X-Gm-Message-State":"AJcUukdU+sx5tUv0+0RSdAO90qtx2LgTjmKqk96ev7SDuLZUs+oFeEgT\n\tvsJ7Z1ASxwVjWDVzIUbM5g1JDDzqS5g=","X-Google-Smtp-Source":"ALg8bN5HsX/o5QEDIrLOpAxg4JdYbU4HM/wbj2LU/AKMhmIxdpUsKfkXRqj+RD65xBkTJXnFr39rww==","X-Received":"by 2002:a2e:5054:: with SMTP id\n\tv20-v6mr27546807ljd.45.1546551745890; \n\tThu, 03 Jan 2019 13:42:25 -0800 (PST)","Date":"Thu, 3 Jan 2019 22:42:24 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190103214224.GK22790@bigcity.dyn.berto.se>","References":"<20190103013110.6849-1-laurent.pinchart@ideasonboard.com>\n\t<20190103013110.6849-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190103013110.6849-9-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: pipeline_handler:\n\tImprove documentation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 03 Jan 2019 21:42:27 -0000"}}]