[PATCH] Adds dB gain to alsa-driver, alsa-lib.

[PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

Hi,

I have implemented dB gain reporting for ALSA mixer controls.
By applying the attached patches to alsa-driver and alsa-lib, one will 
then see dB values appear in alsamixer. (I added support to alsamixer a 
long time ago)

Currently, only the snd-ca0106 sound card reports the values.

If the method is agreed upon, I will extend it to many more sound cards.

General description:

The only piece of information reported by the kernel sound driver is a 
db_scale value.  It is a unsigned int at application layer, but the 
current IOCTL TLV truncates this to 32bit unsigned int. alsa-lib then 
uses this value to decide which translation function to apply to the 
currently available "volume" value.
I view this "db_scale" value as simply a hint from the low level driver 
at which conversion function should be used.
The conversion function in alsa-lib can then be as simple or as 
complicated as required.

I have implemented a new IOCTL for the mixer "SNDRV_CTL_IOCTL_MISC".
This uses a TLV (Type Length Value) approach. This is endlessly 
extensible, and could be used instead of the "SNDRV_CTL_IOCTL_ELEM_INFO".
It supports nesting of TLVs.

Any comments?

James

[-- Attachment #2: alsa-driver.db.patch --]
[-- Type: text/x-patch, Size: 8926 bytes --]

Index: alsa-driver/alsa-kernel/core/control.c
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/core/control.c,v
retrieving revision 1.69
diff -u -r1.69 control.c
--- alsa-driver/alsa-kernel/core/control.c	20 Nov 2005 14:06:59 -0000	1.69
+++ alsa-driver/alsa-kernel/core/control.c	15 Dec 2005 18:16:43 -0000
@@ -237,6 +237,7 @@
 	kctl.info = ncontrol->info;
 	kctl.get = ncontrol->get;
 	kctl.put = ncontrol->put;
+	kctl.db_scale = ncontrol->db_scale;
 	kctl.private_value = ncontrol->private_value;
 	kctl.private_data = private_data;
 	return snd_ctl_new(&kctl, access);
@@ -547,6 +548,138 @@
 	return 0;
 }
 
+static int uint32_to_message(struct snd_ctl_misc *misc, unsigned int value)
+{
+	unsigned int pointer = misc->length;
+	/* If unsigned int is a 64bit value, we will just ignore the high 32bits. */
+	misc->message[pointer++] = value & 0xff;
+	misc->message[pointer++] = (value >> 8) & 0xff;
+	misc->message[pointer++] = (value >> 16) & 0xff;
+	misc->message[pointer++] = (value >> 24) & 0xff;
+	misc->length = pointer;
+	return 0;
+}
+
+static int message_to_uint32(struct snd_ctl_misc *misc, unsigned int *value)
+{
+	unsigned int pointer = misc->length;
+	unsigned int val;
+	val = misc->message[pointer++] ;
+	val |= ((misc->message[pointer++]) << 8);
+	val |= ((misc->message[pointer++]) << 16);
+	val |= ((misc->message[pointer++]) << 24);
+	misc->length = pointer;
+	*value = val;
+	return 0;
+}
+
+
+static int add_int32_tlv_to_container(struct snd_ctl_misc *misc, unsigned int type, unsigned int value)
+{
+	uint32_to_message(misc, type);
+	uint32_to_message(misc, 4);
+	uint32_to_message(misc, value);
+	/* FIXME: Check for overflows */
+	return 0;
+}
+
+
+static int container_open(struct snd_ctl_misc *misc, unsigned int type)
+{
+	uint32_to_message(misc, type);
+	uint32_to_message(misc, 0); /* This gets updated at container close */
+	return 0;
+}
+
+static int container_close(struct snd_ctl_misc *misc)
+{
+	unsigned int pointer = 4;
+	unsigned int value = misc->length;
+	misc->message[pointer++] = value & 0xff;
+	misc->message[pointer++] = (value >> 8) & 0xff;
+	misc->message[pointer++] = (value >> 16) & 0xff;
+	misc->message[pointer++] = (value >> 24) & 0xff;
+	return 0;
+}
+
+static int snd_ctl_misc_user(struct snd_ctl_file *ctl,
+				  struct snd_ctl_misc __user *_misc)
+{
+	struct snd_ctl_misc *misc;
+	struct snd_card *card = ctl->card;
+	struct snd_kcontrol *kctl;
+	int result;
+	int err=0;
+	int numid;
+	unsigned int received_length;
+	unsigned int tlv_type;
+	unsigned int tlv_length;
+	unsigned int tlv_value;
+	unsigned int container_type;
+	unsigned int container_length;
+	
+	misc = kmalloc(sizeof(*misc), GFP_KERNEL);
+	if (misc == NULL)
+		return -ENOMEM;	
+	if (copy_from_user(misc, _misc, sizeof(*misc))) {
+		result = -EFAULT;
+		goto exit_free;
+	}
+	received_length = misc->length;
+	misc->length=0;
+	message_to_uint32(misc, &container_type);
+	switch (container_type) {
+		case SND_MISC_DB_SCALE:
+			message_to_uint32(misc, &container_length);
+			/* FIXME: Do more sanity checks here */
+			message_to_uint32(misc, &tlv_type);
+			switch (tlv_type) {
+				case SND_MISC_ELEM_NUMID:
+					message_to_uint32(misc, &tlv_length);
+					if (tlv_length != 4) {
+						err = -EINVAL;
+						goto exit_free;
+					}
+					message_to_uint32(misc, &tlv_value);
+					break;
+				default:
+					err = -EINVAL;
+					goto exit_free;
+					break;
+			}
+			break;
+		default:
+			err = -EINVAL;
+			goto exit_free;
+	}
+	numid=tlv_value;
+	down_read(&card->controls_rwsem);
+	kctl = snd_ctl_find_numid(card, numid);
+	if (kctl == NULL) {
+		result = -ENOENT;
+		snd_printk("exit_up1\n");
+		goto exit_up;
+	}
+	if (kctl->db_scale == 0) { /* Value defaults to zero when not implemented. */
+		err = -EINVAL;
+		goto exit_up;
+	}
+	misc->type=1;
+	misc->length=0;
+	container_open(misc, SND_MISC_DB_SCALE);
+	add_int32_tlv_to_container(misc, SND_MISC_ELEM_DB_SCALE, kctl->db_scale);
+	container_close(misc);
+	if (copy_to_user(_misc, misc, sizeof(*misc)))
+		result = -EFAULT;
+	
+exit_up:
+	up_read(&card->controls_rwsem);
+exit_free:
+	kfree(misc);
+exit:
+	return result;
+}
+
 static int snd_ctl_elem_list(struct snd_card *card,
 			     struct snd_ctl_elem_list __user *_list)
 {
@@ -1062,6 +1195,8 @@
 		return snd_ctl_elem_add_user(ctl, argp, 1);
 	case SNDRV_CTL_IOCTL_ELEM_REMOVE:
 		return snd_ctl_elem_remove(ctl, argp);
+	case SNDRV_CTL_IOCTL_MISC:
+		return snd_ctl_misc_user(ctl, argp);
 	case SNDRV_CTL_IOCTL_SUBSCRIBE_EVENTS:
 		return snd_ctl_subscribe_events(ctl, ip);
 	case SNDRV_CTL_IOCTL_POWER:
Index: alsa-driver/alsa-kernel/include/asound.h
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/include/asound.h,v
retrieving revision 1.57
diff -u -r1.57 asound.h
--- alsa-driver/alsa-kernel/include/asound.h	17 Nov 2005 13:51:18 -0000	1.57
+++ alsa-driver/alsa-kernel/include/asound.h	15 Dec 2005 18:16:43 -0000
@@ -818,6 +818,18 @@
         unsigned char reserved[128-sizeof(struct timespec)];
 };
 
+struct snd_ctl_misc {
+	u32 type;
+	u32 length;
+	unsigned char message[2040];
+};
+
+#define SND_MISC_DB_SCALE 2
+/* Request */
+#define SND_MISC_ELEM_NUMID 2
+/* Response */
+#define SND_MISC_ELEM_DB_SCALE 2
+
 enum {
 	SNDRV_CTL_IOCTL_PVERSION = _IOR('U', 0x00, int),
 	SNDRV_CTL_IOCTL_CARD_INFO = _IOR('U', 0x01, struct snd_ctl_card_info),
@@ -833,6 +845,7 @@
 	SNDRV_CTL_IOCTL_ELEM_REMOVE = _IOWR('U', 0x19, struct snd_ctl_elem_id),
 	SNDRV_CTL_IOCTL_HWDEP_NEXT_DEVICE = _IOWR('U', 0x20, int),
 	SNDRV_CTL_IOCTL_HWDEP_INFO = _IOR('U', 0x21, struct snd_hwdep_info),
+	SNDRV_CTL_IOCTL_MISC = _IOR('U', 0x22, struct snd_ctl_misc),
 	SNDRV_CTL_IOCTL_PCM_NEXT_DEVICE = _IOR('U', 0x30, int),
 	SNDRV_CTL_IOCTL_PCM_INFO = _IOWR('U', 0x31, struct snd_pcm_info),
 	SNDRV_CTL_IOCTL_PCM_PREFER_SUBDEVICE = _IOW('U', 0x32, int),
Index: alsa-driver/alsa-kernel/include/control.h
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/include/control.h,v
retrieving revision 1.14
diff -u -r1.14 control.h
--- alsa-driver/alsa-kernel/include/control.h	17 Nov 2005 13:53:23 -0000	1.14
+++ alsa-driver/alsa-kernel/include/control.h	15 Dec 2005 18:16:43 -0000
@@ -42,6 +42,7 @@
 	snd_kcontrol_info_t *info;
 	snd_kcontrol_get_t *get;
 	snd_kcontrol_put_t *put;
+	unsigned int db_scale;
 	unsigned long private_value;
 };
 
@@ -58,6 +59,7 @@
 	snd_kcontrol_info_t *info;
 	snd_kcontrol_get_t *get;
 	snd_kcontrol_put_t *put;
+	unsigned int db_scale;
 	unsigned long private_value;
 	void *private_data;
 	void (*private_free)(struct snd_kcontrol *kcontrol);
Index: alsa-driver/alsa-kernel/pci/ca0106/ca0106_mixer.c
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/pci/ca0106/ca0106_mixer.c,v
retrieving revision 1.9
diff -u -r1.9 ca0106_mixer.c
--- alsa-driver/alsa-kernel/pci/ca0106/ca0106_mixer.c	17 Nov 2005 14:55:40 -0000	1.9
+++ alsa-driver/alsa-kernel/pci/ca0106/ca0106_mixer.c	15 Dec 2005 18:16:43 -0000
@@ -329,37 +329,38 @@
 	return 1;
 }
 
