CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[John L. Utz III]

Hi Clemens and all;

( To those following along at home, kindly open
alsa-driver-1.0.18a/alsa-kernel/pci/oxygen/oxygen_mixer.c and look at 
dac_volume_get() and dac_volume_put() )


It appears to me that these functions are attempting to work on all of 
the channels at once instead of just choosing Master, Front, Rear, 
Surround, or what have you:

static int dac_volume_get(struct snd_kcontrol *ctl,
			  struct snd_ctl_elem_value *value)
{
	struct oxygen *chip = ctl->private_data;
	unsigned int i;

	mutex_lock(&chip->mutex);
	for (i = 0; i < chip->model.dac_channels; ++i)
		value->value.integer.value[i] = chip->dac_volume[i];
	mutex_unlock(&chip->mutex);
	return 0;
}


This is then surfaced to userland thusly in the controls array:

static const struct snd_kcontrol_new controls[] = {
	{
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Master Playback Volume",
		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
		.info = dac_volume_info,
		.get = dac_volume_get,
		.put = dac_volume_put,
	},
	{
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Master Playback Switch",
		.info = snd_ctl_boolean_mono_info,
		.get = dac_mute_get,
		.put = dac_mute_put,
	},
	{
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Stereo Upmixing",
		.info = upmix_info,
		.get = upmix_get,
		.put = upmix_put,
	},
         { blah },
	{ blah },
         { blah }
};


this leads to a rather awkward result in alsamixer where there are 5 
volume controls called "Master":


< Master >  Master    Master    Master    Master


IMHO, it would be more desirable for them to say:


< Master >  Front     C/LFE     Surround  Rear Surr


Our app would certainly be much happier :-)

So, i *think* that i can fix this thusly:

static const struct snd_kcontrol_new controls[] = {
	{
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Master Playback Volume",
		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
		.info = dac_volume_info,
		.get = dac_volume_get,
		.put = dac_volume_put,
		.private_value = 0
	},
	{
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Master Playback Switch",
		.info = snd_ctl_boolean_mono_info,
		.get = dac_mute_get,
		.put = dac_mute_put,
		.private_value = 0
	},
         {
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Front Playback Volume",
		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
		.info = dac_volume_info,
		.get = dac_volume_get,
		.put = dac_volume_put,
		.private_value = 1
	},
	{
		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
		.name = "Front Playback Switch",
		.info = snd_ctl_boolean_mono_info,
		.get = dac_mute_get,
		.put = dac_mute_put,
		.private_value = 1
	},


and alter dac_volume_*() to not use a for() loop and just work off the 
private_value.

anybody have any opinions?

i'll probably have tried this already before anybody responds but it 
seemed reasonable to surface the idea.

tnx!

johnu

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Clemens Ladisch]

John L. Utz III wrote:
> It appears to me that these functions are attempting to work on all of 
> the channels at once instead of just choosing Master, Front, Rear, 
> Surround, or what have you:

Yes, the "Master" control is one control with eight channels.

> this leads to a rather awkward result in alsamixer where there are 5 
> volume controls called "Master":
> 
> < Master >  Master    Master    Master    Master
> 
> IMHO, it would be more desirable for them to say:
> 
> < Master >  Front     C/LFE     Surround  Rear Surr

Yes.

Alsamixer chooses to display an eight-channel control this way.  This
can be considered a bug in alsamixer.


Best regards,
Clemens

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Takashi Iwai]

At Fri, 23 Jan 2009 14:51:31 +0100,
Clemens Ladisch wrote:
> 
> John L. Utz III wrote:
> > It appears to me that these functions are attempting to work on all of 
> > the channels at once instead of just choosing Master, Front, Rear, 
> > Surround, or what have you:
> 
> Yes, the "Master" control is one control with eight channels.
> 
> > this leads to a rather awkward result in alsamixer where there are 5 
> > volume controls called "Master":
> > 
> > < Master >  Master    Master    Master    Master
> > 
> > IMHO, it would be more desirable for them to say:
> > 
> > < Master >  Front     C/LFE     Surround  Rear Surr
> 
> Yes.
> 
> Alsamixer chooses to display an eight-channel control this way.  This
> can be considered a bug in alsamixer.

The current ALSA mixer abstraction splits the multi-channels into
separate paired channels.  But alsamixer can't show the channel name
due to its UI design.  And I guess other mixer apps don't handle them
properly, too (who would care?).

