[libcamera-devel] [PATCH] libcamera: CameraManager, PipelineHandler: Allow multiple devnums per camera
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Jun 5 21:10:42 CEST 2020
Hi Laurent,
On 2020-06-05 21:57:54 +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Fri, Jun 05, 2020 at 07:57:29PM +0200, Niklas Söderlund wrote:
> > Hi Paul,
> >
> > Thanks for your work.
> >
> > On 2020-06-05 21:03:06 +0900, Paul Elder wrote:
> > > The V4L2 compatibility layer uses devnum to match video device nodes to
> > > libcamera Cameras. Some pipeline handlers don't report a devnum for
> > > their camera, which prevents the V4L2 compatibility layer from matching
> > > video device nodes to these cameras. To fix this, we first allow the
> > > camera manager to map multiple devnums to a camera. Next, if the pipeline
> > > handler doesn't report a devnum for a camera, then walk the media device
> > > and entity list and tell the camera manager to map every one of these
> > > devnums to the camera.
> >
> > Out of curiosity how will this work if a pipeline registers two cameras
> > from the same media graph?
>
> This is something I've thought about recently. Given that those cameras
> will most likely not be usable at the same time (at least with the
> pipeline handlers we have now), we could select them with VIDIOC_S_INPUT
> through a single emulated V4L2 device. That's not a perfect solution,
> but I think it could be a good first step. We may want, longer term, to
> support concurrent usage of cameras handled by the same pipeline handler
> instance, and that will require pipeline handlers to give a camera to
> devnode mapping. I would however like to avoid doing so explicitly in
> all pipeline handlers, so I'm considering an optional argument to
> PipelineHandler::registerCamera() to provide explicit mapping, and
> otherwise map to all the capture video nodes. Just brainstorming here,
> so please feel free to propose other options.
I'm fine with this approach. Using VIDIOC_S_INPUT sounds like a nice
idea going forward as we could then possibly remove the optional devnode
mapping argument removing one of the many things a pipeline handler is
responsible to get right :-)
For this patch with the possible ways we can take it in the future,
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > > We considered walking the media entity list and taking the devnum from
> > > just the one with the default flag set, but we found that some drivers
> > > (eg. vimc) don't set this flag for any entity.
> > >
> > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > > ---
> > > include/libcamera/camera_manager.h | 3 ++-
> > > src/libcamera/camera_manager.cpp | 14 ++++++++------
> > > src/libcamera/pipeline_handler.cpp | 12 +++++++++++-
> > > 3 files changed, 21 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> > > index 079f848a..6095b799 100644
> > > --- a/include/libcamera/camera_manager.h
> > > +++ b/include/libcamera/camera_manager.h
> > > @@ -34,7 +34,8 @@ public:
> > > std::shared_ptr<Camera> get(const std::string &name);
> > > std::shared_ptr<Camera> get(dev_t devnum);
> > >
> > > - void addCamera(std::shared_ptr<Camera> camera, dev_t devnum);
> > > + void addCamera(std::shared_ptr<Camera> camera,
> > > + std::vector<dev_t> devnums);
> > > void removeCamera(Camera *camera);
> > >
> > > static const std::string &version() { return version_; }
> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > > index da806fa7..fa0bd6a0 100644
> > > --- a/src/libcamera/camera_manager.cpp
> > > +++ b/src/libcamera/camera_manager.cpp
> > > @@ -36,7 +36,8 @@ public:
> > > Private(CameraManager *cm);
> > >
> > > int start();
> > > - void addCamera(std::shared_ptr<Camera> &camera, dev_t devnum);
> > > + void addCamera(std::shared_ptr<Camera> &camera,
> > > + std::vector<dev_t> devnums);
> > > void removeCamera(Camera *camera);
> > >
> > > /*
> > > @@ -168,7 +169,7 @@ void CameraManager::Private::cleanup()
> > > }
> > >
> > > void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,
> > > - dev_t devnum)
> > > + std::vector<dev_t> devnums)
> > > {
> > > MutexLocker locker(mutex_);
> > >
> > > @@ -183,7 +184,7 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,
> > >
> > > cameras_.push_back(std::move(camera));
> > >
> > > - if (devnum) {
> > > + for (dev_t devnum : devnums) {
> > > unsigned int index = cameras_.size() - 1;
> > > camerasByDevnum_[devnum] = cameras_[index];
> > > }
> > > @@ -374,7 +375,7 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> > > /**
> > > * \brief Add a camera to the camera manager
> > > * \param[in] camera The camera to be added
> > > - * \param[in] devnum The device number to associate with \a camera
> > > + * \param[in] devnums The device numbers to associate with \a camera
> > > *
> > > * This function is called by pipeline handlers to register the cameras they
> > > * handle with the camera manager. Registered cameras are immediately made
> > > @@ -385,11 +386,12 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> > > *
> > > * \context This function shall be called from the CameraManager thread.
> > > */
> > > -void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
> > > +void CameraManager::addCamera(std::shared_ptr<Camera> camera,
> > > + std::vector<dev_t> devnums)
> > > {
> > > ASSERT(Thread::current() == p_.get());
> > >
> > > - p_->addCamera(camera, devnum);
> > > + p_->addCamera(camera, devnums);
> > > }
> > >
> > > /**
> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > > index 53aeebdc..b3824a5f 100644
> > > --- a/src/libcamera/pipeline_handler.cpp
> > > +++ b/src/libcamera/pipeline_handler.cpp
> > > @@ -502,7 +502,17 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
> > > data->camera_ = camera.get();
> > > cameraData_[camera.get()] = std::move(data);
> > > cameras_.push_back(camera);
> > > - manager_->addCamera(std::move(camera), devnum);
> > > +
> > > + std::vector<dev_t> devnums;
> > > + if (devnum != 0)
> > > + devnums.push_back(devnum);
> > > + else
> > > + for (const std::shared_ptr<MediaDevice> media : mediaDevices_)
> > > + for (const MediaEntity *entity : media->entities())
> > > + devnums.push_back(makedev(entity->deviceMajor(),
> > > + entity->deviceMinor()));
> > > +
> > > + manager_->addCamera(std::move(camera), devnums);
> > > }
> > >
> > > /**
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> >
> > --
> > Regards,
> > Niklas Söderlund
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list