[PATCH 0/4][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes

[PATCH 0/4][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes

[Takashi Sakamoto]

Hi,

A few days ago, UAPI header of 4.9-pre kernel newly include existent macros
related to TLV operation for user land. Then, applications are going to know
layout of TLV packet payload in the header.

This patchset is for related changes in ALSA user land library. The macros
are put into 'include/sound/tlv.h', however they're not put into
'include/control.h' yet, because currently I have no better idea for library
APIs to parse threshold level information.

Takashi Sakamoto (4):
  Update include/sound/tlv.h from 4.9-pre kernel uapi
  test: use proper threshold level information for user-ctl-element-set
    test
  ctl: improve API documentation for TLV operation
  ctl: improve documentation about TLV-related APIs

 include/sound/tlv.h         | 80 +++++++++++++++++++++++++++++++++++++++++++++
 src/control/control.c       | 21 ++++++++++--
 test/user-ctl-element-set.c | 41 +++++++++++++++--------
 3 files changed, 125 insertions(+), 17 deletions(-)

-- 
2.7.4

[PATCH 1/4] Update include/sound/tlv.h from 4.9-pre kernel uapi

[Takashi Sakamoto]

The UAPI header in 4.9-pre kernel newly includes existent macros related
to tlv operation, mainly for layout of TLV packet payload.

This commit updates corresponding backport header in this library.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/sound/tlv.h | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/include/sound/tlv.h b/include/sound/tlv.h
index 33d747d..f3c198f 100644
--- a/include/sound/tlv.h
+++ b/include/sound/tlv.h
@@ -20,4 +20,84 @@
 #define SNDRV_CTL_TLVT_DB_MINMAX 4	/* dB scale with min/max */
 #define SNDRV_CTL_TLVT_DB_MINMAX_MUTE 5	/* dB scale with min/max with mute */
 
+/*
+ * channel-mapping TLV items
+ *  TLV length must match with num_channels
+ */
+#define SNDRV_CTL_TLVT_CHMAP_FIXED	0x101	/* fixed channel position */
+#define SNDRV_CTL_TLVT_CHMAP_VAR	0x102	/* channels freely swappable */
+#define SNDRV_CTL_TLVT_CHMAP_PAIRED	0x103	/* pair-wise swappable */
+
+/*
+ * TLV structure is right behind the struct snd_ctl_tlv:
+ *   unsigned int type  	- see SNDRV_CTL_TLVT_*
+ *   unsigned int length
+ *   .... data aligned to sizeof(unsigned int), use
+ *        block_length = (length + (sizeof(unsigned int) - 1)) &
+ *                       ~(sizeof(unsigned int) - 1)) ....
+ */
+#define SNDRV_CTL_TLVD_ITEM(type, ...) \
+	(type), SNDRV_CTL_TLVD_LENGTH(__VA_ARGS__), __VA_ARGS__
+#define SNDRV_CTL_TLVD_LENGTH(...) \
+	((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ }))
+
+#define SNDRV_CTL_TLVD_CONTAINER_ITEM(...) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_CONTAINER, __VA_ARGS__)
+#define SNDRV_CTL_TLVD_DECLARE_CONTAINER(name, ...) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_CONTAINER_ITEM(__VA_ARGS__) \
+	}
+
+#define SNDRV_CTL_TLVD_DB_SCALE_MASK	0xffff
+#define SNDRV_CTL_TLVD_DB_SCALE_MUTE	0x10000
+#define SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_SCALE, \
+			    (min), \
+			    ((step) & SNDRV_CTL_TLVD_DB_SCALE_MASK) | \
+			     ((mute) ? SNDRV_CTL_TLVD_DB_SCALE_MUTE : 0))
+#define SNDRV_CTL_TLVD_DECLARE_DB_SCALE(name, min, step, mute) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
+	}
+
+/* dB scale specified with min/max values instead of step */
+#define SNDRV_CTL_TLVD_DB_MINMAX_ITEM(min_dB, max_dB) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_MINMAX, (min_dB), (max_dB))
+#define SNDRV_CTL_TLVD_DB_MINMAX_MUTE_ITEM(min_dB, max_dB) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_MINMAX_MUTE, (min_dB), (max_dB))
+#define SNDRV_CTL_TLVD_DECLARE_DB_MINMAX(name, min_dB, max_dB) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_MINMAX_ITEM(min_dB, max_dB) \
+	}
+#define SNDRV_CTL_TLVD_DECLARE_DB_MINMAX_MUTE(name, min_dB, max_dB) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_MINMAX_MUTE_ITEM(min_dB, max_dB) \
+	}
+
+/* linear volume between min_dB and max_dB (.01dB unit) */
+#define SNDRV_CTL_TLVD_DB_LINEAR_ITEM(min_dB, max_dB) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_LINEAR, (min_dB), (max_dB))
+#define SNDRV_CTL_TLVD_DECLARE_DB_LINEAR(name, min_dB, max_dB) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_LINEAR_ITEM(min_dB, max_dB) \
+	}
+
+/* dB range container:
+ * Items in dB range container must be ordered by their values and by their
+ * dB values. This implies that larger values must correspond with larger
+ * dB values (which is also required for all other mixer controls).
+ */
+/* Each item is: <min> <max> <TLV> */
+#define SNDRV_CTL_TLVD_DB_RANGE_ITEM(...) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_RANGE, __VA_ARGS__)
+#define SNDRV_CTL_TLVD_DECLARE_DB_RANGE(name, ...) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_RANGE_ITEM(__VA_ARGS__) \
+	}
+/* The below assumes that each item TLV is 4 words like DB_SCALE or LINEAR */
+#define SNDRV_CTL_TLVD_DB_RANGE_HEAD(num) \
+	SNDRV_CTL_TLVT_DB_RANGE, 6 * (num) * sizeof(unsigned int)
+
+#define SNDRV_CTL_TLVD_DB_GAIN_MUTE	-9999999
+
 #endif
