[alsa-devel] RFC: PCM extra attributes

[Takashi Iwai]

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: