[libcamera-devel] [PATCH 01/10] libcamera: media_object: Expose entity type

[Kieran Bingham]

Hi Laurent,

On 05/08/2021 23:24, Laurent Pinchart wrote:
> Add a new field to the MediaEntity class to tell the type of interface

s/tell/identify/

> it exposes to userspace. The MediaEntity constructor is changed to take
> a media_v2_interface pointer instead of just the device node major and
> minor to have access to the interface type.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  include/libcamera/internal/media_object.h | 11 ++++-
>  src/libcamera/media_device.cpp            |  9 +---
>  src/libcamera/media_object.cpp            | 50 +++++++++++++++++++++--
>  3 files changed, 57 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h
> index 2f5d33e1903e..6ae22c679cc5 100644
> --- a/include/libcamera/internal/media_object.h
> +++ b/include/libcamera/internal/media_object.h
> @@ -88,9 +88,17 @@ private:
>  class MediaEntity : public MediaObject
>  {
>  public:
> +	enum class Type {
> +		Invalid,
> +		MediaEntity,
> +		V4L2Subdevice,
> +		V4L2VideoDevice,
> +	};
> +
>  	const std::string &name() const { return name_; }
>  	unsigned int function() const { return function_; }
>  	unsigned int flags() const { return flags_; }
> +	Type type() const { return type_; }
>  	const std::string &deviceNode() const { return deviceNode_; }
>  	unsigned int deviceMajor() const { return major_; }
>  	unsigned int deviceMinor() const { return minor_; }
> @@ -108,13 +116,14 @@ private:
>  	friend class MediaDevice;
>  
>  	MediaEntity(MediaDevice *dev, const struct media_v2_entity *entity,
> -		    unsigned int major = 0, unsigned int minor = 0);
> +		    const struct media_v2_interface *iface);
>  
>  	void addPad(MediaPad *pad);
>  
>  	std::string name_;
>  	unsigned int function_;
>  	unsigned int flags_;
> +	Type type_;
>  	std::string deviceNode_;
>  	unsigned int major_;
>  	unsigned int minor_;
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index fa50264fa253..4d7cb1f566c4 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -652,14 +652,7 @@ bool MediaDevice::populateEntities(const struct media_v2_topology &topology)
>  		 */
>  		struct media_v2_interface *iface =
>  			findInterface(topology, ent->id);
> -
> -		MediaEntity *entity;
> -		if (iface)
> -			entity = new MediaEntity(this, ent,
> -						 iface->devnode.major,
> -						 iface->devnode.minor);
> -		else
> -			entity = new MediaEntity(this, ent);
> +		MediaEntity *entity = new MediaEntity(this, ent, iface);
>  
>  		if (!addObject(entity)) {
>  			delete entity;
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index 815edc8e3b2d..f425d0447241 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -247,6 +247,23 @@ void MediaPad::addLink(MediaLink *link)
>   * may expose a deviceNode().
>   */
>  
> +/**
> + * \enum MediaEntity::Type
> + * \brief The type of the interface exposed by the entity to userspace
> + *
> + * \var MediaEntity::Type::Invalid
> + * \brief Invalid or unsupported entity type
> + *
> + * \var MediaEntity::Type::MediaEntity
> + * \brief Plain media entity with no userspace interface
> + *
> + * \var MediaEntity::Type::V4L2VideoDevice
> + * \brief V4L2 video device with a V4L2 video device node
> + *
> + * \var MediaEntity::Type::V4L2Subdevice
> + * \brief V4L2 subdevice with a V4L2 subdev device node
> + */
> +
>  /**
>   * \fn MediaEntity::name()
>   * \brief Retrieve the entity name
> @@ -273,6 +290,15 @@ void MediaPad::addLink(MediaLink *link)
>   * \return The entity's flags
>   */
>  
> +/**
> + * \fn MediaEntity::type()
> + * \brief Retrieve the entity's type
> + *
> + * The entity type identifies the type of interface exposed to userspace.
> + *
> + * \return The entity's type
> + */
> +
>  /**
>   * \fn MediaEntity::deviceNode()
>   * \brief Retrieve the entity's device node path, if any
> @@ -356,16 +382,32 @@ int MediaEntity::setDeviceNode(const std::string &deviceNode)
>   * \brief Construct a MediaEntity
>   * \param[in] dev The media device this entity belongs to
>   * \param[in] entity The media entity kernel data
> - * \param[in] major The major number of the entity associated interface
> - * \param[in] minor The minor number of the entity associated interface
> + * \param[in] iface The entity interface data (may be null)
>   */
>  MediaEntity::MediaEntity(MediaDevice *dev,
>  			 const struct media_v2_entity *entity,
> -			 unsigned int major, unsigned int minor)
> +			 const struct media_v2_interface *iface)
>  	: MediaObject(dev, entity->id), name_(entity->name),
>  	  function_(entity->function), flags_(entity->flags),
> -	  major_(major), minor_(minor)
> +	  type_(Type::MediaEntity), major_(0), minor_(0)
>  {
> +	if (!iface)
> +		return;
> +
> +	switch (iface->intf_type) {
> +	case MEDIA_INTF_T_V4L_VIDEO:
> +		type_ = Type::V4L2VideoDevice;
> +		break;
> +	case MEDIA_INTF_T_V4L_SUBDEV:
> +		type_ = Type::V4L2Subdevice;
> +		break;
> +	default:
> +		type_ = Type::Invalid;
> +		return;
> +	}
> +
> +	major_ = iface->devnode.major;
> +	minor_ = iface->devnode.minor;
>  }
>  
>  /**
>