[libcamera-devel] [PATCH] libcamera: CameraManager, PipelineHandler: Allow multiple devnums per camera

[Laurent Pinchart]

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.

> > 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