[libcamera-devel] [PATCH v4 5/5] libcamera: pipeline: Register device numbers with camera

[Kieran Bingham]

Quoting Jacopo Mondi (2023-06-16 07:53:14)
> Hi Kieran
> 
> On Thu, Jun 15, 2023 at 06:26:08PM +0100, Kieran Bingham wrote:
> > Register the identified device numbers with each camera as the Devices
> 
> This is now SystemDevices
> 
> > property.
> >
> > This facilitates camera daemons or other systems to identify which
> > devices are being managed by libcamera, and can prevent duplication of
> > camera resources.
> >
> > As the Devices property now provides this list of devices, use it
> 
> ditto :)
> 
> > directly from within the CameraManager when adding a Camera rather than
> > passing it through the internal API.
> >
> > Tested-by: Ashok Sidipotu <ashok.sidipotu at collabora.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > ---
> > v4
> >  - Rename property
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  include/libcamera/internal/camera_manager.h |  3 +--
> >  src/libcamera/camera_manager.cpp            | 14 +++++++++-----
> >  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--
> >  3 files changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h
> > index 84aac499ea13..cdf009a9c626 100644
> > --- a/include/libcamera/internal/camera_manager.h
> > +++ b/include/libcamera/internal/camera_manager.h
> > @@ -35,8 +35,7 @@ public:
> >       Private();
> >
> >       int start();
> > -     void addCamera(std::shared_ptr<Camera> camera,
> > -                    const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
> > +     void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
> >       void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
> >
> >       /*
> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > index cafd7bce574e..31d45c42fde0 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -11,7 +11,9 @@
> >  #include <libcamera/base/utils.h>
> >
> >  #include <libcamera/camera.h>
> > +#include <libcamera/property_ids.h>
> >
> > +#include "libcamera/internal/camera.h"
> >  #include "libcamera/internal/device_enumerator.h"
> >  #include "libcamera/internal/pipeline_handler.h"
> >
> > @@ -151,19 +153,17 @@ void CameraManager::Private::cleanup()
> >  /**
> >   * \brief Add a camera to the camera manager
> >   * \param[in] camera The camera to be added
> > - * \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
> >   * available to the system.
> >   *
> > - * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes
> > - * to Camera instances.
> > + * Device numbers from the Devices property are used by the V4L2 compatibility
> 
> here as well

Ayeee thanks.

I'll hold of a bit longer, but I think that's the last fix before I push
these. If there's nothing else I'll fix those and push.

--
Kieran


> 
> > + * layer to map V4L2 device nodes to Camera instances.
> >   *
> >   * \context This function shall be called from the CameraManager thread.
> >   */
> > -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
> > -                                    const std::vector<dev_t> &devnums)
> > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
> >  {
> >       ASSERT(Thread::current() == this);
> >
> > @@ -178,6 +178,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
> >               }
> >       }
> >
> > +     auto devnums = camera->properties()
> > +                            .get(properties::SystemDevices)
> > +                            .value_or(Span<int64_t>{});
> > +
> >       cameras_.push_back(std::move(camera));
> >
> >       unsigned int index = cameras_.size() - 1;
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 49092ea88a58..9c74c6cfda70 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -17,6 +17,7 @@
> >
> >  #include <libcamera/camera.h>
> >  #include <libcamera/framebuffer.h>
> > +#include <libcamera/property_ids.h>
> >
> >  #include "libcamera/internal/camera.h"
> >  #include "libcamera/internal/camera_manager.h"
> > @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
> >        * Walk the entity list and map the devnums of all capture video nodes
> >        * to the camera.
> >        */
> > -     std::vector<dev_t> devnums;
> > +     std::vector<int64_t> devnums;
> >       for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {
> >               for (const MediaEntity *entity : media->entities()) {
> >                       if (entity->pads().size() == 1 &&
> > @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
> >               }
> >       }
> >
> > -     manager_->_d()->addCamera(std::move(camera), devnums);
> > +     /*
> > +      * Store the associated devices as a property of the camera to allow
> > +      * systems to identify which devices are managed by libcamera.
> > +      */
> > +     Camera::Private *data = camera->_d();
> > +     data->properties_.set(properties::SystemDevices, devnums);
> > +
> > +     manager_->_d()->addCamera(std::move(camera));
> >  }
> >
> >  /**
> > --
> > 2.34.1
> >