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

[Jacopo Mondi]

Hi Laurent,

On Tue, Jan 15, 2019 at 05:57:27PM +0200, Laurent Pinchart wrote:
> 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

I tried summarize here what 'identity' informations a media entity
transports. If that's confusing I'll remove it.

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

They

>  * 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 ?

I didn't get this... The devnode *might* be set.. I think that's
captured in your comment here above, so I'll include that.


>
> >   */
> >
> >  /**
> > @@ -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>
>

Thanks
  j

> > + *
> > + * 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
>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190115/6229c677/attachment.sig>