-#define CA_VOLUME(xname,chid,reg) \
+#define CA_VOLUME(xname,chid,reg,db) \
 {								\
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname,	\
 	.info = snd_ca0106_volume_info,				\
 	.get =          snd_ca0106_volume_get,			\
 	.put =          snd_ca0106_volume_put,			\
-	.private_value = ((chid) << 8) | (reg)			\
+	.private_value = ((chid) << 8) | (reg),			\
+	.db_scale = db						\
 }
 
 
 static struct snd_kcontrol_new snd_ca0106_volume_ctls[] __devinitdata = {
 	CA_VOLUME("Analog Front Playback Volume",
-		  CONTROL_FRONT_CHANNEL, PLAYBACK_VOLUME2),
+		  CONTROL_FRONT_CHANNEL, PLAYBACK_VOLUME2, 1),
         CA_VOLUME("Analog Rear Playback Volume",
-		  CONTROL_REAR_CHANNEL, PLAYBACK_VOLUME2),
+		  CONTROL_REAR_CHANNEL, PLAYBACK_VOLUME2, 1),
 	CA_VOLUME("Analog Center/LFE Playback Volume",
-		  CONTROL_CENTER_LFE_CHANNEL, PLAYBACK_VOLUME2),
+		  CONTROL_CENTER_LFE_CHANNEL, PLAYBACK_VOLUME2, 1),
         CA_VOLUME("Analog Side Playback Volume",
-		  CONTROL_UNKNOWN_CHANNEL, PLAYBACK_VOLUME2),
+		  CONTROL_UNKNOWN_CHANNEL, PLAYBACK_VOLUME2, 1),
 
         CA_VOLUME("SPDIF Front Playback Volume",
-		  CONTROL_FRONT_CHANNEL, PLAYBACK_VOLUME1),
+		  CONTROL_FRONT_CHANNEL, PLAYBACK_VOLUME1, 1),
 	CA_VOLUME("SPDIF Rear Playback Volume",
-		  CONTROL_REAR_CHANNEL, PLAYBACK_VOLUME1),
+		  CONTROL_REAR_CHANNEL, PLAYBACK_VOLUME1, 1),
 	CA_VOLUME("SPDIF Center/LFE Playback Volume",
-		  CONTROL_CENTER_LFE_CHANNEL, PLAYBACK_VOLUME1),
+		  CONTROL_CENTER_LFE_CHANNEL, PLAYBACK_VOLUME1, 1),
 	CA_VOLUME("SPDIF Unknown Playback Volume",
-		  CONTROL_UNKNOWN_CHANNEL, PLAYBACK_VOLUME1),
+		  CONTROL_UNKNOWN_CHANNEL, PLAYBACK_VOLUME1, 1),
 
         CA_VOLUME("CAPTURE feedback Playback Volume",
-		  1, CAPTURE_CONTROL),
+		  1, CAPTURE_CONTROL, 1),
 
 	{
 		.access =	SNDRV_CTL_ELEM_ACCESS_READ,

[-- Attachment #3: alsa-lib.db.patch --]
[-- Type: text/x-patch, Size: 12500 bytes --]

Index: alsa-lib/include/control.h
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/include/control.h,v
retrieving revision 1.101
diff -u -r1.101 control.h
--- alsa-lib/include/control.h	9 Jun 2005 17:12:08 -0000	1.101
+++ alsa-lib/include/control.h	15 Dec 2005 18:17:27 -0000
@@ -53,6 +53,12 @@
 /** CTL card info container */
 typedef struct _snd_ctl_card_info snd_ctl_card_info_t;
 
+/** CTL misc container */
+typedef struct _snd_ctl_misc snd_ctl_misc_t;
+
+/** CTL misc container */
+/* FIXME: Add */
+
 /** CTL element identifier container */
 typedef struct _snd_ctl_elem_id snd_ctl_elem_id_t;
 
@@ -212,6 +218,8 @@
 int snd_ctl_poll_descriptors_revents(snd_ctl_t *ctl, struct pollfd *pfds, unsigned int nfds, unsigned short *revents);
 int snd_ctl_subscribe_events(snd_ctl_t *ctl, int subscribe);
 int snd_ctl_card_info(snd_ctl_t *ctl, snd_ctl_card_info_t *info);
+int snd_ctl_misc(snd_ctl_t *ctl, snd_ctl_misc_t *misc);
+int snd_ctl_elem_get_db_scale(snd_ctl_t *ctl, unsigned int numid, long *db_scale);
 int snd_ctl_elem_list(snd_ctl_t *ctl, snd_ctl_elem_list_t * list);
 int snd_ctl_elem_info(snd_ctl_t *ctl, snd_ctl_elem_info_t *info);
 int snd_ctl_elem_read(snd_ctl_t *ctl, snd_ctl_elem_value_t *value);
@@ -485,6 +493,8 @@
 int snd_hctl_elem_info(snd_hctl_elem_t *elem, snd_ctl_elem_info_t * info);
 int snd_hctl_elem_read(snd_hctl_elem_t *elem, snd_ctl_elem_value_t * value);
 int snd_hctl_elem_write(snd_hctl_elem_t *elem, snd_ctl_elem_value_t * value);
+int snd_hctl_elem_misc(snd_hctl_elem_t *elem, snd_ctl_misc_t * misc);
+int snd_hctl_elem_get_db_gain(snd_hctl_elem_t *elem, long volume, long *db_gain);
 
 snd_hctl_t *snd_hctl_elem_get_hctl(snd_hctl_elem_t *elem);
 
Index: alsa-lib/include/local.h
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/include/local.h,v
retrieving revision 1.44
diff -u -r1.44 local.h
--- alsa-lib/include/local.h	29 Sep 2005 19:11:50 -0000	1.44
+++ alsa-lib/include/local.h	15 Dec 2005 18:17:27 -0000
@@ -46,6 +46,7 @@
 #define _snd_pcm_status sndrv_pcm_status
 
 #define _snd_ctl_card_info sndrv_ctl_card_info
+#define _snd_ctl_misc sndrv_ctl_misc
 #define _snd_ctl_elem_id sndrv_ctl_elem_id
 #define _snd_ctl_elem_list sndrv_ctl_elem_list
 #define _snd_ctl_elem_info sndrv_ctl_elem_info
Index: alsa-lib/include/sound/asound.h
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/include/sound/asound.h,v
retrieving revision 1.22
diff -u -r1.22 asound.h
--- alsa-lib/include/sound/asound.h	16 Aug 2005 12:19:15 -0000	1.22
+++ alsa-lib/include/sound/asound.h	15 Dec 2005 18:17:27 -0000
@@ -847,6 +847,18 @@
         unsigned char reserved[128-sizeof(struct timespec)];
 };
 
+struct sndrv_ctl_misc {
+	unsigned int type;  /* This should be uint32_t */
+	unsigned int length; /* This should be uint32_t */
+	unsigned char message[2040];
+};
+
+#define SND_MISC_DB_SCALE 2
+/* Request */
+#define SND_MISC_ELEM_NUMID 2
+/* Response */
+#define SND_MISC_ELEM_DB_SCALE 2
+
 enum {
 	SNDRV_CTL_IOCTL_PVERSION = _IOR('U', 0x00, int),
 	SNDRV_CTL_IOCTL_CARD_INFO = _IOR('U', 0x01, struct sndrv_ctl_card_info),
@@ -862,6 +874,7 @@
 	SNDRV_CTL_IOCTL_ELEM_REMOVE = _IOWR('U', 0x19, struct sndrv_ctl_elem_id),
 	SNDRV_CTL_IOCTL_HWDEP_NEXT_DEVICE = _IOWR('U', 0x20, int),
 	SNDRV_CTL_IOCTL_HWDEP_INFO = _IOR('U', 0x21, struct sndrv_hwdep_info),
+	SNDRV_CTL_IOCTL_MISC = _IOR('U', 0x22, struct sndrv_ctl_misc),
 	SNDRV_CTL_IOCTL_PCM_NEXT_DEVICE = _IOR('U', 0x30, int),
 	SNDRV_CTL_IOCTL_PCM_INFO = _IOWR('U', 0x31, struct sndrv_pcm_info),
 	SNDRV_CTL_IOCTL_PCM_PREFER_SUBDEVICE = _IOW('U', 0x32, int),
Index: alsa-lib/src/control/control.c
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/src/control/control.c,v
retrieving revision 1.108
diff -u -r1.108 control.c
--- alsa-lib/src/control/control.c	14 Nov 2005 10:18:22 -0000	1.108
+++ alsa-lib/src/control/control.c	15 Dec 2005 18:17:27 -0000
@@ -238,6 +238,138 @@
 }
 
 /**
+ * \brief Get/Set misc related information
+ * \param ctl CTL handle
+ * \param misc pointer
+ * \return 0 on success otherwise a negative error code
+ */
+int snd_ctl_misc(snd_ctl_t *ctl, snd_ctl_misc_t *misc)
+{
+	int err;
+	assert(ctl && misc);
+	err = ctl->ops->misc(ctl, misc);
+	return err;
+}
+
+static int uint32_to_message(snd_ctl_misc_t *misc, unsigned int value)
+{
+	unsigned int pointer = misc->length;
+	/* If unsigned int is a 64bit value, we will just ignore the high 32bits. */
+	misc->message[pointer++] = value & 0xff;
+	misc->message[pointer++] = (value >> 8) & 0xff;
+	misc->message[pointer++] = (value >> 16) & 0xff;
+	misc->message[pointer++] = (value >> 24) & 0xff;
+	misc->length = pointer;
+	return 0;
+}
+
+static int message_to_uint32(snd_ctl_misc_t *misc, unsigned int *value)
+{
+	unsigned int pointer = misc->length;
+	unsigned int val;
+	val = misc->message[pointer++] ;
+	val |= ((misc->message[pointer++]) << 8);
+	val |= ((misc->message[pointer++]) << 16);
+	val |= ((misc->message[pointer++]) << 24);
+	misc->length = pointer;
+	*value = val;
+	return 0;
+}
+
+static int add_int32_tlv_to_container(snd_ctl_misc_t *misc, unsigned int type, unsigned int value)
+{
+	uint32_to_message(misc, type);
+	uint32_to_message(misc, 4);
+	uint32_to_message(misc, value);
+	/* FIXME: Check for overflows */
+	return 0;
+}
+
+static int container_open(snd_ctl_misc_t *misc, unsigned int type)
+{
+        uint32_to_message(misc, type);
+        uint32_to_message(misc, 0); /* This gets updated at container close */
+	return 0;
+}
+
+static int container_close(snd_ctl_misc_t *misc)
+{
+	unsigned int pointer = 4;
+	unsigned int value = misc->length;
+	misc->message[pointer++] = value & 0xff;
+	misc->message[pointer++] = (value >> 8) & 0xff;
+	misc->message[pointer++] = (value >> 16) & 0xff;
+	misc->message[pointer++] = (value >> 24) & 0xff;
+	return 0;
+}
+
+/**
+ * \brief Get/Set misc related information
+ * \param ctl CTL handle
+ * \param misc pointer
+ * \return 0 on success otherwise a negative error code
+ * db_scale: index into volume to db gain function.
+ */
+int snd_ctl_elem_get_db_scale(snd_ctl_t *ctl, unsigned int numid, long *db_scale)
+{
+	snd_ctl_misc_t *misc;
+	int err=0;
+	unsigned int received_length;
+	unsigned int tlv_type;
+	unsigned int tlv_length;
+	unsigned int tlv_value;
+	unsigned int container_type;
+	unsigned int container_length;
+
+	assert(ctl && db_scale);
+	misc=malloc(sizeof(*misc));
+	if (misc == NULL) {
+		err = -ENOMEM;
+		return err;
+	}
+	misc->type=1;
+	misc->length=0;
+	container_open(misc, SND_MISC_DB_SCALE);
+	add_int32_tlv_to_container(misc, SND_MISC_ELEM_NUMID, numid);
+	container_close(misc);
+	
+	err = ctl->ops->misc(ctl, misc);
+	received_length = misc->length;
+	misc->length=0;
+	message_to_uint32(misc, &container_type);
+	switch (container_type) {
+		case SND_MISC_DB_SCALE:
+			message_to_uint32(misc, &container_length);
+			/* FIXME: Do more sanity checks here */
+			message_to_uint32(misc, &tlv_type);
+			switch (tlv_type) {
+				case SND_MISC_ELEM_DB_SCALE:
+					message_to_uint32(misc, &tlv_length);
+					if (tlv_length != 4) {
+						err = -EINVAL;
+						goto _err;
+					}
+					message_to_uint32(misc, &tlv_value);
+					break;
+				default:
+					err = -EINVAL;
+					goto _err;
+					break;
+			}
+			break;
+		default:
+			err = -EINVAL;
+			goto _err;
+	}
+	*db_scale=tlv_value;
+
+_err:
+	free(misc);
+	return err;
+}
+
+
+/**
  * \brief Get a list of element identifiers
  * \param ctl CTL handle
  * \param list CTL element identifiers list pointer
Index: alsa-lib/src/control/control_hw.c
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/src/control/control_hw.c,v
retrieving revision 1.43
diff -u -r1.43 control_hw.c
--- alsa-lib/src/control/control_hw.c	9 Jun 2005 17:14:22 -0000	1.43
+++ alsa-lib/src/control/control_hw.c	15 Dec 2005 18:17:27 -0000
@@ -128,6 +128,16 @@
 	return 0;
 }
 
+static int snd_ctl_hw_misc(snd_ctl_t *handle, snd_ctl_misc_t *misc)
+{
+	snd_ctl_hw_t *hw = handle->private_data;
+	if (ioctl(hw->fd, SNDRV_CTL_IOCTL_MISC, misc) < 0) {
+		//SYSERR("SNDRV_CTL_IOCTL_MISC failed");
+		return -errno;
+	}
+	return 0;
+}
+
 static int snd_ctl_hw_elem_list(snd_ctl_t *handle, snd_ctl_elem_list_t *list)
 {
 	snd_ctl_hw_t *hw = handle->private_data;
@@ -296,6 +306,7 @@
 	.async = snd_ctl_hw_async,
 	.subscribe_events = snd_ctl_hw_subscribe_events,
 	.card_info = snd_ctl_hw_card_info,
+	.misc = snd_ctl_hw_misc,
 	.element_list = snd_ctl_hw_elem_list,
 	.element_info = snd_ctl_hw_elem_info,
 	.element_add = snd_ctl_hw_elem_add,
Index: alsa-lib/src/control/control_local.h
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/src/control/control_local.h,v
retrieving revision 1.31
diff -u -r1.31 control_local.h
--- alsa-lib/src/control/control_local.h	9 Jun 2005 17:09:33 -0000	1.31
+++ alsa-lib/src/control/control_local.h	15 Dec 2005 18:17:27 -0000
@@ -27,6 +27,7 @@
 	int (*async)(snd_ctl_t *handle, int sig, pid_t pid);
 	int (*subscribe_events)(snd_ctl_t *handle, int subscribe);
 	int (*card_info)(snd_ctl_t *handle, snd_ctl_card_info_t *info);
+	int (*misc)(snd_ctl_t *handle, snd_ctl_misc_t *info);
 	int (*element_list)(snd_ctl_t *handle, snd_ctl_elem_list_t *list);
 	int (*element_info)(snd_ctl_t *handle, snd_ctl_elem_info_t *info);
 	int (*element_add)(snd_ctl_t *handle, snd_ctl_elem_info_t *info);
Index: alsa-lib/src/control/hcontrol.c
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/src/control/hcontrol.c,v
retrieving revision 1.36
diff -u -r1.36 hcontrol.c
--- alsa-lib/src/control/hcontrol.c	28 Jun 2005 10:24:44 -0000	1.36
+++ alsa-lib/src/control/hcontrol.c	15 Dec 2005 18:17:27 -0000
@@ -759,6 +759,42 @@
 }
 
 /**
+ * \brief Get misc information for an HCTL element
+ * \param elem HCTL element
+ * \param info HCTL element information
+ * \return 0 otherwise a negative error code on failure
+ */
+int snd_hctl_elem_get_db_gain(snd_hctl_elem_t *elem, long volume, long *db_gain)
+{
+	int err;
+	long db_scale;
+	assert(elem);
+	assert(elem->hctl);
+	if ((err = snd_ctl_elem_get_db_scale(elem->hctl->ctl, elem->id.numid, &db_scale)) < 0)
+		return err;
+	switch (db_scale) {
+	case 1:  /* Scale from -51.50 dB to +12.00 dB in steps of 0.75 dB. 256 steps. */
+		if (volume == 0) {
+			*db_gain=-9999999; /* Muted */
+			break;
+		}
+		*db_gain=(volume * 25) - 5175; /* For ca0106 controls */
+		break;
+	case 2:  /* Scale from -39.60 dB to 0.00 dB in steps of 0.4 dB. 100 steps. */
+		if (volume == 0) {
+			*db_gain=-9999999; /* Muted */
+			break;
+		}
+		*db_gain=(volume * 40) - 4000; /* For emu10k1 dsp controls */
+		break;
+	default:
+		*db_gain=-9999999; /* Muted */
+		return 1;
+	}
+	return 0;
+}
+
+/**
  * \brief Get information for an HCTL element
  * \param elem HCTL element
  * \param info HCTL element information
Index: alsa-lib/src/mixer/simple_none.c
===================================================================
RCS file: /cvsroot/alsa/alsa-lib/src/mixer/simple_none.c,v
retrieving revision 1.8
diff -u -r1.8 simple_none.c
--- alsa-lib/src/mixer/simple_none.c	2 Dec 2005 13:39:24 -0000	1.8
+++ alsa-lib/src/mixer/simple_none.c	15 Dec 2005 18:17:27 -0000
@@ -971,12 +971,36 @@
 	return 0;
 }
 
-static int get_dB_ops(snd_mixer_elem_t *elem ATTRIBUTE_UNUSED,
-		      int dir ATTRIBUTE_UNUSED,
-		      snd_mixer_selem_channel_id_t channel ATTRIBUTE_UNUSED,
-		      long *value ATTRIBUTE_UNUSED)
+static int get_dB_ops(snd_mixer_elem_t *elem,
+		      int dir,
+		      snd_mixer_selem_channel_id_t channel,
+		      long *value)
 {
-	return -ENXIO;
+	selem_none_t *s = snd_mixer_elem_get_private(elem);
+	selem_ctl_t *c;
+	int err=0;
+	int n;
+	long volume, db_gain;
+	if (dir==SM_PLAY) {
+		c = &s->ctls[CTL_PLAYBACK_VOLUME];
+		if (c->type==2) {
+			if (get_volume_ops(elem, dir, channel, &volume)) 
+				return -EINVAL;
+			if ((err = snd_hctl_elem_get_db_gain(c->elem, volume, &db_gain)) < 0)
+				goto _err;
+		}
+	}
+        //for (n=0;n<=CTL_LAST;n++) {
+	//	c = &s->ctls[n];
+	//	printf("%d:c->inactive=%d\n",n, c->inactive);
+	//}
+	//misc=malloc(2048);
+	//if ((err = snd_hctl_misc(elem->ctl, misc)) < 0)
+	//	return err;
+	//printf("misc=%p, *misc=%d\n",misc, (int) *misc);
+	*value=db_gain;
+_err:
+	return err;
 }
 
 static int get_switch_ops(snd_mixer_elem_t *elem, int dir,

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Lee Revell]

On Thu, 2005-12-15 at 18:34 +0000, James Courtier-Dutton wrote:
> Hi,
> 
> I have implemented dB gain reporting for ALSA mixer controls.
> By applying the attached patches to alsa-driver and alsa-lib, one will 
> then see dB values appear in alsamixer. (I added support to alsamixer a 
> long time ago)
> 
> Currently, only the snd-ca0106 sound card reports the values.
> 
> If the method is agreed upon, I will extend it to many more sound cards.

Can you briefly explain the process for adding this feature to an
existing driver (IOW say you have no docs just a vendor-obfuscated or
RE'd driver, how do you determine db_scale)?

Lee



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Lee Revell wrote:
> On Thu, 2005-12-15 at 18:34 +0000, James Courtier-Dutton wrote:
> 
>>Hi,
>>
>>I have implemented dB gain reporting for ALSA mixer controls.
>>By applying the attached patches to alsa-driver and alsa-lib, one will 
>>then see dB values appear in alsamixer. (I added support to alsamixer a 
>>long time ago)
>>
>>Currently, only the snd-ca0106 sound card reports the values.
>>
>>If the method is agreed upon, I will extend it to many more sound cards.
> 
> 
> Can you briefly explain the process for adding this feature to an
> existing driver (IOW say you have no docs just a vendor-obfuscated or
> RE'd driver, how do you determine db_scale)?
> 
> Lee
> 

There is no point in RE the windows driver, because windows does not 
display dB values.
You do it simply by measurements.
You calibrate a line-in from a sound card that has documentation.
Then connect it to the suspect sound card's output and measure it.

Same for mic/line in.

I will be implementing this dB support on creative cards first, as I 
have some details for them. We have docs for all AC97 controls. So 
hopefully that won't leave many other controls to do.

Please note, my last patch has a few errors in it, so that it does not 
always supress the dB display with unsupported cards. I have fixed this 
in my own tree, but I would like full feedback first.

Which sound card do you have?

James



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Lee Revell]

On Thu, 2005-12-15 at 19:06 +0000, James Courtier-Dutton wrote:
> Lee Revell wrote:
> > On Thu, 2005-12-15 at 18:34 +0000, James Courtier-Dutton wrote:
> > 
> >>Hi,
> >>
> >>I have implemented dB gain reporting for ALSA mixer controls.
> >>By applying the attached patches to alsa-driver and alsa-lib, one will 
> >>then see dB values appear in alsamixer. (I added support to alsamixer a 
> >>long time ago)
> >>
> >>Currently, only the snd-ca0106 sound card reports the values.
> >>
> >>If the method is agreed upon, I will extend it to many more sound cards.
> > 
> > 
> > Can you briefly explain the process for adding this feature to an
> > existing driver (IOW say you have no docs just a vendor-obfuscated or
> > RE'd driver, how do you determine db_scale)?
> > 
> > Lee
> > 
> 
> There is no point in RE the windows driver, because windows does not 
> display dB values.
> You do it simply by measurements.
> You calibrate a line-in from a sound card that has documentation.
> Then connect it to the suspect sound card's output and measure it.
> 
> Same for mic/line in.
> 
> I will be implementing this dB support on creative cards first, as I 
> have some details for them. We have docs for all AC97 controls. So 
> hopefully that won't leave many other controls to do.
> 
> Please note, my last patch has a few errors in it, so that it does not 
> always supress the dB display with unsupported cards. I have fixed this 
> in my own tree, but I would like full feedback first.
> 
> Which sound card do you have?

Two Creative cards (an SBLive platinum CT4760 and Audigy2 ZS platinum)

Lee



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Jaroslav Kysela]

On Thu, 15 Dec 2005, James Courtier-Dutton wrote:

> I have implemented dB gain reporting for ALSA mixer controls.
> By applying the attached patches to alsa-driver and alsa-lib, one will then
> see dB values appear in alsamixer. (I added support to alsamixer a long time
> ago)
> 
> Currently, only the snd-ca0106 sound card reports the values.

NACK.

1) The midlevel code should not know anything about dB scale or so...
   It should handle only universal containter / TLV stuff per control.
   I'm talking about dB_scale members etc. The TLV should be obtained
   from the lowlevel driver using a callback like get. You can provide
   the library function to manage TLV in the control midlevel code.
2) The ca0106 driver is not candidate for such info. The info should
   be static somewhere in alsa-lib, probably in the src/control/control.c
   and added only when TLV stuff is requested.
3) I'm still not sure to use integers as keys in the TLV 
   tree. Of course, it's more efficient than string handling, but
   I think that it's not an issue because the amount of transferred
   data is low.
4) Nesting. Do we need this on the driver side? The key might contain
   multiple "sub-keys". If we have this information static, it might be
   more efficient than your implementation.

So, what about this (use integers as keys):

- one key - max 8 * 32-bit subkeys
- value   - depending on subkeys, store only the size, might be binary

or (use strings as keys):

- one key - max 8 * 16 byte subkeys (ASCII string)
- value   - depending on subkeys, store only the size, might be binary

So the key lookup function will handle max. 256 bytes which is no so bad.
The lowlevel driver might use some key-lookup library functions in the 
control midlevel code like snd_control_key_is_db_scale() or so and put
directly the data in the get_key() callback to value area.

Perhaps, we can use 8 byte strings (it's enough) or use 64-bit values for 
sub-keys which might represent strings (I can imagine that first sub-key 
will be string and rest of sub-keys might be indexes, strings, 64-bit 
values, 64-bit masks etc.).

In future, if 8-subkeys are not enough, we may create a new ioctl which 
will add more space to the whole key. The old ioctl might be simply 
emulated with the conversion to new one.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Jaroslav Kysela wrote:
> On Thu, 15 Dec 2005, James Courtier-Dutton wrote:
> 
> 
>>I have implemented dB gain reporting for ALSA mixer controls.
>>By applying the attached patches to alsa-driver and alsa-lib, one will then
>>see dB values appear in alsamixer. (I added support to alsamixer a long time
>>ago)
>>
>>Currently, only the snd-ca0106 sound card reports the values.
> 
> 
> NACK.
> 
> 1) The midlevel code should not know anything about dB scale or so...
>    It should handle only universal containter / TLV stuff per control.
>    I'm talking about dB_scale members etc. The TLV should be obtained
>    from the lowlevel driver using a callback like get. You can provide
>    the library function to manage TLV in the control midlevel code.

I was viewing the TLV as simply a transport across the IOCTL interface.
If one needs to use a new TLV, one would have to implement a new 
function in control.c in alsa-lib.
I have put the volume to db_gain calculation in hcontrol.c, but it could 
just as easily be in simple_none.c.

> 2) The ca0106 driver is not candidate for such info. The info should
>    be static somewhere in alsa-lib, probably in the src/control/control.c
>    and added only when TLV stuff is requested.

What "info" are you talking about? If you are talking about the db_scale 
information, then I disagree with you on this point and I explained why 
some time ago in a previous thread. I think Takashi agreed with me in 
the end, once I managed to fully explain the reasons.

> 3) I'm still not sure to use integers as keys in the TLV 
>    tree. Of course, it's more efficient than string handling, but
>    I think that it's not an issue because the amount of transferred
>    data is low.

It if flexible. TLV allows for the value to be integer, string, 
ip-address, anything in fact.
If you mean "key" where I say "Type" in the TLV, then integers is 
certainly the best way to go. Then the nesting provides for re-use of 
"Type" numbers. Using strings as the "Type" or "key" just adds bloat, 
with no real benefit.

> 4) Nesting. Do we need this on the driver side? The key might contain
>    multiple "sub-keys". If we have this information static, it might be
>    more efficient than your implementation.

Efficient in what way?
The current method is efficient in that it is implemented using very few 
lines of code, and thus not much code bloat.

> 
> So, what about this (use integers as keys):
> 
> - one key - max 8 * 32-bit subkeys
> - value   - depending on subkeys, store only the size, might be binary
> 
> or (use strings as keys):
> 
> - one key - max 8 * 16 byte subkeys (ASCII string)
> - value   - depending on subkeys, store only the size, might be binary
>

Why do you want to place limits on the size of the request/response?

> So the key lookup function will handle max. 256 bytes which is no so bad.
> The lowlevel driver might use some key-lookup library functions in the 
> control midlevel code like snd_control_key_is_db_scale() or so and put
> directly the data in the get_key() callback to value area.
> 
> Perhaps, we can use 8 byte strings (it's enough) or use 64-bit values for 
> sub-keys which might represent strings (I can imagine that first sub-key 
> will be string and rest of sub-keys might be indexes, strings, 64-bit 
> values, 64-bit masks etc.).
> 
> In future, if 8-subkeys are not enough, we may create a new ioctl which 
> will add more space to the whole key. The old ioctl might be simply 
> emulated with the conversion to new one.
> 
> 						Jaroslav
> 
> -----
> Jaroslav Kysela <perex@suse.cz>
> Linux Kernel Sound Maintainer
> ALSA Project, SUSE Labs
> 
> 



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Takashi Iwai]

At Thu, 15 Dec 2005 19:45:57 +0000,
James Courtier-Dutton wrote:
> 
> > 2) The ca0106 driver is not candidate for such info. The info should
> >    be static somewhere in alsa-lib, probably in the src/control/control.c
> >    and added only when TLV stuff is requested.
> 
> What "info" are you talking about? If you are talking about the db_scale 
> information, then I disagree with you on this point and I explained why 
> some time ago in a previous thread. I think Takashi agreed with me in 
> the end, once I managed to fully explain the reasons.

Err, then it's a misunderstanding.  I'm also against inclusion of
unnecessary code in the kernel.  That is, if the dB information for
each control is static and unchanged, it shouldn't be embedded in the
driver.

I agree to add a new ioctl for drivers with dynamic controls or
the drivers taking the dB information from the codec chip.
The former case is emu10k1, which mixer can be changed via ld10k1.
The latter case is usb-audio and hda-intel.

Since the number of drivers requring this ioctl should be relatively
small, I suppose that the implementation would be hardware-dependent.
So, a generic "hint" and a dedicated parser in alsa-lib would suffice,
rather than defining a too complicated struct to cover everyhing about
dB.

The case of ac97 needs a further investigation.  I believe it can be
implemented well in alsa-lib, but the implementation on the driver
side _might_ be more efficient.  Let's see the code.


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Takashi Iwai wrote:
> At Thu, 15 Dec 2005 19:45:57 +0000,
> James Courtier-Dutton wrote:
> 
>>>2) The ca0106 driver is not candidate for such info. The info should
>>>   be static somewhere in alsa-lib, probably in the src/control/control.c
>>>   and added only when TLV stuff is requested.
>>
>>What "info" are you talking about? If you are talking about the db_scale 
>>information, then I disagree with you on this point and I explained why 
>>some time ago in a previous thread. I think Takashi agreed with me in 
>>the end, once I managed to fully explain the reasons.
> 
> 
> Err, then it's a misunderstanding.  I'm also against inclusion of
> unnecessary code in the kernel.  That is, if the dB information for
> each control is static and unchanged, it shouldn't be embedded in the
> driver.
> 
> I agree to add a new ioctl for drivers with dynamic controls or
> the drivers taking the dB information from the codec chip.
> The former case is emu10k1, which mixer can be changed via ld10k1.
> The latter case is usb-audio and hda-intel.
> 
> Since the number of drivers requring this ioctl should be relatively
> small, I suppose that the implementation would be hardware-dependent.
> So, a generic "hint" and a dedicated parser in alsa-lib would suffice,
> rather than defining a too complicated struct to cover everyhing about
> dB.
> 
> The case of ac97 needs a further investigation.  I believe it can be
> implemented well in alsa-lib, but the implementation on the driver
> side _might_ be more efficient.  Let's see the code.
> 
> 
> Takashi
> 
> 

Ok, somone else can write this. I give up.

Your solution is going to break down in the edge cases, and mine adds 
just 4 bytes to each mixer control structure in the kernel. No extra 
code at all to each driver. Just some lines of extra core code to handle 
the new ioctl.

Your suggestions will add considerably to code bloat in the alsa-lib, 
and probably not work very well due to the edge cases.

James


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Takashi Iwai]

At Fri, 16 Dec 2005 10:47:06 +0000,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> > At Thu, 15 Dec 2005 19:45:57 +0000,
> > James Courtier-Dutton wrote:
> > 
> >>>2) The ca0106 driver is not candidate for such info. The info should
> >>>   be static somewhere in alsa-lib, probably in the src/control/control.c
> >>>   and added only when TLV stuff is requested.
> >>
> >>What "info" are you talking about? If you are talking about the db_scale 
> >>information, then I disagree with you on this point and I explained why 
> >>some time ago in a previous thread. I think Takashi agreed with me in 
> >>the end, once I managed to fully explain the reasons.
> > 
> > 
> > Err, then it's a misunderstanding.  I'm also against inclusion of
> > unnecessary code in the kernel.  That is, if the dB information for
> > each control is static and unchanged, it shouldn't be embedded in the
> > driver.
> > 
> > I agree to add a new ioctl for drivers with dynamic controls or
> > the drivers taking the dB information from the codec chip.
> > The former case is emu10k1, which mixer can be changed via ld10k1.
> > The latter case is usb-audio and hda-intel.
> > 
> > Since the number of drivers requring this ioctl should be relatively
> > small, I suppose that the implementation would be hardware-dependent.
> > So, a generic "hint" and a dedicated parser in alsa-lib would suffice,
> > rather than defining a too complicated struct to cover everyhing about
> > dB.
> > 
> > The case of ac97 needs a further investigation.  I believe it can be
> > implemented well in alsa-lib, but the implementation on the driver
> > side _might_ be more efficient.  Let's see the code.
> > 
> > 
> > Takashi
> > 
> > 
> 
> Ok, somone else can write this. I give up.
> 
> Your solution is going to break down in the edge cases, and mine adds 
> just 4 bytes to each mixer control structure in the kernel. No extra 
> code at all to each driver. Just some lines of extra core code to handle 
> the new ioctl.
> 
> Your suggestions will add considerably to code bloat in the alsa-lib, 
> and probably not work very well due to the edge cases.

The code won't be compliex nor bload so much as you thought.
Imagine that you just move the table of dB information into alsa-lib
like

	static struct db_info_lookup table[] = {
		{ "ca0106", db_info_ca0106 },
		...
	};

The static table could be implemented as an external file, too, of
course.

OTOH, the lookup-table in alsa-lib has a drawback that the table must
includes the whole snd_ctl_elem_id, thus the memory usage about the
control name will be doubled.  So the actual size might be bigger.


... OK, now I've been slowly convinced to add minimal dB information
to the kernel side.  The further question is the implementation.

IMO, your method, everything-about-parser-is-in-alsa-lib, is a bit
exaggerated.  In that way, you'll have to add a different type for
each driver and chip and end up with endless bloat of new types.

As Liam suggested, most cases are linear.  They can be embedded in 4
bytes, including min, max, step and 0dB-offset.  Take a look at
HD-audio codec or USB-audio specification.
(BTW, the floating point expression is (still) prohibitied in the
 kernel code.)

One exception is the volume muting at the lowest value as often seen
in ac97 codecs.  This needs a different type.
More than 90% cases could be coverted by these two types (linear and
linear+mute), I guess.


In addition, some suggestions/questions about your patch:

- The name snd_ctl_misc sounds too ambiguous

- (in kernel) Move the dB-specific stuff out of snd_ctl_elem_misc_user()

- TLV parser (e.g. uint32_to_message) should be more optimized :)

- As Jaroslav pointed, the parsing of dB information should be in the
  mixer layer instead of [h]control.

- How to implement the reverse conversion, from dB to raw?


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Jaroslav Kysela]

On Fri, 16 Dec 2005, Takashi Iwai wrote:

> OTOH, the lookup-table in alsa-lib has a drawback that the table must
> includes the whole snd_ctl_elem_id, thus the memory usage about the
> control name will be doubled.  So the actual size might be bigger.

I don't think so. The card specific code can be called using a callback
and the callback might provide the data lookup quite efficiently, so
not all or only partial parts from snd_ctl_elem_id can be there.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Takashi Iwai wrote:
> 
> 
> The code won't be compliex nor bload so much as you thought.
> Imagine that you just move the table of dB information into alsa-lib
> like
> 
> 	static struct db_info_lookup table[] = {
> 		{ "ca0106", db_info_ca0106 },
> 		...
> 	};
> 
> The static table could be implemented as an external file, too, of
> course.
> 
> OTOH, the lookup-table in alsa-lib has a drawback that the table must
> includes the whole snd_ctl_elem_id, thus the memory usage about the
> control name will be doubled.  So the actual size might be bigger.
> 
> 
> ... OK, now I've been slowly convinced to add minimal dB information
> to the kernel side.  The further question is the implementation.
> 
> IMO, your method, everything-about-parser-is-in-alsa-lib, is a bit
> exaggerated.  In that way, you'll have to add a different type for
> each driver and chip and end up with endless bloat of new types.
> 
> As Liam suggested, most cases are linear.  They can be embedded in 4
> bytes, including min, max, step and 0dB-offset.  Take a look at
> HD-audio codec or USB-audio specification.
> (BTW, the floating point expression is (still) prohibitied in the
>  kernel code.)
> 
> One exception is the volume muting at the lowest value as often seen
> in ac97 codecs.  This needs a different type.
> More than 90% cases could be coverted by these two types (linear and
> linear+mute), I guess.
> 

In my thoughts, I started with just the hint, then tried what Liam 
described (long before Liam described it. I think we all discussed it a 
some time ago.), and then when back to hint.
The "hint" is also a lot neater on the kernel side.
I am also looking forward to when we finally do have a card-mixer.conf 
config file. We would then put all the conversion function information 
in there. So, when a new card driver appears, one also adds the alsa-lib 
card-pcm.conf and card-mixer.conf and you are done. We already have to 
add the card-pcm.conf file with each new sound card type, so it is not 
adding much bloat.
The "hint" is also perfect for the "reverse conversion" requirement. The 
same "hint" can be the index into the "reverse conversion function" 
lookup as well as the "forward conversion function".

> 
> In addition, some suggestions/questions about your patch:
> 
> - The name snd_ctl_misc sounds too ambiguous

The name "snd_ctl_misc" could be anything. It is not dB specific, 
because that same ioctl could be used for other things. Don't know what 
yet, but some future requirement might need it. I suppose we could call 
is "snd_ctl_tlv" but that is about as descriptive as "snd_ctl_misc"

> 
> - (in kernel) Move the dB-specific stuff out of snd_ctl_elem_misc_user()

Do you just mean move the dB-specific stuff out to it's own function.
I just figured to leave it there for now, as it is quite short. If it 
gets more complicated, we can break it up.

> 
> - TLV parser (e.g. uint32_to_message) should be more optimized :)

True, but hardly important at this stage.

> 
> - As Jaroslav pointed, the parsing of dB information should be in the
>   mixer layer instead of [h]control.

No problem, so move the stuff from
hcontrol.c:snd_hctl_elem_get_db_gain
to
simple_none.c:get_dB_ops
or some extra sub function in simple_none.c

If you mean move the parsing done in
control.c:snd_ctl_elem_get_db_scale
out to simple_none.c, then I don't think we can do that.
The snd_ctl_misc_t is only visible to control.c.

> 
> - How to implement the reverse conversion, from dB to raw?
> 

The "hint" is also perfect for the "reverse conversion" requirement. The 
same "hint" can be the index into the "reverse conversion function" 
lookup as well as the "forward conversion function".

> 
> Takashi
> 
> 

James


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Takashi Iwai]

At Fri, 16 Dec 2005 12:43:21 +0000,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> > 
> > 
> > The code won't be compliex nor bload so much as you thought.
> > Imagine that you just move the table of dB information into alsa-lib
> > like
> > 
> > 	static struct db_info_lookup table[] = {
> > 		{ "ca0106", db_info_ca0106 },
> > 		...
> > 	};
> > 
> > The static table could be implemented as an external file, too, of
> > course.
> > 
> > OTOH, the lookup-table in alsa-lib has a drawback that the table must
> > includes the whole snd_ctl_elem_id, thus the memory usage about the
> > control name will be doubled.  So the actual size might be bigger.
> > 
> > 
> > ... OK, now I've been slowly convinced to add minimal dB information
> > to the kernel side.  The further question is the implementation.
> > 
> > IMO, your method, everything-about-parser-is-in-alsa-lib, is a bit
> > exaggerated.  In that way, you'll have to add a different type for
> > each driver and chip and end up with endless bloat of new types.
> > 
> > As Liam suggested, most cases are linear.  They can be embedded in 4
> > bytes, including min, max, step and 0dB-offset.  Take a look at
> > HD-audio codec or USB-audio specification.
> > (BTW, the floating point expression is (still) prohibitied in the
> >  kernel code.)
> > 
> > One exception is the volume muting at the lowest value as often seen
> > in ac97 codecs.  This needs a different type.
> > More than 90% cases could be coverted by these two types (linear and
> > linear+mute), I guess.
> > 
> 
> In my thoughts, I started with just the hint, then tried what Liam 
> described (long before Liam described it. I think we all discussed it a 
> some time ago.), and then when back to hint.
> The "hint" is also a lot neater on the kernel side.

Well, the composition of 4-bytes info can be done via a macro, so it's
even more readable than a strange index.

> I am also looking forward to when we finally do have a card-mixer.conf 
> config file. We would then put all the conversion function information 
> in there.

This will result in the bloat exactly as same as you pointed in your
last mail.

>  So, when a new card driver appears, one also adds the alsa-lib 
> card-pcm.conf and card-mixer.conf and you are done. We already have to 
> add the card-pcm.conf file with each new sound card type, so it is not 
> adding much bloat.

If you introduce the extra data and the extra parsing/handling codes
for each driver/control type, then there is no merit to embed the
information in the kernel at all.  The only merit to have information
in the kernel is to reduce the total size.

> The "hint" is also perfect for the "reverse conversion" requirement. The 
> same "hint" can be the index into the "reverse conversion function" 
> lookup as well as the "forward conversion function".

Ditto for the composed info.  It's pretty reversible.

The composed information is not perfect at all.  But it can conver
most things.  That's the point.


> > In addition, some suggestions/questions about your patch:
> > 
> > - The name snd_ctl_misc sounds too ambiguous
> 
> The name "snd_ctl_misc" could be anything. It is not dB specific, 
> because that same ioctl could be used for other things. Don't know what 
> yet, but some future requirement might need it. I suppose we could call 
> is "snd_ctl_tlv" but that is about as descriptive as "snd_ctl_misc"

Sure, I understood that it's to be generic.  But, snd_ctl_misc...
misc what?  I'd better understand if it's called snd_ctl_misc_info.
Anyway, I leave this decision to native english speaker.

> > - (in kernel) Move the dB-specific stuff out of snd_ctl_elem_misc_user()
> 
> Do you just mean move the dB-specific stuff out to it's own function.
> I just figured to leave it there for now, as it is quite short. If it 
> gets more complicated, we can break it up.

The current code looks messy and far from the concenpt to be generic.

> > - TLV parser (e.g. uint32_to_message) should be more optimized :)
> 
> True, but hardly important at this stage.

For example, you call kmalloc at each time for this ioctl.  This can
be better done by direct read. 
Such implementation details may influence on the definition,
e.g. reducing the struct size, etc.

> > - As Jaroslav pointed, the parsing of dB information should be in the
> >   mixer layer instead of [h]control.
> 
> No problem, so move the stuff from
> hcontrol.c:snd_hctl_elem_get_db_gain
> to
> simple_none.c:get_dB_ops
> or some extra sub function in simple_none.c
> 
> If you mean move the parsing done in
> control.c:snd_ctl_elem_get_db_scale
> out to simple_none.c, then I don't think we can do that.
> The snd_ctl_misc_t is only visible to control.c.

How about to simply pass snd_ctl_misc_t to the higher layer?
We don't have to export this function to external.


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Takashi Iwai wrote:
>>In my thoughts, I started with just the hint, then tried what Liam 
>>described (long before Liam described it. I think we all discussed it a 
>>some time ago.), and then when back to hint.
>>The "hint" is also a lot neater on the kernel side.
> 
> 
> Well, the composition of 4-bytes info can be done via a macro, so it's
> even more readable than a strange index.

How do you propose that 4 separate bytes composed will contain enough 
information. Some of the current volume controls range more than 0-255.

> 
> 
>>I am also looking forward to when we finally do have a card-mixer.conf 
>>config file. We would then put all the conversion function information 
>>in there.
> 
> 
> This will result in the bloat exactly as same as you pointed in your
> last mail.

But a lot less bloat than the alternative or user land only.

> 
> 
>> So, when a new card driver appears, one also adds the alsa-lib 
>>card-pcm.conf and card-mixer.conf and you are done. We already have to 
>>add the card-pcm.conf file with each new sound card type, so it is not 
>>adding much bloat.
> 
> 
> If you introduce the extra data and the extra parsing/handling codes
> for each driver/control type, then there is no merit to embed the
> information in the kernel at all.  The only merit to have information
> in the kernel is to reduce the total size.

Untrue, it will still result is a lot less bloat than a user land only 
solution

> 
> 
>>The "hint" is also perfect for the "reverse conversion" requirement. The 
>>same "hint" can be the index into the "reverse conversion function" 
>>lookup as well as the "forward conversion function".
> 
> 
> Ditto for the composed info.  It's pretty reversible.
> 
> The composed information is not perfect at all.  But it can conver
> most things.  That's the point.

We can't ignore the edge cases where the composed information will not 
be suitable. How do you propose to support them?

> 
> 
> 
>>>In addition, some suggestions/questions about your patch:
>>>
>>>- The name snd_ctl_misc sounds too ambiguous
>>
>>The name "snd_ctl_misc" could be anything. It is not dB specific, 
>>because that same ioctl could be used for other things. Don't know what 
>>yet, but some future requirement might need it. I suppose we could call 
>>is "snd_ctl_tlv" but that is about as descriptive as "snd_ctl_misc"
> 
> 
> Sure, I understood that it's to be generic.  But, snd_ctl_misc...
> misc what?  I'd better understand if it's called snd_ctl_misc_info.
> Anyway, I leave this decision to native english speaker.
> 
> 
>>>- (in kernel) Move the dB-specific stuff out of snd_ctl_elem_misc_user()
>>
>>Do you just mean move the dB-specific stuff out to it's own function.
>>I just figured to leave it there for now, as it is quite short. If it 
>>gets more complicated, we can break it up.
> 
> 
> The current code looks messy and far from the concenpt to be generic.
> 
> 
>>>- TLV parser (e.g. uint32_to_message) should be more optimized :)
>>
>>True, but hardly important at this stage.
> 
> 
> For example, you call kmalloc at each time for this ioctl.  This can
> be better done by direct read. 
> Such implementation details may influence on the definition,
> e.g. reducing the struct size, etc.
> 
> 
>>>- As Jaroslav pointed, the parsing of dB information should be in the
>>>  mixer layer instead of [h]control.
>>
>>No problem, so move the stuff from
>>hcontrol.c:snd_hctl_elem_get_db_gain
>>to
>>simple_none.c:get_dB_ops
>>or some extra sub function in simple_none.c
>>
>>If you mean move the parsing done in
>>control.c:snd_ctl_elem_get_db_scale
>>out to simple_none.c, then I don't think we can do that.
>>The snd_ctl_misc_t is only visible to control.c.
> 
> 
> How about to simply pass snd_ctl_misc_t to the higher layer?
> We don't have to export this function to external.
> 

I thought one of the points of the layers was to hide stuff below it.
Your suggestion just breaks that.

> 
> Takashi
> 
> 



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Takashi Iwai]

At Fri, 16 Dec 2005 13:47:43 +0000,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> >>In my thoughts, I started with just the hint, then tried what Liam 
> >>described (long before Liam described it. I think we all discussed it a 
> >>some time ago.), and then when back to hint.
> >>The "hint" is also a lot neater on the kernel side.
> > 
> > 
> > Well, the composition of 4-bytes info can be done via a macro, so it's
> > even more readable than a strange index.
> 
> How do you propose that 4 separate bytes composed will contain enough 
> information. Some of the current volume controls range more than 0-255.

The trick is to define "step".  Then min/max can be usually within
8 bits.  I recommend you to take a look at HD-audio specification, for
example.


> >>I am also looking forward to when we finally do have a card-mixer.conf 
> >>config file. We would then put all the conversion function information 
> >>in there.
> > 
> > 
> > This will result in the bloat exactly as same as you pointed in your
> > last mail.
> 
> But a lot less bloat than the alternative or user land only.
> 
> >> So, when a new card driver appears, one also adds the alsa-lib 
> >>card-pcm.conf and card-mixer.conf and you are done. We already have to 
> >>add the card-pcm.conf file with each new sound card type, so it is not 
> >>adding much bloat.
> > 
> > 
> > If you introduce the extra data and the extra parsing/handling codes
> > for each driver/control type, then there is no merit to embed the
> > information in the kernel at all.  The only merit to have information
> > in the kernel is to reduce the total size.
> 
> Untrue, it will still result is a lot less bloat than a user land only 
> solution

I disagree.


> >>The "hint" is also perfect for the "reverse conversion" requirement. The 
> >>same "hint" can be the index into the "reverse conversion function" 
> >>lookup as well as the "forward conversion function".
> > 
> > 
> > Ditto for the composed info.  It's pretty reversible.
> > 
> > The composed information is not perfect at all.  But it can conver
> > most things.  That's the point.
> 
> We can't ignore the edge cases where the composed information will not 
> be suitable. How do you propose to support them?

In such a case, they must be handled differently, of course.  Either a
special type as you implemented, or completely on user-space.

What I mention is that a uniqe type index and the correponding
parser/handling code for each control is inefficient, especially from 
the code-amount viewpoint.  You have to add a code to alsa-lib at each
time you add a new control in the kernel side.  This eliminates the
merit to embe the info into the kernel.


> >>>In addition, some suggestions/questions about your patch:
> >>>
> >>>- The name snd_ctl_misc sounds too ambiguous
> >>
> >>The name "snd_ctl_misc" could be anything. It is not dB specific, 
> >>because that same ioctl could be used for other things. Don't know what 
> >>yet, but some future requirement might need it. I suppose we could call 
> >>is "snd_ctl_tlv" but that is about as descriptive as "snd_ctl_misc"
> > 
> > 
> > Sure, I understood that it's to be generic.  But, snd_ctl_misc...
> > misc what?  I'd better understand if it's called snd_ctl_misc_info.
> > Anyway, I leave this decision to native english speaker.
> > 
> > 
> >>>- (in kernel) Move the dB-specific stuff out of snd_ctl_elem_misc_user()
> >>
> >>Do you just mean move the dB-specific stuff out to it's own function.
> >>I just figured to leave it there for now, as it is quite short. If it 
> >>gets more complicated, we can break it up.
> > 
> > 
> > The current code looks messy and far from the concenpt to be generic.
> > 
> > 
> >>>- TLV parser (e.g. uint32_to_message) should be more optimized :)
> >>
> >>True, but hardly important at this stage.
> > 
> > 
> > For example, you call kmalloc at each time for this ioctl.  This can
> > be better done by direct read. 
> > Such implementation details may influence on the definition,
> > e.g. reducing the struct size, etc.
> > 
> > 
> >>>- As Jaroslav pointed, the parsing of dB information should be in the
> >>>  mixer layer instead of [h]control.
> >>
> >>No problem, so move the stuff from
> >>hcontrol.c:snd_hctl_elem_get_db_gain
> >>to
> >>simple_none.c:get_dB_ops
> >>or some extra sub function in simple_none.c
> >>
> >>If you mean move the parsing done in
> >>control.c:snd_ctl_elem_get_db_scale
> >>out to simple_none.c, then I don't think we can do that.
> >>The snd_ctl_misc_t is only visible to control.c.
> > 
> > 
> > How about to simply pass snd_ctl_misc_t to the higher layer?
> > We don't have to export this function to external.
> > 
> 
> I thought one of the points of the layers was to hide stuff below it.

No, we don't hide the functionality at all.  We hide the definition of
each struct, but this is just for external apps.  And, I think the TLV
expression is extensible and generic enough, thus see no big reason to
hide it.

Remember that the dB stuff is only for mixer.  It's not for gneric
control stuff.  Therefore, it's natural that the mixer part parses the
contents.


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Jaroslav Kysela]

On Fri, 16 Dec 2005, Takashi Iwai wrote:

> The trick is to define "step".  Then min/max can be usually within
> 8 bits.  I recommend you to take a look at HD-audio specification, for
> example.

Just short note: Wouldn't be better to use a RPN expression to describe dB 
<-> raw conversions like rrdtool uses? Just idea (of course that everybody 
tells me that it's overkill).

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Liam Girdwood]

On Fri, 2005-12-16 at 15:11 +0100, Jaroslav Kysela wrote:
> On Fri, 16 Dec 2005, Takashi Iwai wrote:
> 
> > The trick is to define "step".  Then min/max can be usually within
> > 8 bits.  I recommend you to take a look at HD-audio specification, for
> > example.
> 
> Just short note: Wouldn't be better to use a RPN expression to describe dB 
> <-> raw conversions like rrdtool uses? Just idea (of course that everybody 
> tells me that it's overkill).
> 

I think it might just be overkill :) as whoever is writing the driver
code will probably just lift the values from the codec data sheet. The
data sheets I've seen typically just specify the dB range and step size.

Liam  



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Liam Girdwood]

On Fri, 2005-12-16 at 11:27 +0100, Takashi Iwai wrote:

> 
> Err, then it's a misunderstanding.  I'm also against inclusion of
> unnecessary code in the kernel.  That is, if the dB information for
> each control is static and unchanged, it shouldn't be embedded in the
> driver.
> 

I also agree that some dB information shouldn't be in the kernel (e.g.
lookup tables), however I think it makes sense for simple controls with
a linear dB step size to be within the driver. 

I'd like to be able to do something like:-

SOC_SINGLE("Left ADC Capture Volume", WM8753_LADC, 0, 63, 0, DB(-17.25,
0.75),

This describes a control with a dB gain starting at -17.25 and in
increments of 0.75dB. The dB info could be represented in hex as
follows:-

0.75db = 0xC
1.00 db = 0x10 
-1.00 dB = 0x8010 (assuming we are using a short)

With a little extra effort the dB code could be extended to pass ALC
timeout information.

Liam



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Jaroslav Kysela]

On Thu, 15 Dec 2005, James Courtier-Dutton wrote:

> Efficient in what way?
> The current method is efficient in that it is implemented using very few lines
> of code, and thus not much code bloat.

That's a question. I'm trying to propose something simple which can be
handled in most easy way.

> Why do you want to place limits on the size of the request/response?

No, the limits will be similar as yours. If you have 2048 bytes for ioctl 
request, there is plenty space for the response values.

My question is what is better: The static structure or dynamically created
containers.

With my proposal, the lowlevel driver can handle only such callbacks:

struct snd_ctl_extra {
	unsigned char key[8][8];
	unsigned char value[2048-256];
};

static int usb_control_get_extra_info(struct snd_kcontrol *kcontrol, 
				      struct snd_ctl_extra *extra)
{
	if (!strcmp(extra->key[0], "dB Scale")) {
		*((unsigned int *)extra) = 0x11223344;
		return 0;
	}
	return -ENOENT;
}

As you see:

1) no value size encoding / decoding (size is already known, because it is 
   assigned to key)
2) the callback can be shared among many controls (ususally one lowlevel
   driver has many controls with same dB scale values)
3) extra code for specific controls might be handled in the extra callback
   as 'if (kcontrol == driver->my_specific_kcontrol) { do_it; return 0; }'

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Jaroslav Kysela wrote:
> On Thu, 15 Dec 2005, James Courtier-Dutton wrote:
> 
> 
>>Efficient in what way?
>>The current method is efficient in that it is implemented using very few lines
>>of code, and thus not much code bloat.
> 
> 
> That's a question. I'm trying to propose something simple which can be
> handled in most easy way.
> 
> 
>>Why do you want to place limits on the size of the request/response?
> 
> 
> No, the limits will be similar as yours. If you have 2048 bytes for ioctl 
> request, there is plenty space for the response values.
> 
> My question is what is better: The static structure or dynamically created
> containers.
> 
> With my proposal, the lowlevel driver can handle only such callbacks:
> 
> struct snd_ctl_extra {
> 	unsigned char key[8][8];
> 	unsigned char value[2048-256];
> };
> 
> static int usb_control_get_extra_info(struct snd_kcontrol *kcontrol, 
> 				      struct snd_ctl_extra *extra)
> {
> 	if (!strcmp(extra->key[0], "dB Scale")) {
> 		*((unsigned int *)extra) = 0x11223344;
> 		return 0;
> 	}
> 	return -ENOENT;
> }
> 
> As you see:
> 
> 1) no value size encoding / decoding (size is already known, because it is 
>    assigned to key)
> 2) the callback can be shared among many controls (ususally one lowlevel
>    driver has many controls with same dB scale values)
> 3) extra code for specific controls might be handled in the extra callback
>    as 'if (kcontrol == driver->my_specific_kcontrol) { do_it; return 0; }'
> 
> 						Jaroslav
> 

Once the call gets into the kernel, whether it gathers the required 
information via a callback or just grabbing a value from a kernel 
structure makes very little difference. For the db_scale "hint", I 
though a callback was overkill, but some other value might be more 
complex and thus need a callback.

Currently my struct snd_ctl_misc is:
struct sndrv_ctl_misc {
         unsigned int type;
         unsigned int length;
         unsigned char message[2040];
};

What I was hoping for was something like this:
struct sndrv_ctl_misc {
         unsigned int type;
         unsigned int length;
         unsigned char message[];   /* Size variable */
};

