[{"id":414,"web_url":"https://patchwork.libcamera.org/comment/414/","msgid":"<20190118160604.GU6484@bigcity.dyn.berto.se>","date":"2019-01-18T16:06:04","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","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-18 01:59:15 +0200, Laurent Pinchart wrote:\n> Cameras are listed through a double indirection, first iterating over\n> all available pipeline handlers, and then listing the cameras they each\n> support. To simplify the API make the pipeline handlers register the\n> cameras with the camera manager directly, which lets the camera manager\n> easily expose the list of all available cameras.\n> \n> The PipelineHandler API gets simplified as the handlers don't need to\n> expose the list of cameras they have created.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/camera_manager.h       |  5 ++-\n>  src/libcamera/camera_manager.cpp         | 44 +++++++++++-------------\n>  src/libcamera/include/pipeline_handler.h |  6 ++--\n>  src/libcamera/pipeline/vimc.cpp          | 30 ++++------------\n>  src/libcamera/pipeline_handler.cpp       | 21 +++--------\n>  test/list-cameras.cpp                    |  5 +--\n>  6 files changed, 40 insertions(+), 71 deletions(-)\n> \n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index b82a8ce95b9f..4b941fd9183b 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -24,9 +24,11 @@ public:\n>  \tint start();\n>  \tvoid stop();\n>  \n> -\tstd::vector<std::string> list() const;\n> +\tconst std::vector<Camera *> &cameras() const { return cameras_; }\n>  \tCamera *get(const std::string &name);\n>  \n> +\tvoid addCamera(Camera *camera);\n> +\n>  \tstatic CameraManager *instance();\n>  \n>  \tvoid setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);\n> @@ -40,6 +42,7 @@ private:\n>  \n>  \tstd::unique_ptr<DeviceEnumerator> enumerator_;\n>  \tstd::vector<PipelineHandler *> pipes_;\n> +\tstd::vector<Camera *> cameras_;\n>  \n>  \tstd::unique_ptr<EventDispatcher> dispatcher_;\n>  };\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 9806e399f92b..852e5ed70fe3 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -93,7 +93,7 @@ int CameraManager::start()\n>  \t\t */\n>  \t\twhile (1) {\n>  \t\t\tPipelineHandler *pipe = factory->create();\n> -\t\t\tif (!pipe->match(enumerator_.get())) {\n> +\t\t\tif (!pipe->match(this, enumerator_.get())) {\n>  \t\t\t\tdelete pipe;\n>  \t\t\t\tbreak;\n>  \t\t\t}\n> @@ -132,26 +132,14 @@ void CameraManager::stop()\n>  }\n>  \n>  /**\n> - * \\brief List all detected cameras\n> + * \\fn CameraManager::cameras()\n> + * \\brief Retrieve all available cameras\n>   *\n>   * Before calling this function the caller is responsible for ensuring that\n>   * the camera manger is running.\n>   *\n> - * \\return List of names for all detected cameras\n> + * \\return List of all available cameras\n>   */\n> -std::vector<std::string> CameraManager::list() const\n> -{\n> -\tstd::vector<std::string> list;\n> -\n> -\tfor (PipelineHandler *pipe : pipes_) {\n> -\t\tfor (unsigned int i = 0; i < pipe->count(); i++) {\n> -\t\t\tCamera *cam = pipe->camera(i);\n> -\t\t\tlist.push_back(cam->name());\n> -\t\t}\n> -\t}\n> -\n> -\treturn list;\n> -}\n>  \n>  /**\n>   * \\brief Get a camera based on name\n> @@ -167,19 +155,27 @@ std::vector<std::string> CameraManager::list() const\n>   */\n>  Camera *CameraManager::get(const std::string &name)\n>  {\n> -\tfor (PipelineHandler *pipe : pipes_) {\n> -\t\tfor (unsigned int i = 0; i < pipe->count(); i++) {\n> -\t\t\tCamera *cam = pipe->camera(i);\n> -\t\t\tif (cam->name() == name) {\n> -\t\t\t\tcam->get();\n> -\t\t\t\treturn cam;\n> -\t\t\t}\n> -\t\t}\n> +\tfor (Camera *camera : cameras_) {\n> +\t\tif (camera->name() == name)\n> +\t\t\treturn camera;\n>  \t}\n>  \n>  \treturn nullptr;\n>  }\n>  \n> +/**\n> + * \\brief Add a camera to the camera manager\n> + * \\param[in] camera The camera to be added\n> + *\n> + * This function is called by pipeline handlers to register the cameras they\n> + * handle with the camera manager. Registered cameras are immediately made\n> + * available to the system.\n> + */\n> +void CameraManager::addCamera(Camera *camera)\n> +{\n> +\tcameras_.push_back(camera);\n\nI wonder if we should not add a runtime check here to make sure the \ncamera name is unique before adding it to the vector here? The get() \nfunction retrieves a Camera object by name there might be odd \nconsequences otherwise :-)\n\nWith this addressed,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> +}\n> +\n>  /**\n>   * \\brief Retrieve the camera manager instance\n>   *\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index e976aaa13546..f05f201f7ca8 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -15,6 +15,7 @@\n>  \n>  namespace libcamera {\n>  \n> +class CameraManager;\n>  class DeviceEnumerator;\n>  \n>  class PipelineHandler\n> @@ -22,10 +23,7 @@ class PipelineHandler\n>  public:\n>  \tvirtual ~PipelineHandler() { };\n>  \n> -\tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> -\n> -\tvirtual unsigned int count() = 0;\n> -\tvirtual Camera *camera(unsigned int id) = 0;\n> +\tvirtual bool match(CameraManager *manager, DeviceEnumerator *enumerator) = 0;\n>  };\n>  \n>  class PipelineHandlerFactory\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 720d9c2031c9..8742e0bae9a8 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -6,6 +6,7 @@\n>   */\n>  \n>  #include <libcamera/camera.h>\n> +#include <libcamera/camera_manager.h>\n>  \n>  #include \"device_enumerator.h\"\n>  #include \"media_device.h\"\n> @@ -19,44 +20,24 @@ public:\n>  \tPipeHandlerVimc();\n>  \t~PipeHandlerVimc();\n>  \n> -\tbool match(DeviceEnumerator *enumerator);\n> -\n> -\tunsigned int count();\n> -\tCamera *camera(unsigned int id) final;\n> +\tbool match(CameraManager *manager, DeviceEnumerator *enumerator);\n>  \n>  private:\n>  \tMediaDevice *dev_;\n> -\tCamera *camera_;\n>  };\n>  \n>  PipeHandlerVimc::PipeHandlerVimc()\n> -\t: dev_(nullptr), camera_(nullptr)\n> +\t: dev_(nullptr)\n>  {\n>  }\n>  \n>  PipeHandlerVimc::~PipeHandlerVimc()\n>  {\n> -\tif (camera_)\n> -\t\tcamera_->put();\n> -\n>  \tif (dev_)\n>  \t\tdev_->release();\n>  }\n>  \n> -unsigned int PipeHandlerVimc::count()\n> -{\n> -\treturn 1;\n> -}\n> -\n> -Camera *PipeHandlerVimc::camera(unsigned int id)\n> -{\n> -\tif (id != 0)\n> -\t\treturn nullptr;\n> -\n> -\treturn camera_;\n> -}\n> -\n> -bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> +bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator)\n>  {\n>  \tDeviceMatch dm(\"vimc\");\n>  \n> @@ -83,7 +64,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n>  \t * will be chosen depends on how the Camera\n>  \t * object is modeled.\n>  \t */\n> -\tcamera_ = new Camera(\"Dummy VIMC Camera\");\n> +\tCamera *camera = new Camera(\"Dummy VIMC Camera\");\n> +\tmanager->addCamera(camera);\n>  \n>  \treturn true;\n>  }\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index c19ab94f1774..f2e08a6a7315 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -35,13 +35,15 @@ namespace libcamera {\n>  /**\n>   * \\fn PipelineHandler::match(DeviceEnumerator *enumerator)\n>   * \\brief Match media devices and create camera instances\n> + * \\param manager The camera manager\n>   * \\param enumerator The enumerator providing all media devices found in the\n>   * system\n>   *\n>   * This function is the main entry point of the pipeline handler. It is called\n> - * by the camera manager with the \\a enumerator passed as an argument. It\n> - * shall acquire from the \\a enumerator all the media devices it needs for a\n> - * single pipeline and create one or multiple Camera instances.\n> + * by the camera manager with the \\a manager and \\a enumerator passed as\n> + * arguments. It shall acquire from the \\a enumerator all the media devices it\n> + * needs for a single pipeline, create one or multiple Camera instances and\n> + * register them with the \\a manager.\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> @@ -62,19 +64,6 @@ namespace libcamera {\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 Registration of PipelineHandler classes and creation of instances\n> diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp\n> index e2026c99c5b8..fdbbda0957b2 100644\n> --- a/test/list-cameras.cpp\n> +++ b/test/list-cameras.cpp\n> @@ -7,6 +7,7 @@\n>  \n>  #include <iostream>\n>  \n> +#include <libcamera/camera.h>\n>  #include <libcamera/camera_manager.h>\n>  \n>  #include \"test.h\"\n> @@ -29,8 +30,8 @@ protected:\n>  \t{\n>  \t\tunsigned int count = 0;\n>  \n> -\t\tfor (auto name : cm->list()) {\n> -\t\t\tcout << \"- \" << name << endl;\n> +\t\tfor (Camera *camera : cm->cameras()) {\n> +\t\t\tcout << \"- \" << camera->name() << endl;\n>  \t\t\tcount++;\n>  \t\t}\n>  \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-lf1-x129.google.com (mail-lf1-x129.google.com\n\t[IPv6:2a00:1450:4864:20::129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7038960B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 17:06:06 +0100 (CET)","by mail-lf1-x129.google.com with SMTP id z13so10844470lfe.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 08:06:06 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\to14-v6sm790430lji.70.2019.01.18.08.06.04\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 18 Jan 2019 08:06:04 -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=IVFImodPBKik9TjjUjArdi9SdNpknFCE8sZT6Q402Bw=;\n\tb=pzne7PfSVwiKcPugk8UfXnCSb0/Wnu+5aJxuO32+Eu6TZ9zZ2rZQvV16JWMu04R3D6\n\tBJ8wwmhUASezEn7ShiegAA7nFSPPDq8zfy+IhlaCs6nzmX1Kr6poN7RXMMfYcAnSYrAJ\n\td1MmRjwe5WK41e88fxzgUjKrmrzRTsjthVbm0z/Yl2tZbHSuFmDESbtec44YeY4BALuy\n\taFaWhBR13Tcwae3aV0YU2d5MRsjXxSwoMHvuL3FOkeeKNGUktMymeVxBvx7F0+XFpiLL\n\t3wrj4m7X8fp0gGOLDzEMVDlNzcVotV5HZhlp7CkFnzM+bHD3ZyBGeSjz5IKWAX9Z/xvp\n\tTGOw==","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=IVFImodPBKik9TjjUjArdi9SdNpknFCE8sZT6Q402Bw=;\n\tb=dYn6TSxDEEmTqvfEaJAclWgxTzeG++2weJ8IQuhuzTjM5AtFvMz+RUs4wZbs929/5L\n\tdnjQW82r1svv+vFLVv2TrMpb8nRCpx/6jLmSoX4P2AaQPgRoP5YmN7/5Vba83BCFJZz3\n\tx3UzQ2ejXYcHJKMmEYSt6Xb4uLoyzqIDrKioneKgd4VRrRGsjlPUVupMe3jd3Z1J4tQs\n\tnsUAwHUHHN23cmw4/cXvLnGVem/5YBkU003STrSgOb+qfTz+w80NIiJXKiYiiBADcL9k\n\tanCvgq3MeaoqIsLMPTOZe49uFq/m8uIgzYt8pPJL3fF6wI8Q/lwkzWN1g4/cBI3Rxmaq\n\tHPsA==","X-Gm-Message-State":"AJcUukcQMpnDXyLZQ/6/enXUFFlJLxiEoHVIep2lpFRE4frV8WzaH6SV\n\tcGcyoahexM7bZgcTO3mdhR4HzA==","X-Google-Smtp-Source":"ALg8bN5793g9SQ+910yQCWzZaJAqz+dVmYRTRQoxwZ1p8NcKMw73lAgTtEb97q+NBWLsqp/YZBNMEg==","X-Received":"by 2002:a19:7919:: with SMTP id\n\tu25mr12618751lfc.18.1547827565520; \n\tFri, 18 Jan 2019 08:06:05 -0800 (PST)","Date":"Fri, 18 Jan 2019 17:06:04 +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":"<20190118160604.GU6484@bigcity.dyn.berto.se>","References":"<20190117235916.1906-1-laurent.pinchart@ideasonboard.com>\n\t<20190117235916.1906-4-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":"<20190117235916.1906-4-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","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":"Fri, 18 Jan 2019 16:06:06 -0000"}},{"id":416,"web_url":"https://patchwork.libcamera.org/comment/416/","msgid":"<20190118163010.GJ5275@pendragon.ideasonboard.com>","date":"2019-01-18T16:30:10","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, Jan 18, 2019 at 05:06:04PM +0100, Niklas Söderlund wrote:\n> On 2019-01-18 01:59:15 +0200, Laurent Pinchart wrote:\n> > Cameras are listed through a double indirection, first iterating over\n> > all available pipeline handlers, and then listing the cameras they each\n> > support. To simplify the API make the pipeline handlers register the\n> > cameras with the camera manager directly, which lets the camera manager\n> > easily expose the list of all available cameras.\n> > \n> > The PipelineHandler API gets simplified as the handlers don't need to\n> > expose the list of cameras they have created.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/camera_manager.h       |  5 ++-\n> >  src/libcamera/camera_manager.cpp         | 44 +++++++++++-------------\n> >  src/libcamera/include/pipeline_handler.h |  6 ++--\n> >  src/libcamera/pipeline/vimc.cpp          | 30 ++++------------\n> >  src/libcamera/pipeline_handler.cpp       | 21 +++--------\n> >  test/list-cameras.cpp                    |  5 +--\n> >  6 files changed, 40 insertions(+), 71 deletions(-)\n\n[snip]\n\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 9806e399f92b..852e5ed70fe3 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n\n[snip]\n\n> > +/**\n> > + * \\brief Add a camera to the camera manager\n> > + * \\param[in] camera The camera to be added\n> > + *\n> > + * This function is called by pipeline handlers to register the cameras they\n> > + * handle with the camera manager. Registered cameras are immediately made\n> > + * available to the system.\n> > + */\n> > +void CameraManager::addCamera(Camera *camera)\n> > +{\n> > +\tcameras_.push_back(camera);\n> \n> I wonder if we should not add a runtime check here to make sure the \n> camera name is unique before adding it to the vector here? The get() \n> function retrieves a Camera object by name there might be odd \n> consequences otherwise :-)\n\nGood point. I'll add\n\n        for (Camera *c : cameras_) {\n                if (c->name() == camera->name()) {\n                        LOG(Warning) << \"Registering camera with duplicate name '\"\n                                     << camera->name() << \"'\";\n                        break;\n                }\n        }\n\nOr should it be LOG(Error) ? I don't think it should be a fatal error in\nany case (so no ASSERT) as it doesn't prevent from using the other\ncameras, or even the one with a duplicate name when retrieved directly\nfrom the vector of cameras (introduced in patch 3/4).\n\n> With this addressed,\n> \n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> > +}\n\n[snip]","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 773E560B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 17:30:11 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D186153E;\n\tFri, 18 Jan 2019 17:30:10 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547829011;\n\tbh=5qx7KAehvpYcQiB4DVAU3iDSu2HfAr2yYiSfbnMaXQk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HbUIuK74Jh56+0jfOdJE0KT4iwUxHhN7aW8EiVZIMucCGch0ndRQXNBSLvmgpqvlK\n\t7IEJfNpA6meC8jPGQQARqbqxzBpT5Z6k7uA9nUKMYbtBdHte299103bnZVLSqOoS9n\n\twDAWke4j/OvqIYdgqxwDh67SAqRXn3ONLSIlA3jY=","Date":"Fri, 18 Jan 2019 18:30:10 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190118163010.GJ5275@pendragon.ideasonboard.com>","References":"<20190117235916.1906-1-laurent.pinchart@ideasonboard.com>\n\t<20190117235916.1906-4-laurent.pinchart@ideasonboard.com>\n\t<20190118160604.GU6484@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190118160604.GU6484@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","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":"Fri, 18 Jan 2019 16:30:11 -0000"}},{"id":418,"web_url":"https://patchwork.libcamera.org/comment/418/","msgid":"<20190118164731.GV6484@bigcity.dyn.berto.se>","date":"2019-01-18T16:47:31","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2019-01-18 18:30:10 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Fri, Jan 18, 2019 at 05:06:04PM +0100, Niklas Söderlund wrote:\n> > On 2019-01-18 01:59:15 +0200, Laurent Pinchart wrote:\n> > > Cameras are listed through a double indirection, first iterating over\n> > > all available pipeline handlers, and then listing the cameras they each\n> > > support. To simplify the API make the pipeline handlers register the\n> > > cameras with the camera manager directly, which lets the camera manager\n> > > easily expose the list of all available cameras.\n> > > \n> > > The PipelineHandler API gets simplified as the handlers don't need to\n> > > expose the list of cameras they have created.\n> > > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > >  include/libcamera/camera_manager.h       |  5 ++-\n> > >  src/libcamera/camera_manager.cpp         | 44 +++++++++++-------------\n> > >  src/libcamera/include/pipeline_handler.h |  6 ++--\n> > >  src/libcamera/pipeline/vimc.cpp          | 30 ++++------------\n> > >  src/libcamera/pipeline_handler.cpp       | 21 +++--------\n> > >  test/list-cameras.cpp                    |  5 +--\n> > >  6 files changed, 40 insertions(+), 71 deletions(-)\n> \n> [snip]\n> \n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 9806e399f92b..852e5ed70fe3 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> \n> [snip]\n> \n> > > +/**\n> > > + * \\brief Add a camera to the camera manager\n> > > + * \\param[in] camera The camera to be added\n> > > + *\n> > > + * This function is called by pipeline handlers to register the cameras they\n> > > + * handle with the camera manager. Registered cameras are immediately made\n> > > + * available to the system.\n> > > + */\n> > > +void CameraManager::addCamera(Camera *camera)\n> > > +{\n> > > +\tcameras_.push_back(camera);\n> > \n> > I wonder if we should not add a runtime check here to make sure the \n> > camera name is unique before adding it to the vector here? The get() \n> > function retrieves a Camera object by name there might be odd \n> > consequences otherwise :-)\n> \n> Good point. I'll add\n> \n>         for (Camera *c : cameras_) {\n>                 if (c->name() == camera->name()) {\n>                         LOG(Warning) << \"Registering camera with duplicate name '\"\n>                                      << camera->name() << \"'\";\n>                         break;\n>                 }\n>         }\n> \n> Or should it be LOG(Error) ? I don't think it should be a fatal error in\n> any case (so no ASSERT) as it doesn't prevent from using the other\n> cameras, or even the one with a duplicate name when retrieved directly\n> from the vector of cameras (introduced in patch 3/4).\n\nI think this should be LOG(Error) and that we should not add the Camera \nobject to the list but instead return after LOG().\n\nI don't think it's a good idea to try and function in a state which have \na possibility of unknown behavior when we can in a safe way inform the \nuser that something is wrong and keep the fully functional previous \nstate. That being said, I'm not totally against your solution here and \nif you can think of a good use-case where it's more desirable to do so I \nwould be OK with that.\n\n> \n> > With this addressed,\n> > \n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > \n> > > +}\n> \n> [snip]\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CC6860B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 17:47:33 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id n18-v6so12180455lji.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 08:47:33 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tb25-v6sm815430lji.94.2019.01.18.08.47.31\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 18 Jan 2019 08:47:31 -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=BWoZoRfWPsmaMCC4uoiUTiSnzIhzymRt2WS6j+x/zEI=;\n\tb=CnPbEEOvXBCxoqBbxT9NaVoRWFpOWMG7QMGJB9tZoZbGbljQsRt/rvCt9Vc8lZRCYU\n\t1PjHrh58YZku/ubUtQqmMc2CgB7urOCMItPrw32bZ0aYnu0BGHrDmO7WVO7Ugo/3HYq9\n\ta47uCXpXnbuTOdrYXmt2SGeJ/Vza6FJSSD3DXvy3qyvXpkVmBD0FOgyi4jb3ZN9pG5cr\n\tqILweIAyAYz9Okt39dg731U+T3CX54necxaXc3F0uopHTDf8EjZqbkfKIdGvHYqJz2ZR\n\tp8cl24fZPjPJTCwFUjwE7xJhqzPxtRQR53ATSnNGBU5Hl4l0ByMiv8xQ1qofI5WE9tUB\n\tSnKA==","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=BWoZoRfWPsmaMCC4uoiUTiSnzIhzymRt2WS6j+x/zEI=;\n\tb=hm+YEljZiZvNYj+y4IGIW0lo7Vr1mSUgOiDGGNz/8Gfhu+ISdJqbEGMWIllpsAxuap\n\t+mQrl5y+DaVcca9meX4hEptHryg0rN2ArndTrtetCw7V5iG4MzEnfj5ZsfkW1jS5A6VT\n\t6fA0i2RYmJXTdPDGqsN4xBQZW7sHNpsTlCeDYJuvqIahWj7zsQcmKRH3BKJdGyzRJ68a\n\tGjdZyRY1yI44DRbCHmoE0x4ro7a5CU2BLA/1gFJN5OhXw3Yu0W0rD+G6RbfWy+VsXXcP\n\tPA07IsZ9+TgwSdPXY+WjcLOWhhoXx6MYOTcF1ivjDXgxzQlMJdEBfTbLGIi21JtZqRY5\n\tdACg==","X-Gm-Message-State":"AJcUukfrH7r3VGkDPEyHR7tnI15uGg2e+4mmbUpiMS5lTQaAxnfc8021\n\tbraCQxySMklGfTxky8OsZfBQYxv5tH4=","X-Google-Smtp-Source":"ALg8bN70fpB9G7re6qJE3qiqTQa+98k7Jh+uOQB4pdvM5oSa0z4osT2YqgETYgG4Bq6Z1mYY1USTKQ==","X-Received":"by 2002:a2e:3218:: with SMTP id\n\ty24-v6mr13380418ljy.157.1547830052284; \n\tFri, 18 Jan 2019 08:47:32 -0800 (PST)","Date":"Fri, 18 Jan 2019 17:47:31 +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":"<20190118164731.GV6484@bigcity.dyn.berto.se>","References":"<20190117235916.1906-1-laurent.pinchart@ideasonboard.com>\n\t<20190117235916.1906-4-laurent.pinchart@ideasonboard.com>\n\t<20190118160604.GU6484@bigcity.dyn.berto.se>\n\t<20190118163010.GJ5275@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190118163010.GJ5275@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","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":"Fri, 18 Jan 2019 16:47:33 -0000"}},{"id":426,"web_url":"https://patchwork.libcamera.org/comment/426/","msgid":"<20190118225750.GC1799@pendragon.ideasonboard.com>","date":"2019-01-18T22:57:50","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, Jan 18, 2019 at 05:47:31PM +0100, Niklas Söderlund wrote:\n>  On 2019-01-18 18:30:10 +0200, Laurent Pinchart wrote:\n> > On Fri, Jan 18, 2019 at 05:06:04PM +0100, Niklas Söderlund wrote:\n> >> On 2019-01-18 01:59:15 +0200, Laurent Pinchart wrote:\n> >>> Cameras are listed through a double indirection, first iterating over\n> >>> all available pipeline handlers, and then listing the cameras they each\n> >>> support. To simplify the API make the pipeline handlers register the\n> >>> cameras with the camera manager directly, which lets the camera manager\n> >>> easily expose the list of all available cameras.\n> >>> \n> >>> The PipelineHandler API gets simplified as the handlers don't need to\n> >>> expose the list of cameras they have created.\n> >>> \n> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >>> ---\n> >>> include/libcamera/camera_manager.h       |  5 ++-\n> >>> src/libcamera/camera_manager.cpp         | 44 +++++++++++-------------\n> >>> src/libcamera/include/pipeline_handler.h |  6 ++--\n> >>> src/libcamera/pipeline/vimc.cpp          | 30 ++++------------\n> >>> src/libcamera/pipeline_handler.cpp       | 21 +++--------\n> >>> test/list-cameras.cpp                    |  5 +--\n> >>> 6 files changed, 40 insertions(+), 71 deletions(-)\n> > \n> > [snip]\n> > \n> >>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> >>> index 9806e399f92b..852e5ed70fe3 100644\n> >>> --- a/src/libcamera/camera_manager.cpp\n> >>> +++ b/src/libcamera/camera_manager.cpp\n> > \n> > [snip]\n> > \n> >>> +/**\n> >>> + * \\brief Add a camera to the camera manager\n> >>> + * \\param[in] camera The camera to be added\n> >>> + *\n> >>> + * This function is called by pipeline handlers to register the cameras they\n> >>> + * handle with the camera manager. Registered cameras are immediately made\n> >>> + * available to the system.\n> >>> + */\n> >>> +void CameraManager::addCamera(Camera *camera)\n> >>> +{\n> >>> +\tcameras_.push_back(camera);\n> >> \n> >> I wonder if we should not add a runtime check here to make sure the \n> >> camera name is unique before adding it to the vector here? The get() \n> >> function retrieves a Camera object by name there might be odd \n> >> consequences otherwise :-)\n> > \n> > Good point. I'll add\n> > \n> > for (Camera *c : cameras_) {\n> > if (c->name() == camera->name()) {\n> > LOG(Warning) << \"Registering camera with duplicate name '\"\n> > << camera->name() << \"'\";\n> > break;\n> > }\n> > }\n> > \n> > Or should it be LOG(Error) ? I don't think it should be a fatal error in\n> > any case (so no ASSERT) as it doesn't prevent from using the other\n> > cameras, or even the one with a duplicate name when retrieved directly\n> > from the vector of cameras (introduced in patch 3/4).\n>  \n>  I think this should be LOG(Error) and that we should not add the Camera \n>  object to the list but instead return after LOG().\n>  \n>  I don't think it's a good idea to try and function in a state which have \n>  a possibility of unknown behavior when we can in a safe way inform the \n>  user that something is wrong and keep the fully functional previous \n>  state. That being said, I'm not totally against your solution here and \n>  if you can think of a good use-case where it's more desirable to do so I \n>  would be OK with that.\n\nThis would require returning an error to the pipeline handler and\npropagating (until where ?), while we can still function normally when\nmultiple cameras have the same name. Only the CameraManager::get()\nfunction will be affected, and will return the first camera with the\nrequested name. That's why I think it would be simpler for pipeline\nmanagers if the addCamera() call didn't fail. We should certainly log a\nwarning or error as the problem should be fixed, but it won't cause us\nto fail (which is why I went for a warning originally, to denote an\nissue that is recoverable but still needs to be fixed).\n\nI expect the camera lookup API offered by the CameraManager to still\nevolve, so I think this will be revisited anyway. If you strongly think\nit needs to be addressed now then I'll see what can be done.\n\n> > \n> >> With this addressed,\n> >> \n> >> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >> \n> >>> +}\n> > \n> > [snip]","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54E0C60B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 23:57:51 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B662B53E;\n\tFri, 18 Jan 2019 23:57:50 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547852270;\n\tbh=dQIaxb3Yj89f9M9PxbikOTWBMOirpzUMiDCic0oOKQQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s3wi7SVNhPCteU+GhiYr4nvWkW2APQygusczupFGnfYecP+dTHUtCRFZAfydLIrle\n\tNSfKQLdVbJtLQZ3o9hNFYvGc6flSio+KPlLCcrpZ6BPB8bPmWOXQ1honi8MBsgFllE\n\tz7ZPxWKJ70kvK9tybV5eZneXTmR1sDu2+fV2BCbk=","Date":"Sat, 19 Jan 2019 00:57:50 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190118225750.GC1799@pendragon.ideasonboard.com>","References":"<20190117235916.1906-1-laurent.pinchart@ideasonboard.com>\n\t<20190117235916.1906-4-laurent.pinchart@ideasonboard.com>\n\t<20190118160604.GU6484@bigcity.dyn.berto.se>\n\t<20190118163010.GJ5275@pendragon.ideasonboard.com>\n\t<20190118164731.GV6484@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190118164731.GV6484@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","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":"Fri, 18 Jan 2019 22:57:51 -0000"}},{"id":428,"web_url":"https://patchwork.libcamera.org/comment/428/","msgid":"<20190120135037.GA6484@bigcity.dyn.berto.se>","date":"2019-01-20T13:50:37","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2019-01-19 00:57:50 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Fri, Jan 18, 2019 at 05:47:31PM +0100, Niklas Söderlund wrote:\n> >  On 2019-01-18 18:30:10 +0200, Laurent Pinchart wrote:\n> > > On Fri, Jan 18, 2019 at 05:06:04PM +0100, Niklas Söderlund wrote:\n> > >> On 2019-01-18 01:59:15 +0200, Laurent Pinchart wrote:\n> > >>> Cameras are listed through a double indirection, first iterating over\n> > >>> all available pipeline handlers, and then listing the cameras they each\n> > >>> support. To simplify the API make the pipeline handlers register the\n> > >>> cameras with the camera manager directly, which lets the camera manager\n> > >>> easily expose the list of all available cameras.\n> > >>> \n> > >>> The PipelineHandler API gets simplified as the handlers don't need to\n> > >>> expose the list of cameras they have created.\n> > >>> \n> > >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > >>> ---\n> > >>> include/libcamera/camera_manager.h       |  5 ++-\n> > >>> src/libcamera/camera_manager.cpp         | 44 +++++++++++-------------\n> > >>> src/libcamera/include/pipeline_handler.h |  6 ++--\n> > >>> src/libcamera/pipeline/vimc.cpp          | 30 ++++------------\n> > >>> src/libcamera/pipeline_handler.cpp       | 21 +++--------\n> > >>> test/list-cameras.cpp                    |  5 +--\n> > >>> 6 files changed, 40 insertions(+), 71 deletions(-)\n> > > \n> > > [snip]\n> > > \n> > >>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > >>> index 9806e399f92b..852e5ed70fe3 100644\n> > >>> --- a/src/libcamera/camera_manager.cpp\n> > >>> +++ b/src/libcamera/camera_manager.cpp\n> > > \n> > > [snip]\n> > > \n> > >>> +/**\n> > >>> + * \\brief Add a camera to the camera manager\n> > >>> + * \\param[in] camera The camera to be added\n> > >>> + *\n> > >>> + * This function is called by pipeline handlers to register the cameras they\n> > >>> + * handle with the camera manager. Registered cameras are immediately made\n> > >>> + * available to the system.\n> > >>> + */\n> > >>> +void CameraManager::addCamera(Camera *camera)\n> > >>> +{\n> > >>> +\tcameras_.push_back(camera);\n> > >> \n> > >> I wonder if we should not add a runtime check here to make sure the \n> > >> camera name is unique before adding it to the vector here? The get() \n> > >> function retrieves a Camera object by name there might be odd \n> > >> consequences otherwise :-)\n> > > \n> > > Good point. I'll add\n> > > \n> > > for (Camera *c : cameras_) {\n> > > if (c->name() == camera->name()) {\n> > > LOG(Warning) << \"Registering camera with duplicate name '\"\n> > > << camera->name() << \"'\";\n> > > break;\n> > > }\n> > > }\n> > > \n> > > Or should it be LOG(Error) ? I don't think it should be a fatal error in\n> > > any case (so no ASSERT) as it doesn't prevent from using the other\n> > > cameras, or even the one with a duplicate name when retrieved directly\n> > > from the vector of cameras (introduced in patch 3/4).\n> >  \n> >  I think this should be LOG(Error) and that we should not add the Camera \n> >  object to the list but instead return after LOG().\n> >  \n> >  I don't think it's a good idea to try and function in a state which have \n> >  a possibility of unknown behavior when we can in a safe way inform the \n> >  user that something is wrong and keep the fully functional previous \n> >  state. That being said, I'm not totally against your solution here and \n> >  if you can think of a good use-case where it's more desirable to do so I \n> >  would be OK with that.\n> \n> This would require returning an error to the pipeline handler and\n> propagating (until where ?), while we can still function normally when\n> multiple cameras have the same name. Only the CameraManager::get()\n> function will be affected, and will return the first camera with the\n> requested name. That's why I think it would be simpler for pipeline\n> managers if the addCamera() call didn't fail. We should certainly log a\n> warning or error as the problem should be fixed, but it won't cause us\n> to fail (which is why I went for a warning originally, to denote an\n> issue that is recoverable but still needs to be fixed).\n> \n> I expect the camera lookup API offered by the CameraManager to still\n> evolve, so I think this will be revisited anyway. If you strongly think\n> it needs to be addressed now then I'll see what can be done.\n\nI'm fine with moving forward with the current solution until we get the \nCameraManager API in a more stable state.\n\n> \n> > > \n> > >> With this addressed,\n> > >> \n> > >> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > >> \n> > >>> +}\n> > > \n> > > [snip]\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C6E5960C81\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Jan 2019 14:50:39 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id x85-v6so15349754ljb.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Jan 2019 05:50:39 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\th12-v6sm1687203ljb.80.2019.01.20.05.50.37\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 20 Jan 2019 05:50:37 -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=ks4HAS8HF5wbKFy0D7sO1UCXRwPZjcS5xbvMppH7Leo=;\n\tb=pf9ifDLP/HoJB8SeMh/hGH+fKjLyJ6HOrn+oOL8nfrtwZ0YjZpgnhobOJzN5cg5lHY\n\twCUtNhCZG9fGeuRvsy0IGYUArlMVqqQDVQmI/waAWawmrgV2hLutqs+477Fah5qFH/C5\n\tJtaNS/CjoccIFBiVXiD8MGlfxYF7UDkgJeq2E/lRyalOKRf72/kmi4Dlu5ui8HGRBGdP\n\tgUB9I7SM8y46DYcHb3YQLBxRfAWGBxd56DROvV1nHl2ouvLGV6vykYuRfxAwJFSUVOMb\n\tqnOmCFNt7hUG4p6W2WxNfdxj0MMFJGnlFqT4t6nzAKZcIQ7q3kuFWpErD+NOf6pKdOKg\n\tNZWg==","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=ks4HAS8HF5wbKFy0D7sO1UCXRwPZjcS5xbvMppH7Leo=;\n\tb=r073rqrvVB6zjt3/fRH+/W+oBO/8AwuS/9T5XjPQ2tUuxQVoB+3vCROtgjtZHRwcXY\n\tXVKwLSJTrcr+0sHMxqnXAGdo7LAorc0+OUL6gQArE7EA2x4qxJC2aJC3/1WJpG+1oAin\n\tX1peh8j1rcmu8fxR6DIpNJxJQE1LM+IYyd0tsNA298zf4CpwWtdvIij1qHIGyN5K9Enz\n\tX6kACVxjfiRMwUA78mYtu0YMRwddyE65uaJWeW8GRIpm4ke3fqhTOOWdjBGOopEzYwqz\n\tk53JE/zcFqqXcAIZriikHWD1WgDTUKrKMzlpdQSP+9Khq19UnnkoqD2taW5WkdPxXobW\n\tl4IQ==","X-Gm-Message-State":"AJcUukfI4P+Lp3TKyTij0zUGs4Va73DaOvM6VqQoWbiqqzQyJF8WdP1F\n\tn1xHVeeS/de75Xp3UmIEZ4qqbkF0z7I=","X-Google-Smtp-Source":"ALg8bN4YUveBKzNcI8TJfFKsnQX+C4Y7mkS2k4cAPMqmkYI1GKOBZhGIXmIOS2wOcUYFaLBySnm2CA==","X-Received":"by 2002:a2e:9c7:: with SMTP id\n\t190-v6mr14383331ljj.120.1547992238775; \n\tSun, 20 Jan 2019 05:50:38 -0800 (PST)","Date":"Sun, 20 Jan 2019 14:50:37 +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":"<20190120135037.GA6484@bigcity.dyn.berto.se>","References":"<20190117235916.1906-1-laurent.pinchart@ideasonboard.com>\n\t<20190117235916.1906-4-laurent.pinchart@ideasonboard.com>\n\t<20190118160604.GU6484@bigcity.dyn.berto.se>\n\t<20190118163010.GJ5275@pendragon.ideasonboard.com>\n\t<20190118164731.GV6484@bigcity.dyn.berto.se>\n\t<20190118225750.GC1799@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190118225750.GC1799@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tRegister cameras with the camera manager","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":"Sun, 20 Jan 2019 13:50:40 -0000"}}]