ask about the assumption of min volume and max volume got from ALSA

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Colin,

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.

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
>



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux