Hi,
'Twas brillig, and xing wang at 26/05/11 04:52 did gyre and gimble:
> Would you please give some comments about Guanqun's patch?
> We're working on Pulseaudio and meet some bugs, i think we will run
> faster with community's help.
Absolutely!
To be honest tho', I was hoping Tanu or David would give their takes on
this patch (which they may still do) as they have a better grasp of the
audio control side than I do.
One thing I will say is that PA will use the info from alsa in such a
way as to provide a consistent dB range further up the stack.
We take the alsa-provided 0dB point and mark that as our "Base volume"
this is exposed e.g. in the pavucontrol and gnome-volume-control GUI's
This base volume is meant to indicate the "cleanest" path through the h/w.
This might not be super relevant to the specific issue in question, but
that's a little background on the dB levels exposed by alsa, but I get
the impression the volumes you're talking about are not dB related?
Looking at your patch, it seems logical enough (although for internal
code, I'd use pa_bool_t + TRUE/FALSE values, but that's just a minor
observation), so if Tanu ACKs it, I'd happily merge it.
Cheers
Col
> thanks
> --xingchao
>
> 2011/5/25 Lu Guanqun <guanqun.lu at intel.com>:
>> 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
>> _______________________________________________
>> pulseaudio-discuss mailing list
>> pulseaudio-discuss at mail.0pointer.de
>> https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
>>
--
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/]
