On Fri, 19 Jun 2009, Jaroslav Kysela wrote:
> On Fri, 19 Jun 2009, Takashi Iwai wrote:
>
>> At Fri, 19 Jun 2009 10:47:30 +0200 (CEST),
>> Jaroslav Kysela wrote:
>>>
>>> On Fri, 19 Jun 2009, Takashi Iwai wrote:
>>>
>>>> Hi,
>>>>
>>>> this is yet another topic I'm (currently) working on -- the addition
>>>> of PCM ioctls to get/set some extra attributes. Basically, it adds
>>>> two simple ioctls for getting/setting extra attributes to the PCM
>>>> substream. The attribute has a sort of TLV form,
>>>>
>>>> /* PCM extra attributes */
>>>> struct snd_pcm_attr {
>>>> unsigned int type; /* SNDRC_PCM_TYPE_ATTR_XXX */
>>>> unsigned int len; /* GET R: the max elements in value array
>>>> * W: the actually written # elements
>>>> * SET R/W: # elements to store
>>>> */
>>>> unsigned int value[0]; /* value(s) to read / write */
>>>> };
>>>>
>>>> And corresponding two ioctls
>>>> #define SNDRV_PCM_IOCTL_GET_ATTR _IOWR('A', 0x14, struct snd_pcm_attr)
>>>> #define SNDRV_PCM_IOCTL_SET_ATTR _IOWR('A', 0x15, struct snd_pcm_attr)
>>>
>>> I would prefer to implement similar TLV implementation as for the control
>>> API. The amount of information for reading (get) will be small, so
>>> filtering in this direction is not necessary. Also, common parts of
>>> implementation (future merging of more TLVs to compounds) can be shared.
>>
>> Actually it's a sort of TLV. You see exactly it in snd_pcm_attr
>> struct, no? :)
>>
>> And, thinking twice after posting (that's a good effect of posting to
>> ML, BTW), I feel that using a callback would be a better way, such as
>> re-using the existing ops->ioctl with a new cmd tag rather than the
>> statically assigned thing.
>>
>> A similar method like control TLV can be used, too. However, a
>> distinct from the existing control TLV is that this is intended for
>> just one type of information while the control TLV is supposed to
>> contain everything in a single shot.
>>
>> That is, this is a query with a key. In that sense, sharing a small
>> amount of control TLV code (about 10 lines) doesn't give a big
>> benefit. In anyways, it's a implementation detail, so one could
>> optimize somehow, though...
>
> I don't mean current implementation. TLVs can be nested. In this case, we
> need a set of functions which operates with TLVs (merging). These
> functions can be shared. It's also possible to share TLV code in
> the user space (search). But it's really implementation detail. We should
> focus on ioctl definitions now.
>
> I would defined 'struct snd_pcm_attr' as 'struct snd_tlv' - it's same as
> for control API.
>
> The control API has:
>
> SNDRV_CTL_IOCTL_TLV_READ - read all static information
> SNDRV_CTL_IOCTL_TLV_WRITE - write static information (userspace controls)
> SNDRV_CTL_IOCTL_TLV_COMMAND - change some setup
>
> So, SNDRV_CTL_IOCTL_TLV_COMMAND == SNDRV_PCM_IOCTL_SET_ATTR in your
> proposal.
>
> SNDRV_CTL_IOCTL_TLV_WRITE is not probably useable unless we have virtual
> user-space PCM interface kernel implementation.
>
> SNDRV_CTL_IOCTL_TLV_READ might make sense for static-only information
> which don't change between open()/close() syscalls for given substream.
>
> SNDRV_PCM_IOCTL_GET_ATTR cannot be mapped at this time. Might be something
> like TLV_READONE, TLV_CONFIG, TLV_SETUP, TLV_GET or so - what's better
> for COMMAND word, if we agree on common names for all kernel interfaces.
BTW: It's also question, if to divide TLVs to static/configuration ones.
TLV_READ might just return all TLVs and TLV_READONE filter only one, if
user space does not want to obtain all information.
I would like to preserve TLV_READ to obtain all TLVs for possible user
space enumeration (for example for debugging purposes) rather that do a
single query for all possible TLV types.
Jaroslav
-----
Jaroslav Kysela <perex@xxxxxxxx>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
