[libcamera-devel,v5,06/19] libcamera: v4l2_device: Create device from entity name

Message ID 20190326083902.26121-7-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Add ImgU support
Related show

Commit Message

Jacopo Mondi March 26, 2019, 8:38 a.m. UTC
Add a static method to V4L2Device class to create a V4L2Device instance
from a media entity name.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/include/v4l2_device.h |  4 ++++
 src/libcamera/v4l2_device.cpp       | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Laurent Pinchart April 1, 2019, 10:01 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tue, Mar 26, 2019 at 09:38:49AM +0100, Jacopo Mondi wrote:
> Add a static method to V4L2Device class to create a V4L2Device instance
> from a media entity name.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/include/v4l2_device.h |  4 ++++
>  src/libcamera/v4l2_device.cpp       | 22 ++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 258deee8d461..482d7891dd49 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -22,6 +22,7 @@ namespace libcamera {
>  class Buffer;
>  class BufferPool;
>  class EventNotifier;
> +class MediaDevice;
>  class MediaEntity;
>  
>  struct V4L2Capability final : v4l2_capability {
> @@ -107,6 +108,9 @@ public:
>  class V4L2Device : protected Loggable
>  {
>  public:
> +	static V4L2Device *fromEntityName(const MediaDevice *media,
> +					  const std::string &name);
> +

I would move this down, as static methods are usually put after the
non-static ones.

I wonder if there would be an advantage in turning this into a
constructor for the V4L2Device class, but in any case that could be done
later, so, with the above fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	explicit V4L2Device(const std::string &deviceNode);
>  	explicit V4L2Device(const MediaEntity *entity);
>  	V4L2Device(const V4L2Device &) = delete;
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 93d81517fedb..68be5a064258 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -19,6 +19,7 @@
>  #include <libcamera/event_notifier.h>
>  
>  #include "log.h"
> +#include "media_device.h"
>  #include "media_object.h"
>  #include "v4l2_device.h"
>  
> @@ -264,6 +265,27 @@ const std::string V4L2DeviceFormat::toString() const
>   * released.
>   */
>  
> +/**
> + * \brief Create a new video device instance from entity with \a name in
> + * media device \a media
> + * \param[in] media The media device where the entity is registered
> + * \param[in] name The media entity name
> + *
> + * Releasing memory of the newly created instance is responsibility of the
> + * caller of this function.
> + *
> + * \return A newly created V4L2Device on success, nullptr otherwise
> + */
> +V4L2Device *V4L2Device::fromEntityName(const MediaDevice *media,
> +				       const std::string &name)
> +{
> +	MediaEntity *entity = media->getEntityByName(name);
> +	if (!entity)
> +		return nullptr;
> +
> +	return new V4L2Device(entity);
> +}
> +
>  /**
>   * \brief Construct a V4L2Device
>   * \param deviceNode The file-system path to the video device node
Jacopo Mondi April 2, 2019, 8:23 a.m. UTC | #2
Hi Laurent,

On Tue, Apr 02, 2019 at 01:01:37AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Mar 26, 2019 at 09:38:49AM +0100, Jacopo Mondi wrote:
> > Add a static method to V4L2Device class to create a V4L2Device instance
> > from a media entity name.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/include/v4l2_device.h |  4 ++++
> >  src/libcamera/v4l2_device.cpp       | 22 ++++++++++++++++++++++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> > index 258deee8d461..482d7891dd49 100644
> > --- a/src/libcamera/include/v4l2_device.h
> > +++ b/src/libcamera/include/v4l2_device.h
> > @@ -22,6 +22,7 @@ namespace libcamera {
> >  class Buffer;
> >  class BufferPool;
> >  class EventNotifier;
> > +class MediaDevice;
> >  class MediaEntity;
> >
> >  struct V4L2Capability final : v4l2_capability {
> > @@ -107,6 +108,9 @@ public:
> >  class V4L2Device : protected Loggable
> >  {
> >  public:
> > +	static V4L2Device *fromEntityName(const MediaDevice *media,
> > +					  const std::string &name);
> > +
>
> I would move this down, as static methods are usually put after the
> non-static ones.
>

I haven't found this rule anywhere in the code style guides we refer
to. I can move them down if that's how we generally do things.

> I wonder if there would be an advantage in turning this into a
> constructor for the V4L2Device class, but in any case that could be done
> later, so, with the above fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks
  j

>
> >  	explicit V4L2Device(const std::string &deviceNode);
> >  	explicit V4L2Device(const MediaEntity *entity);
> >  	V4L2Device(const V4L2Device &) = delete;
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 93d81517fedb..68be5a064258 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -19,6 +19,7 @@
> >  #include <libcamera/event_notifier.h>
> >
> >  #include "log.h"
> > +#include "media_device.h"
> >  #include "media_object.h"
> >  #include "v4l2_device.h"
> >
> > @@ -264,6 +265,27 @@ const std::string V4L2DeviceFormat::toString() const
> >   * released.
> >   */
> >
> > +/**
> > + * \brief Create a new video device instance from entity with \a name in
> > + * media device \a media
> > + * \param[in] media The media device where the entity is registered
> > + * \param[in] name The media entity name
> > + *
> > + * Releasing memory of the newly created instance is responsibility of the
> > + * caller of this function.
> > + *
> > + * \return A newly created V4L2Device on success, nullptr otherwise
> > + */
> > +V4L2Device *V4L2Device::fromEntityName(const MediaDevice *media,
> > +				       const std::string &name)
> > +{
> > +	MediaEntity *entity = media->getEntityByName(name);
> > +	if (!entity)
> > +		return nullptr;
> > +
> > +	return new V4L2Device(entity);
> > +}
> > +
> >  /**
> >   * \brief Construct a V4L2Device
> >   * \param deviceNode The file-system path to the video device node
>
> --
> Regards,
>
> Laurent Pinchart

Patch

diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
index 258deee8d461..482d7891dd49 100644
--- a/src/libcamera/include/v4l2_device.h
+++ b/src/libcamera/include/v4l2_device.h
@@ -22,6 +22,7 @@  namespace libcamera {
 class Buffer;
 class BufferPool;
 class EventNotifier;
+class MediaDevice;
 class MediaEntity;
 
 struct V4L2Capability final : v4l2_capability {
@@ -107,6 +108,9 @@  public:
 class V4L2Device : protected Loggable
 {
 public:
+	static V4L2Device *fromEntityName(const MediaDevice *media,
+					  const std::string &name);
+
 	explicit V4L2Device(const std::string &deviceNode);
 	explicit V4L2Device(const MediaEntity *entity);
 	V4L2Device(const V4L2Device &) = delete;
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 93d81517fedb..68be5a064258 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -19,6 +19,7 @@ 
 #include <libcamera/event_notifier.h>
 
 #include "log.h"
+#include "media_device.h"
 #include "media_object.h"
 #include "v4l2_device.h"
 
@@ -264,6 +265,27 @@  const std::string V4L2DeviceFormat::toString() const
  * released.
  */
 
+/**
+ * \brief Create a new video device instance from entity with \a name in
+ * media device \a media
+ * \param[in] media The media device where the entity is registered
+ * \param[in] name The media entity name
+ *
+ * Releasing memory of the newly created instance is responsibility of the
+ * caller of this function.
+ *
+ * \return A newly created V4L2Device on success, nullptr otherwise
+ */
+V4L2Device *V4L2Device::fromEntityName(const MediaDevice *media,
+				       const std::string &name)
+{
+	MediaEntity *entity = media->getEntityByName(name);
+	if (!entity)
+		return nullptr;
+
+	return new V4L2Device(entity);
+}
+
 /**
  * \brief Construct a V4L2Device
  * \param deviceNode The file-system path to the video device node