[alsa-lib][PATCH 0/5] ctl: support extra information for user-defined element set

[alsa-lib][PATCH 0/5] ctl: support extra information for user-defined element set

[Takashi Sakamoto]

Hi,

This patchset adds support extra information to the type-specific for
user-defined element set. Currently, 'dimension' is such an extra
information.

In ALSA kernel/userspace interface, information to an element is described
in this structure.

struct snd_ctl_elem_info {
    struct snd_ctl_elem_id id;
    unsigned int access;
    unsigned int count;
    pid_t owner;
    union value;

    union dimen;
    unsigned char reserved[64 - 4 * sizeof(unsigned short)];
};

This structure includes reserved fields, thus it's possible to add
more fields for future extension.

Currently, APIs to add user-defined element set don't support such extra
fields. Meanwhile, supporting just the dimension field is not good as
stable library APIs.

This patchset changes prototype of the APIs with 'snd_ctl_elem_info_t'
instead of adding more parameters. Callers are expected to fill the parameter
with identification information and the extra information.


Takashi Sakamoto (5):
  ctl: support extra information in user-defined element set
  ctl: add an API to set dimension levels to element information
  ctl: optimize a test for user-defined element set to older kernels
  ctl: optimize a test for user-defined element set to changes of APIs
  ctl: support dimension test for user-defined element set

 include/control.h           |  12 ++-
 src/control/control.c       | 219 +++++++++++++++++++++++---------------------
 src/pcm/pcm_softvol.c       |  10 +-
 test/user-ctl-element-set.c | 122 ++++++++++++++++++------
 4 files changed, 220 insertions(+), 143 deletions(-)

-- 
2.7.4

[PATCH 1/5] ctl: support extra information to user-defined element set

[Takashi Sakamoto]

In ALSA control feature, information of an element includes extra fields
to type-specific parameters; i.e. dimension. The fields can be extended in
future.

Meanwhile, current APIs to add user-defined element set can not support
such an extended fields. This may cause inconveniences in future.

This commit supports the fields, by changing APIs for element set.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/control.h     |  10 +--
 src/control/control.c | 189 ++++++++++++++++++++++----------------------------
 src/pcm/pcm_softvol.c |  10 +--
 3 files changed, 94 insertions(+), 115 deletions(-)

diff --git a/include/control.h b/include/control.h
index 13b0d4e..b14edee 100644
--- a/include/control.h
+++ b/include/control.h
@@ -423,24 +423,24 @@ void snd_ctl_elem_info_set_subdevice(snd_ctl_elem_info_t *obj, unsigned int val)
 void snd_ctl_elem_info_set_name(snd_ctl_elem_info_t *obj, const char *val);
 void snd_ctl_elem_info_set_index(snd_ctl_elem_info_t *obj, unsigned int val);
 
-int snd_ctl_elem_add_integer_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_integer_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				 unsigned int element_count,
 				 unsigned int member_count,
 				 long min, long max, long step);
-int snd_ctl_elem_add_integer64_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_integer64_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				   unsigned int element_count,
 				   unsigned int member_count,
 				   long long min, long long max,
 				   long long step);
-int snd_ctl_elem_add_boolean_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_boolean_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				 unsigned int element_count,
 				 unsigned int member_count);
-int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				    unsigned int element_count,
 				    unsigned int member_count,
 				    unsigned int items,
 				    const char *const labels[]);
-int snd_ctl_elem_add_bytes_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_bytes_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 			       unsigned int element_count,
 			       unsigned int member_count);
 
diff --git a/src/control/control.c b/src/control/control.c
index c7fcbd2..70b166b 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -305,7 +305,8 @@ int snd_ctl_elem_info(snd_ctl_t *ctl, snd_ctl_elem_info_t *info)
 /**
  * \brief Create and add some user-defined control elements of integer type.
  * \param ctl A handle of backend module for control interface.
- * \param id ID of the first new element.
+ * \param info Common iformation for a new element set, with ID of the first new
+ *	       element.
  * \param element_count The number of elements added by this operation.
  * \param member_count The number of members which a element has to
  *			   represent its states.
@@ -342,38 +343,36 @@ int snd_ctl_elem_info(snd_ctl_t *ctl, snd_ctl_elem_info_t *info)
  * \par Compatibility:
  * This function is added in version 1.1.2.
  */
