[libcamera-devel] [PATCH 3/4] libcamera: media_object: Add functions to entities

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tuesday, 15 January 2019 16:07:48 EET Jacopo Mondi wrote:
> Media entities convey information about their main function in the
> 'function' field of 'struct media_v2_entity'.
> 
> Store the main function in the MediaEntity function_ class member and
> provide a getter function for that.
> 
> While at there update comments and remove a stale TODO entry.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/media_object.h |  2 ++
>  src/libcamera/media_object.cpp       | 18 +++++++++++++-----
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/include/media_object.h
> b/src/libcamera/include/media_object.h index 7fc4441..a10f7e1 100644
> --- a/src/libcamera/include/media_object.h
> +++ b/src/libcamera/include/media_object.h
> @@ -84,6 +84,7 @@ class MediaEntity : public MediaObject
>  {
>  public:
>  	const std::string &name() const { return name_; }
> +	unsigned int function() const { return function_; }
>  	unsigned int deviceMajor() const { return major_; }
>  	unsigned int deviceMinor() const { return minor_; }
> 
> @@ -103,6 +104,7 @@ private:
>  	~MediaEntity();
> 
>  	std::string name_;
> +	unsigned int function_;
>  	std::string devnode_;
>  	unsigned int major_;
>  	unsigned int minor_;
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index cb3af85..c47fb53 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -243,10 +243,8 @@ void MediaPad::addLink(MediaLink *link)
>   * API in the media_v2_entity structure. They reference the pads() they
> contain. *
>   * In addition to its graph id, every media graph entity is identified by a
> - * name() unique in the media device context.
> - *
> - * \todo Add support for associating a devnode to the entity when
> integrating - * with DeviceEnumerator.
> + * name() unique in the media device context, a function() and its
> associated + * devnode, if any.

The entity isn't "identified" by its function or devnode. How about

 * In addition to their graph id, media graph entities are identified by a
 * name() unique in the media device context. Their implement a function(),
 * and may expose a devnode().

By the way the devnode isn't even set in the existing code, neither it is 
accessed :-) Is that an oversight ?

>   */
> 
>  /**
> @@ -255,6 +253,16 @@ void MediaPad::addLink(MediaLink *link)
>   * \return The entity name
>   */
> 
> +/**
> + * \fn MediaEntity::function()
> + * \brief Retrieve the entity main function

s/entity/entity's/ ?

Apart from that,

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

> + *
> + * Media entity functions are expressed using the MEDIA_ENT_F_* macros
> + * defined by the Media Controller API.
> + *
> + * \return The entity function
> + */
> +
>  /**
>   * \fn MediaEntity::deviceMajor()
>   * \brief Retrieve the major number of the interface associated with the
> entity @@ -336,7 +344,7 @@ MediaEntity::MediaEntity(MediaDevice *dev,
>  			 const struct media_v2_entity *entity,
>  			 unsigned int major, unsigned int minor)
>  	: MediaObject(dev, entity->id), name_(entity->name),
> -	  major_(major), minor_(minor)
> +	  function_(entity->function), major_(major), minor_(minor)
>  {
>  }

-- 
Regards,

Laurent Pinchart