lkml.org 
[lkml]   [2010]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links
Date
Hi Clemens,

On Monday 13 December 2010 17:10:51 Clemens Ladisch wrote:
> I wrote:
> > I'll see if I can draw up the ALSA-specific media stuff over the weekend.
>
> Sorry, wrong weekend.
>
> Anyway, below are some remarks and a patch.

Thank you. Please see my comments inline.

> * Entity types
>
> TYPE_NODE was renamed to TYPE_DEVICE because "node" sounds like a node
> in a graph, which does not distinguish it from other entity types
> because all entities are part of the topology graph. I chose "device"
> as this type describes entities that are visible as some device node to
> other software.

What this type describes is a device node. Both NODE and DEVICE can be
confusing in my opinion, but DEVICE_NODE is a bit long.

> TYPE_EXT describes entities that represent some interface to the
> external world, TYPE_INT those that are internal to the entire device.
> (I'm not sure if that distinction is very useful, but TYPE_SUBDEV seems
> to be an even more meaningless name.)

SUBDEV comes from the V4L2 world, and I agree that it might not be a very good
name.

I'm not sure I would split entities in internal/external categories. I would
create a category for connectors though.

> ALSA mixer controls are not directly represented; a better fit for the
> architecture of actual devices is that one or more mixer controls can be
> associated with an entity. (This can be done with a field of the mixer
> control.)

Agreed.

> * Entity properties
>
> There needs to be a mechanism to associate meta-information (properties)
> with entities. This information should be optional and extensible, but,
> when being handled inside the kernel, doesn't need to be more than
> a read-only blob. I think that something like ALSA's TLV format (used
> for mixer controls) can be used here. (I'm not mentioning the X-word
> here, except to note that the "M" stands for "markup".)

I've been thinking of adding a new ioctl for that. It's something we need to
draft. The UVC driver will need it, and I'm pretty sure other V4L2 drivers
would find it useful as well.

> * Entity subtypes
>
> EXT_JACK_ANALOG represents any analog audio and/or video connector.
> Properties for audio jacks would be jack type (TRS/RCA), color code,
> line level, position, etc.
>
> EXT_JACK_DIGITAL represents a digital connector like S/PDIF (coax/
> TOSLINK), ADAT, TDIF, or MADI.
>
> EXT_JACK_BUS represents a bus like FireWire and comes from the USB audio
> spec. (I doubt that any devices with this entitiy will ever exist.)
>
> EXT_INSTRUMENT represents something like an e-guitar, keyboard, or MIDI
> controller. (Instrument entities are typically audio sources and MIDI
> sources and sinks, but can also be audio sinks.)
>
> EXT_SPEAKER also includes headphones; there might be made a case for
> having those as a separate subtype.

Shouldn't headphones be represented by an EXT_JACK_ANALOG ?

> EXT_PLAYER represents a device like a CD/DVD/tape player. Recorders can
> also write to that device, so "player" might not be an ideal name.
>
> EXT_BROADCAST represents devices like TV tuners, satellite receivers,
> cable tuners, or radios.

There's clearly an overlap with V4L here. Hopefully someone from the linux-
media list can comment on this.

> INT_SYNTHESIZER converts MIDI to audio.
>
> INT_NOISE_SOURCE comes from the USB audio spec; this is not an attempt
> to describe the characteristics of consumer-grade devices :-) , but
> represents an internal noise source for level calibration or measurements.
>
> INT_CONTROLS may have multiple independent controls (this is USB's
> Feature Unit); INT_EFFECT may have multiple controls that affect one
> single algorithm.

I'd describe this as a feature unit/processing unit then.

> INT_CHANNEL_SPLIT/MERGE are needed for HDAudio devices, whose topology
> information has only stereo links.

Some of those INT entities could also be implemented in dedicated chips, so I
really think the EXT/INT split doesn't make too much sense. Should we have an
AUDIO category ?

> * Entity specifications
>
> While TYPE_DEVICE entities can be identified by their device node, other
> entities typcially have just a numeric ID.

In V4L2 sub-devices have (or rather will have once the media controller
patches will be integrated) device nodes as well, so exposing that information
is required.

> For that, it would be useful to make do without separate identification and
> let the driver choose the entity ID.

How would drivers do that ? What if you have two instances of the same chip (a
video sensor, audio mixer, ...) on the same board ?

> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>
> --- linux/include/linux/media.h
> +++ linux/include/linux/media.h
> @@ -46,16 +46,36 @@ struct media_device_info {
> #define MEDIA_ENTITY_TYPE_MASK 0x00ff0000
> #define MEDIA_ENTITY_SUBTYPE_MASK 0x0000ffff
>
> -#define MEDIA_ENTITY_TYPE_NODE (1 << MEDIA_ENTITY_TYPE_SHIFT)
> -#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_DEVICE (1 << MEDIA_ENTITY_TYPE_SHIFT)
> +#define MEDIA_ENTITY_TYPE_DEVICE_V4L (MEDIA_ENTITY_TYPE_DEVICE + 1)
> +#define MEDIA_ENTITY_TYPE_DEVICE_FB (MEDIA_ENTITY_TYPE_DEVICE + 2)
> +#define MEDIA_ENTITY_TYPE_DEVICE_DVB (MEDIA_ENTITY_TYPE_DEVICE + 3)
> +#define MEDIA_ENTITY_TYPE_DEVICE_ALSA_PCM (MEDIA_ENTITY_TYPE_DEVICE + 4)
> +#define MEDIA_ENTITY_TYPE_DEVICE_ALSA_MIDI (MEDIA_ENTITY_TYPE_DEVICE + 5)
>
> -#define MEDIA_ENTITY_TYPE_SUBDEV (2 << MEDIA_ENTITY_TYPE_SHIFT)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_SENSOR (MEDIA_ENTITY_TYPE_SUBDEV + 1)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_FLASH (MEDIA_ENTITY_TYPE_SUBDEV + 2)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_LENS (MEDIA_ENTITY_TYPE_SUBDEV + 3)
> +#define MEDIA_ENTITY_TYPE_EXT (2 << MEDIA_ENTITY_TYPE_SHIFT)
> +#define MEDIA_ENTITY_TYPE_EXT_SENSOR (MEDIA_ENTITY_TYPE_EXT + 1)
> +#define MEDIA_ENTITY_TYPE_EXT_FLASH (MEDIA_ENTITY_TYPE_EXT + 2)
> +#define MEDIA_ENTITY_TYPE_EXT_LENS (MEDIA_ENTITY_TYPE_EXT + 3)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_MIDI (MEDIA_ENTITY_TYPE_EXT + 4)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_ANALOG (MEDIA_ENTITY_TYPE_EXT + 5)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_DIGITAL (MEDIA_ENTITY_TYPE_EXT + 6)
> +#define MEDIA_ENTITY_TYPE_EXT_JACK_BUS (MEDIA_ENTITY_TYPE_EXT + 7)
> +#define MEDIA_ENTITY_TYPE_EXT_INSTRUMENT (MEDIA_ENTITY_TYPE_EXT + 8)
> +#define MEDIA_ENTITY_TYPE_EXT_SPEAKER (MEDIA_ENTITY_TYPE_EXT + 9)
> +#define MEDIA_ENTITY_TYPE_EXT_MICROPHONE (MEDIA_ENTITY_TYPE_EXT + 10)
> +#define MEDIA_ENTITY_TYPE_EXT_PLAYER (MEDIA_ENTITY_TYPE_EXT + 11)
> +#define MEDIA_ENTITY_TYPE_EXT_BROADCAST (MEDIA_ENTITY_TYPE_EXT + 12)
> +
> +#define MEDIA_ENTITY_TYPE_INT (3 << MEDIA_ENTITY_TYPE_SHIFT)
> +#define MEDIA_ENTITY_TYPE_INT_SYNTHESIZER (MEDIA_ENTITY_TYPE_INT + 1)
> +#define MEDIA_ENTITY_TYPE_INT_NOISE_SOURCE (MEDIA_ENTITY_TYPE_INT + 2)
> +#define MEDIA_ENTITY_TYPE_INT_MIXER (MEDIA_ENTITY_TYPE_INT + 3)
> +#define MEDIA_ENTITY_TYPE_INT_SELECTOR (MEDIA_ENTITY_TYPE_INT + 4)
> +#define MEDIA_ENTITY_TYPE_INT_CONTROLS (MEDIA_ENTITY_TYPE_INT + 5)
> +#define MEDIA_ENTITY_TYPE_INT_EFFECT (MEDIA_ENTITY_TYPE_INT + 6)
> +#define MEDIA_ENTITY_TYPE_INT_CHANNEL_SPLIT (MEDIA_ENTITY_TYPE_INT + 7)
> +#define MEDIA_ENTITY_TYPE_INT_CHANNEL_MERGE (MEDIA_ENTITY_TYPE_INT + 8)
>
> #define MEDIA_ENTITY_FLAG_DEFAULT (1 << 0)
>
> @@ -72,7 +92,7 @@ struct media_entity_desc {
> __u32 reserved[4];
>
> union {
> - /* Node specifications */
> + /* Device specifications */
> struct {
> __u32 major;
> __u32 minor;
> @@ -81,11 +101,15 @@ struct media_entity_desc {
> __u32 major;
> __u32 minor;
> } fb;
> - int alsa;
> + struct {
> + __u32 card;
> + __u32 device;
> + __s32 subdevice;
> + } alsa;

I will already incorporate this change, and I'll wait for other opinions on
the types before changing them.

> int dvb;
>
> /* Sub-device specifications */
> /* Nothing needed yet */
> __u8 raw[184];
> };
> };

--
Regards,

Laurent Pinchart


\
 
 \ /
  Last update: 2010-12-14 13:03    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site