The length would then contain the length of the message buffer that user 
land has allocated.

I did not actually try it yet though. That would at least make this api 
totally future proof.

Your static structure idea seems a little more constraining than my 
method. Although, at the moment I have no idea if in future we will need 
more than 8 keys or not, or whether they need to be nested or not, but 
at least my method does not rule those out.
I was thinking that a possible addition to my method would be a batch 
request. I.e. alsa-lib makes one request for all mixer elements, and my 
method would then need nesting in order to return the multiple element 
information fields.
Or, if one wanted the db_scale and some other information about a 
particular mixer element, then nesting would be needed.

James


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Jaroslav Kysela]

On Fri, 16 Dec 2005, James Courtier-Dutton wrote:

> Once the call gets into the kernel, whether it gathers the required
> information via a callback or just grabbing a value from a kernel structure
> makes very little difference.

It makes. The memory usage is a bit different. In your case you'll eat
a bit more memory, my solution eats a bit more code. Your solution cannot
be optimized with a programmer, my solution (code) can be nicely 
optimized.

> For the db_scale "hint", I though a callback was overkill, but some 
> other value might be more complex and thus need a callback.

Really? Imagine that all your control will use one callback (the hint is 
same). The code might use less or equal amount of memory than yours 
without extra runtime memory in the kcontrol structure.

