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

[Laurent Pinchart]

Hi Jacopo,

On Tuesday, 15 January 2019 18:06:58 EET Jacopo Mondi wrote:
> On Tue, Jan 15, 2019 at 05:57:27PM +0200, Laurent Pinchart wrote:
> > 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(-)

[snip]

> >> 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.

I'm just nitpicking on the wording :-)

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

Oops.

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

Unless I'm mistaken, setDeviceNode() doesn't assign devnode_, and devnode_ is 
never used.

> >>   */

[snip]

-- 
Regards,

Laurent Pinchart