'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/]
From 3a8e7005a980bcbdca7c56b2c236ecbcb6b0b7ca Mon Sep 17 00:00:00 2001
From: Colin Guthrie <cguthrie@xxxxxxxxxxxx>
Date: Fri, 8 Oct 2010 16:46:43 +0100
Subject: [PATCH] tlv: Ensure that min-is-mute flag is honored in snd_tlv_convert_from_dB()
Thanks to Clemens Ladisch for identifying the issue and the original patch.
Discussion here: http://thread.gmane.org/gmane.linux.alsa.devel/77688
---
src/control/tlv.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/control/tlv.c b/src/control/tlv.c
index ba52752..d525f29 100644
--- a/src/control/tlv.c
+++ b/src/control/tlv.c
@@ -326,16 +326,21 @@ int snd_tlv_convert_from_dB(unsigned int *tlv, long rangemin, long rangemax,
min = tlv[2];
step = (tlv[3] & 0xffff);
max = min + (int)(step * (rangemax - rangemin));
- if (db_gain <= min)
- *value = rangemin;
- else if (db_gain >= max)
+ 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;
- *value = v;
+ if (db_gain <= min)
+ *value = rangemin;
+ else {
+ long v = (db_gain - min) * (rangemax - rangemin);
+ if (xdir > 0)
+ v += (max - min) - 1;
+ v = v / (max - min) + rangemin;
+ *value = v;
+ }
+ if (*value == rangemin && xdir > 0 && (tlv[3] & 0x10000) &&
+ db_gain > SND_CTL_TLV_DB_GAIN_MUTE)
+ (*value)++;
}
return 0;
}
--
1.7.3.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
