[pulseaudio-discuss] [PATCH 5/5] alsa-sink: Don't assume we were able to enable hw-volume or sync-volume

[Colin Guthrie]

'Twas brillig, and oku at iki.fi at 20/12/10 16:47 did gyre and gimble:
> From: Jyri Sarha <jyri.sarha at nokia.com>
> 
> ---
>  src/modules/alsa/alsa-sink.c |   39 ++++++++++++++++++++-------------------
>  1 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
> index a8ba363..fd418fc 100644
> --- a/src/modules/alsa/alsa-sink.c
> +++ b/src/modules/alsa/alsa-sink.c
> @@ -1762,29 +1762,30 @@ static int setup_mixer(struct userdata *u, pa_bool_t ignore_dB, pa_bool_t sync_v
>          pa_log_info("Using hardware mute control.");
>      }
>  
> -    if (sync_volume) {
> -        u->mixer_pd = pa_alsa_mixer_pdata_new();
> -        mixer_callback = io_mixer_callback;
> -
> -        if (pa_alsa_set_mixer_rtpoll(u->mixer_pd, u->mixer_handle, u->rtpoll) < 0) {
> -            pa_log("Failed to initialize file descriptor monitoring");
> -            return -1;
> -        }
> +    if (u->sink->flags & PA_SINK_HW_VOLUME_CTRL) {
> +        if (u->sink->flags & PA_SINK_SYNC_VOLUME) {
> +            u->mixer_pd = pa_alsa_mixer_pdata_new();
> +            mixer_callback = io_mixer_callback;
>  
> -    } else {
> -        u->mixer_fdl = pa_alsa_fdlist_new();
> -        mixer_callback = ctl_mixer_callback;
> +            if (pa_alsa_set_mixer_rtpoll(u->mixer_pd, u->mixer_handle, u->rtpoll) < 0) {
> +                pa_log("Failed to initialize file descriptor monitoring");
> +                return -1;
> +            }
> +        } else {
> +            u->mixer_fdl = pa_alsa_fdlist_new();
> +            mixer_callback = ctl_mixer_callback;
>  
> -        if (pa_alsa_fdlist_set_mixer(u->mixer_fdl, u->mixer_handle, u->core->mainloop) < 0) {
> -            pa_log("Failed to initialize file descriptor monitoring");
> -            return -1;
> +            if (pa_alsa_fdlist_set_mixer(u->mixer_fdl, u->mixer_handle, u->core->mainloop) < 0) {
> +                pa_log("Failed to initialize file descriptor monitoring");
> +                return -1;
> +            }
>          }
> -    }
>  
> -    if (u->mixer_path_set)
> -        pa_alsa_path_set_set_callback(u->mixer_path_set, u->mixer_handle, mixer_callback, u);
> -    else
> -        pa_alsa_path_set_callback(u->mixer_path, u->mixer_handle, mixer_callback, u);
> +        if (u->mixer_path_set)
> +            pa_alsa_path_set_set_callback(u->mixer_path_set, u->mixer_handle, mixer_callback, u);
> +        else
> +            pa_alsa_path_set_callback(u->mixer_path, u->mixer_handle, mixer_callback, u);
> +    }
>  
>      return 0;
>  }

While I'm not 100% sure here, I think the mixer_callback is needed when
u->sink->flags & PA_SINK_HW_MUTE_CTRL, but you now only activate this
when u->sink->flags & PA_SINK_HW_VOLUME_CTRL. While it's quite unlikely
that some h/w exsist that has PA_SINK_HW_MUTE_CTRL but not
PA_SINK_HW_VOLUME_CTRL I think this should either be set unconditionally
(as before) or at least within an if block that checks for either flag.

WDYT?

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]