[alsa-devel] [RFC] usb-audio: Use a table of mixer controls

[Mark Hills]

On Fri, 11 May 2012, Takashi Iwai wrote:

[...]
> >  /*
> > + * Create a set of standard UAC controls from a table
> > + */
> > +static int snd_create_std_mono_table(struct usb_mixer_interface *mixer,
> > +				struct std_mono_table *t)
> > +{
> > +	int err;
> > +
> > +	while (t->name != NULL) {
> > +		err = snd_create_std_mono_ctl(mixer, t->unitid, t->control,
> > +				t->cmask, t->val_type, t->name, t->tlv_callback);
> > +		if (err < 0)
> > +			return err;
> 
> t++ is missing?

Hmm, yes. A clear indication that the code I must have not tested the 
correct code (originally I used ARRAY_SIZE)

I'll make sure to confirm this before I submit a proper patch.
 
[...]
> > +/*
> > + * The mixer units for Ebox-44 are corrupt, and even where they
> > + * are valid they presents mono controls as L and R channels of
> > + * stereo. So we create provide a good mixer here.
> > + */
> > +struct std_mono_table ebox44_table[] = {
> > +	{ 4, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Headphone Playback Switch", NULL },
> > +	{ 4, 2, 0x1, USB_MIXER_S16, "Headphone A Mix Playback Volume", NULL },
> > +	{ 4, 2, 0x2, USB_MIXER_S16, "Headphone B Mix Playback Volume", NULL },
> > +
> > +	{ 7, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Output Playback Switch", NULL },
> > +	{ 7, 2, 0x1, USB_MIXER_S16, "Output A Playback Volume", NULL },
> > +	{ 7, 2, 0x2, USB_MIXER_S16, "Output B Playback Volume", NULL },
> > +
> > +	{ 10, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Input Capture Switch", NULL },
> > +	{ 10, 2, 0x1, USB_MIXER_S16, "Input A Capture Volume", NULL },
> > +	{ 10, 2, 0x2, USB_MIXER_S16, "Input B Capture Volume", NULL },
> > +
> > +	{ }
> 
> C99 style struct initialization is preferred.  Then NULL settings can
> be omittted.

Do you mean like this?

  ebox44_table[] = {
      {
          .unitid = 4,
          .control =  1
          .cmask = 0x0
          /* etc. */
      }

Because then it becomes a lot less concise and a lot less readable? Of 
course, if there are more optional params in the future, then maybe.

IIRC, I could omit the NULL anyway, because static memory is initialised 
to contain zeroes (hence the "{ }" works). Am I correct? It's late, I 
might be mistaken.

> Other than that, it looks fine to me.

Thanks for comments.

-- 
Mark