At Sat, 10 Jun 2006 10:24:26 +0100,
James Courtier-Dutton wrote:
>
> Jaroslav Kysela wrote:
> > changeset: 4278:d194ae01b0ba9d5614ff29a82f4aacae9edd1212
> > tag: tip
> > user: perex
> > date: Thu Jun 1 18:34:01 2006 +0200
> > files: core/control.c include/asound.h include/control.h include/tlv.h pci/ca0106/ca0106_mixer.c
> > description:
> > Control API - TLV implementation for additional information like dB scale
> >
> > This patch implements a TLV mechanism to transfer an additional information
> > like dB scale to the user space. The types might be extended in future.
> >
> > Acked-by: Takashi Iwai <tiwai@xxxxxxx>
> >
> >
> > +struct snd_ctl_tlv {
> > + unsigned int numid; /* control element numeric identification */
> > + unsigned int length; /* in bytes aligned to 4 */
> > + unsigned int tlv[0]; /* first TLV */
> > +};
> > +
>
> > +static int snd_ctl_tlv_read(struct snd_card *card,
> > + struct snd_ctl_tlv __user *_tlv)
> > +{
> > + struct snd_ctl_tlv tlv;
> > + struct snd_kcontrol *kctl;
> > + unsigned int len;
> > + int err = 0;
> > +
> > + if (copy_from_user(&tlv, _tlv, sizeof(tlv)))
> > + return -EFAULT;
> > + if (tlv.length < sizeof(unsigned int) * 3)
> > + return -EINVAL;
> > + down_read(&card->controls_rwsem);
> > + kctl = snd_ctl_find_numid(card, tlv.numid);
> > + if (kctl == NULL) {
> > + err = -ENOENT;
> > + goto __kctl_end;
> > + }
> > + if (kctl->tlv == NULL) {
> > + err = -ENXIO;
> > + goto __kctl_end;
> > + }
> > + len = kctl->tlv[1] + 2 * sizeof(unsigned int);
> > + if (tlv.length < len) {
> > + err = -ENOMEM;
> > + goto __kctl_end;
> > + }
> > + if (copy_to_user(_tlv->tlv, kctl->tlv, len))
> > + err = -EFAULT;
> > + __kctl_end:
> > + up_read(&card->controls_rwsem);
> > + return err;
> > +}
>
> I have been away for the last week, so I was not able to comment on this
> commit.
I also didn't expect that the patch was merged so quickly without
testing...
> How do you intend to expand this for requests for anything apart from
> db_scale? For example, how could we get it to return all the current
> "info" (min,max etc.) information instead of the db_scale? Or, are you
> keeping it simple, and make a general request for info from the numid,
> and receive a group of TLVs back for all the different information that
> the numid might contain? I.e. one gets the entire group of information,
> regardless of what is actually needed.
I understand so. Get TLV composites and parse them in alsa-lib.
> My intention was to place a TLV or multiple TLVs in the request to
> select the information needed, and then receive the TLVs in return.
>
> For example, audio professionals don't like any volume steps at all.
> They like continuous volume gain adjustment. For example, they request
> +1dB up, the software then gradually increases the actual gain in really
> small steps until a +1dB is reached. This permits totally click free
> volume adjustments. This is all achievable on any sound cards with DSP
> based volume control. (i.e. not cheep hardware based volume control with
> defined steps.). To achieve this, one would pass the raw linear gain
> values from the DSP control (.e.g. 32bit values), and then use some more
> complicated userland (alsa-lib), possibly floating point, conversion
> function to convert it to dB gain levels (e.g.linear to log conversion).
> In this case, one might want the raw linear gain via the
> snd_ctl_tlv_read IOCTL, instead of the stepwise value currently returned
> using the current snd_ca0106_volume_get API. In which case, one would
> need a TLV in the request, to differentiate it from the db_scale request.
Until now, such a hardware-specific thing was supposed to be processed
over hwdep device. Maybe implementing a new ioctl in control API
might make the alsa-lib code a bit smaller.
> What I am trying to say, is that your "simplification" of the API, has
> resulted in this sort of functionality being excluded, so I would need
> to implement yet another IOCTL to support it! My whole point of using
> the full request/response TLV api, was to not restrict us. I was also
> intending to maybe use the TLVs to set values, as well as just get
> values. For example, another way to implement the "continuous gain"
> feature would be to use the TLV api to set a flag, so that the 32bit
> values are returned via the current snd_ca0106_volume_get(), but only
> for this particular mixer open/close cycle, for backward compatibility
> with the current api.
>
> Do you prefer separate new IOCTLs for all these features?
We can modify the implementation. For example, the patch like one
below. tlv_rw callback is completely free for own implementation.
Each callback is in change of reading/writing the given user-space
pointer arbitrarily.
Takashi
diff -r 08577d0b45ef core/control.c
--- a/core/control.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/control.c Wed Jun 14 16:09:50 2006 +0200
@@ -241,6 +241,7 @@ struct snd_kcontrol *snd_ctl_new1(const
kctl.info = ncontrol->info;
kctl.get = ncontrol->get;
kctl.put = ncontrol->put;
+ kctl.tlv_rw = ncontrol->tlv_rw;
kctl.tlv = ncontrol->tlv;
kctl.private_value = ncontrol->private_value;
kctl.private_data = private_data;
@@ -1086,17 +1087,21 @@ static int snd_ctl_tlv_read(struct snd_c
err = -ENOENT;
goto __kctl_end;
}
- if (kctl->tlv == NULL) {
- err = -ENXIO;
- goto __kctl_end;
- }
- len = kctl->tlv[1] + 2 * sizeof(unsigned int);
- if (tlv.length < len) {
- err = -ENOMEM;
- goto __kctl_end;
- }
- if (copy_to_user(_tlv->tlv, kctl->tlv, len))
- err = -EFAULT;
+ if (kctl->tlv_rw)
+ err = kctl->tlv_rw(kctl, tlv.length, _tlv->tlv);
+ else {
+ if (kctl->tlv == NULL) {
+ err = -ENXIO;
+ goto __kctl_end;
+ }
+ len = kctl->tlv[1] + 2 * sizeof(unsigned int);
+ if (tlv.length < len) {
+ err = -ENOMEM;
+ goto __kctl_end;
+ }
+ if (copy_to_user(_tlv->tlv, kctl->tlv, len))
+ err = -EFAULT;
+ }
__kctl_end:
up_read(&card->controls_rwsem);
return err;
diff -r 08577d0b45ef include/control.h
--- a/include/control.h Tue Jun 13 12:01:14 2006 +0200
+++ b/include/control.h Wed Jun 14 16:09:50 2006 +0200
@@ -30,6 +30,10 @@ typedef int (snd_kcontrol_info_t) (struc
typedef int (snd_kcontrol_info_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_info * uinfo);
typedef int (snd_kcontrol_get_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
typedef int (snd_kcontrol_put_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
+typedef int (snd_kcontrol_tlv_rw_t)(struct snd_kcontrol *kcontrol,
+ unsigned int size,
+ unsigned int __user *tlv);
+
struct snd_kcontrol_new {
snd_ctl_elem_iface_t iface; /* interface identifier */
@@ -42,6 +46,7 @@ struct snd_kcontrol_new {
snd_kcontrol_info_t *info;
snd_kcontrol_get_t *get;
snd_kcontrol_put_t *put;
+ snd_kcontrol_tlv_rw_t *tlv_rw;
unsigned int *tlv;
unsigned long private_value;
};
@@ -59,6 +64,7 @@ struct snd_kcontrol {
snd_kcontrol_info_t *info;
snd_kcontrol_get_t *get;
snd_kcontrol_put_t *put;
+ snd_kcontrol_tlv_rw_t *tlv_rw;
unsigned int *tlv;
unsigned long private_value;
void *private_data;
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
