Hi Sakari,

On Friday 30 July 2010 16:00:31 Sakari Ailus wrote:
> Laurent Pinchart wrote:

[snip]

> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > new file mode 100644
> > index 0000000..37a25bf
> > --- /dev/null
> > +++ b/include/media/media-entity.h
> > @@ -0,0 +1,85 @@
> > +#ifndef _MEDIA_ENTITY_H
> > +#define _MEDIA_ENTITY_H
> > +
> > +#include <linux/list.h>
> > +
> > +#define MEDIA_ENTITY_TYPE_NODE                     (1 << 16)
> 
> About that 16 there, could that be replaced by a #define instead? Like
> MEDIA_ENTITY_TYPE_SHIFT? (I don't think we'd need
> MEDIA_ENTITY_SUBTYPE_SHIFT.)

Agreed.

> > +#define MEDIA_ENTITY_TYPE_NODE_V4L         (MEDIA_ENTITY_TYPE_NODE + 1)
> > +#define MEDIA_ENTITY_TYPE_NODE_FB          (MEDIA_ENTITY_TYPE_NODE + 2)
> > +#define MEDIA_ENTITY_TYPE_NODE_ALSA                (MEDIA_ENTITY_TYPE_NODE 
> > + 3)
> > +#define MEDIA_ENTITY_TYPE_NODE_DVB         (MEDIA_ENTITY_TYPE_NODE + 4)
> > +
> > +#define MEDIA_ENTITY_TYPE_SUBDEV           (2 << 16)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER       
> > (MEDIA_ENTITY_TYPE_SUBDEV +
> > 1) +#define
> > MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER        (MEDIA_ENTITY_TYPE_SUBDEV + 2)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_MISC              
> > (MEDIA_ENTITY_TYPE_SUBDEV + 3) 
+
> > +#define MEDIA_LINK_FLAG_ACTIVE                     (1 << 0)
> > +#define MEDIA_LINK_FLAG_IMMUTABLE          (1 << 1)
> > +
> > +#define MEDIA_PAD_FLAG_INPUT                       (1 << 0)
> > +#define MEDIA_PAD_FLAG_OUTPUT                      (1 << 1)
> > +
> > +struct media_link {
> > +   struct media_pad *source;       /* Source pad */
> > +   struct media_pad *sink;         /* Sink pad  */
> > +   struct media_link *other;       /* Link in the reverse direction */
> > +   unsigned long flags;            /* Link flags (MEDIA_LINK_FLAG_*) */
> > +};
> > +
> > +struct media_pad {
> > +   struct media_entity *entity;    /* Entity this pad belongs to */
> > +   u16 index;                      /* Pad index in the entity pads array */
> > +   unsigned long flags;            /* Pad flags (MEDIA_PAD_FLAG_*) */
> > +};
> > +
> > +struct media_entity {
> > +   struct list_head list;
> > +   struct media_device *parent;    /* Media device this entity belongs to*/
> > +   u32 id;                         /* Entity ID, unique in the parent media
> > +                                    * device context */
> > +   const char *name;               /* Entity name */
> > +   u32 type;                       /* Entity type (MEDIA_ENTITY_TYPE_*) */
> > +
> > +   u8 num_pads;                    /* Number of input and output pads */
> 
> Hans suggested u16 for pads. This is a kernel structure but still it'd
> be good to keep them the same IMO, even if that u16 was there for the
> future.
> 
> u8 is used on some function arguments as well for these purposes.

Will fix.

> > +   u8 num_links;                   /* Number of existing links, both active
> > +                                    * and inactive */
> > +   u8 num_backlinks;               /* Number of backlinks */
> > +   u8 max_links;                   /* Maximum number of links */
> 
> Same for these.

Agreed. The number of links is expected to be >= number of pads. I'll change 
num_links, num_backlinks and max_links to u16 as well.

> > +   struct media_pad *pads;         /* Pads array (num_pads elements) */
> > +   struct media_link *links;       /* Links array (max_links elements)*/
> > +
> > +   union {
> > +           /* Node specifications */
> > +           struct {
> > +                   u32 major;
> > +                   u32 minor;
> 
> How about dev_t here?

dev_t doesn't seem to be a public kernel type.

> > +           } v4l;
> > +           struct {
> > +                   u32 major;
> > +                   u32 minor;
> 
> And here.
> 
> > +           } fb;
> > +           int alsa;
> > +           int dvb;
> > +
> > +           /* Sub-device specifications */
> > +           /* Nothing needed yet */
> > +   };
> > +};

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to