[alsa-devel] RFC: PCM extra attributes
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
The caller sets type and len fields. For getting attributes, the len points the max buffer length, and in return, the actual size is stored there. For setting, it's the size to set.
Now, for getting the control ids associated to a PCM substream, it'll be like
unsigned int buf[32]; buf[0] = SNDRV_PCM_ATTR_ASSOC_CTLS; buf[1] = ARRAY_SIZE(buf) - 2; /* minus header size */ ioctl(fd, SNDRV_PCM_IOCTL_GET_ATTR, buf);
The kernel side implementation (a core part) is attached below. In this prototype, each driver is supposed to allocate and set substream->assoc_ctls and substream->num_assoc_ctls appropriately. I chose it since the control ids are statically associated to the PCM substreams at the driver loading time. It could be, of course, a liked list with kmalloc, but I thought it's overkill.
Well, that's a pretty subtle thing and can be changed at any time. But, we need to define ABI properly from the beginning.
So, if you have any comments on ABI or on the whole design, please speak up.
An important note is that I'm planning to use this framework for getting/setting the PCM (surround) channel mapping. It's straightforward for this ABI. The internal implementation may vary, but the interface is the question now. For example, one could imagine that the channel mapping belongs to hw_params. But, it looks difficult because of the nature of channel mapping; it doesn't match with a simple integer parameter as hw_parmas has.
The PCM channel mapping feature will be certainly useful for PA (and even for normal apps with a new alsa-lib plugin to convert the correct the channel mapping properly on demand). So, this is rather more interesting stuff to me, too...
thanks,
Takashi
--- diff --git a/include/sound/asound.h b/include/sound/asound.h index 82aed3f..159826b 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -445,6 +445,21 @@ struct snd_xfern { snd_pcm_uframes_t frames; };
+/* PCM extra attributes */ +struct snd_pcm_attr { + unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ + unsigned int len; /* GET R: the max elements in value array + * W: the actually written # elements + * SET R/W: # elements to store + */ + unsigned int value[0]; /* value(s) to read / write */ +}; + +/* PCM attribute types */ +enum { + SNDRV_PCM_ATTR_ASSOC_CTLS, /* array of associated ctl ids */ +}; + enum { SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0, /* gettimeofday equivalent */ SNDRV_PCM_TSTAMP_TYPE_MONOTONIC, /* posix_clock_monotonic equivalent */ @@ -459,6 +474,8 @@ enum { #define SNDRV_PCM_IOCTL_HW_PARAMS _IOWR('A', 0x11, struct snd_pcm_hw_params) #define SNDRV_PCM_IOCTL_HW_FREE _IO('A', 0x12) #define SNDRV_PCM_IOCTL_SW_PARAMS _IOWR('A', 0x13, struct snd_pcm_sw_params) +#define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) +#define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_STATUS _IOR('A', 0x20, struct snd_pcm_status) #define SNDRV_PCM_IOCTL_DELAY _IOR('A', 0x21, snd_pcm_sframes_t) #define SNDRV_PCM_IOCTL_HWSYNC _IO('A', 0x22) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 2389352..98416b0 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -392,6 +392,10 @@ struct snd_pcm_substream { struct snd_info_entry *proc_prealloc_entry; struct snd_info_entry *proc_prealloc_max_entry; #endif + /* associated controls */ + unsigned int num_assoc_ctls; + unsigned int *assoc_ctls; + /* misc flags */ unsigned int hw_opened: 1; }; diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 145931a..7a46674 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -748,6 +748,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr) substream_next = substream->next; snd_pcm_timer_done(substream); snd_pcm_substream_proc_done(substream); + kfree(substream->assoc_ctls); kfree(substream); substream = substream_next; } diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index 08bfed5..07512b3 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -485,6 +485,8 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_PCM_IOCTL_TTSTAMP: case SNDRV_PCM_IOCTL_HWSYNC: case SNDRV_PCM_IOCTL_PREPARE: + case SNDRV_PCM_IOCTL_GET_ATTR: + case SNDRV_PCM_IOCTL_SET_ATTR: case SNDRV_PCM_IOCTL_RESET: case SNDRV_PCM_IOCTL_START: case SNDRV_PCM_IOCTL_DROP: diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 84da3ba..46129c7 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2492,6 +2492,43 @@ static int snd_pcm_tstamp(struct snd_pcm_substream *substream, int __user *_arg) return 0; } +/* store back the associated ctl ids to the given PCM substream */ +static int snd_pcm_get_attr_assoc_ctls(struct snd_pcm_substream *substream, + unsigned int __user *arg) +{ + unsigned int len, i; + + arg++; + if (get_user(len, arg)) + return -EFAULT; + for (i = 0; i < len && i < substream->num_assoc_ctls; i++) + put_user(substream->assoc_ctls[i], arg + i + 1); + put_user(i, arg); + return 0; +} + +/* GET_ATTR ioctl */ +static int snd_pcm_get_attr(struct snd_pcm_substream *substream, + unsigned int __user *arg) +{ + unsigned int type; + + if (get_user(type, arg)) + return -EFAULT; + switch (type) { + case SNDRV_PCM_ATTR_ASSOC_CTLS: + return snd_pcm_get_attr_assoc_ctls(substream, arg); + } + return -ENOENT; +} + +/* SET_ATTR ioctl */ +static int snd_pcm_set_attr(struct snd_pcm_substream *substream, + struct snd_pcm_attr __user *arg) +{ + return -ENOENT; +} + static int snd_pcm_common_ioctl1(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) @@ -2519,6 +2556,10 @@ static int snd_pcm_common_ioctl1(struct file *file, return snd_pcm_channel_info_user(substream, arg); case SNDRV_PCM_IOCTL_PREPARE: return snd_pcm_prepare(substream, file); + case SNDRV_PCM_IOCTL_GET_ATTR: + return snd_pcm_get_attr(substream, arg); + case SNDRV_PCM_IOCTL_SET_ATTR: + return snd_pcm_set_attr(substream, arg); case SNDRV_PCM_IOCTL_RESET: return snd_pcm_reset(substream); case SNDRV_PCM_IOCTL_START:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
thanks,
Takashi
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Fri, 19 Jun 2009 12:29:17 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
Well, you still missed the difference between the control TLV and the PCM attr I proposed. The former is one-shot information without any query key while the latter is a reply to a query key. The former doesn't work well if you wan to get dynamic data.
Also, see that the data value structure is undefined. The returned data itself can be nested TLV components, if any.
thanks,
Takashi
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 12:29:17 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
Well, you still missed the difference between the control TLV and the PCM attr I proposed. The former is one-shot information without any query key while the latter is a reply to a query key. The former doesn't work well if you wan to get dynamic data.
It will work, but in some cases it's not prefered, of course. I'm just trying to identify common parts of API.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones. TLV_READ might just return all TLVs and TLV_READONE filter only one, if user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user space enumeration (for example for debugging purposes) rather that do a single query for all possible TLV types.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Fri, 19 Jun 2009 12:45:04 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones. TLV_READ might just return all TLVs and TLV_READONE filter only one, if user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user space enumeration (for example for debugging purposes) rather that do a single query for all possible TLV types.
I disagree with "all-in-on-TLV" strategy. That makes life much harder. Sorry, it's no go.
The current control TLV works because it's only for dB information. If we were to give more different data types such as volatile data, it would have been a nightmare. The PCM attributes can be such a dynamic one. For example the channel map can be changed differently when you change runtime->channels or formats. "One-for-all" really doesn't server well even for such a practical case...
thanks,
Takashi
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 12:45:04 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
/* PCM extra attributes */ struct snd_pcm_attr { unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ unsigned int len; /* GET R: the max elements in value array * W: the actually written # elements * SET R/W: # elements to store */ unsigned int value[0]; /* value(s) to read / write */ };
And corresponding two ioctls #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones. TLV_READ might just return all TLVs and TLV_READONE filter only one, if user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user space enumeration (for example for debugging purposes) rather that do a single query for all possible TLV types.
I disagree with "all-in-on-TLV" strategy. That makes life much harder. Sorry, it's no go.
Sorry, the implementation can be more simple than you think. Imagine just a TLV callback in the lowlevel driver and switch/case statement in the callback. We can define a special type in kernel that queries for all present TLV types (bitmap) and the ioctl TLV_READ implementation can just compose results from single queries. So, the code in the lowlevel driver will grow only with 4 (or less) lines:
case TLV_TYPES: tlv.length = 4; tlv.data[0] = (1<<TLV_CHANNEL_MAP) | (1<<TLV_CONTROL_IDS); return 0;
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Fri, 19 Jun 2009 13:14:15 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 12:45:04 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
> Hi, > > this is yet another topic I'm (currently) working on -- the addition > of PCM ioctls to get/set some extra attributes. Basically, it adds > two simple ioctls for getting/setting extra attributes to the PCM > substream. The attribute has a sort of TLV form, > > /* PCM extra attributes */ > struct snd_pcm_attr { > unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ > unsigned int len; /* GET R: the max elements in value array > * W: the actually written # elements > * SET R/W: # elements to store > */ > unsigned int value[0]; /* value(s) to read / write */ > }; > > And corresponding two ioctls > #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) > #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
I would prefer to implement similar TLV implementation as for the control API. The amount of information for reading (get) will be small, so filtering in this direction is not necessary. Also, common parts of implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones. TLV_READ might just return all TLVs and TLV_READONE filter only one, if user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user space enumeration (for example for debugging purposes) rather that do a single query for all possible TLV types.
I disagree with "all-in-on-TLV" strategy. That makes life much harder. Sorry, it's no go.
Sorry, the implementation can be more simple than you think. Imagine just a TLV callback in the lowlevel driver and switch/case statement in the callback. We can define a special type in kernel that queries for all present TLV types (bitmap) and the ioctl TLV_READ implementation can just compose results from single queries. So, the code in the lowlevel driver will grow only with 4 (or less) lines:
case TLV_TYPES: tlv.length = 4; tlv.data[0] = (1<<TLV_CHANNEL_MAP) | (1<<TLV_CONTROL_IDS); return 0;
Sorry I can't draw a picture from your explanation. How can it compose a "all-in-one" TLV at each time?
What I'm talking is like the scenario below:
- app open PCM, do hw_params - app requires the channel map, get one - app changes hw_params again, then get channel map again
With "all-in-one" TLV, you have to read all the information at each time you get channel maps because the channel map is to be generated dynamically. At each time, the kernel needs to gather all information, compose into a single big TLV, and copy it. Then user-space decomposes the big TLV, look for a specific tag, then picks up the value there.
Why not simply query a value and get a value a la normal ioctl?
Takashi
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 13:14:15 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 12:45:04 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), Jaroslav Kysela wrote: > > On Fri, 19 Jun 2009, Takashi Iwai wrote: > >> Hi, >> >> this is yet another topic I'm (currently) working on -- the addition >> of PCM ioctls to get/set some extra attributes. Basically, it adds >> two simple ioctls for getting/setting extra attributes to the PCM >> substream. The attribute has a sort of TLV form, >> >> /* PCM extra attributes */ >> struct snd_pcm_attr { >> unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ >> unsigned int len; /* GET R: the max elements in value array >> * W: the actually written # elements >> * SET R/W: # elements to store >> */ >> unsigned int value[0]; /* value(s) to read / write */ >> }; >> >> And corresponding two ioctls >> #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) >> #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr) > > I would prefer to implement similar TLV implementation as for the control > API. The amount of information for reading (get) will be small, so > filtering in this direction is not necessary. Also, common parts of > implementation (future merging of more TLVs to compounds) can be shared.
Actually it's a sort of TLV. You see exactly it in snd_pcm_attr struct, no? :)
And, thinking twice after posting (that's a good effect of posting to ML, BTW), I feel that using a callback would be a better way, such as re-using the existing ops->ioctl with a new cmd tag rather than the statically assigned thing.
A similar method like control TLV can be used, too. However, a distinct from the existing control TLV is that this is intended for just one type of information while the control TLV is supposed to contain everything in a single shot.
That is, this is a query with a key. In that sense, sharing a small amount of control TLV code (about 10 lines) doesn't give a big benefit. In anyways, it's a implementation detail, so one could optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones. TLV_READ might just return all TLVs and TLV_READONE filter only one, if user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user space enumeration (for example for debugging purposes) rather that do a single query for all possible TLV types.
I disagree with "all-in-on-TLV" strategy. That makes life much harder. Sorry, it's no go.
Sorry, the implementation can be more simple than you think. Imagine just a TLV callback in the lowlevel driver and switch/case statement in the callback. We can define a special type in kernel that queries for all present TLV types (bitmap) and the ioctl TLV_READ implementation can just compose results from single queries. So, the code in the lowlevel driver will grow only with 4 (or less) lines:
case TLV_TYPES: tlv.length = 4; tlv.data[0] = (1<<TLV_CHANNEL_MAP) | (1<<TLV_CONTROL_IDS); return 0;
Sorry I can't draw a picture from your explanation. How can it compose a "all-in-one" TLV at each time?
The tlv_read ioctl will be something like this (simplified without proper length and allocation handling):
tlv_types.type = TLV_TYPES; tlv_callback(tlv_types); idx = 0; while (tlv_types[0] && (tlv_types[0] & (1 << idx)) != 0) { tlv.type = idx; tlv_callback(tlv); merge_tlv(result, tlv); tlv_types[0] &= ~(1 << idx); idx++; }
What I'm talking is like the scenario below:
- app open PCM, do hw_params
- app requires the channel map, get one
- app changes hw_params again, then get channel map again
With "all-in-one" TLV, you have to read all the information at each time you get channel maps because the channel map is to be generated dynamically. At each time, the kernel needs to gather all information, compose into a single big TLV, and copy it. Then user-space decomposes the big TLV, look for a specific tag, then picks up the value there.
Why not simply query a value and get a value a la normal ioctl?
I'm talking about to have both "all-in-one" and single value ioctls. I see your optimization when one value is required to read multiple times. I just prefer to make things similar to the control interface (although single value read is not implemented in current control API, but it might be added later, if required). I would like to see also common naming in interfaces like:
SNDRV_PCM_IOCTL_TLV_READ # read all SNDRV_PCM_IOCTL_TLV_COMMAND # write one or more TLVs (COMPOUND type) SNDRV_PCM_IOCTL_TLV_TREAD # read only one TLV specified by type # TREAD might be also READONE or so..
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Fri, 19 Jun 2009 18:58:46 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 13:14:15 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
At Fri, 19 Jun 2009 12:45:04 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
On Fri, 19 Jun 2009, Takashi Iwai wrote:
> At Fri, 19 Jun 2009 10:47:30 +0200 (CEST), > Jaroslav Kysela wrote: >> >> On Fri, 19 Jun 2009, Takashi Iwai wrote: >> >>> Hi, >>> >>> this is yet another topic I'm (currently) working on -- the addition >>> of PCM ioctls to get/set some extra attributes. Basically, it adds >>> two simple ioctls for getting/setting extra attributes to the PCM >>> substream. The attribute has a sort of TLV form, >>> >>> /* PCM extra attributes */ >>> struct snd_pcm_attr { >>> unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */ >>> unsigned int len; /* GET R: the max elements in value array >>> * W: the actually written # elements >>> * SET R/W: # elements to store >>> */ >>> unsigned int value[0]; /* value(s) to read / write */ >>> }; >>> >>> And corresponding two ioctls >>> #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr) >>> #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr) >> >> I would prefer to implement similar TLV implementation as for the control >> API. The amount of information for reading (get) will be small, so >> filtering in this direction is not necessary. Also, common parts of >> implementation (future merging of more TLVs to compounds) can be shared. > > Actually it's a sort of TLV. You see exactly it in snd_pcm_attr > struct, no? :) > > And, thinking twice after posting (that's a good effect of posting to > ML, BTW), I feel that using a callback would be a better way, such as > re-using the existing ops->ioctl with a new cmd tag rather than the > statically assigned thing. > > A similar method like control TLV can be used, too. However, a > distinct from the existing control TLV is that this is intended for > just one type of information while the control TLV is supposed to > contain everything in a single shot. > > That is, this is a query with a key. In that sense, sharing a small > amount of control TLV code (about 10 lines) doesn't give a big > benefit. In anyways, it's a implementation detail, so one could > optimize somehow, though...
I don't mean current implementation. TLVs can be nested. In this case, we need a set of functions which operates with TLVs (merging). These functions can be shared. It's also possible to share TLV code in the user space (search). But it's really implementation detail. We should focus on ioctl definitions now.
I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as for control API.
The control API has:
SNDRV_CTL_IOCTL_TLV_READ - read all static information SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls) SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your proposal.
SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual user-space PCM interface kernel implementation.
SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information which don't change between open()/close() syscalls for given substream.
SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones. TLV_READ might just return all TLVs and TLV_READONE filter only one, if user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user space enumeration (for example for debugging purposes) rather that do a single query for all possible TLV types.
I disagree with "all-in-on-TLV" strategy. That makes life much harder. Sorry, it's no go.
Sorry, the implementation can be more simple than you think. Imagine just a TLV callback in the lowlevel driver and switch/case statement in the callback. We can define a special type in kernel that queries for all present TLV types (bitmap) and the ioctl TLV_READ implementation can just compose results from single queries. So, the code in the lowlevel driver will grow only with 4 (or less) lines:
case TLV_TYPES: tlv.length = 4; tlv.data[0] = (1<<TLV_CHANNEL_MAP) | (1<<TLV_CONTROL_IDS); return 0;
Sorry I can't draw a picture from your explanation. How can it compose a "all-in-one" TLV at each time?
The tlv_read ioctl will be something like this (simplified without proper length and allocation handling):
tlv_types.type = TLV_TYPES; tlv_callback(tlv_types); idx = 0; while (tlv_types[0] && (tlv_types[0] & (1 << idx)) != 0) { tlv.type = idx; tlv_callback(tlv); merge_tlv(result, tlv); tlv_types[0] &= ~(1 << idx); idx++; }
What I'm talking is like the scenario below:
- app open PCM, do hw_params
- app requires the channel map, get one
- app changes hw_params again, then get channel map again
With "all-in-one" TLV, you have to read all the information at each time you get channel maps because the channel map is to be generated dynamically. At each time, the kernel needs to gather all information, compose into a single big TLV, and copy it. Then user-space decomposes the big TLV, look for a specific tag, then picks up the value there.
Why not simply query a value and get a value a la normal ioctl?
I'm talking about to have both "all-in-one" and single value ioctls. I see your optimization when one value is required to read multiple times. I just prefer to make things similar to the control interface (although single value read is not implemented in current control API, but it might be added later, if required).
The single read *is* required. That's why I don't want "all-in-one" TLV at all because it makes thing more complicated in such a case.
I would like to see also common naming in interfaces like:
SNDRV_PCM_IOCTL_TLV_READ # read all SNDRV_PCM_IOCTL_TLV_COMMAND # write one or more TLVs (COMPOUND type) SNDRV_PCM_IOCTL_TLV_TREAD # read only one TLV specified by type # TREAD might be also READONE or so..
We can forget about the compatibility with the control API first. The benefit comes only if the all-in-one TLV API fits better than other simpler ones.
The advantage of all-in-one TLV is when the app wants to know *everything*, like a number 42. But, is such a situation really often? No, mostly you want to know a certain information.
Or, think from a different direction: what's wrong when the driver accepts a special query key "GIVE_ME_ALL" that returns the all-in-one TLV? Then we don't need three calls but just two.
thanks,
Takashi
2009/6/19 Takashi Iwai tiwai@suse.de:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
How is the association done between the pcm and the control? I would have thought an easy approach could be (from userland): snd_pcm_open( snd_pcm_t **handle, ...); snd_pcm_get_attr( handle, int attribute_id, *attributeX ); snd_pcm_set_attr( handle, int attribute_id, *attributeX );
I.e Use the snd_pcm_t handle from the snd_pcm_open call to query and set any controls associated with the pcm. We could then remove those controls from being viewable in alsamixer and leave it to the application to control them. The only controls left in alsamixer would then be the global controls. E.g. Speaker arrangement, master gain control etc. The result being that applications would never need to access the alsamixer controls, and instead only need to use the snd_pcm_get/set_attr interface on a per PCM basis that is much more in line with what applications actually need.
An important note is that I'm planning to use this framework for getting/setting the PCM (surround) channel mapping.
Why would we want this channel mapping info in user space? Can't we just standardize on a single channel mapping as seen from user space, and get the driver to do any adaption if needed?
Kind Regards
James
At Fri, 19 Jun 2009 12:58:21 +0100, James Courtier-Dutton wrote:
2009/6/19 Takashi Iwai tiwai@suse.de:
Hi,
this is yet another topic I'm (currently) working on -- the addition of PCM ioctls to get/set some extra attributes. Basically, it adds two simple ioctls for getting/setting extra attributes to the PCM substream. The attribute has a sort of TLV form,
How is the association done between the pcm and the control? I would have thought an easy approach could be (from userland): snd_pcm_open( snd_pcm_t **handle, ...); snd_pcm_get_attr( handle, int attribute_id, *attributeX ); snd_pcm_set_attr( handle, int attribute_id, *attributeX );
The ioctls aren't visible explicitly on alsa-lib. What I initially thought of is the function like:
int snd_pcm_get_assoc_ctls(snd_pcm_t *, int num_ids, snd_ctl_id_t *ids);
I.e Use the snd_pcm_t handle from the snd_pcm_open call to query and set any controls associated with the pcm. We could then remove those controls from being viewable in alsamixer and leave it to the application to control them.
The alsamixer view isn't the issue I'm concerned. These spontaneous controls should be IFACE_PCM in the first place. They shouldn't be IFACE_MIXER. For example, IEC958 status bits can be associated to a dedicated PCM substream.
One question is the visible volume control such as VIA DXS Volume. But, I feel we should move them also to IFACE_PCM space and hide from the alsamixer view.
The only controls left in alsamixer would then be the global controls. E.g. Speaker arrangement, master gain control etc.
There will be still many strange controls that don't belong to specific PCM substreams :)
The result being that applications would never need to access the alsamixer controls, and instead only need to use the snd_pcm_get/set_attr interface on a per PCM basis that is much more in line with what applications actually need.
An important note is that I'm planning to use this framework for getting/setting the PCM (surround) channel mapping.
Why would we want this channel mapping info in user space?
Because we don't know what channel to assign to which slot. Whether the third channel is a center or a rear-left?
Can't we just standardize on a single channel mapping as seen from user space, and get the driver to do any adaption if needed?
We assume the standard channel map, but unfortunately many hardwares don't follow and are unable to remap. Thus currently it's done in alsa-lib route plugin.
However, there are devices (e.g. HD-audio) that require different channel maps for different streams depending on chip. Since the alsa-lib config is static, it can't be changed well on the fly. Thus, the driver needs first to provide some information about the channel map so that alsa-lib (or PA) can handle the remapping appropriately.
In addition, there are demands from apps to use different chanenl maps. FL/FR/C/LFE/RL/RR is far more standard except for ALSA. We are definitely minor. So, sometimes it's good to adapt the majority's choice...
thanks,
Takashi
2009/6/19 Takashi Iwai tiwai@suse.de:
The alsamixer view isn't the issue I'm concerned. These spontaneous controls should be IFACE_PCM in the first place. They shouldn't be IFACE_MIXER. For example, IEC958 status bits can be associated to a dedicated PCM substream.
Would an option could be to stream the IEC958 status bits with the PCM stream? I.e. As an extra channel of the PCM multichannel stream. Another option could be to stream the gain control values with the PCM stream. I.e. Instead of attributes attached to the PCM, stream those attributes with the PCM stream.
At Fri, 19 Jun 2009 13:43:08 +0100, James Courtier-Dutton wrote:
2009/6/19 Takashi Iwai tiwai@suse.de:
The alsamixer view isn't the issue I'm concerned. These spontaneous controls should be IFACE_PCM in the first place. They shouldn't be IFACE_MIXER. For example, IEC958 status bits can be associated to a dedicated PCM substream.
Would an option could be to stream the IEC958 status bits with the PCM stream? I.e. As an extra channel of the PCM multichannel stream.
I don't buy it because the status bits don't need resend at each time. And for compatibility reason, this is a huge gap.
If any, we have already IEC958 raw subframe support that embeds the status bits...
Another option could be to stream the gain control values with the PCM stream. I.e. Instead of attributes attached to the PCM, stream those attributes with the PCM stream.
Hrm how can it be done? Could you elaborate?
Takashi
2009/6/19 Takashi Iwai tiwai@suse.de:
Another option could be to stream the gain control values with the PCM stream. I.e. Instead of attributes attached to the PCM, stream those attributes with the PCM stream.
Hrm how can it be done? Could you elaborate?
You said that you were adding a method to identify the channel maps. I.e. Is Front the first of second channel in the stream. Add extra channels in the stream that can be tagged as non-audio data and also tag the sort of non audio data they are. They could be gain values, time stamps, etc. So, basically just an extra way to capture metadata on a per sample basis. For example, when capturing this sample, the analog input gain was set to X. Obviously we don't have the functionality in the driver itself yet, but making sure the api could permit this would be useful.
At Wed, 24 Jun 2009 18:18:48 +0100, James Courtier-Dutton wrote:
2009/6/19 Takashi Iwai tiwai@suse.de:
Another option could be to stream the gain control values with the PCM stream. I.e. Instead of attributes attached to the PCM, stream those attributes with the PCM stream.
Hrm how can it be done? Could you elaborate?
You said that you were adding a method to identify the channel maps. I.e. Is Front the first of second channel in the stream. Add extra channels in the stream that can be tagged as non-audio data and also tag the sort of non audio data they are. They could be gain values, time stamps, etc. So, basically just an extra way to capture metadata on a per sample basis. For example, when capturing this sample, the analog input gain was set to X. Obviously we don't have the functionality in the driver itself yet, but making sure the api could permit this would be useful.
I've discussed with V4L guys about a similar idea to sending timestamps together with the PCM stream. The concern was how to handle it, who multiplexes and who decodes. Anyway, they weren't impressed much by this.
This kind of thing is indeed technically interesting, but there aren't so many real use cases. It reminds me the fact that SPDIF raw format is rarely used :) But, timestamping would be a good candidate requiring this kind of extension.
thanks,
Takashi
participants (3)
-
James Courtier-Dutton -
Jaroslav Kysela -
Takashi Iwai