[pulseaudio-discuss] [PATCH] alsa-mixer: Fix element_is_subset().

[Tanu Kaskinen]

On Mon, 2011-08-01 at 14:13 +0200, David Henningsson wrote:
> On 2011-07-22 19:20, Tanu Kaskinen wrote:
> > Untested code, don't commit willy-nilly... I'll be away for
> > the weekend, and I thought I'd already send this patch for
> > review. I'll test this some day next week.

Did that week pass already? :(

> > diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> > index f6a2a20..8f1fb4a 100644
> > --- a/src/modules/alsa/alsa-mixer.c
> > +++ b/src/modules/alsa/alsa-mixer.c
> > @@ -1344,6 +1344,82 @@ static int check_required(pa_alsa_element *e, snd_mixer_elem_t *me) {
> >       return 0;
> >   }
> >
> > +static int element_alsa_dB_to_step(pa_alsa_element *e, snd_mixer_elem_t *me, long *dB, long *step) {
> > +    int r;
> > +
> > +    pa_assert(e);
> > +    pa_assert(me);
> > +    pa_assert(dB);
> > +    pa_assert(step);
> > +
> > +    if (e->direction == PA_ALSA_DIRECTION_OUTPUT) {
> > +        if ((r = snd_mixer_selem_ask_playback_dB_vol(me, *dB, +1, step))>= 0) {
> > +            *step = (e->volume_limit>= 0) ? PA_MIN(*step, e->volume_limit) : *step;
> 
> But can't volume_limit can be less than zero and still valid?

The current code assumes that it can't be less than zero. That's an
invalid assumption, but making the assumption here doesn't break
anything that wasn't broken already.

Bug filed: https://bugs.freedesktop.org/show_bug.cgi?id=39723

> > +
> > +            if ((r = snd_mixer_selem_ask_playback_vol_dB(me, *step, dB)<  0)) {
> > +                pa_log("snd_mixer_selem_ask_playback_vol_dB() failed: %s", pa_alsa_strerror(r));
> > +                return -1;
> > +            }
> 
> Nitpick: could be worth adding a comment about why you need to ask first 
> dB -> vol and then vol -> dB again,

Agreed.

-- 
Tanu