-int snd_ctl_elem_add_integer_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_integer_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				 unsigned int element_count,
 				 unsigned int member_count,
 				 long min, long max, long step)
 {
-	snd_ctl_elem_info_t info = {0};
 	snd_ctl_elem_value_t data = {0};
 	unsigned int i;
 	unsigned int j;
 	unsigned int numid;
 	int err;
 
-	assert(ctl && id && id->name[0]);
+	assert(ctl && info && info->id.name[0]);
 
-	info.id = *id;
-	info.type = SND_CTL_ELEM_TYPE_INTEGER;
-	info.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
-	              SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
-	              SNDRV_CTL_ELEM_ACCESS_USER;
-	info.owner = element_count;
-	info.count = member_count;
-	info.value.integer.min = min;
-	info.value.integer.max = max;
-	info.value.integer.step = step;
-
-	err = ctl->ops->element_add(ctl, &info);
+	info->type = SND_CTL_ELEM_TYPE_INTEGER;
+	info->access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_USER;
+	info->owner = element_count;
+	info->count = member_count;
+	info->value.integer.min = min;
+	info->value.integer.max = max;
+	info->value.integer.step = step;
+
+	err = ctl->ops->element_add(ctl, info);
 	if (err < 0)
 		return err;
-	numid = snd_ctl_elem_id_get_numid(&info.id);
+	numid = snd_ctl_elem_id_get_numid(&info->id);
 
 	/* Set initial value to all of members in all of added elements. */
-	data.id = info.id;
+	data.id = info->id;
 	for (i = 0; i < element_count; i++) {
 		snd_ctl_elem_id_set_numid(&data.id, numid + i);
 
@@ -385,14 +384,14 @@ int snd_ctl_elem_add_integer_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
 			return err;
 	}
 
-	*id = info.id;
 	return 0;
 }
 
 /**
  * \brief Create and add some user-defined control elements of integer64 type.
  * \param ctl A handle of backend module for control interface.
- * \param id ID of the first new control element.
+ * \param info Common iformation for a new element set, with ID of the first new
+ *	       element.
  * \param element_count The number of elements added by this operation.
  * \param member_count The number of members which a element has to
  *	    	   represent its states.
@@ -429,38 +428,36 @@ int snd_ctl_elem_add_integer_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
  * \par Compatibility:
  * This function is added in version 1.1.2.
  */
-int snd_ctl_elem_add_integer64_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_integer64_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				   unsigned int element_count,
 				   unsigned int member_count,
 				   long long min, long long max, long long step)
 {
-	snd_ctl_elem_info_t info = {0};
 	snd_ctl_elem_value_t data = {0};
 	unsigned int i;
 	unsigned int j;
 	unsigned int numid;
 	int err;
 
-	assert(ctl && id && id->name[0]);
+	assert(ctl && info && info->id.name[0]);
 
-	info.id = *id;
-	info.type = SND_CTL_ELEM_TYPE_INTEGER64;
-	info.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_USER;
-	info.owner = element_count;
-	info.count = member_count;
-	info.value.integer64.min = min;
-	info.value.integer64.max = max;
-	info.value.integer64.step = step;
-
-	err = ctl->ops->element_add(ctl, &info);
+	info->type = SND_CTL_ELEM_TYPE_INTEGER64;
+	info->access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_USER;
+	info->owner = element_count;
+	info->count = member_count;
+	info->value.integer64.min = min;
+	info->value.integer64.max = max;
+	info->value.integer64.step = step;
+
+	err = ctl->ops->element_add(ctl, info);
 	if (err < 0)
 		return err;
-	numid = snd_ctl_elem_id_get_numid(&info.id);
+	numid = snd_ctl_elem_id_get_numid(&info->id);
 
 	/* Set initial value to all of members in all of added elements. */
-	data.id = info.id;
+	data.id = info->id;
 	for (i = 0; i < element_count; i++) {
 		snd_ctl_elem_id_set_numid(&data.id, numid + i);
 
@@ -472,14 +469,14 @@ int snd_ctl_elem_add_integer64_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
 			return err;
 	}
 
-	*id = info.id;
 	return 0;
 }
 
 /**
  * \brief Create and add some user-defined control elements of boolean type.
  * \param ctl A handle of backend module for control interface.
- * \param id ID of the new control element.
+ * \param info Common iformation for a new element set, with ID of the first new
+ *	       element.
  * \param element_count The number of elements added by this operation.
  * \param member_count The number of members which a element has to
  *			   represent its states.
@@ -512,36 +509,29 @@ int snd_ctl_elem_add_integer64_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
  * \par Compatibility:
  * This function is added in version 1.1.2.
  */
-int snd_ctl_elem_add_boolean_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_boolean_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				 unsigned int element_count,
 				 unsigned int member_count)
 {
-	snd_ctl_elem_info_t info = {0};
-	int err;
-
-	assert(ctl && id && id->name[0]);
+	assert(ctl && info && info->id.name[0]);
 
-	info.id = *id;
-	info.type = SND_CTL_ELEM_TYPE_BOOLEAN;
-	info.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_USER;
-	info.owner = element_count;
-	info.count = member_count;
-	info.value.integer.min = 0;
-	info.value.integer.max = 1;
-
-	err = ctl->ops->element_add(ctl, &info);
-	if (err >= 0)
-		*id = info.id;
+	info->type = SND_CTL_ELEM_TYPE_BOOLEAN;
+	info->access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_USER;
+	info->owner = element_count;
+	info->count = member_count;
+	info->value.integer.min = 0;
+	info->value.integer.max = 1;
 
-	return err;
+	return ctl->ops->element_add(ctl, info);
 }
 
 /**
  * \brief Create and add some user-defined control elements of enumerated type.
  * \param ctl A handle of backend module for control interface.
- * \param id ID of the first new element.
+ * \param info Common iformation for a new element set, with ID of the first new
+ *	       element.
  * \param element_count The number of elements added by this operation.
  * \param member_count The number of members which a element has to
  *	    	   represent its states.
@@ -579,27 +569,25 @@ int snd_ctl_elem_add_boolean_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
  * \par Compatibility:
  * This function is added in version 1.1.2.
  */
