[PATCH 1/3] libcamera: v4l2: Support fromEntityName with shared_ptr<MediaDevice>

[Kieran Bingham]

Quoting Laurent Pinchart (2025-04-06 19:00:08)
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Fri, Apr 04, 2025 at 04:46:21PM +0900, Paul Elder wrote:
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > To facilitate expanding the use of a shared_ptr<MediaDevice> in
> > preference of a raw MediaDevice* pointer, add an additional
> > implementation for fromEntityName for both of v4l2_videodevice and
> > v4l2_subdevice to support this type.
> 
> Is this really worth it, to just avoid a .get() call in the caller ?

I felt it was cleaner/less churn, and meant that both types could be
supported - with shorter lines in the case of a shared_ptr.

(Note that this patch was preceeding a conversion of call locations
from being a MediaDevice*  to a shared_ptr<const MediaDevice>)

Would you rather see this patch dropped and have the next patch convert
all current users of fromEntityName() call .get on MediaDevice pointers?

> 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  include/libcamera/internal/v4l2_subdevice.h   |  2 ++
> >  include/libcamera/internal/v4l2_videodevice.h |  2 ++
> >  src/libcamera/v4l2_subdevice.cpp              | 15 +++++++++++++++
> >  src/libcamera/v4l2_videodevice.cpp            | 15 +++++++++++++++
> >  4 files changed, 34 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index fa2a4a21eefb..08216cd56bc3 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -163,6 +163,8 @@ public:
> >  
> >       static std::unique_ptr<V4L2Subdevice>
> >       fromEntityName(const MediaDevice *media, const std::string &entity);
> > +     static std::unique_ptr<V4L2Subdevice>
> > +     fromEntityName(std::shared_ptr<const MediaDevice>, const std::string &entity);
> >  
> >  protected:
> >       std::string logPrefix() const override;
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index ae6a76cb0209..e35fb11bc99f 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -228,6 +228,8 @@ public:
> >  
> >       static std::unique_ptr<V4L2VideoDevice>
> >       fromEntityName(const MediaDevice *media, const std::string &entity);
> > +     static std::unique_ptr<V4L2VideoDevice>
> > +     fromEntityName(std::shared_ptr<const MediaDevice>, const std::string &entity);
> >  
> >       V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const;
> >  
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 33279654db8c..13f5316a3fc3 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -1766,6 +1766,21 @@ V4L2Subdevice::fromEntityName(const MediaDevice *media,
> >       return std::make_unique<V4L2Subdevice>(mediaEntity);
> >  }
> >  
> > +/**
> > + * \brief Create a new video subdevice instance from \a entity in media device
> > + * \a media
> > + * \param[in] media The media device where the entity is registered
> > + * \param[in] entity The media entity name
> > + *
> > + * \return A newly created V4L2Subdevice on success, nullptr otherwise
> > + */
> > +std::unique_ptr<V4L2Subdevice>
> > +V4L2Subdevice::fromEntityName(std::shared_ptr<const MediaDevice> media,
> > +                           const std::string &entity)
> > +{
> > +     return fromEntityName(media.get(), entity);
> > +}
> > +
> >  std::string V4L2Subdevice::logPrefix() const
> >  {
> >       return "'" + entity_->name() + "'";
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index f5b3fa09d9a0..ce7be7deb91f 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -2109,6 +2109,21 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,
> >       return std::make_unique<V4L2VideoDevice>(mediaEntity);
> >  }
> >  
> > +/**
> > + * \brief Create a new video device instance from \a entity in media device
> > + * \a media
> > + * \param[in] media The media device where the entity is registered
> > + * \param[in] entity The media entity name
> > + *
> > + * \return A newly created V4L2VideoDevice on success, nullptr otherwise
> > + */
> > +std::unique_ptr<V4L2VideoDevice>
> > +V4L2VideoDevice::fromEntityName(std::shared_ptr<const MediaDevice> media,
> > +                             const std::string &entity)
> > +{
> > +     return fromEntityName(media.get(), entity);
> > +}
> > +
> >  /**
> >   * \brief Convert \a PixelFormat to a V4L2PixelFormat supported by the device
> >   * \param[in] pixelFormat The PixelFormat to convert
> 
> -- 
> Regards,
> 
> Laurent Pinchart