A solution would be make individual control like "Front", and create a
virtual master control.


Takashi

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Clemens Ladisch]

Takashi Iwai wrote:
> Clemens Ladisch wrote:
> > Alsamixer chooses to display an eight-channel control this way.  This
> > can be considered a bug in alsamixer.
> 
> The current ALSA mixer abstraction splits the multi-channels into
> separate paired channels.  But alsamixer can't show the channel name
> due to its UI design.

The its UI design should be changed.

> A solution would be make individual control like "Front", and create a
> virtual master control.

I am *not* going to change the driver just to accommodate to alsamixer.
I am going to change alsamixer to display the channel names (which it
already knows).


Best regards,
Clemens

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Takashi Iwai]

At Fri, 23 Jan 2009 15:13:31 +0100,
Clemens Ladisch wrote:
> 
> Takashi Iwai wrote:
> > Clemens Ladisch wrote:
> > > Alsamixer chooses to display an eight-channel control this way.  This
> > > can be considered a bug in alsamixer.
> > 
> > The current ALSA mixer abstraction splits the multi-channels into
> > separate paired channels.  But alsamixer can't show the channel name
> > due to its UI design.
> 
> The its UI design should be changed.

Feel free to fix that chaotic code one could ever think :)


Takashi

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Takashi Iwai]

At Fri, 23 Jan 2009 15:18:32 +0100,
I wrote:
> 
> At Fri, 23 Jan 2009 15:13:31 +0100,
> Clemens Ladisch wrote:
> > 
> > Takashi Iwai wrote:
> > > Clemens Ladisch wrote:
> > > > Alsamixer chooses to display an eight-channel control this way.  This
> > > > can be considered a bug in alsamixer.
> > > 
> > > The current ALSA mixer abstraction splits the multi-channels into
> > > separate paired channels.  But alsamixer can't show the channel name
> > > due to its UI design.
> > 
> > The its UI design should be changed.
> 
> Feel free to fix that chaotic code one could ever think :)

And after fixing alsamixer, don't forget to fix kmix, gnome-mixer,
wm-mixer applet and whatever, too...


Takashi

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[John Utz]

-----Original Message-----
From: Clemens Ladisch [mailto:clemens@ladisch.de]
Sent: Fri 1/23/2009 6:13 AM
To: Takashi Iwai
Cc: ALSA Developers; John Utz
Subject: Re: [alsa-devel] CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())
 
Takashi Iwai wrote:
> Clemens Ladisch wrote:
> > Alsamixer chooses to display an eight-channel control this way.  This
> > can be considered a bug in alsamixer.
> 
> The current ALSA mixer abstraction splits the multi-channels into
> separate paired channels.  But alsamixer can't show the channel name
> due to its UI design.

The its UI design should be changed.

JU It's *not* the UI, amixer shows the same thing

> A solution would be make individual control like "Front", and create a
> virtual master control.

I am *not* going to change the driver just to accommodate to alsamixer.

