[libcamera-devel,v2,1/6] libcamera: v4l2_subdevice: Store media entity
diff mbox series

Message ID 20190225121037.11415-2-jacopo@jmondi.org
State Superseded
Headers show
Series
  • v4l2_(sub)dev: improvements and tests
Related show

Commit Message

Jacopo Mondi Feb. 25, 2019, 12:10 p.m. UTC 
Store the media entity backing the V4L2Subdevice and add a deviceName()
method to retrieve 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 | 5 ++++-
 src/libcamera/v4l2_subdevice.cpp       | 9 ++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Niklas Söderlund Feb. 26, 2019, 2:30 a.m. UTC | #1 
Hi Jacopo,

Thanks for your patch.

On 2019-02-25 13:10:32 +0100, Jacopo Mondi wrote:
> Store the media entity backing the V4L2Subdevice and add a deviceName()
> method to retrieve 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 | 5 ++++-
>  src/libcamera/v4l2_subdevice.cpp       | 9 ++++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index 82fa6685ab52..fcfbee5af106 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -9,9 +9,10 @@
>  
>  #include <string>
>  
> +#include "media_object.h"
> +
>  namespace libcamera {
>  
> -class MediaEntity;
>  struct Rectangle;
>  
>  struct V4L2SubdeviceFormat {
> @@ -32,6 +33,7 @@ public:
>  	void close();
>  
>  	std::string deviceNode() const { return deviceNode_; }
> +	std::string deviceName() const { return entity_->name(); }
>  
>  	int setCrop(unsigned int pad, Rectangle *rect);
>  	int setCompose(unsigned int pad, Rectangle *rect);
> @@ -43,6 +45,7 @@ private:
>  	int setSelection(unsigned int pad, unsigned int target,
>  			 Rectangle *rect);
>  
> +	const MediaEntity *entity_;
>  	std::string deviceNode_;
>  	int fd_;
>  };
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index b436f73cc75f..ebf87f0124cb 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)
>   * path
>   */
>  V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
> -	: deviceNode_(entity->deviceNode()), fd_(-1)
> +	: entity_(entity), deviceNode_(entity->deviceNode()), fd_(-1)

This looks odd. Can't you remove the deviceNode_ member of 
V4L2SubdeviceFormat and use the same schema to retrieve it as 
deviceName?

    std::string deviceNode() const { return entity_->deviceNode(); }
    std::string deviceName() const { return entity_->name(); }

>  {
>  }
>  
> @@ -147,6 +147,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
Jacopo Mondi Feb. 26, 2019, 8:18 a.m. UTC | #2 
Hi Niklas,

On Tue, Feb 26, 2019 at 03:30:25AM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2019-02-25 13:10:32 +0100, Jacopo Mondi wrote:
> > Store the media entity backing the V4L2Subdevice and add a deviceName()
> > method to retrieve 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 | 5 ++++-
> >  src/libcamera/v4l2_subdevice.cpp       | 9 ++++++++-
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index 82fa6685ab52..fcfbee5af106 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -9,9 +9,10 @@
> >
> >  #include <string>
> >
> > +#include "media_object.h"
> > +
> >  namespace libcamera {
> >
> > -class MediaEntity;
> >  struct Rectangle;
> >
> >  struct V4L2SubdeviceFormat {
> > @@ -32,6 +33,7 @@ public:
> >  	void close();
> >
> >  	std::string deviceNode() const { return deviceNode_; }
> > +	std::string deviceName() const { return entity_->name(); }
> >
> >  	int setCrop(unsigned int pad, Rectangle *rect);
> >  	int setCompose(unsigned int pad, Rectangle *rect);
> > @@ -43,6 +45,7 @@ private:
> >  	int setSelection(unsigned int pad, unsigned int target,
> >  			 Rectangle *rect);
> >
> > +	const MediaEntity *entity_;
> >  	std::string deviceNode_;
> >  	int fd_;
> >  };
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index b436f73cc75f..ebf87f0124cb 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)
> >   * path
> >   */
> >  V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
> > -	: deviceNode_(entity->deviceNode()), fd_(-1)
> > +	: entity_(entity), deviceNode_(entity->deviceNode()), fd_(-1)
>
> This looks odd. Can't you remove the deviceNode_ member of
> V4L2SubdeviceFormat and use the same schema to retrieve it as
> deviceName?
>
>     std::string deviceNode() const { return entity_->deviceNode(); }
>     std::string deviceName() const { return entity_->name(); }

deviceNode is used in several places, I considered it was better to
store it to avoid going through accessing the entity_.

I'm sure the advantage is not that significant, so if you prefer so,
I'll change it.

Thanks
  j

>
> >  {
> >  }
> >
> > @@ -147,6 +147,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
>
> --
> Regards,
> Niklas Söderlund

Patch
diff mbox series 

diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
index 82fa6685ab52..fcfbee5af106 100644
--- a/src/libcamera/include/v4l2_subdevice.h
+++ b/src/libcamera/include/v4l2_subdevice.h
@@ -9,9 +9,10 @@ 
 
 #include <string>
 
+#include "media_object.h"
+
 namespace libcamera {
 
-class MediaEntity;
 struct Rectangle;
 
 struct V4L2SubdeviceFormat {
@@ -32,6 +33,7 @@  public:
 	void close();
 
 	std::string deviceNode() const { return deviceNode_; }
+	std::string deviceName() const { return entity_->name(); }
 
 	int setCrop(unsigned int pad, Rectangle *rect);
 	int setCompose(unsigned int pad, Rectangle *rect);
@@ -43,6 +45,7 @@  private:
 	int setSelection(unsigned int pad, unsigned int target,
 			 Rectangle *rect);
 
+	const MediaEntity *entity_;
 	std::string deviceNode_;
 	int fd_;
 };
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index b436f73cc75f..ebf87f0124cb 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -88,7 +88,7 @@  LOG_DEFINE_CATEGORY(V4L2Subdev)
  * path
  */
 V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
-	: deviceNode_(entity->deviceNode()), fd_(-1)
+	: entity_(entity), deviceNode_(entity->deviceNode()), fd_(-1)
 {
 }
 
@@ -147,6 +147,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