> Your static structure idea seems a little more constraining than my method.

Sure, but the kernel API is not about to solve everything 100 years ahead.

> I was thinking that a possible addition to my method would be a batch 
> request.

It's not problem with my solution, too. Imagine a new ioctl() which 
requests a key and value size - which should be collected for application 
and a pointer to large user space memory area and you can do collecting, 
too. But these optimizations can be implemented on demand.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Takashi Iwai]

At Thu, 15 Dec 2005 20:13:57 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 15 Dec 2005, James Courtier-Dutton wrote:
> 
> > I have implemented dB gain reporting for ALSA mixer controls.
> > By applying the attached patches to alsa-driver and alsa-lib, one will then
> > see dB values appear in alsamixer. (I added support to alsamixer a long time
> > ago)
> > 
> > Currently, only the snd-ca0106 sound card reports the values.
> 
> NACK.
> 
> 1) The midlevel code should not know anything about dB scale or so...
>    It should handle only universal containter / TLV stuff per control.
>    I'm talking about dB_scale members etc. The TLV should be obtained
>    from the lowlevel driver using a callback like get. You can provide
>    the library function to manage TLV in the control midlevel code.
> 2) The ca0106 driver is not candidate for such info. The info should
>    be static somewhere in alsa-lib, probably in the src/control/control.c
>    and added only when TLV stuff is requested.
> 3) I'm still not sure to use integers as keys in the TLV 
>    tree. Of course, it's more efficient than string handling, but
>    I think that it's not an issue because the amount of transferred
>    data is low.
> 4) Nesting. Do we need this on the driver side? The key might contain
>    multiple "sub-keys". If we have this information static, it might be
>    more efficient than your implementation.
> 
> So, what about this (use integers as keys):
> 
> - one key - max 8 * 32-bit subkeys
> - value   - depending on subkeys, store only the size, might be binary
> 
> or (use strings as keys):
> 
> - one key - max 8 * 16 byte subkeys (ASCII string)
> - value   - depending on subkeys, store only the size, might be binary
> 
> So the key lookup function will handle max. 256 bytes which is no so bad.
> The lowlevel driver might use some key-lookup library functions in the 
> control midlevel code like snd_control_key_is_db_scale() or so and put
> directly the data in the get_key() callback to value area.
> 
> Perhaps, we can use 8 byte strings (it's enough) or use 64-bit values for 
> sub-keys which might represent strings (I can imagine that first sub-key 
> will be string and rest of sub-keys might be indexes, strings, 64-bit 
> values, 64-bit masks etc.).
> 
> In future, if 8-subkeys are not enough, we may create a new ioctl which 
> will add more space to the whole key. The old ioctl might be simply 
> emulated with the conversion to new one.

Well, I also can't image that so complicated TLV expressions will be
ever used.  James' implementation already includes the type tag at the
very beginning, so it's almost one-key-to-one-conent mapping.  The
record to be used would very likely only the unnested ones with fixed
sizes.

OTOH, the string or 256 byte keys look too much to me for our
purpuses.  It's hard to believe that we'll have so many different
keys.


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Liam Girdwood]