JU nobody is asking you to, if my employer decides that we will use the card (not entirely clear, it's expensive and IMHO the sound quality isnt worth the money) then i will be tasked with making this work.
JU i will figure out a functional solution that allows the CMI8788 to appear to our app as a 'ALSA traditional' mixer api, be it compiled code or  alsa card config fu.

I am going to change alsamixer to display the channel names (which it
already knows).

JU I am thinking that this perspective is not likely to lead to a successful outcome.
JU Do keep in mind that your current implementation *works* for *humans*, they can figure out which is the right slider to move.
JU *Programs* are hosed tho, because they depend on unique control identification.
JU I will save my rant about how misguided it was to put english strings in a driver for another day. :-(
JU We are stuck with it. The only way that will change will be if a new sound project where to be invented, just like ALSA replaced OSS.

Best regards,
Clemens

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Takashi Iwai]

At Fri, 23 Jan 2009 11:21:57 -0800,
John Utz wrote:
> 
> [1  <text/plain; iso-8859-1 (quoted-printable)>]
> 
> [2  <text/html; iso-8859-1 (quoted-printable)>]
> -----Original Message-----
> From: Clemens Ladisch [mailto:clemens@ladisch.de]
> Sent: Fri 1/23/2009 6:13 AM
> To: Takashi Iwai
> Cc: ALSA Developers; John Utz
> Subject: Re: [alsa-devel] CMI8788/AV200/Oxygen Volume Controls in alsamixer
> (oxygen_mixer.c:dac_volume_get())
> 
> > A solution would be make individual control like "Front", and create a
> > virtual master control.
> 
> I am *not* going to change the driver just to accommodate to alsamixer.
> 
> JU nobody is asking you to, if my employer decides that we will use the card
> (not entirely clear, it's expensive and IMHO the sound quality isnt worth the
> money) then i will be tasked with making this work.
> JU i will figure out a functional solution that allows the CMI8788 to appear
> to our app as a 'ALSA traditional' mixer api, be it compiled code or  alsa
> card config fu.
> 
> I am going to change alsamixer to display the channel names (which it
> already knows).
> 
> JU I am thinking that this perspective is not likely to lead to a successful
> outcome.
> JU Do keep in mind that your current implementation *works* for *humans*, they
> can figure out which is the right slider to move.
> JU *Programs* are hosed tho, because they depend on unique control
> identification.

Well, there are a few issues behind this problem.

- Existing mixer apps handle only mono or pair-stereo volume controls.
- Thus alsa-lib mixer API (or alsamixer) tries to split them up

This works if all the apps cope with this policy -- checking the channel 
and appending the channel suffix appropriately.  But, in practice,
it's not.

In addition, there is another thing as "de facto standard":

- A typical mixer app shows one mixer element as the master (e.g. kmix
  shows one slider with a left click), and this is *one* slider.

This brings us a question, what is "Master" control.  In common sense,
it is a control that covers all levels.  And, currently existing apps
expect that this is either a mono or a pair-stereo volume, not
multi-channel volumes.

Now what we can do for this?  Let all apps compute a mono master
volume based on multi-channel master volumes?  Or introduce a virtual
master in the sound subsystem level?

In the case of former solution, it'd be a really hardwork.  In the
latter case, a framework is already there in the sound driver core,
and will be a 5-minutes job to adapt it for virtuoso/oxygen.  That's
why I suggested it as *a* solution, not *the* solution.


> JU I will save my rant about how misguided it was to put english strings in a
> driver for another day. :-(
> JU We are stuck with it. The only way that will change will be if a new sound
> project where to be invented, just like ALSA replaced OSS.

It's not about putting the english strings.  This works fine with
other OS like Windows.  The problem comes just along with the
multi-channel handling.


thanks,

Takashi

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Clemens Ladisch]

John Utz wrote:
> if my employer decides that we will use the card

If you're thinking about the Xonar D2/D2X: please note that I removed
the "Master" control in the latest kernel in the hope of fixing some
unexplained bug where the card's EEPROM would be deleted.  (I think that
I may be able to prove that the Master control isn't related to this
bug, and revert that change.)

> (not entirely clear, it's expensive and IMHO the sound quality isnt
> worth the money)

The Xonar D1/DX has almost the same capabilities as the D2/D2X (in Linux)
and is more reasonably priced.

What are your requirements?


Best regards,
Clemens

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[John L. Utz III]

Clemens Ladisch wrote:
> John Utz wrote:
>> if my employer decides that we will use the card
> 
> If you're thinking about the Xonar D2/D2X: please note that I removed
> the "Master" control in the latest kernel in the hope of fixing some
> unexplained bug where the card's EEPROM would be deleted.  (I think that
> I may be able to prove that the Master control isn't related to this
> bug, and revert that change.)

Good to know, but not relevant in our case, we are exploring the D1.

>> (not entirely clear, it's expensive and IMHO the sound quality isnt
>> worth the money)
> 
> The Xonar D1/DX has almost the same capabilities as the D2/D2X (in Linux)
> and is more reasonably priced.

It's definitely reasonable when compared to the D2, but it's still 
relatively expensive.

> What are your requirements?

Low price, good power output on *all* channels, good fidelity. robust 
linux multichannel support.

not a big list, eh?. :-)

onboard stuff tends to only provide amplification on the front channels 
and expects that the user will provide a separate amplifier for rear and 
surround.

Sound *cards* tend to have onboard amps for all the channels because 
they have the pcb real-estate to spare for that sort of thing.

> 
> Best regards,
> Clemens
> 

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[John Utz]

using exchange webmail today, sorry.

