[pulseaudio-discuss] [PATCH] alsa-mixer: Ignore volume with unexpected number of channels

[Alexander E. Patrakov]

19.10.2015 14:16, David Henningsson wrote:
> We currently only support one and two channels for volumes, and
> bail out otherwise. This makes Xonar users unhappy because they
> have a volume with eight channels, and bailing out means they
> don't have a path/port at all.
>
> This way they will at least have a port, which will in turn make
> the gnome/unity UI behave better.
>
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=84983
> BugLink: https://bugzilla.gnome.org/show_bug.cgi?id=745017
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>
> This is a resend of https://bugs.freedesktop.org/attachment.cgi?id=114250
> There is a competing patch by Raymond in that bug, but I think his
> patch is broken, so I'm going to push mine in a week if there are no
> reviews.

Well, here is my review :)

While this should work, this patch means that all those users are 
essentially limited to software-based volume inside of PulseAudio. Also, 
the master (or whatever multichannel) slider will not be touched by 
PulseAudio at all. Which means that it will stay at whatever position 
that alsactl init (or alsactl restore) left it at. I.e. a limited volume 
range (note: this is not a regression). Which is still better than what 
we have now.

Counterproposal: if that's easier to implement than full multi-channel 
volume, treat all multichannel volume controls as mono internally, 
duplicate the volume across all channels when setting it, and use the 
first channel when getting. I.e., instead of full software volume, make 
"software balance".

As this counterproposal comes without a patch, and especially since the 
"counterprpopsed" improvement can be done later, this should not block 
the "going to push in one week" statement.

-- 
Alexander E. Patrakov

>
>   src/modules/alsa/alsa-mixer.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> index 9e06ba4..486da83 100644
> --- a/src/modules/alsa/alsa-mixer.c
> +++ b/src/modules/alsa/alsa-mixer.c
> @@ -1743,10 +1743,10 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t *m) {
>   
>                       if (e->n_channels <= 0) {
>                           pa_log_warn("Volume element %s with no channels?", e->alsa_name);
> -                        return -1;
> +                        e->volume_use = PA_ALSA_VOLUME_IGNORE;
>                       }
>   
> -                    if (e->n_channels > 2) {
> +                    else if (e->n_channels > 2) {
>                           /* FIXME: In some places code like this is used:
>                            *
>                            *     e->masks[alsa_channel_ids[p]][e->n_channels-1]
> @@ -1759,7 +1759,7 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t *m) {
>                            * don't support elements with more than two
>                            * channels... */
>                           pa_log_warn("Volume element %s has %u channels. That's too much! I can't handle that!", e->alsa_name, e->n_channels);
> -                        return -1;
> +                        e->volume_use = PA_ALSA_VOLUME_IGNORE;
>                       }
>   
>                       if (!e->override_map) {