| Message ID | 20190219165620.2385-3-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your patch. On 2019-02-19 17:56:19 +0100, Jacopo Mondi wrote: > Add a deviceName_ field that contains the human readable name of the > subdevice, which is created using the name of the associated media > entity. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/v4l2_subdevice.h | 2 ++ > src/libcamera/v4l2_subdevice.cpp | 9 ++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h > index c7045776555c..40becd0ca99b 100644 > --- a/src/libcamera/include/v4l2_subdevice.h > +++ b/src/libcamera/include/v4l2_subdevice.h > @@ -42,6 +42,7 @@ public: > void close(); > > std::string deviceNode() const { return deviceNode_; } > + std::string deviceName() const { return deviceName_; } > > int setCrop(unsigned int pad, Rectangle *rect); > int setCompose(unsigned int pad, Rectangle *rect); > @@ -55,6 +56,7 @@ private: > Rectangle *rect); > > std::string deviceNode_; > + std::string deviceName_; > int fd_; > }; > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index 5665154a2762..4411ffa51460 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -134,7 +134,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev) > * path > */ > V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity) > - : deviceNode_(entity->deviceNode()), fd_(-1) > + : deviceNode_(entity->deviceNode()), deviceName_(entity->name()), fd_(-1) > { > } > > @@ -193,6 +193,13 @@ void V4L2Subdevice::close() > * \return The subdevice's device node system path > */ > > +/** > + * \fn V4L2Subdevice::deviceName() > + * \brief Retrieve the name of the media entity associated with the subdevice > + * > + * \return The name of the media entity the subdevice is associated to > + */ > + > /** > * \brief Set a crop rectangle on one of the V4L2 subdevice pads > * \param[in] pad The 0-indexed pad number the rectangle is to be applied to > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thank you for the patch. On Tue, Feb 19, 2019 at 05:56:19PM +0100, Jacopo Mondi wrote: > Add a deviceName_ field that contains the human readable name of the > subdevice, which is created using the name of the associated media > entity. How about naming this name() instead of deviceName() ? I also wonder if it would make sense to store a pointer to the media entity, in order to allow easy access to other fields, instead of duplicating the information. We wouldn't need to store deviceNode_ in that case, just return entity_->deviceNode(). > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/include/v4l2_subdevice.h | 2 ++ > src/libcamera/v4l2_subdevice.cpp | 9 ++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h > index c7045776555c..40becd0ca99b 100644 > --- a/src/libcamera/include/v4l2_subdevice.h > +++ b/src/libcamera/include/v4l2_subdevice.h > @@ -42,6 +42,7 @@ public: > void close(); > > std::string deviceNode() const { return deviceNode_; } > + std::string deviceName() const { return deviceName_; } > > int setCrop(unsigned int pad, Rectangle *rect); > int setCompose(unsigned int pad, Rectangle *rect); > @@ -55,6 +56,7 @@ private: > Rectangle *rect); > > std::string deviceNode_; > + std::string deviceName_; > int fd_; > }; > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index 5665154a2762..4411ffa51460 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -134,7 +134,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev) > * path > */ > V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity) > - : deviceNode_(entity->deviceNode()), fd_(-1) > + : deviceNode_(entity->deviceNode()), deviceName_(entity->name()), fd_(-1) > { > } > > @@ -193,6 +193,13 @@ void V4L2Subdevice::close() > * \return The subdevice's device node system path > */ > > +/** > + * \fn V4L2Subdevice::deviceName() > + * \brief Retrieve the name of the media entity associated with the subdevice > + * > + * \return The name of the media entity the subdevice is associated to > + */ > + > /** > * \brief Set a crop rectangle on one of the V4L2 subdevice pads > * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
Hi Jacopo, Laurent, On 2019-02-22 01:19:49 +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Tue, Feb 19, 2019 at 05:56:19PM +0100, Jacopo Mondi wrote: > > Add a deviceName_ field that contains the human readable name of the > > subdevice, which is created using the name of the associated media > > entity. > > How about naming this name() instead of deviceName() ? > > I also wonder if it would make sense to store a pointer to the media > entity, in order to allow easy access to other fields, instead of > duplicating the information. We wouldn't need to store deviceNode_ in > that case, just return entity_->deviceNode(). I like the idea. > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/include/v4l2_subdevice.h | 2 ++ > > src/libcamera/v4l2_subdevice.cpp | 9 ++++++++- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h > > index c7045776555c..40becd0ca99b 100644 > > --- a/src/libcamera/include/v4l2_subdevice.h > > +++ b/src/libcamera/include/v4l2_subdevice.h > > @@ -42,6 +42,7 @@ public: > > void close(); > > > > std::string deviceNode() const { return deviceNode_; } > > + std::string deviceName() const { return deviceName_; } > > > > int setCrop(unsigned int pad, Rectangle *rect); > > int setCompose(unsigned int pad, Rectangle *rect); > > @@ -55,6 +56,7 @@ private: > > Rectangle *rect); > > > > std::string deviceNode_; > > + std::string deviceName_; > > int fd_; > > }; > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > index 5665154a2762..4411ffa51460 100644 > > --- a/src/libcamera/v4l2_subdevice.cpp > > +++ b/src/libcamera/v4l2_subdevice.cpp > > @@ -134,7 +134,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev) > > * path > > */ > > V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity) > > - : deviceNode_(entity->deviceNode()), fd_(-1) > > + : deviceNode_(entity->deviceNode()), deviceName_(entity->name()), fd_(-1) > > { > > } > > > > @@ -193,6 +193,13 @@ void V4L2Subdevice::close() > > * \return The subdevice's device node system path > > */ > > > > +/** > > + * \fn V4L2Subdevice::deviceName() > > + * \brief Retrieve the name of the media entity associated with the subdevice > > + * > > + * \return The name of the media entity the subdevice is associated to > > + */ > > + > > /** > > * \brief Set a crop rectangle on one of the V4L2 subdevice pads > > * \param[in] pad The 0-indexed pad number the rectangle is to be applied to > > -- > Regards, > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h index c7045776555c..40becd0ca99b 100644 --- a/src/libcamera/include/v4l2_subdevice.h +++ b/src/libcamera/include/v4l2_subdevice.h @@ -42,6 +42,7 @@ public: void close(); std::string deviceNode() const { return deviceNode_; } + std::string deviceName() const { return deviceName_; } int setCrop(unsigned int pad, Rectangle *rect); int setCompose(unsigned int pad, Rectangle *rect); @@ -55,6 +56,7 @@ private: Rectangle *rect); std::string deviceNode_; + std::string deviceName_; int fd_; }; diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp index 5665154a2762..4411ffa51460 100644 --- a/src/libcamera/v4l2_subdevice.cpp +++ b/src/libcamera/v4l2_subdevice.cpp @@ -134,7 +134,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev) * path */ V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity) - : deviceNode_(entity->deviceNode()), fd_(-1) + : deviceNode_(entity->deviceNode()), deviceName_(entity->name()), fd_(-1) { } @@ -193,6 +193,13 @@ void V4L2Subdevice::close() * \return The subdevice's device node system path */ +/** + * \fn V4L2Subdevice::deviceName() + * \brief Retrieve the name of the media entity associated with the subdevice + * + * \return The name of the media entity the subdevice is associated to + */ + /** * \brief Set a crop rectangle on one of the V4L2 subdevice pads * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
Add a deviceName_ field that contains the human readable name of the subdevice, which is created using the name of the associated media entity. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/include/v4l2_subdevice.h | 2 ++ src/libcamera/v4l2_subdevice.cpp | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)