Message ID | 20250404074624.2975182-2-paul.elder@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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@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 ? > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Paul Elder <paul.elder@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
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@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@ideasonboard.com> > > Signed-off-by: Paul Elder <paul.elder@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
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