-int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 				    unsigned int element_count,
 				    unsigned int member_count,
 				    unsigned int items,
 				    const char *const labels[])
 {
-	snd_ctl_elem_info_t info = {0};
 	unsigned int i, bytes;
 	char *buf, *p;
 	int err;
 
-	assert(ctl && id && id->name[0] && labels);
+	assert(ctl && info && info->id.name[0] && labels);
 
-	info.id = *id;
-	info.type = SND_CTL_ELEM_TYPE_ENUMERATED;
-	info.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_USER;
-	info.owner = element_count;
-	info.count = member_count;
-	info.value.enumerated.items = items;
+	info->type = SND_CTL_ELEM_TYPE_ENUMERATED;
+	info->access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_USER;
+	info->owner = element_count;
+	info->count = member_count;
+	info->value.enumerated.items = items;
 
 	bytes = 0;
 	for (i = 0; i < items; ++i)
@@ -609,17 +597,15 @@ int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
 	buf = malloc(bytes);
 	if (buf == NULL)
 		return -ENOMEM;
-	info.value.enumerated.names_ptr = (uintptr_t)buf;
-	info.value.enumerated.names_length = bytes;
+	info->value.enumerated.names_ptr = (uintptr_t)buf;
+	info->value.enumerated.names_length = bytes;
 	p = buf;
 	for (i = 0; i < items; ++i) {
 		strcpy(p, labels[i]);
 		p += strlen(labels[i]) + 1;
 	}
 
-	err = ctl->ops->element_add(ctl, &info);
-	if (err >= 0)
-		*id = info.id;
+	err = ctl->ops->element_add(ctl, info);
 
 	free(buf);
 
@@ -629,7 +615,8 @@ int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
 /**
  * \brief Create and add some user-defined control elements of bytes type.
  * \param ctl A handle of backend module for control interface.
- * \param id ID of the first new element.
+ * \param info Common iformation for a new element set, with ID of the first new
+ *	       element.
  * \param element_count The number of elements added by this operation.
  * \param member_count The number of members which a element has to
  *			   represent its states.
@@ -663,28 +650,20 @@ int snd_ctl_elem_add_enumerated_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
  * \par Compatibility:
  * This function is added in version 1.1.2.
  */
-int snd_ctl_elem_add_bytes_set(snd_ctl_t *ctl, snd_ctl_elem_id_t *id,
+int snd_ctl_elem_add_bytes_set(snd_ctl_t *ctl, snd_ctl_elem_info_t *info,
 			       unsigned int element_count,
 			       unsigned int member_count)
 {
-	snd_ctl_elem_info_t info = {0};
-	int err;
-
-	assert(ctl && id && id->name[0]);
+	assert(ctl && info && info->id.name[0]);
 
-	info.id = *id;
-	info.type = SND_CTL_ELEM_TYPE_BYTES;
-	info.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
-		      SNDRV_CTL_ELEM_ACCESS_USER;
-	info.owner = element_count;
-	info.count = member_count;
-
-	err = ctl->ops->element_add(ctl, &info);
-	if (err >= 0)
-		*id = info.id;
+	info->type = SND_CTL_ELEM_TYPE_BYTES;
+	info->access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
+		       SNDRV_CTL_ELEM_ACCESS_USER;
+	info->owner = element_count;
+	info->count = member_count;
 
-	return err;
+	return ctl->ops->element_add(ctl, info);
 }
 
 /**
@@ -698,11 +677,11 @@ int snd_ctl_elem_add_integer(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			     unsigned int member_count,
 			     long min, long max, long step)
 {
-	snd_ctl_elem_id_t local_id = {0};
+	snd_ctl_elem_info_t info = {0};
 
-	local_id = *id;
+	info.id = *id;
 
-	return snd_ctl_elem_add_integer_set(ctl, &local_id, 1, member_count,
+	return snd_ctl_elem_add_integer_set(ctl, &info, 1, member_count,
 					    min, max, step);
 }
 
@@ -717,11 +696,11 @@ int snd_ctl_elem_add_integer64(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			       unsigned int member_count,
 			       long long min, long long max, long long step)
 {
-	snd_ctl_elem_id_t local_id = {0};
+	snd_ctl_elem_info_t info = {0};
 
-	local_id = *id;
+	info.id = *id;
 
-	return snd_ctl_elem_add_integer64_set(ctl, &local_id, 1, member_count,
+	return snd_ctl_elem_add_integer64_set(ctl, &info, 1, member_count,
 					      min, max, step);
 }
 
@@ -735,11 +714,11 @@ int snd_ctl_elem_add_integer64(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 int snd_ctl_elem_add_boolean(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			     unsigned int member_count)
 {
-	snd_ctl_elem_id_t local_id = {0};
+	snd_ctl_elem_info_t info = {0};
 
-	local_id = *id;
+	info.id = *id;
 
-	return snd_ctl_elem_add_boolean_set(ctl, &local_id, 1, member_count);
+	return snd_ctl_elem_add_boolean_set(ctl, &info, 1, member_count);
 }
 
 /**
@@ -755,11 +734,11 @@ int snd_ctl_elem_add_enumerated(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 				unsigned int member_count, unsigned int items,
 				const char *const labels[])
 {
-	snd_ctl_elem_id_t local_id = {0};
+	snd_ctl_elem_info_t info = {0};
 
-	local_id = *id;
+	info.id = *id;
 
-	return snd_ctl_elem_add_enumerated_set(ctl, &local_id, 1, member_count,
+	return snd_ctl_elem_add_enumerated_set(ctl, &info, 1, member_count,
 					       items, labels);
 }
 
diff --git a/src/pcm/pcm_softvol.c b/src/pcm/pcm_softvol.c
index 459ff8e..a667c85 100644
--- a/src/pcm/pcm_softvol.c
+++ b/src/pcm/pcm_softvol.c
@@ -663,18 +663,18 @@ static int add_tlv_info(snd_pcm_softvol_t *svol, snd_ctl_elem_info_t *cinfo)
 	return snd_ctl_elem_tlv_write(svol->ctl, &cinfo->id, tlv);
 }
 
-static int add_user_ctl(snd_pcm_softvol_t *svol, snd_ctl_elem_info_t *cinfo, int count)
+static int add_user_ctl(snd_pcm_softvol_t *svol, snd_ctl_elem_info_t *cinfo,
+			int count)
 {
 	int err;
 	int i;
 	unsigned int def_val;
 	
 	if (svol->max_val == 1)
-		err = snd_ctl_elem_add_boolean_set(svol->ctl, &cinfo->id, 1,
-						   count);
+		err = snd_ctl_elem_add_boolean_set(svol->ctl, cinfo, 1, count);
 	else
-		err = snd_ctl_elem_add_integer_set(svol->ctl, &cinfo->id, 1,
-						   count, 0, svol->max_val, 0);
+		err = snd_ctl_elem_add_integer_set(svol->ctl, cinfo, 1, count,
+						   0, svol->max_val, 0);
 	if (err < 0)
 		return err;
 	if (svol->max_val == 1)
-- 
2.7.4

[PATCH 2/5] ctl: add an API to set dimension levels to element information

[Takashi Sakamoto]

In a former commit, 'struct snd_ctl_elem_info' is used as a 'container' to
transfer extra fields of element information for APIs to add an element
set. The extra fields should be filled in advance of call of the APIs.
Currently, dimension level is in the extra fields and no APIs to set it.

This commit adds an API to set dimension level to the information
structure. This API is expected to be used in advance of usage of APIs
to add an element set, for nothing others. When the information structure
is extended in future, then the similar APIs shall be added for the new
feature.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/control.h     |  2 ++
 src/control/control.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/control.h b/include/control.h
index b14edee..cd60d90 100644
--- a/include/control.h
+++ b/include/control.h
@@ -408,6 +408,8 @@ void snd_ctl_elem_info_set_item(snd_ctl_elem_info_t *obj, unsigned int val);
 const char *snd_ctl_elem_info_get_item_name(const snd_ctl_elem_info_t *obj);
 int snd_ctl_elem_info_get_dimensions(const snd_ctl_elem_info_t *obj);
 int snd_ctl_elem_info_get_dimension(const snd_ctl_elem_info_t *obj, unsigned int idx);
+int snd_ctl_elem_info_set_dimension(snd_ctl_elem_info_t *info,
+				    const int dimension[4]);
 void snd_ctl_elem_info_get_id(const snd_ctl_elem_info_t *obj, snd_ctl_elem_id_t *ptr);
 unsigned int snd_ctl_elem_info_get_numid(const snd_ctl_elem_info_t *obj);
 snd_ctl_elem_iface_t snd_ctl_elem_info_get_interface(const snd_ctl_elem_info_t *obj);
diff --git a/src/control/control.c b/src/control/control.c
index 70b166b..2ad3e0d 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -2458,6 +2458,36 @@ int snd_ctl_elem_info_get_dimension(const snd_ctl_elem_info_t *obj, unsigned int
 use_default_symbol_version(__snd_ctl_elem_info_get_dimension, snd_ctl_elem_info_get_dimension, ALSA_0.9.3);
 
 /**
+ * \brief Set width to a specified dimension level of given element information.
+ * \param info Information of an element.
+ * \param dimension Dimension width for each level by member unit.
+ * \return Zero on success, otherwise a negative error code.
+ *
+ * \par Errors:
+ * <dl>
+ * <dt>-EINVAL
+ * <dd>Invalid arguments are given as parameters.
+ * </dl>
+ */
+int snd_ctl_elem_info_set_dimension(snd_ctl_elem_info_t *info,
+				    const int dimension[4])
+{
+	unsigned int i;
+
+	if (info == NULL)
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(info->dimen.d); i++) {
+		if (dimension[i] < 0)
+			return -EINVAL;
+
+		info->dimen.d[i] = dimension[i];
+	}
+
+	return 0;
+}
+
+/**
  * \brief Get CTL element identifier of a CTL element id/info
  * \param obj CTL element id/info
  * \param ptr Pointer to returned CTL element identifier
-- 
2.7.4

[PATCH 3/5] ctl: optimize a test for user-defined element set to older kernels

[Takashi Sakamoto]

In Linux 4.0 or former, call of ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD
doesn't fill all of identical information in an argument; i.e. numid.
With the kernel, a test of user-defined element set fails.

This commit fixes the bug. The 'numid' field in identical information
is always zero when adding an element set, therefore zero check has an
effect.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 test/user-ctl-element-set.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c
index 09c7929..bfb11d7 100644
--- a/test/user-ctl-element-set.c
+++ b/test/user-ctl-element-set.c
@@ -279,6 +279,7 @@ static int check_elem_set_props(struct elem_set_trial *trial)
 	snd_ctl_elem_id_t *id;
 	snd_ctl_elem_info_t *info;
 	unsigned int numid;
+	unsigned int index;
 	unsigned int i;
 	int err;
 
@@ -286,10 +287,18 @@ static int check_elem_set_props(struct elem_set_trial *trial)
 	snd_ctl_elem_info_alloca(&info);
 
 	snd_ctl_elem_info_set_id(info, trial->id);
-	numid = snd_ctl_elem_info_get_numid(info);
+	numid = snd_ctl_elem_id_get_numid(trial->id);
+	index = snd_ctl_elem_id_get_index(trial->id);
 
 	for (i = 0; i < trial->element_count; ++i) {
-		snd_ctl_elem_info_set_numid(info, numid + i);
+		snd_ctl_elem_info_set_index(info, index + i);
+
+		/*
+		 * In Linux 4.0 or former, ioctl(SNDRV_CTL_IOCTL_ELEM_ADD)
+		 * doesn't fill all of fields for identification.
+		 */
+		if (numid > 0)
+			snd_ctl_elem_info_set_numid(info, numid + i);
 
 		err = snd_ctl_elem_info(trial->handle, info);
 		if (err < 0)
@@ -335,25 +344,25 @@ static int check_elems(struct elem_set_trial *trial)
 {
 	snd_ctl_elem_value_t *data;
 	unsigned int numid;
+	unsigned int index;
 	unsigned int i;
 	int err;
 
 	snd_ctl_elem_value_alloca(&data);
 
 	snd_ctl_elem_value_set_id(data, trial->id);
-
-	/*
-	 * Get the value of numid field in identical information for the first
-	 * element of this element set.
-	 */
-	numid = snd_ctl_elem_value_get_numid(data);
+	numid = snd_ctl_elem_id_get_numid(trial->id);
+	index = snd_ctl_elem_id_get_index(trial->id);
 
 	for (i = 0; i < trial->element_count; ++i) {
+		snd_ctl_elem_value_set_index(data, index + i);
+
 		/*
-		 * Here, I increment numid, while we can also increment offset
-		 * to enumerate each element in this element set.
+		 * In Linux 4.0 or former, ioctl(SNDRV_CTL_IOCTL_ELEM_ADD)
+		 * doesn't fill all of fields for identification.
 		 */
-		snd_ctl_elem_value_set_numid(data, numid + i);
+		if (numid > 0)
+			snd_ctl_elem_value_set_numid(data, numid + i);
 
 		err = snd_ctl_elem_read(trial->handle, data);
 		if (err < 0)
-- 
2.7.4

[PATCH 4/5] ctl: optimize a test for user-defined element set to changes of APIs

[Takashi Sakamoto]

In former commits, APIs to add an element set are changed, while a test
program for user-defined element set doesn't follow them.

This commit add support the change.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 test/user-ctl-element-set.c | 54 +++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c
index bfb11d7..51e850c 100644
--- a/test/user-ctl-element-set.c
+++ b/test/user-ctl-element-set.c
@@ -18,7 +18,8 @@ struct elem_set_trial {
 
 	snd_ctl_elem_id_t *id;
 
-	int (*add_elem_set)(struct elem_set_trial *trial);
+	int (*add_elem_set)(struct elem_set_trial *trial,
+			    snd_ctl_elem_info_t *info);
 	int (*check_elem_props)(struct elem_set_trial *trial,
 				snd_ctl_elem_info_t *info);
 	void (*change_elem_members)(struct elem_set_trial *trial,
@@ -26,9 +27,10 @@ struct elem_set_trial {
 };
 
 /* Operations for elements in an element set with boolean type. */
-static int add_bool_elem_set(struct elem_set_trial *trial)
+static int add_bool_elem_set(struct elem_set_trial *trial,
+			     snd_ctl_elem_info_t *info)
 {
-	return snd_ctl_elem_add_boolean_set(trial->handle, trial->id,
+	return snd_ctl_elem_add_boolean_set(trial->handle, info,
 				trial->element_count, trial->member_count);
 }
 
@@ -45,9 +47,10 @@ static void change_bool_elem_members(struct elem_set_trial *trial,
 }
 
 /* Operations for elements in an element set with integer type. */
-static int add_int_elem_set(struct elem_set_trial *trial)
+static int add_int_elem_set(struct elem_set_trial *trial,
+			    snd_ctl_elem_info_t *info)
 {
-	return snd_ctl_elem_add_integer_set(trial->handle, trial->id,
+	return snd_ctl_elem_add_integer_set(trial->handle, info,
 				trial->element_count, trial->member_count,
 				0, 99, 1);
 }
@@ -86,9 +89,10 @@ static const char *const labels[] = {
 	"xenial",
 };
 
-static int add_enum_elem_set(struct elem_set_trial *trial)
+static int add_enum_elem_set(struct elem_set_trial *trial,
+			     snd_ctl_elem_info_t *info)
 {
-	return snd_ctl_elem_add_enumerated_set(trial->handle, trial->id,
+	return snd_ctl_elem_add_enumerated_set(trial->handle, info,
 				trial->element_count, trial->member_count,
 				sizeof(labels) / sizeof(labels[0]),
 				labels);
@@ -134,9 +138,10 @@ static void change_enum_elem_members(struct elem_set_trial *trial,
 }
 
 /* Operations for elements in an element set with bytes type. */
-static int add_bytes_elem_set(struct elem_set_trial *trial)
+static int add_bytes_elem_set(struct elem_set_trial *trial,
+			      snd_ctl_elem_info_t *info)
 {
-	return snd_ctl_elem_add_bytes_set(trial->handle, trial->id,
+	return snd_ctl_elem_add_bytes_set(trial->handle, info,
 				trial->element_count, trial->member_count);
 }
 
@@ -153,9 +158,22 @@ static void change_bytes_elem_members(struct elem_set_trial *trial,
 }
 
 /* Operations for elements in an element set with iec958 type. */
-static int add_iec958_elem_set(struct elem_set_trial *trial)
+static int add_iec958_elem_set(struct elem_set_trial *trial,
+			       snd_ctl_elem_info_t *info)
 {
-	return snd_ctl_elem_add_iec958(trial->handle, trial->id);
+	int err;
+
+	snd_ctl_elem_info_get_id(info, trial->id);
+
+	err = snd_ctl_elem_add_iec958(trial->handle, trial->id);
+	if (err < 0)
+	        return err;
+
+	/*
+	 * In historical reason, the above API is not allowed to fill all of
+	 * fields in identification data.
+	 */
+	return snd_ctl_elem_info(trial->handle, info);
 }
 
 static void change_iec958_elem_members(struct elem_set_trial *trial,
@@ -173,9 +191,10 @@ static void change_iec958_elem_members(struct elem_set_trial *trial,
 }
 
 /* Operations for elements in an element set with integer64 type. */
-static int add_int64_elem_set(struct elem_set_trial *trial)
+static int add_int64_elem_set(struct elem_set_trial *trial,
+			      snd_ctl_elem_info_t *info)
 {
-	return snd_ctl_elem_add_integer64_set(trial->handle, trial->id,
+	return snd_ctl_elem_add_integer64_set(trial->handle, info,
 				trial->element_count, trial->member_count,
 				100, 10000, 30);
 }
@@ -208,16 +227,17 @@ static void change_int64_elem_members(struct elem_set_trial *trial,
 /* Common operations. */
 static int add_elem_set(struct elem_set_trial *trial)
 {
+	snd_ctl_elem_info_t *info;
 	char name[64] = {0};
 
 	snprintf(name, 64, "userspace-control-element-%s",
 		 snd_ctl_elem_type_name(trial->type));
 
-	memset(trial->id, 0, snd_ctl_elem_id_sizeof());
-	snd_ctl_elem_id_set_interface(trial->id, SND_CTL_ELEM_IFACE_MIXER);
-	snd_ctl_elem_id_set_name(trial->id, name);
+	snd_ctl_elem_info_alloca(&info);
+	snd_ctl_elem_info_set_interface(info, SND_CTL_ELEM_IFACE_MIXER);
+	snd_ctl_elem_info_set_name(info, name);
 
-	return trial->add_elem_set(trial);
+	return trial->add_elem_set(trial, info);
 }
 
 static int check_event(struct elem_set_trial *trial, unsigned int mask,
-- 
2.7.4

[PATCH 5/5] ctl: support dimension test for user-defined element set

[Takashi Sakamoto]

In former commits, APIs to add an element set are extended to support extra
fields to information structure. Currently, the fields are mainly used to
describe dimension level.

This commit adds tests to check the dimension level.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 test/user-ctl-element-set.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c
index 51e850c..c346e03 100644
--- a/test/user-ctl-element-set.c
+++ b/test/user-ctl-element-set.c
@@ -17,6 +17,7 @@ struct elem_set_trial {
 	unsigned int element_count;
 
 	snd_ctl_elem_id_t *id;
+	int dimension[4];
 
 	int (*add_elem_set)(struct elem_set_trial *trial,
 			    snd_ctl_elem_info_t *info);
@@ -229,6 +230,7 @@ static int add_elem_set(struct elem_set_trial *trial)
 {
 	snd_ctl_elem_info_t *info;
 	char name[64] = {0};
+	int err;
 
 	snprintf(name, 64, "userspace-control-element-%s",
 		 snd_ctl_elem_type_name(trial->type));
@@ -236,8 +238,13 @@ static int add_elem_set(struct elem_set_trial *trial)
 	snd_ctl_elem_info_alloca(&info);
 	snd_ctl_elem_info_set_interface(info, SND_CTL_ELEM_IFACE_MIXER);
 	snd_ctl_elem_info_set_name(info, name);
+	snd_ctl_elem_info_set_dimension(info, trial->dimension);
+
+	err = trial->add_elem_set(trial, info);
+	if (err >= 0)
+		snd_ctl_elem_info_get_id(info, trial->id);
 
-	return trial->add_elem_set(trial, info);
+	return err;
 }
 
 static int check_event(struct elem_set_trial *trial, unsigned int mask,
@@ -301,6 +308,7 @@ static int check_elem_set_props(struct elem_set_trial *trial)
 	unsigned int numid;
 	unsigned int index;
 	unsigned int i;
+	unsigned int j;
 	int err;
 
 	snd_ctl_elem_id_alloca(&id);
@@ -329,6 +337,11 @@ static int check_elem_set_props(struct elem_set_trial *trial)
 			return -EIO;
 		if (snd_ctl_elem_info_get_count(info) != trial->member_count)
 			return -EIO;
+		for (j = 0; j < 4; ++j) {
+			if (snd_ctl_elem_info_get_dimension(info, j) !=
+							trial->dimension[j])
+				return -EIO;
+		}
 
 		/*
 		 * In a case of IPC, this is the others. But in this case,
@@ -464,6 +477,10 @@ int main(void)
 		case SND_CTL_ELEM_TYPE_BOOLEAN:
 			trial.element_count = 900;
 			trial.member_count = 128;
+			trial.dimension[0] = 4;
+			trial.dimension[1] = 4;
+			trial.dimension[2] = 8;
+			trial.dimension[3] = 0;
 			trial.add_elem_set = add_bool_elem_set;
 			trial.check_elem_props = NULL;
 			trial.change_elem_members = change_bool_elem_members;
@@ -471,6 +488,10 @@ int main(void)
 		case SND_CTL_ELEM_TYPE_INTEGER:
 			trial.element_count = 900;
 			trial.member_count = 128;
+			trial.dimension[0] = 128;
+			trial.dimension[1] = 0;
+			trial.dimension[2] = 0;
+			trial.dimension[3] = 0;
 			trial.add_elem_set = add_int_elem_set;
 			trial.check_elem_props = check_int_elem_props;
 			trial.change_elem_members = change_int_elem_members;
@@ -478,6 +499,10 @@ int main(void)
 		case SND_CTL_ELEM_TYPE_ENUMERATED:
 			trial.element_count = 900;
 			trial.member_count = 128;
+			trial.dimension[0] = 16;
+			trial.dimension[1] = 8;
+			trial.dimension[2] = 0;
+			trial.dimension[3] = 0;
 			trial.add_elem_set = add_enum_elem_set;
 			trial.check_elem_props = check_enum_elem_props;
 			trial.change_elem_members = change_enum_elem_members;
@@ -485,6 +510,10 @@ int main(void)
 		case SND_CTL_ELEM_TYPE_BYTES:
 			trial.element_count = 900;
 			trial.member_count = 512;
+			trial.dimension[0] = 8;
+			trial.dimension[1] = 4;
+			trial.dimension[2] = 8;
+			trial.dimension[3] = 4;
 			trial.add_elem_set = add_bytes_elem_set;
 			trial.check_elem_props = NULL;
 			trial.change_elem_members = change_bytes_elem_members;
@@ -492,6 +521,10 @@ int main(void)
 		case SND_CTL_ELEM_TYPE_IEC958:
 			trial.element_count = 1;
 			trial.member_count = 1;
+			trial.dimension[0] = 0;
+			trial.dimension[1] = 0;
+			trial.dimension[2] = 0;
+			trial.dimension[3] = 0;
 			trial.add_elem_set = add_iec958_elem_set;
 			trial.check_elem_props = NULL;
 			trial.change_elem_members = change_iec958_elem_members;
@@ -500,6 +533,10 @@ int main(void)
 		default:
 			trial.element_count = 900;
 			trial.member_count = 64;
+			trial.dimension[0] = 0;
+			trial.dimension[1] = 0;
+			trial.dimension[2] = 0;
+			trial.dimension[3] = 0;
 			trial.add_elem_set = add_int64_elem_set;
 			trial.check_elem_props = check_int64_elem_props;
 			trial.change_elem_members = change_int64_elem_members;
-- 
2.7.4

Re: [alsa-lib][PATCH 0/5] ctl: support extra information for user-defined element set

[Takashi Iwai]

On Wed, 29 Jun 2016 15:42:58 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset adds support extra information to the type-specific for
> user-defined element set. Currently, 'dimension' is such an extra
> information.
> 
> In ALSA kernel/userspace interface, information to an element is described
> in this structure.
> 
> struct snd_ctl_elem_info {
>     struct snd_ctl_elem_id id;
>     unsigned int access;
>     unsigned int count;
>     pid_t owner;
>     union value;
> 
>     union dimen;
>     unsigned char reserved[64 - 4 * sizeof(unsigned short)];
> };
> 
> This structure includes reserved fields, thus it's possible to add
> more fields for future extension.
> 
> Currently, APIs to add user-defined element set don't support such extra
> fields. Meanwhile, supporting just the dimension field is not good as
> stable library APIs.
> 
> This patchset changes prototype of the APIs with 'snd_ctl_elem_info_t'
> instead of adding more parameters. Callers are expected to fill the parameter
> with identification information and the extra information.

Applied, thanks.

But at the next time, try to avoid changing the API functions you
added.  They must not be changed once when published, in general.


Takashi

Re: [alsa-lib][PATCH 0/5] ctl: support extra information for user-defined element set

[Takashi Sakamoto]

Hi,

On Jun 30 2016 15:54, Takashi Iwai wrote:
> On Wed, 29 Jun 2016 15:42:58 +0200,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> This patchset adds support extra information to the type-specific for
>> user-defined element set. Currently, 'dimension' is such an extra
>> information.
>>
>> In ALSA kernel/userspace interface, information to an element is described
>> in this structure.
>>
>> struct snd_ctl_elem_info {
>>     struct snd_ctl_elem_id id;
>>     unsigned int access;
>>     unsigned int count;
>>     pid_t owner;
>>     union value;
>>
>>     union dimen;
>>     unsigned char reserved[64 - 4 * sizeof(unsigned short)];
>> };
>>
>> This structure includes reserved fields, thus it's possible to add
>> more fields for future extension.
>>
>> Currently, APIs to add user-defined element set don't support such extra
>> fields. Meanwhile, supporting just the dimension field is not good as
>> stable library APIs.
>>
>> This patchset changes prototype of the APIs with 'snd_ctl_elem_info_t'
>> instead of adding more parameters. Callers are expected to fill the parameter
>> with identification information and the extra information.
> 
> Applied, thanks.
> 
> But at the next time, try to avoid changing the API functions you
> added.  They must not be changed once when published, in general.

In general, it's true. But it's in development cycle now. We don't
publish the APIs yet. I think it better to permit to change APIs just
included in the same development period, till releasing. Unless, the
cost to develop this userspace library becomes quite expensive, at least
to me.

In this time, I realized the probability of extension of information
structure, after committing previous patchset, several days after
reading a thread about extension of data structure which Intel
developers proposed[1]. I'm not so clever developer, and you're not so,
too. We should have enough rest to assure the changes.

[1]
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-November/069483.html


Thanks

Takashi Sakamoto

Re: [alsa-lib][PATCH 0/5] ctl: support extra information for user-defined element set

[Takashi Iwai]

On Thu, 30 Jun 2016 16:22:35 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Jun 30 2016 15:54, Takashi Iwai wrote:
> > On Wed, 29 Jun 2016 15:42:58 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> >> This patchset adds support extra information to the type-specific for
> >> user-defined element set. Currently, 'dimension' is such an extra
> >> information.
> >>
> >> In ALSA kernel/userspace interface, information to an element is described
> >> in this structure.
> >>
> >> struct snd_ctl_elem_info {
> >>     struct snd_ctl_elem_id id;
> >>     unsigned int access;
> >>     unsigned int count;
> >>     pid_t owner;
> >>     union value;
> >>
> >>     union dimen;
> >>     unsigned char reserved[64 - 4 * sizeof(unsigned short)];
> >> };
> >>
> >> This structure includes reserved fields, thus it's possible to add
> >> more fields for future extension.
> >>
> >> Currently, APIs to add user-defined element set don't support such extra
> >> fields. Meanwhile, supporting just the dimension field is not good as
> >> stable library APIs.
> >>
> >> This patchset changes prototype of the APIs with 'snd_ctl_elem_info_t'
> >> instead of adding more parameters. Callers are expected to fill the parameter
> >> with identification information and the extra information.
> > 
> > Applied, thanks.
> > 
> > But at the next time, try to avoid changing the API functions you
> > added.  They must not be changed once when published, in general.
> 
> In general, it's true.

Yes, and the story is basically over at this point...

> But it's in development cycle now. We don't
> publish the APIs yet. I think it better to permit to change APIs just
> included in the same development period, till releasing. Unless, the
> cost to develop this userspace library becomes quite expensive, at least
> to me.
>
> In this time, I realized the probability of extension of information
> structure, after committing previous patchset, several days after
> reading a thread about extension of data structure which Intel
> developers proposed[1]. I'm not so clever developer, and you're not so,
> too. We should have enough rest to assure the changes.

As I already pulled, it wasn't a big problem in this case.  Otherwise
I didn't pull it.

However, the fact that we had to change the API function means that
the previous design was bad.  Let's accept this truth.  We should be
careful enough not to make such a change happening again.
This is what my previous comment implies.  Nothing more than that.


thanks,

Takashi