[{"id":5052,"web_url":"https://patchwork.libcamera.org/comment/5052/","msgid":"<20200605175729.GC5864@oden.dyn.berto.se>","date":"2020-06-05T17:57:29","subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2020-06-05 21:03:06 +0900, Paul Elder wrote:\n> The V4L2 compatibility layer uses devnum to match video device nodes to\n> libcamera Cameras. Some pipeline handlers don't report a devnum for\n> their camera, which prevents the V4L2 compatibility layer from matching\n> video device nodes to these cameras. To fix this, we first allow the\n> camera manager to map multiple devnums to a camera. Next, if the pipeline\n> handler doesn't report a devnum for a camera, then walk the media device\n> and entity list and tell the camera manager to map every one of these\n> devnums to the camera.\n\nOut of curiosity how will this work if a pipeline registers two cameras \nfrom the same media graph?\n\n> \n> We considered walking the media entity list and taking the devnum from\n> just the one with the default flag set, but we found that some drivers\n> (eg. vimc) don't set this flag for any entity.\n> \n> Signed-off-by: Paul Elder \n> ---\n> include/libcamera/camera_manager.h | 3 ++-\n> src/libcamera/camera_manager.cpp | 14 ++++++++------\n> src/libcamera/pipeline_handler.cpp | 12 +++++++++++-\n> 3 files changed, 21 insertions(+), 8 deletions(-)\n> \n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 079f848a..6095b799 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -34,7 +34,8 @@ public:\n> \tstd::shared_ptr get(const std::string &name);\n> \tstd::shared_ptr get(dev_t devnum);\n> \n> -\tvoid addCamera(std::shared_ptr camera, dev_t devnum);\n> +\tvoid addCamera(std::shared_ptr camera,\n> +\t\t std::vector devnums);\n> \tvoid removeCamera(Camera *camera);\n> \n> \tstatic const std::string &version() { return version_; }\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index da806fa7..fa0bd6a0 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -36,7 +36,8 @@ public:\n> \tPrivate(CameraManager *cm);\n> \n> \tint start();\n> -\tvoid addCamera(std::shared_ptr &camera, dev_t devnum);\n> +\tvoid addCamera(std::shared_ptr &camera,\n> +\t\t std::vector devnums);\n> \tvoid removeCamera(Camera *camera);\n> \n> \t/*\n> @@ -168,7 +169,7 @@ void CameraManager::Private::cleanup()\n> }\n> \n> void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> -\t\t\t\t dev_t devnum)\n> +\t\t\t\t std::vector devnums)\n> {\n> \tMutexLocker locker(mutex_);\n> \n> @@ -183,7 +184,7 @@ void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> \n> \tcameras_.push_back(std::move(camera));\n> \n> -\tif (devnum) {\n> +\tfor (dev_t devnum : devnums) {\n> \t\tunsigned int index = cameras_.size() - 1;\n> \t\tcamerasByDevnum_[devnum] = cameras_[index];\n> \t}\n> @@ -374,7 +375,7 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> /**\n> * \\brief Add a camera to the camera manager\n> * \\param[in] camera The camera to be added\n> - * \\param[in] devnum The device number to associate with \\a camera\n> + * \\param[in] devnums The device numbers to associate with \\a camera\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> @@ -385,11 +386,12 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> *\n> * \\context This function shall be called from the CameraManager thread.\n> */\n> -void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum)\n> +void CameraManager::addCamera(std::shared_ptr camera,\n> +\t\t\t std::vector devnums)\n> {\n> \tASSERT(Thread::current() == p_.get());\n> \n> -\tp_->addCamera(camera, devnum);\n> +\tp_->addCamera(camera, devnums);\n> }\n> \n> /**\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 53aeebdc..b3824a5f 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr camera,\n> \tdata->camera_ = camera.get();\n> \tcameraData_[camera.get()] = std::move(data);\n> \tcameras_.push_back(camera);\n> -\tmanager_->addCamera(std::move(camera), devnum);\n> +\n> +\tstd::vector devnums;\n> +\tif (devnum != 0)\n> +\t\tdevnums.push_back(devnum);\n> +\telse\n> +\t\tfor (const std::shared_ptr media : mediaDevices_)\n> +\t\t\tfor (const MediaEntity *entity : media->entities())\n> +\t\t\t\tdevnums.push_back(makedev(entity->deviceMajor(),\n> +\t\t\t\t\t\t\t entity->deviceMinor()));\n> +\n> +\tmanager_->addCamera(std::move(camera), devnums);\n> }\n> \n> /**\n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D7640603C6\n\tfor ;\n\tFri, 5 Jun 2020 19:57:31 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id c17so12778604lji.11\n\tfor ;\n\tFri, 05 Jun 2020 10:57:31 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tm11sm1058647lji.51.2020.06.05.10.57.29\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 05 Jun 2020 10:57:30 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"XJvrDzfq\"; \n\tdkim-atps=neutral","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\tbh=LKrOrW6dfB+LrACQPXnAOvAG2QKJrXP0rkEakHwsNBc=;\n\tb=XJvrDzfq+yEDTRP0iKaeAeDzCtCKYs3TDXH+k5QzuFRXl7ZETx72PfVGt5gui2P13Z\n\t/wzlGhWw5t2Bw/xS1x2HAqrDjYpsRE3b3BBib5Q90CfdPYgsJ1kKJOJ44dUsINuPeVqe\n\tTKVARl1K4bp9mtbnV1mJsowPWqBU7ULBwu50jYM5bQ0K3zX+rGm+fnqCqViqPmI74fFl\n\tLVT9RDKmEC5YE7NClpxOIFdrkkHfZTsGQ/MdokA3TCW+hfT7uGK/Gc6dG/ma3f2VyDKY\n\txaSNvyErgZdryV2xgu9yPrJyhc6pPTuwqx/d+uwYdbCZ23yooqb266Hc9Ds3Kuro8IUe\n\tOhXA==","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;\n\tbh=LKrOrW6dfB+LrACQPXnAOvAG2QKJrXP0rkEakHwsNBc=;\n\tb=k0GBTzdrKhAqOGdpbrTTCXcn6S4YK8J9+oEQ5xGuJg5yg5KkPeQzhT110r2jE5vJQF\n\tijDWl3TqmEaTnGuZw0BsqWSr0R3hiOodNQ1lrUH2Y078l1mtkQnUoWeiq1RaDiGpSX3f\n\tDwQeRAeDmIIbCcUDTKUCUdKHW2f580wEcL5T975q70kHBFlVeBkYDH5QCAvz1KAwxWJj\n\t94SakIqb2ZJ79lmK1hOvS2qzBhTlKRUpqmDM+pCQ9bdofLHjltchzxuoz2oHC4Bl9YXi\n\tOG9buZ3H0VO4OolFdIisWE/WVuuvXYC8FOVw29pgLVxsFpOz9kKgN7xSsbQ7UNPyRR19\n\tWOFw==","X-Gm-Message-State":"AOAM533FuYLEtxYjKgSA0L2vGzfaWAGGn/kfrEO3ARY/JBZBIDa7KCNQ\n\txTRj5Wk9nmphsAlUjpCjxOfUo9AvviympQ==","X-Google-Smtp-Source":"ABdhPJz2e9z9A8qy7wXD8hPKf6hr9KxZdOOaEjeqcRtIMMZIuZb2y8uB5Wqk+6M8T1oJjJKgLKTI6A==","X-Received":"by 2002:a05:651c:333:: with SMTP id\n\tb19mr5324368ljp.204.1591379851086; \n\tFri, 05 Jun 2020 10:57:31 -0700 (PDT)","Date":"Fri, 5 Jun 2020 19:57:29 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200605175729.GC5864@oden.dyn.berto.se>","References":"<20200605120306.25529-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200605120306.25529-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 05 Jun 2020 17:57:32 -0000"}},{"id":5058,"web_url":"https://patchwork.libcamera.org/comment/5058/","msgid":"<20200605185754.GE26752@pendragon.ideasonboard.com>","date":"2020-06-05T18:57:54","subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","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, Jun 05, 2020 at 07:57:29PM +0200, Niklas Söderlund wrote:\n> Hi Paul,\n> \n> Thanks for your work.\n> \n> On 2020-06-05 21:03:06 +0900, Paul Elder wrote:\n> > The V4L2 compatibility layer uses devnum to match video device nodes to\n> > libcamera Cameras. Some pipeline handlers don't report a devnum for\n> > their camera, which prevents the V4L2 compatibility layer from matching\n> > video device nodes to these cameras. To fix this, we first allow the\n> > camera manager to map multiple devnums to a camera. Next, if the pipeline\n> > handler doesn't report a devnum for a camera, then walk the media device\n> > and entity list and tell the camera manager to map every one of these\n> > devnums to the camera.\n> \n> Out of curiosity how will this work if a pipeline registers two cameras \n> from the same media graph?\n\nThis is something I've thought about recently. Given that those cameras\nwill most likely not be usable at the same time (at least with the\npipeline handlers we have now), we could select them with VIDIOC_S_INPUT\nthrough a single emulated V4L2 device. That's not a perfect solution,\nbut I think it could be a good first step. We may want, longer term, to\nsupport concurrent usage of cameras handled by the same pipeline handler\ninstance, and that will require pipeline handlers to give a camera to\ndevnode mapping. I would however like to avoid doing so explicitly in\nall pipeline handlers, so I'm considering an optional argument to\nPipelineHandler::registerCamera() to provide explicit mapping, and\notherwise map to all the capture video nodes. Just brainstorming here,\nso please feel free to propose other options.\n\n> > We considered walking the media entity list and taking the devnum from\n> > just the one with the default flag set, but we found that some drivers\n> > (eg. vimc) don't set this flag for any entity.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > include/libcamera/camera_manager.h | 3 ++-\n> > src/libcamera/camera_manager.cpp | 14 ++++++++------\n> > src/libcamera/pipeline_handler.cpp | 12 +++++++++++-\n> > 3 files changed, 21 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> > index 079f848a..6095b799 100644\n> > --- a/include/libcamera/camera_manager.h\n> > +++ b/include/libcamera/camera_manager.h\n> > @@ -34,7 +34,8 @@ public:\n> > \tstd::shared_ptr get(const std::string &name);\n> > \tstd::shared_ptr get(dev_t devnum);\n> > \n> > -\tvoid addCamera(std::shared_ptr camera, dev_t devnum);\n> > +\tvoid addCamera(std::shared_ptr camera,\n> > +\t\t std::vector devnums);\n> > \tvoid removeCamera(Camera *camera);\n> > \n> > \tstatic const std::string &version() { return version_; }\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index da806fa7..fa0bd6a0 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -36,7 +36,8 @@ public:\n> > \tPrivate(CameraManager *cm);\n> > \n> > \tint start();\n> > -\tvoid addCamera(std::shared_ptr &camera, dev_t devnum);\n> > +\tvoid addCamera(std::shared_ptr &camera,\n> > +\t\t std::vector devnums);\n> > \tvoid removeCamera(Camera *camera);\n> > \n> > \t/*\n> > @@ -168,7 +169,7 @@ void CameraManager::Private::cleanup()\n> > }\n> > \n> > void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> > -\t\t\t\t dev_t devnum)\n> > +\t\t\t\t std::vector devnums)\n> > {\n> > \tMutexLocker locker(mutex_);\n> > \n> > @@ -183,7 +184,7 @@ void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> > \n> > \tcameras_.push_back(std::move(camera));\n> > \n> > -\tif (devnum) {\n> > +\tfor (dev_t devnum : devnums) {\n> > \t\tunsigned int index = cameras_.size() - 1;\n> > \t\tcamerasByDevnum_[devnum] = cameras_[index];\n> > \t}\n> > @@ -374,7 +375,7 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> > /**\n> > * \\brief Add a camera to the camera manager\n> > * \\param[in] camera The camera to be added\n> > - * \\param[in] devnum The device number to associate with \\a camera\n> > + * \\param[in] devnums The device numbers to associate with \\a camera\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> > @@ -385,11 +386,12 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> > *\n> > * \\context This function shall be called from the CameraManager thread.\n> > */\n> > -void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum)\n> > +void CameraManager::addCamera(std::shared_ptr camera,\n> > +\t\t\t std::vector devnums)\n> > {\n> > \tASSERT(Thread::current() == p_.get());\n> > \n> > -\tp_->addCamera(camera, devnum);\n> > +\tp_->addCamera(camera, devnums);\n> > }\n> > \n> > /**\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 53aeebdc..b3824a5f 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr camera,\n> > \tdata->camera_ = camera.get();\n> > \tcameraData_[camera.get()] = std::move(data);\n> > \tcameras_.push_back(camera);\n> > -\tmanager_->addCamera(std::move(camera), devnum);\n> > +\n> > +\tstd::vector devnums;\n> > +\tif (devnum != 0)\n> > +\t\tdevnums.push_back(devnum);\n> > +\telse\n> > +\t\tfor (const std::shared_ptr media : mediaDevices_)\n> > +\t\t\tfor (const MediaEntity *entity : media->entities())\n> > +\t\t\t\tdevnums.push_back(makedev(entity->deviceMajor(),\n> > +\t\t\t\t\t\t\t entity->deviceMinor()));\n> > +\n> > +\tmanager_->addCamera(std::move(camera), devnums);\n> > }\n> > \n> > /**\n> > -- \n> > 2.20.1\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> \n> -- \n> Regards,\n> Niklas Söderlund\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 EF222603C6\n\tfor ;\n\tFri, 5 Jun 2020 20:58:13 +0200 (CEST)","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 602B6F9;\n\tFri, 5 Jun 2020 20:58:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"q0msBXsK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591383493;\n\tbh=dz+BDy/79N8mfqOqlaMf/BySUWDAakR9KG6R6XwbqD8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q0msBXsKS7vwIyqCekdpih+d85FOpXMBl52Tlr+A+167bgKLXkEeIMxtfKexvb0e0\n\ttM7BzMaLbDYcACAAaetTqGcbtIyEv4wT48eSZD6MjZVX/W2nASZZGbF2c8Pl+zsJgT\n\tMUu/8+2SlrdzcbVGspgzcovRKtHtgYI46RoKiEL4=","Date":"Fri, 5 Jun 2020 21:57:54 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200605185754.GE26752@pendragon.ideasonboard.com>","References":"<20200605120306.25529-1-paul.elder@ideasonboard.com>\n\t<20200605175729.GC5864@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200605175729.GC5864@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 05 Jun 2020 18:58:14 -0000"}},{"id":5059,"web_url":"https://patchwork.libcamera.org/comment/5059/","msgid":"<20200605190237.GF26752@pendragon.ideasonboard.com>","date":"2020-06-05T19:02:37","subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Jun 05, 2020 at 09:03:06PM +0900, Paul Elder wrote:\n> The V4L2 compatibility layer uses devnum to match video device nodes to\n> libcamera Cameras. Some pipeline handlers don't report a devnum for\n> their camera, which prevents the V4L2 compatibility layer from matching\n> video device nodes to these cameras. To fix this, we first allow the\n> camera manager to map multiple devnums to a camera. Next, if the pipeline\n> handler doesn't report a devnum for a camera, then walk the media device\n> and entity list and tell the camera manager to map every one of these\n> devnums to the camera.\n> \n> We considered walking the media entity list and taking the devnum from\n> just the one with the default flag set, but we found that some drivers\n> (eg. vimc) don't set this flag for any entity.\n> \n> Signed-off-by: Paul Elder \n> ---\n> include/libcamera/camera_manager.h | 3 ++-\n> src/libcamera/camera_manager.cpp | 14 ++++++++------\n> src/libcamera/pipeline_handler.cpp | 12 +++++++++++-\n> 3 files changed, 21 insertions(+), 8 deletions(-)\n> \n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 079f848a..6095b799 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -34,7 +34,8 @@ public:\n> \tstd::shared_ptr get(const std::string &name);\n> \tstd::shared_ptr get(dev_t devnum);\n> \n> -\tvoid addCamera(std::shared_ptr camera, dev_t devnum);\n> +\tvoid addCamera(std::shared_ptr camera,\n> +\t\t std::vector devnums);\n\nMake this a const std::vector<> & to avoid copies.\n\n> \tvoid removeCamera(Camera *camera);\n> \n> \tstatic const std::string &version() { return version_; }\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index da806fa7..fa0bd6a0 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -36,7 +36,8 @@ public:\n> \tPrivate(CameraManager *cm);\n> \n> \tint start();\n> -\tvoid addCamera(std::shared_ptr &camera, dev_t devnum);\n> +\tvoid addCamera(std::shared_ptr &camera,\n> +\t\t std::vector devnums);\n\nSame here.\n\n> \tvoid removeCamera(Camera *camera);\n> \n> \t/*\n> @@ -168,7 +169,7 @@ void CameraManager::Private::cleanup()\n> }\n> \n> void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> -\t\t\t\t dev_t devnum)\n> +\t\t\t\t std::vector devnums)\n> {\n> \tMutexLocker locker(mutex_);\n> \n> @@ -183,7 +184,7 @@ void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> \n> \tcameras_.push_back(std::move(camera));\n> \n> -\tif (devnum) {\n> +\tfor (dev_t devnum : devnums) {\n> \t\tunsigned int index = cameras_.size() - 1;\n\nYou can move this outside of the loop.\n\n> \t\tcamerasByDevnum_[devnum] = cameras_[index];\n> \t}\n> @@ -374,7 +375,7 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> /**\n> * \\brief Add a camera to the camera manager\n> * \\param[in] camera The camera to be added\n> - * \\param[in] devnum The device number to associate with \\a camera\n> + * \\param[in] devnums The device numbers to associate with \\a camera\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> @@ -385,11 +386,12 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> *\n> * \\context This function shall be called from the CameraManager thread.\n> */\n> -void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum)\n> +void CameraManager::addCamera(std::shared_ptr camera,\n> +\t\t\t std::vector devnums)\n> {\n> \tASSERT(Thread::current() == p_.get());\n> \n> -\tp_->addCamera(camera, devnum);\n> +\tp_->addCamera(camera, devnums);\n> }\n> \n> /**\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 53aeebdc..b3824a5f 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr camera,\n> \tdata->camera_ = camera.get();\n> \tcameraData_[camera.get()] = std::move(data);\n> \tcameras_.push_back(camera);\n> -\tmanager_->addCamera(std::move(camera), devnum);\n> +\n> +\tstd::vector devnums;\n> +\tif (devnum != 0)\n> +\t\tdevnums.push_back(devnum);\n\nShould we allow this, or always use all the capture video nodes ?\n\n> +\telse\n> +\t\tfor (const std::shared_ptr media : mediaDevices_)\n> +\t\t\tfor (const MediaEntity *entity : media->entities())\n> +\t\t\t\tdevnums.push_back(makedev(entity->deviceMajor(),\n> +\t\t\t\t\t\t\t entity->deviceMinor()));\n\nYou need to limit entities to the capture video nodes. You can handle\nthat through a combination of entity flags (to check if it's a video\nnode) and pad flags (to check if it's a capture video node, by looking\nat the direction of the pad).\n\nA short comment to explain what's going on would be useful.\n\n> +\n> +\tmanager_->addCamera(std::move(camera), devnums);\n> }\n> \n> /**","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A7797603C6\n\tfor ;\n\tFri, 5 Jun 2020 21:02:57 +0200 (CEST)","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 A2B4F27C;\n\tFri, 5 Jun 2020 21:02:56 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WjXBG4PE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591383777;\n\tbh=b4Etw0MU/hc7JAEZtl2xM/7kWtrcRuhJMzJryqJWtng=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WjXBG4PEJ0T/tLQobdd2EsC0xw62m5S4RH5cybuH5lSWEZQizBkc7xZrnGD39UQ3L\n\tlQUdd1lMseh6LeQJdgBCX9TZjurPOaYvplX3vU0OpkTS2oCiGpnLHMc0ANpa1SNr2B\n\tgj6CqN2ftGAlwORLHov90I7FNX8upkXeyGeEHrJU=","Date":"Fri, 5 Jun 2020 22:02:37 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200605190237.GF26752@pendragon.ideasonboard.com>","References":"<20200605120306.25529-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200605120306.25529-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 05 Jun 2020 19:02:57 -0000"}},{"id":5060,"web_url":"https://patchwork.libcamera.org/comment/5060/","msgid":"<20200605191042.GI5864@oden.dyn.berto.se>","date":"2020-06-05T19:10:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","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 2020-06-05 21:57:54 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Fri, Jun 05, 2020 at 07:57:29PM +0200, Niklas Söderlund wrote:\n> > Hi Paul,\n> > \n> > Thanks for your work.\n> > \n> > On 2020-06-05 21:03:06 +0900, Paul Elder wrote:\n> > > The V4L2 compatibility layer uses devnum to match video device nodes to\n> > > libcamera Cameras. Some pipeline handlers don't report a devnum for\n> > > their camera, which prevents the V4L2 compatibility layer from matching\n> > > video device nodes to these cameras. To fix this, we first allow the\n> > > camera manager to map multiple devnums to a camera. Next, if the pipeline\n> > > handler doesn't report a devnum for a camera, then walk the media device\n> > > and entity list and tell the camera manager to map every one of these\n> > > devnums to the camera.\n> > \n> > Out of curiosity how will this work if a pipeline registers two cameras \n> > from the same media graph?\n> \n> This is something I've thought about recently. Given that those cameras\n> will most likely not be usable at the same time (at least with the\n> pipeline handlers we have now), we could select them with VIDIOC_S_INPUT\n> through a single emulated V4L2 device. That's not a perfect solution,\n> but I think it could be a good first step. We may want, longer term, to\n> support concurrent usage of cameras handled by the same pipeline handler\n> instance, and that will require pipeline handlers to give a camera to\n> devnode mapping. I would however like to avoid doing so explicitly in\n> all pipeline handlers, so I'm considering an optional argument to\n> PipelineHandler::registerCamera() to provide explicit mapping, and\n> otherwise map to all the capture video nodes. Just brainstorming here,\n> so please feel free to propose other options.\n\nI'm fine with this approach. Using VIDIOC_S_INPUT sounds like a nice \nidea going forward as we could then possibly remove the optional devnode \nmapping argument removing one of the many things a pipeline handler is \nresponsible to get right :-)\n\nFor this patch with the possible ways we can take it in the future,\n\nReviewed-by: Niklas Söderlund \n\n> \n> > > We considered walking the media entity list and taking the devnum from\n> > > just the one with the default flag set, but we found that some drivers\n> > > (eg. vimc) don't set this flag for any entity.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > include/libcamera/camera_manager.h | 3 ++-\n> > > src/libcamera/camera_manager.cpp | 14 ++++++++------\n> > > src/libcamera/pipeline_handler.cpp | 12 +++++++++++-\n> > > 3 files changed, 21 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> > > index 079f848a..6095b799 100644\n> > > --- a/include/libcamera/camera_manager.h\n> > > +++ b/include/libcamera/camera_manager.h\n> > > @@ -34,7 +34,8 @@ public:\n> > > \tstd::shared_ptr get(const std::string &name);\n> > > \tstd::shared_ptr get(dev_t devnum);\n> > > \n> > > -\tvoid addCamera(std::shared_ptr camera, dev_t devnum);\n> > > +\tvoid addCamera(std::shared_ptr camera,\n> > > +\t\t std::vector devnums);\n> > > \tvoid removeCamera(Camera *camera);\n> > > \n> > > \tstatic const std::string &version() { return version_; }\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index da806fa7..fa0bd6a0 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> > > @@ -36,7 +36,8 @@ public:\n> > > \tPrivate(CameraManager *cm);\n> > > \n> > > \tint start();\n> > > -\tvoid addCamera(std::shared_ptr &camera, dev_t devnum);\n> > > +\tvoid addCamera(std::shared_ptr &camera,\n> > > +\t\t std::vector devnums);\n> > > \tvoid removeCamera(Camera *camera);\n> > > \n> > > \t/*\n> > > @@ -168,7 +169,7 @@ void CameraManager::Private::cleanup()\n> > > }\n> > > \n> > > void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> > > -\t\t\t\t dev_t devnum)\n> > > +\t\t\t\t std::vector devnums)\n> > > {\n> > > \tMutexLocker locker(mutex_);\n> > > \n> > > @@ -183,7 +184,7 @@ void CameraManager::Private::addCamera(std::shared_ptr &camera,\n> > > \n> > > \tcameras_.push_back(std::move(camera));\n> > > \n> > > -\tif (devnum) {\n> > > +\tfor (dev_t devnum : devnums) {\n> > > \t\tunsigned int index = cameras_.size() - 1;\n> > > \t\tcamerasByDevnum_[devnum] = cameras_[index];\n> > > \t}\n> > > @@ -374,7 +375,7 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> > > /**\n> > > * \\brief Add a camera to the camera manager\n> > > * \\param[in] camera The camera to be added\n> > > - * \\param[in] devnum The device number to associate with \\a camera\n> > > + * \\param[in] devnums The device numbers to associate with \\a camera\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> > > @@ -385,11 +386,12 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> > > *\n> > > * \\context This function shall be called from the CameraManager thread.\n> > > */\n> > > -void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum)\n> > > +void CameraManager::addCamera(std::shared_ptr camera,\n> > > +\t\t\t std::vector devnums)\n> > > {\n> > > \tASSERT(Thread::current() == p_.get());\n> > > \n> > > -\tp_->addCamera(camera, devnum);\n> > > +\tp_->addCamera(camera, devnums);\n> > > }\n> > > \n> > > /**\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 53aeebdc..b3824a5f 100644\n> > > --- a/src/libcamera/pipeline_handler.cpp\n> > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr camera,\n> > > \tdata->camera_ = camera.get();\n> > > \tcameraData_[camera.get()] = std::move(data);\n> > > \tcameras_.push_back(camera);\n> > > -\tmanager_->addCamera(std::move(camera), devnum);\n> > > +\n> > > +\tstd::vector devnums;\n> > > +\tif (devnum != 0)\n> > > +\t\tdevnums.push_back(devnum);\n> > > +\telse\n> > > +\t\tfor (const std::shared_ptr media : mediaDevices_)\n> > > +\t\t\tfor (const MediaEntity *entity : media->entities())\n> > > +\t\t\t\tdevnums.push_back(makedev(entity->deviceMajor(),\n> > > +\t\t\t\t\t\t\t entity->deviceMinor()));\n> > > +\n> > > +\tmanager_->addCamera(std::move(camera), devnums);\n> > > }\n> > > \n> > > /**\n> > > -- \n> > > 2.20.1\n> > > \n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n> > \n> > -- \n> > Regards,\n> > Niklas Söderlund\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2106E603CA\n\tfor ;\n\tFri, 5 Jun 2020 21:10:44 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id z206so6458294lfc.6\n\tfor ;\n\tFri, 05 Jun 2020 12:10:44 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t130sm1298026lfl.37.2020.06.05.12.10.42\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 05 Jun 2020 12:10:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"ZX54EWav\"; \n\tdkim-atps=neutral","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\tbh=SOHZyiOuHB3uWbW4Imq7xeU/uQsh9sIa+MD+Uxz8ff0=;\n\tb=ZX54EWav9muYWxD5lrBDb1AnCmA8KKqgvxeAFFdmmc3ERJ4xYJ8btNY/iGtS5qGSgu\n\tFLskYoFMYF3udENhfKuWrpKDOAZo7SmAB6NhqsJcpRlhKR4vwnN1GN3rqBLeQyFKF16V\n\tUZBeyM19H2xtEehqlCVUOGpfRmVJtwz+HDtCaCAr7wjFsTNiCmoS/Gc7Ci7Y75yMytXQ\n\tTgjG1Qo37owcwgOY1VRh0Nv0oJvNIyEgAUA8ur2pcc3sb/AAPc77Hoj45a8r8DWa5oif\n\tHs7LRqMmor26jVkAOOnDri8z4A67HzskiuP0c5uklykY9sMkowz8+9DoH2ET69kVc/EX\n\t7xoA==","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;\n\tbh=SOHZyiOuHB3uWbW4Imq7xeU/uQsh9sIa+MD+Uxz8ff0=;\n\tb=AQRWbS4JZPEP+PpZWchOLqBbdzw4zCAReNrrKUdsnVg2svqz8xSnnNaQVFlVpHmuLK\n\talocbUmvzxLvevtI0dQWEZDgy9y0qWjn+986YTIRPftcALOg498IPeoD8ZgaS123tqW1\n\tQfI0cfG28IHRYglYRRRtBsZlJPeifjj/LNi9x4q+mILq78SJCojZ/AOOWL28sSs7K80X\n\tSvyrab/pZZ1SplBKZmdBwVogFcRQws1N1WQHAB5XtpnIVMQBhg++vSYsDuRnSE1By6h4\n\tLF95gLaOgYFJ8RzhI3UQop2tw0KhA76UQwvkvpwFWCc2Y81LFyFnp0V1wEv57X9G+tdr\n\te52w==","X-Gm-Message-State":"AOAM5317doBTCa+rWvdq4xa+YiEQEMaJgkzCZ0CF9d++uPO71+10En9c\n\tgI9v+xYJ5fYZoVekk7pdd+NFqHivBnolUw==","X-Google-Smtp-Source":"ABdhPJxloeeGVCspfb7TrbMpKvX+uZwWYkKQHLuh0yrda5MVcENSWV5yi7R9xgiEjA8pt/4TA97Nug==","X-Received":"by 2002:ac2:560f:: with SMTP id\n\tv15mr6151966lfd.160.1591384243318; \n\tFri, 05 Jun 2020 12:10:43 -0700 (PDT)","Date":"Fri, 5 Jun 2020 21:10:42 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200605191042.GI5864@oden.dyn.berto.se>","References":"<20200605120306.25529-1-paul.elder@ideasonboard.com>\n\t<20200605175729.GC5864@oden.dyn.berto.se>\n\t<20200605185754.GE26752@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":"<20200605185754.GE26752@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 05 Jun 2020 19:10:44 -0000"}},{"id":5084,"web_url":"https://patchwork.libcamera.org/comment/5084/","msgid":"<20200606074834.GB2814@emerald.amanokami.net>","date":"2020-06-06T07:48:34","subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the review.\n\nOn Fri, Jun 05, 2020 at 10:02:37PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n\n\n\n> > /**\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 53aeebdc..b3824a5f 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr camera,\n> > \tdata->camera_ = camera.get();\n> > \tcameraData_[camera.get()] = std::move(data);\n> > \tcameras_.push_back(camera);\n> > -\tmanager_->addCamera(std::move(camera), devnum);\n> > +\n> > +\tstd::vector devnums;\n> > +\tif (devnum != 0)\n> > +\t\tdevnums.push_back(devnum);\n> \n> Should we allow this, or always use all the capture video nodes ?\n\nIt doesn't seem to me that applications are very likely to use\nCameraManager::get(devnum) to get cameras, and only those that deal with\nV4L2 to a certain extent (eg. the V4L2 compatibility layer), will. In\nthat sense, I think that it won't be bad to disallow declaring a devnum\nand always map the devnums automatically like below. In addition, having\nall the capture video nodes mapped to a camera would be prevent\nconfusion on the rules surrounding which \"video node is mapped to which\ncamera when using the V4L2 compatibility layer\".\n\nOn the other hand, I think it's good to still provide the ability to\npipeline handlers to declare their own mapping if they want to. The\ndefault (ie. less work) is the auto-mapping anyway. This might cause\nconfusion to users of the V4L2 compatilibity layer, if they expect that\nall capture nodes can be used when the pipeline handler has disagreed.\n\nSo it's ease of use for the users of the V4L2 compatiblity layer, or\nfreedom to pipeline handlers. Maybe user-friendliness is more important,\nand the pipeline handlers don't care to declare devnum mappings. I think\nI'll go with the former then, until there are objections.\n\n\n> > +\telse\n> > +\t\tfor (const std::shared_ptr media : mediaDevices_)\n> > +\t\t\tfor (const MediaEntity *entity : media->entities())\n> > +\t\t\t\tdevnums.push_back(makedev(entity->deviceMajor(),\n> > +\t\t\t\t\t\t\t entity->deviceMinor()));\n> \n> You need to limit entities to the capture video nodes. You can handle\n> that through a combination of entity flags (to check if it's a video\n> node) and pad flags (to check if it's a capture video node, by looking\n> at the direction of the pad).\n> \n> A short comment to explain what's going on would be useful.\n\nack\n\n> > +\n> > +\tmanager_->addCamera(std::move(camera), devnums);\n> > }\n> > \n> > /**\n\n\nThanks,\n\nPaul","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C734600F7\n\tfor ;\n\tSat, 6 Jun 2020 09:48:43 +0200 (CEST)","from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp\n\t[118.238.243.68])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 79C01F9;\n\tSat, 6 Jun 2020 09:48:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Kc85V7/k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591429722;\n\tbh=rXT4av+AQ4vAD8pVjslACYO//6RkG27/iosKcIwWCuM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kc85V7/kDkP9QQK/5E7eYFMDFNRRMqYrhLyjB+zpaGwq8J5sQZnGt9cNW0fpX6BsB\n\t6TxVeBD/FCn3t6SM6py4OPGzawA6YLaLcJRIdpa5Bjn/M+88pTvxrimIx/NqvsF8ta\n\tVUbmUNUn4WMHvRcSMl95R24edtdNAShF+DnyZUFw=","Date":"Sat, 6 Jun 2020 16:48:34 +0900","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200606074834.GB2814@emerald.amanokami.net>","References":"<20200605120306.25529-1-paul.elder@ideasonboard.com>\n\t<20200605190237.GF26752@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200605190237.GF26752@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 06 Jun 2020 07:48:43 -0000"}},{"id":5089,"web_url":"https://patchwork.libcamera.org/comment/5089/","msgid":"<20200606110848.GA7339@pendragon.ideasonboard.com>","date":"2020-06-06T11:08:48","subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Sat, Jun 06, 2020 at 04:48:34PM +0900, Paul Elder wrote:\n> Hi Laurent,\n> \n> Thank you for the review.\n> \n> On Fri, Jun 05, 2020 at 10:02:37PM +0300, Laurent Pinchart wrote:\n> > Hi Paul,\n> > \n> > Thank you for the patch.\n> > \n> \n> \n> \n> > > /**\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 53aeebdc..b3824a5f 100644\n> > > --- a/src/libcamera/pipeline_handler.cpp\n> > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr camera,\n> > > \tdata->camera_ = camera.get();\n> > > \tcameraData_[camera.get()] = std::move(data);\n> > > \tcameras_.push_back(camera);\n> > > -\tmanager_->addCamera(std::move(camera), devnum);\n> > > +\n> > > +\tstd::vector devnums;\n> > > +\tif (devnum != 0)\n> > > +\t\tdevnums.push_back(devnum);\n> > \n> > Should we allow this, or always use all the capture video nodes ?\n> \n> It doesn't seem to me that applications are very likely to use\n> CameraManager::get(devnum) to get cameras, and only those that deal with\n> V4L2 to a certain extent (eg. the V4L2 compatibility layer), will. In\n> that sense, I think that it won't be bad to disallow declaring a devnum\n> and always map the devnums automatically like below. In addition, having\n> all the capture video nodes mapped to a camera would be prevent\n> confusion on the rules surrounding which \"video node is mapped to which\n> camera when using the V4L2 compatibility layer\".\n> \n> On the other hand, I think it's good to still provide the ability to\n> pipeline handlers to declare their own mapping if they want to. The\n> default (ie. less work) is the auto-mapping anyway. This might cause\n> confusion to users of the V4L2 compatilibity layer, if they expect that\n> all capture nodes can be used when the pipeline handler has disagreed.\n> \n> So it's ease of use for the users of the V4L2 compatiblity layer, or\n> freedom to pipeline handlers. Maybe user-friendliness is more important,\n> and the pipeline handlers don't care to declare devnum mappings. I think\n> I'll go with the former then, until there are objections.\n\nSounds good to me. We may reconsider this later when (if) we'll have\npipelines supporting multiple cameras concurrently, but for now I think\nit's totally fine.\n\n> > > +\telse\n> > > +\t\tfor (const std::shared_ptr media : mediaDevices_)\n> > > +\t\t\tfor (const MediaEntity *entity : media->entities())\n> > > +\t\t\t\tdevnums.push_back(makedev(entity->deviceMajor(),\n> > > +\t\t\t\t\t\t\t entity->deviceMinor()));\n> > \n> > You need to limit entities to the capture video nodes. You can handle\n> > that through a combination of entity flags (to check if it's a video\n> > node) and pad flags (to check if it's a capture video node, by looking\n> > at the direction of the pad).\n> > \n> > A short comment to explain what's going on would be useful.\n> \n> ack\n> \n> > > +\n> > > +\tmanager_->addCamera(std::move(camera), devnums);\n> > > }\n> > > \n> > > /**","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F292D61167\n\tfor ;\n\tSat, 6 Jun 2020 13:09:06 +0200 (CEST)","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 6F09C27C;\n\tSat, 6 Jun 2020 13:09:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"khtEHYST\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591441746;\n\tbh=7RQoKtGZQiJ2wZFWhYd9bDBQuLzVoJJYt26n32OIV8o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=khtEHYSTQ/6FdTj8xZ31IztnEDymZqJ6vwwS+BtsDotIieZnuAxUoa3THHpwfYaOo\n\tyProZPgFFKr2XOYiEvNkl+YIP7BObIhP0Vn7/ADZzlBn+58lKaLWchfh6FU9i2S+tI\n\tEsNBeyICEsh2ZEE3y84k1UVzNFP7IhF3p0OUPVl0=","Date":"Sat, 6 Jun 2020 14:08:48 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200606110848.GA7339@pendragon.ideasonboard.com>","References":"<20200605120306.25529-1-paul.elder@ideasonboard.com>\n\t<20200605190237.GF26752@pendragon.ideasonboard.com>\n\t<20200606074834.GB2814@emerald.amanokami.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200606074834.GB2814@emerald.amanokami.net>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: CameraManager,\n\tPipelineHandler: Allow multiple devnums per camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 06 Jun 2020 11:09:07 -0000"}}]