On Thu, 2005-12-15 at 18:34 +0000, James Courtier-Dutton wrote:

> 
> The only piece of information reported by the kernel sound driver is a 
> db_scale value.  It is a unsigned int at application layer, but the 
> current IOCTL TLV truncates this to 32bit unsigned int. alsa-lib then 
> uses this value to decide which translation function to apply to the 
> currently available "volume" value.
> I view this "db_scale" value as simply a hint from the low level driver 
> at which conversion function should be used.
> The conversion function in alsa-lib can then be as simple or as 
> complicated as required.
> 

I'm wondering whether it might be a better idea to use the "db_scale" to
store the specifics of the gain rather than just a hint.

IMHO we need to be able to specify:-

 o number of steps in control
 o starting dB setting
 o step size (including negative bit)

This could either be divided up into the 32 bits of db_scale or by
adding another member(s) to the struct.

This would then mean alsa lib doesn't need to know about the dB settings
for each codec/card (as they are quite different) and the dB information
would be in the driver where it belongs.

Liam

 



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[James Courtier-Dutton]

Liam Girdwood wrote:
> 
> I'm wondering whether it might be a better idea to use the "db_scale" to
> store the specifics of the gain rather than just a hint.
> 
> IMHO we need to be able to specify:-
> 
>  o number of steps in control
>  o starting dB setting
>  o step size (including negative bit)
> 
> This could either be divided up into the 32 bits of db_scale or by
> adding another member(s) to the struct.
> 
> This would then mean alsa lib doesn't need to know about the dB settings
> for each codec/card (as they are quite different) and the dB information
> would be in the driver where it belongs.
> 
> Liam
> 

We discussed this point some time ago on the list.
the "hint" is better, because it then lets alsa-lib decide which 
conversion function to use.
Problems with your approach were:
1) Control with non-linear dB steps. E.g. attenuation using a different 
scale than gain.
2) Complicates controls where only a lookup table will do.

James



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.

[Liam Girdwood]

On Thu, 2005-12-15 at 19:48 +0000, James Courtier-Dutton wrote:

> Problems with your approach were:
> 1) Control with non-linear dB steps. E.g. attenuation using a different 
> scale than gain.

In my experience, the majority of controls (certainly in the embedded
space) use a linear dB scale to adjust the gain/attenuation and would be
better served by a value rather than a hint. A hint would still be
required for the non linear controls.

How do you feel about adding a further field called db_hint. This way
the hint method could be used for the more complex and non linear
controls whilst the value method could be used for the simple ones. 

This would save a lot effort, as driver developers wouldn't need to add
any hints to alsa-lib for simple controls.
 
> 2) Complicates controls where only a lookup table will do.
> 

I agree, but with both fields this isn't an issue.

Further to this, I realised this code could also be used for other
controls that are unitised. i.e. some ALC controls use ms for timeouts.
This hint and scale could also be used to describe them.

Liam 



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click