-----Original Message-----
From: Takashi Iwai [mailto:tiwai@suse.de]
Sent: Fri 1/23/2009 5:56 AM
To: Clemens Ladisch
Cc: John Utz; ALSA Developers
Subject: Re: [alsa-devel] CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())
 
At Fri, 23 Jan 2009 14:51:31 +0100,
Clemens Ladisch wrote:
> 
> John L. Utz III wrote:
> > It appears to me that these functions are attempting to work on all of 
> > the channels at once instead of just choosing Master, Front, Rear, 
> > Surround, or what have you:
> 
> Yes, the "Master" control is one control with eight channels.
> 
> > this leads to a rather awkward result in alsamixer where there are 5 
> > volume controls called "Master":
> > 
> > < Master >  Master    Master    Master    Master
> > 
> > IMHO, it would be more desirable for them to say:
> > 
> > < Master >  Front     C/LFE     Surround  Rear Surr
> 
> Yes.
> 
> Alsamixer chooses to display an eight-channel control this way.  This
> can be considered a bug in alsamixer.

The current ALSA mixer abstraction splits the multi-channels into
separate paired channels.  But alsamixer can't show the channel name
due to its UI design.  And I guess other mixer apps don't handle them
properly, too (who would care?).

A solution would be make individual control like "Front", and create a
virtual master control.


JU a virtual control in the conf file or in the driver?

Takashi

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[Takashi Iwai]

At Fri, 23 Jan 2009 11:08:23 -0800,
John Utz wrote:
> 
> using exchange webmail today, sorry.
> 
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Fri 1/23/2009 5:56 AM
> To: Clemens Ladisch
> Cc: John Utz; ALSA Developers
> Subject: Re: [alsa-devel] CMI8788/AV200/Oxygen Volume Controls in alsamixer
> (oxygen_mixer.c:dac_volume_get())
> 
> At Fri, 23 Jan 2009 14:51:31 +0100,
> Clemens Ladisch wrote:
> >
> > John L. Utz III wrote:
> > > It appears to me that these functions are attempting to work on all of
> > > the channels at once instead of just choosing Master, Front, Rear,
> > > Surround, or what have you:
> >
> > Yes, the "Master" control is one control with eight channels.
> >
> > > this leads to a rather awkward result in alsamixer where there are 5
> > > volume controls called "Master":
> > >
> > > < Master >  Master    Master    Master    Master
> > >
> > > IMHO, it would be more desirable for them to say:
> > >
> > > < Master >  Front     C/LFE     Surround  Rear Surr
> >
> > Yes.
> >
> > Alsamixer chooses to display an eight-channel control this way.  This
> > can be considered a bug in alsamixer.
> 
> The current ALSA mixer abstraction splits the multi-channels into
> separate paired channels.  But alsamixer can't show the channel name
> due to its UI design.  And I guess other mixer apps don't handle them
> properly, too (who would care?).
> 
> A solution would be make individual control like "Front", and create a
> virtual master control.
> 
> JU a virtual control in the conf file or in the driver?

In the driver.  See sound/core/vmaster.c.


Takashi

Re: CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())

[John Utz]

Sorry in advance about how i write mail today, i am working from home so i have to use exchange webmail.

-----Original Message-----
From: Clemens Ladisch [mailto:clemens@ladisch.de]
Sent: Fri 1/23/2009 5:51 AM
To: John Utz
Cc: ALSA Developers
Subject: Re: [alsa-devel] CMI8788/AV200/Oxygen Volume Controls in alsamixer (oxygen_mixer.c:dac_volume_get())
 
John L. Utz III wrote:
> It appears to me that these functions are attempting to work on all of 
> the channels at once instead of just choosing Master, Front, Rear, 
> Surround, or what have you:

Yes, the "Master" control is one control with eight channels.

JU Argh, yet another implementation strategy from a vendor. Too bad, but remember, this is why we use ALSA now instead of OSS. :-)

> this leads to a rather awkward result in alsamixer where there are 5 
> volume controls called "Master":
> 
> < Master >  Master    Master    Master    Master
> 
> IMHO, it would be more desirable for them to say:
> 
> < Master >  Front     C/LFE     Surround  Rear Surr

Yes.

Alsamixer chooses to display an eight-channel control this way.  This
can be considered a bug in alsamixer.

JU It *could*, but that's probably an impractical perspective. zillions of other drivers present this data in the way that amixer, alsamixer and many other mixers expect. 

Best regards,
Clemens