[libcamera-devel,v2,2/5] libcamera: media_object: Add functions to entities

251 diff mbox series

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, keep the MediaPad description in sync
with the MediaEntity one and remove a stale TODO entry.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/include/media_object.h |  2 ++
 src/libcamera/media_object.cpp       | 22 +++++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

Hi Jacopo,

Thanks for your work.

On 2019-01-16 14:59:46 +0100, 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, keep the MediaPad description in sync
> with the MediaEntity one and remove a stale TODO entry.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

I would have split this patch in two, one updating the documentation and 
one adding the new functionality. If you choose to keep this as one or 
split it in to feel free to add to (both)

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/include/media_object.h |  2 ++
>  src/libcamera/media_object.cpp       | 22 +++++++++++++++-------
>  2 files changed, 17 insertions(+), 7 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..76dd326 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -166,7 +166,7 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,
>   * Pads are created from the information provided by the Media Controller API
>   * in the media_v2_pad structure. They reference the entity() they belong to.
>   *
> - * In addition to its graph id, every media graph pad is identified by an index
> + * In addition to its graph id, media graph pads are identified by an index
>   * unique in the context of the entity the pad belongs to.
>   *
>   * A pad can be either a 'source' pad or a 'sink' pad. This information is
> @@ -242,11 +242,9 @@ void MediaPad::addLink(MediaLink *link)
>   * Entities are created from the information provided by the Media Controller
>   * 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.
> + * In addition to its graph id, media graph entities are identified by a
> + * name() unique in the media device context. They implement a function() and
> + * may expose a devnode().
>   */
>  
>  /**
> @@ -255,6 +253,16 @@ void MediaPad::addLink(MediaLink *link)
>   * \return The entity name
>   */
>  
> +/**
> + * \fn MediaEntity::function()
> + * \brief Retrieve the entity's main function
> + *
> + * Media entity functions are expressed using the MEDIA_ENT_F_* macros
> + * defined by the Media Controller API.
> + *
> + * \return The entity's 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)
>  {
>  }
>  
> -- 
> 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 Wed, Jan 16, 2019 at 02:59:46PM +0100, 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, keep the MediaPad description in sync
> with the MediaEntity one and remove a stale TODO entry.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/include/media_object.h |  2 ++
>  src/libcamera/media_object.cpp       | 22 +++++++++++++++-------
>  2 files changed, 17 insertions(+), 7 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..76dd326 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -166,7 +166,7 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,
>   * Pads are created from the information provided by the Media Controller API
>   * in the media_v2_pad structure. They reference the entity() they belong to.
>   *
> - * In addition to its graph id, every media graph pad is identified by an index
> + * In addition to its graph id, media graph pads are identified by an index

s/its/their/

>   * unique in the context of the entity the pad belongs to.
>   *
>   * A pad can be either a 'source' pad or a 'sink' pad. This information is
> @@ -242,11 +242,9 @@ void MediaPad::addLink(MediaLink *link)
>   * Entities are created from the information provided by the Media Controller
>   * 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.
> + * In addition to its graph id, media graph entities are identified by a

Ditto.

With these fixed you can keep my ack and push this patch.

> + * name() unique in the media device context. They implement a function() and
> + * may expose a devnode().
>   */
>  
>  /**
> @@ -255,6 +253,16 @@ void MediaPad::addLink(MediaLink *link)
>   * \return The entity name
>   */
>  
> +/**
> + * \fn MediaEntity::function()
> + * \brief Retrieve the entity's main function
> + *
> + * Media entity functions are expressed using the MEDIA_ENT_F_* macros
> + * defined by the Media Controller API.
> + *
> + * \return The entity's 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)
>  {
>  }
>