[alsa-devel] alsactl adds volume controls?

[Colin Guthrie]

'Twas brillig, and Colin Guthrie at 08/10/10 16:29 did gyre and gimble:
> 'Twas brillig, and Colin Guthrie at 08/10/10 16:25 did gyre and gimble:
>> 'Twas brillig, and Clemens Ladisch at 08/10/10 15:42 did gyre and gimble:
>>> Colin Guthrie wrote:
>>>> Hmm, just thinking about this (as I don't know the volume control logic
>>>> particularly well in PA), the call snd_mixer_selem_set_playback_dB() is
>>>> used with a dir argument of +1.
>>>>
>>>> From what I understand, this would allow me to say "set the volume to
>>>> 50dB" and due to the +1 dir, it should select -46.499999 ( because -46.5
>>>> mutes it).
>>>>
>>>> In this case however, the value of -99999.999dB is ultimately selected
>>>> (aka 0).
>>>>
>>>> I'm wondering if the TLV fix actually affects how
>>>> snd_mixer_selem_set_playback_dB() call works with the dir argument.
>>>
>>> Uh, oh.  snd_tlv_convert_from_dB() ignores the minimum-is-mute flag.
>>
>> Ahh good, I analysed that kinda correctly then :D
>>
>>> Please try this hack:
>>>
>>> --- alsa-lib/src/control/tlv.c
>>> +++ alsa-lib/src/control/tlv.c
>>> @@ -335,6 +335,9 @@ int snd_tlv_convert_from_dB(unsigned int *tlv, long rangemin, long rangemax,
>>>  			if (xdir > 0)
>>>  				v += (max - min) - 1;
>>>  			v = v / (max - min) + rangemin;
>>> +			if (v == rangemin && xdir > 0 && (tlv[3] & 0x10000) &&
>>> +			    db_gain > SND_CTL_TLV_DB_GAIN_MUTE)
>>> +				v++;
>>>  			*value = v;
>>>  		}
>>>  		return 0;
>>
>>
>> Didn't help sadly, but then looking at the code there it seems a little odd.
>>
>> Firstly I tried this (extended) patch - I'm not sure it's needed but
>> when the first one didn't work I tried to experiment a bit:
>>
>> --- a/src/control/tlv.c
>> +++ b/src/control/tlv.c
>> @@ -323,15 +323,20 @@ int snd_tlv_convert_from_dB(unsigned int *tlv,
>> long rangemin, long rangemax,
>>                 int min, max;
>>                 min = tlv[2];
>>                 max = tlv[3];
>> -               if (db_gain <= min)
>> +               if (db_gain <= min) {
>>                         *value = rangemin;
>> -               else if (db_gain >= max)
>> +                       if (xdir > 0 && (tlv[3] & 0x10000) && db_gain >
>> SND_CTL_TLV_DB_GAIN_MUTE)
>> +                               *value = rangemin + 1;
>> +               } else if (db_gain >= max)
>>                         *value = rangemax;
>>                 else {
>>                         long v = (db_gain - min) * (rangemax - rangemin);
>>                         if (xdir > 0)
>>                                 v += (max - min) - 1;
>>                         v = v / (max - min) + rangemin;
>> +                       if (v == rangemin && xdir > 0 && (tlv[3] &
>> 0x10000) &&
>> +                           db_gain > SND_CTL_TLV_DB_GAIN_MUTE)
>> +                               v++;
>>                         *value = v;
>>                 }
>>                 return 0;
>>
>>
>> This also catches the case when I try to set the volume to e.g. -5000
>> (-50.00dB) as it is < min (which I presume is still -4650, but actually
>> didn't check) then it will be set to rangemin and no further checks are
>> done.
>>
>> What is odd about this tho', is that the flag for min is mute (0x10000)
>> is checked on tlv[3] which is also used here for the "max" value.
>>
>> Is the use of position 3 correct (both here and on the kernel side) or
>> perhaps the max line needs to be:
>>
>>                 max = (tlv[3] & 0xffff);
>>
>> instead? Either way it didn't help, but I thought the strangeness with
>> the tlv[3] value was worth pointing out.
>>
>> Many thanks for debugging this with me :)
>>
>> Col
>>
>> PS, happy to chat on IRC if you prefer - coling
> 
> Ignore this mail... I applied the patch to the wrong tree and it put the
> hunks in a different place as the context was a 100% match...
> 
> I'll retest with this applied correctly! Sorry :D

OK, applying the patch correctly helps a lot :D

This is now working perfectly thanks.

I've attached the final patch I used. I modified your one slightly to
correct case when passing in a value that is <= min (e.g. -5000dB on my
setup) to also fix that up. As I'm not 100% sure it's needed, I'll leave
it up to you to decide which patch to apply.

Many thanks once again.

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/]
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0103-tlv-Ensure-that-min-is-mute-flag-is-honored-in-snd_t.patch
Url: http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20101008/c6beb166/attachment.bat