-- 
2.7.4

[PATCH 2/4] test: use proper threshold level information for user-ctl-element-set test

[Takashi Sakamoto]

Currently, this test program uses invalid array for threshold level
information.

This commit replaces the array with valid information, which consists of
newly exported TLV macros from kernel land.

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

diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c
index 9b9dc59..7d22ca6 100644
--- a/test/user-ctl-element-set.c
+++ b/test/user-ctl-element-set.c
@@ -8,6 +8,7 @@
  */
 
 #include "../include/asoundlib.h"
+#include <sound/tlv.h>
 
 struct elem_set_trial {
 	snd_ctl_t *handle;
@@ -414,22 +415,34 @@ static int check_elems(struct elem_set_trial *trial)
 
 static int check_tlv(struct elem_set_trial *trial)
 {
-	unsigned int orig[8], curr[8];
-	int err;
-
 	/*
-	 * See a layout of 'struct snd_ctl_tlv'. I don't know the reason to
-	 * construct this buffer with the same layout. It should be abstracted
-	 * inner userspace library...
+	 * Use unlikely data for threshold level.
+	 *
+	 *  Linear: dB range (coeff)
+	 *  0<-> 4: -5940<->-5636 (44)
+	 *  4<->22: -5636<->-4556 (60)
+	 * 22<->33: -4556<->-4072 (76)
+	 * 33<->37: -4072<->-3832 (44)
+	 * 37<->48: -3832<->-2996 (76)
+	 * 48<->66: -2996<->-2204 (60)
+	 * 66<->84: -2204<-> -836 (44)
+	 * 84<->95:  -836<-> -176 (60)
+	 * 95<->99:  -176<->    0 (76)
 	 */
-	orig[0] = snd_ctl_elem_id_get_numid(trial->id);
-	orig[1] = 6 * sizeof(orig[0]);
-	orig[2] = 'a';
-	orig[3] = 'b';
-	orig[4] = 'c';
-	orig[5] = 'd';
-	orig[6] = 'e';
-	orig[7] = 'f';
+	static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(orig,
+		SNDRV_CTL_TLVD_DB_RANGE_HEAD(9),
+		 0,   4, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-5940, 44, 1),
+		 4,  22, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-5636, 60, 0),
+		22,  33, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-4556, 76, 0),
+		33,  37, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-4072, 44, 0),
+		37,  48, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-3832, 76, 0),
+		48,  66, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-2996, 60, 0),
+		66,  84, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-2204, 44, 0),
+		84,  95, SNDRV_CTL_TLVD_DB_SCALE_ITEM( -836, 60, 0),
+		95,  99, SNDRV_CTL_TLVD_DB_SCALE_ITEM( -176, 76, 0),
+	);
+	unsigned int curr[sizeof(orig)/sizeof(orig[0])];
+	int err;
 
 	/*
 	 * In in-kernel implementation, write and command operations are the
-- 
2.7.4

[PATCH 3/4] ctl: fix API explanation for TLV operation

[Takashi Sakamoto]

A commit fe1b08803db6 ('ctl: improve API documentation for threshold level
operations') changes documentations for some TLV-related APIs with wrong
explanations.

This commit fix it with better explanations.

Fixes: fe1b08803db6('ctl: improve API documentation for threshold level operations')
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 src/control/control.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/control/control.c b/src/control/control.c
index 6c00b8e..383f401 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -912,7 +912,7 @@ static int snd_ctl_tlv_do(snd_ctl_t *ctl, int op_flag,
 }
 
 /**
- * \brief Set given data to an element as threshold level.
+ * \brief Read threshold level from an element to given buffer.
  * \param ctl A handle of backend module for control interface.
  * \param id ID of an element.
  * \param tlv An array with members of unsigned int type.
@@ -940,7 +940,7 @@ int snd_ctl_elem_tlv_read(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 }
 
 /**
- * \brief Set given data to an element as threshold level.
+ * \brief Write threshold level from given buffer to an element.
  * \param ctl A handle of backend module for control interface.
  * \param id ID of an element.
  * \param tlv An array with members of unsigned int type. The second member
@@ -957,7 +957,7 @@ int snd_ctl_elem_tlv_write(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 }
 
 /**
- * \brief Set given data to an element as threshold level.
+ * \brief Process threshold level from given buffer for an element.
  * \param ctl A handle of backend module for control interface.
  * \param id ID of an element.
  * \param tlv An array with members of unsigned int type. The second member
-- 
2.7.4

[PATCH 4/4] ctl: improve documentation about TLV-related APIs

[Takashi Sakamoto]

The documentation gives no hints to users about content of threshold level
information.

This commit add hints to parse the information.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 src/control/control.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/control/control.c b/src/control/control.c
index 383f401..7165222 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -918,6 +918,11 @@ static int snd_ctl_tlv_do(snd_ctl_t *ctl, int op_flag,
  * \param tlv An array with members of unsigned int type.
  * \param tlv_size The length of the array.
  * \return 0 on success otherwise a negative error code
+ *
+ * The format of an array of \a tlv argument is:
+ *   tlv[0]: Data type. One of SND_CTL_TLVT_XXX.
+ *   tlv[1]: Lengh of data in units of byte.
+ *   tlv[2..]: Data. Details are described in <sound/tlv.h>.
  */
 int snd_ctl_elem_tlv_read(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			  unsigned int *tlv, unsigned int tlv_size)
