[pulseaudio-discuss] ask about the assumption of min volume and max volume got from ALSA

[Lu Guanqun]

Hi,

This patch needs a little more love. :)
No one cared?

On Wed, May 25, 2011 at 02:17:18PM +0800, Lu Guanqun wrote:
> Hi list,
> 
> I may not have the background on this issue, but from the code, it seems
> the code assumes min_volume and max_volume should be non negative. Is
> this the right assumption we should take? What about some drivers
> exposes that min_volume is -126 and max_volume is 0? Should we fix the
> driver or generalize pulseaudio code to tolerate these issues?
> 
> If we allow min_volume could be less than zero, and how about the below
> patch in your point of view? Could you have a review of the below patch?
> If that's OK, I can send a standalone patch for it.
> 
> From 1f5c8fa0e80d9fe2e3d56133afa8fe9a5dbe6693 Mon Sep 17 00:00:00 2001
> From: Lu Guanqun <guanqun.lu at intel.com>
> Date: Wed, 25 May 2011 14:12:52 +0800
> Subject: [PATCH] fix the assumption that volume is always positive
> 
> Add a variable to track whether the actual volume is set or not.
> Suppose this:
> 	min volume: -126	max volume: 0
> then when user wants to set some constant volume to -10, it would fail.
> 
> Signed-off-by: Lu Guanqun <guanqun.lu at intel.com>
> ---
>  src/modules/alsa/alsa-mixer.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> index 1bd709a..23651b0 100644
> --- a/src/modules/alsa/alsa-mixer.c
> +++ b/src/modules/alsa/alsa-mixer.c
> @@ -1123,6 +1123,7 @@ static int element_set_constant_volume(pa_alsa_element *e, snd_mixer_t *m) {
>      snd_mixer_selem_id_t *sid = NULL;
>      int r = 0;
>      long volume = -1;
> +    int volume_set = 0;
>  
>      pa_assert(m);
>      pa_assert(e);
> @@ -1136,6 +1137,7 @@ static int element_set_constant_volume(pa_alsa_element *e, snd_mixer_t *m) {
>      switch (e->volume_use) {
>          case PA_ALSA_VOLUME_OFF:
>              volume = e->min_volume;
> +            volume_set = 1;
>              break;
>  
>          case PA_ALSA_VOLUME_ZERO:
> @@ -1143,18 +1145,20 @@ static int element_set_constant_volume(pa_alsa_element *e, snd_mixer_t *m) {
>                  long dB = 0;
>  
>                  volume = decibel_fix_get_step(e->db_fix, &dB, +1);
> +                volume_set = 1;
>              }
>              break;
>  
>          case PA_ALSA_VOLUME_CONSTANT:
>              volume = e->constant_volume;
> +            volume_set = 1;
>              break;
>  
>          default:
>              pa_assert_not_reached();
>      }
>  
> -    if (volume >= 0) {
> +    if (volume_set) {
>          if (e->direction == PA_ALSA_DIRECTION_OUTPUT)
>              r = snd_mixer_selem_set_playback_volume_all(me, volume);
>          else
> -- 
> 1.7.2.3
> 
> -- 
> guanqun
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at mail.0pointer.de
> https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

-- 
guanqun