Adds dB gain to alsa-driver, alsa-lib.

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

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
@@ -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
@@ -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
@@ -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
@@ -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,

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
@@ -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
@@ -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
@@ -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

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
@@ -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
@@ -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
@@ -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
@@ -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,

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

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

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

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 <[email protected]>
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

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.

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.

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.

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.

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

-------------------------------------------------------
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

At Thu, 15 Dec 2005 19:45:57 +0000,

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

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

At Fri, 16 Dec 2005 10:47:06 +0000,

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

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 <[email protected]>
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

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".

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"

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.

True, but hardly important at this stage.

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.

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".

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

At Fri, 16 Dec 2005 12:43:21 +0000,

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

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

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.

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.

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.

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

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.

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

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

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

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

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

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

-------------------------------------------------------
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

At Fri, 16 Dec 2005 13:47:43 +0000,

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 disagree.

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.

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

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 <[email protected]>
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

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

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

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

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 <[email protected]>
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

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

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.

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.

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

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 <[email protected]>
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

At Thu, 15 Dec 2005 20:13:57 +0100 (CET),

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

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

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

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.

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