@@ -948,6 +953,11 @@ int snd_ctl_elem_tlv_read(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
  * \retval 0 on success
  * \retval >0 on success when value was changed
  * \retval <0 a negative error code
+ *
+ * The format of an array of \a tlv argument is:
+ *   tlv[0]: Data type. One of SND_CTL_TLVT_XXX.
+ *   tlv[1]: Lengh of data in units of byte.
+ *   tlv[2..]: Data. Details are described in <sound/tlv.h>.
  */
 int snd_ctl_elem_tlv_write(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			   const unsigned int *tlv)
@@ -965,6 +975,11 @@ int snd_ctl_elem_tlv_write(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
  * \retval 0 on success
  * \retval >0 on success when value was changed
  * \retval <0 a negative error code
+ *
+ * The format of an array of \a tlv argument is:
+ *   tlv[0]: Data type. One of SND_CTL_TLVT_XXX.
+ *   tlv[1]: Lengh of data in units of byte.
+ *   tlv[2..]: Data. Details are described in <sound/tlv.h>.
  */
 int snd_ctl_elem_tlv_command(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			     const unsigned int *tlv)
-- 
2.7.4

Re: [PATCH 2/4] test: use proper threshold level information for user-ctl-element-set test

[Takashi Iwai]

On Sat, 17 Sep 2016 16:49:26 +0200,
Takashi Sakamoto wrote:
> 
> Currently, this test program uses invalid array for threshold level
> information.

What do you mean exactly here as "threshold level"?
This term is fairly unclear to me and I don't remember of it being
defined properly although you seem to have started using in some
comments.


thanks,

Takashi

> 
> This commit replaces the array with valid information, which consists of
> newly exported TLV macros from kernel land.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  test/user-ctl-element-set.c | 41 +++++++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c
> index 9b9dc59..7d22ca6 100644
> --- a/test/user-ctl-element-set.c
> +++ b/test/user-ctl-element-set.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include "../include/asoundlib.h"
> +#include <sound/tlv.h>
>  
>  struct elem_set_trial {
>  	snd_ctl_t *handle;
> @@ -414,22 +415,34 @@ static int check_elems(struct elem_set_trial *trial)
>  
>  static int check_tlv(struct elem_set_trial *trial)
>  {
> -	unsigned int orig[8], curr[8];
> -	int err;
> -
>  	/*
> -	 * See a layout of 'struct snd_ctl_tlv'. I don't know the reason to
> -	 * construct this buffer with the same layout. It should be abstracted
> -	 * inner userspace library...
> +	 * Use unlikely data for threshold level.
> +	 *
> +	 *  Linear: dB range (coeff)
> +	 *  0<-> 4: -5940<->-5636 (44)
> +	 *  4<->22: -5636<->-4556 (60)
> +	 * 22<->33: -4556<->-4072 (76)
> +	 * 33<->37: -4072<->-3832 (44)
> +	 * 37<->48: -3832<->-2996 (76)
> +	 * 48<->66: -2996<->-2204 (60)
> +	 * 66<->84: -2204<-> -836 (44)
> +	 * 84<->95:  -836<-> -176 (60)
> +	 * 95<->99:  -176<->    0 (76)
>  	 */
> -	orig[0] = snd_ctl_elem_id_get_numid(trial->id);
> -	orig[1] = 6 * sizeof(orig[0]);
> -	orig[2] = 'a';
> -	orig[3] = 'b';
> -	orig[4] = 'c';
> -	orig[5] = 'd';
> -	orig[6] = 'e';
> -	orig[7] = 'f';
> +	static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(orig,
> +		SNDRV_CTL_TLVD_DB_RANGE_HEAD(9),
> +		 0,   4, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-5940, 44, 1),
> +		 4,  22, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-5636, 60, 0),
> +		22,  33, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-4556, 76, 0),
> +		33,  37, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-4072, 44, 0),
> +		37,  48, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-3832, 76, 0),
> +		48,  66, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-2996, 60, 0),
> +		66,  84, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-2204, 44, 0),
> +		84,  95, SNDRV_CTL_TLVD_DB_SCALE_ITEM( -836, 60, 0),
> +		95,  99, SNDRV_CTL_TLVD_DB_SCALE_ITEM( -176, 76, 0),
> +	);
> +	unsigned int curr[sizeof(orig)/sizeof(orig[0])];
> +	int err;
>  
>  	/*
>  	 * In in-kernel implementation, write and command operations are the
> -- 
> 2.7.4
> 

Re: [PATCH 2/4] test: use proper threshold level information for user-ctl-element-set test

[Takashi Sakamoto]

On 2016年09月20日 05:48, Takashi Iwai wrote:
> On Sat, 17 Sep 2016 16:49:26 +0200,
> Takashi Sakamoto wrote:
>>
>> Currently, this test program uses invalid array for threshold level
>> information.
> 
> What do you mean exactly here as "threshold level"?
> This term is fairly unclear to me and I don't remember of it being
> defined properly although you seem to have started using in some
> comments.

Aha. Now I got it.

I interpreted 'TLV' as 'Threshold LeVel', and it's completely my
misreading. Actually, TLV is mainly used to describe the relationship
between Linear/dB representation, However, in a design of ALSA control
interface, 'TLV' means a data structure with Type/Length/Value. So this
line expect the TLV packet is larger than two members for the T and the L.
http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/control.c?h=v4.8-rc6#n1440

I have scarcely realized my mistakes. I'll revise this patchset with a
new one to update control interface description...


Thanks!

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel