Re: [PATCH 5/6] control: add a function to add an element of bytes type

[Takashi Iwai <tiwai@suse.de>]

On Tue, 23 Feb 2016 10:22:50 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Feb 23 2016 17:31, Clemens Ladisch wrote:
> > Takashi Sakamoto wrote:
> >> ALSA Ctl core allows userspace applications to add elements of bytes type,
> >> while there's no APIs for this purpose in alsa-lib.
> >>
> >> This commit adds the missing function.
> >
> >>   /**
> >> + * \brief Create and add an user-defined control element of bytes type.
> >> + * \param[in] ctl CTL handle.
> >> + * \param[in,out] id ID of the new control element.
> >> + * \param[in] channels The number of channels which a control element includes.
> >
> > For this control type, "count" might be a better name.
> > Or at least say in the description that this is the number of bytes.
> 
> For the name of this variable, I don't mind using 'count'. But then, in 
> next commit, snd_ctl_elem_add_bytes_set() has two arguments named 
> 'count'. And the consistency of API design is lost a bit.

How about naming it a bit more descriptive way, e.g. count_xxx?
"channel" isn't clearer than "count", IMO.


> I think that naming the variables depends on interpretation of 'struct 
> snd_ctl_elem_value', therefore it mostly depends on taste of each 
> developers. For example, to 'bytes' type element set, we can interpret 
> 'struct snd_ctl_elem_value.bytes' as either 'byte array in an element' 
> or 'data for each channels with int8_t (=char) value for an element'.
> 
> So I propose the design of ALSA control core. If possible, I'd like to 
> follow the design when adding new APIs for a consistent representation. 
> If the proposed design is not propper, we can change the design for 
> better shape. (but it will sometimes be a dull work depending on taste 
> of each developers.)
> 
> And, I note that the name of 'channel' is picked up from Mixer APIs in 
> alsa-lib. It doesn't come from my brain ;)

The mixer API is indeed focused on the mixer element, thus the concept
of channel would match in most cases (but not all, sure).  But control
API is designed a bit more generically, so I understand Clemens'
concern, too.


thanks,

Takashi