Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
On Tue, 2011-05-17 at 12:13 +0100, Colin Guthrie wrote: > Thanks, I'll test this patch and see if it helps with my test case. > > Assuming that it does, is this something we should really push to alsa > lib? When I first encountered the problem I did start hacking away at > alsa lib looking for the problem. > > Perhaps a new direction argument -2 and +2 for "nearest below bias" and > "nearest above bias" respectively? (the bias only coming into play when > there you are exactly inbetween two possible values). Nah, if we need rounding to the nearest step only for working around rounding errors that are caused by ourselves due to not-entirely-necessary format conversions, I don't think alsa-lib should be changed just for that. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
On Tue, 2011-05-17 at 19:40 +0100, Colin Guthrie wrote: > Just to report that a modified version of that patch does indeed fix the > problem for me. > > What do you think overall? Should we try and hunt down the problems with > the conversion or should this patch be included? I think it would make sense to save the intermediate volume as integer millibels - that way there couldn't be any rounding errors, because no rounding would happen. The patch works, though, so I'm not against applying it. That would be the easier way forward - from a quick glance at the code it seems that changing the volume format will involve more than just changing a variable type. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
'Twas brillig, and Colin Guthrie at 17/05/11 12:13 did gyre and gimble: > 'Twas brillig, and Tanu Kaskinen at 17/05/11 10:37 did gyre and gimble: >> On Mon, 2011-05-16 at 21:22 +0100, Colin Guthrie wrote: >>> OK, here is the debug output from my run using this volume approach >>> >>> As you can see there are several points where the message: >>> >>> "Written HW volume did not match with the request" is shown. This is >>> shown quite clearly when writing the volumes to h/w >>> >>> I suspect this problem arrises due to the use of pa_cvolume_divide() >>> which is based around PA_VOLUME_NORM, when the second argument (the >>> software volume) is > PA_VOLUME_NORM. >>> >>> Any thoughts on this are welcome. >> >> I'll quote the log: >> >> D: protocol-native.c: Client pavucontrol changes volume of source >> alsa_input.pci-_00_1b.0.analog-stereo. >> D: alsa-source.c: Requested volume: 0: 45% 1: 45% >> D: alsa-source.c:in dB: 0: -20.71 dB 1: -20.71 dB >> D: alsa-source.c: Got hardware volume: 0: 45% 1: 45% >> D: alsa-source.c: in dB: 0: -21.00 dB 1: -21.00 dB >> D: alsa-source.c: Calculated software volume: 0: 101% 1: 101% >> (accurate-enough=no) >> D: alsa-source.c: in dB: 0: 0.29 dB 1: 0.29 dB >> D: source.c: Volume going up to 29273 at 270475970821 >> D: source.c: Volume change to 29273 at 270475970821 was written 34 usec late >> D: alsa-source.c: Written HW volume did not match with the request: 0: 45% >> 1: 45% (request) != 0: 42% 1: 42% >> D: alsa-source.c:in dB: 0: >> -21.00 dB 1: -21.00 dB (request) != 0: -22.50 dB 1: -22.50 dB >> >> Looking at the last line, the requested volume seems to hit exactly the >> right step (-21.00dB), but for some reason Alsa decides to choose >> something else. I'm pretty sure that this happens because of rounding >> errors. In the first phase we ask Alsa what dB value we should set, and >> it returns -21.00 dB. The value is given as a long int, but we convert >> that to pa_cvolume. Then when we set the volume, we convert the >> pa_cvolume value back to a long integer. At this point I believe it gets >> converted to -2101. This is not visible in the debug message for some >> reason - the rounding algorithm must be different from what was used >> with the pa_cvolume -> long conversion. >> >> Here's a patch that should fix the problem: >> >> http://meego.gitorious.org/maemo-multimedia/pulseaudio/commit/a88c2b1d9f8c2088a430a257470a70efeaaf4b34 > > Thanks, I'll test this patch and see if it helps with my test case. > > Assuming that it does, is this something we should really push to alsa > lib? When I first encountered the problem I did start hacking away at > alsa lib looking for the problem. > > Perhaps a new direction argument -2 and +2 for "nearest below bias" and > "nearest above bias" respectively? (the bias only coming into play when > there you are exactly inbetween two possible values). Just to report that a modified version of that patch does indeed fix the problem for me. What do you think overall? Should we try and hunt down the problems with the conversion or should this patch be included? 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/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
'Twas brillig, and Tanu Kaskinen at 17/05/11 10:37 did gyre and gimble: > On Mon, 2011-05-16 at 21:22 +0100, Colin Guthrie wrote: >> OK, here is the debug output from my run using this volume approach >> >> As you can see there are several points where the message: >> >> "Written HW volume did not match with the request" is shown. This is >> shown quite clearly when writing the volumes to h/w >> >> I suspect this problem arrises due to the use of pa_cvolume_divide() >> which is based around PA_VOLUME_NORM, when the second argument (the >> software volume) is > PA_VOLUME_NORM. >> >> Any thoughts on this are welcome. > > I'll quote the log: > > D: protocol-native.c: Client pavucontrol changes volume of source > alsa_input.pci-_00_1b.0.analog-stereo. > D: alsa-source.c: Requested volume: 0: 45% 1: 45% > D: alsa-source.c:in dB: 0: -20.71 dB 1: -20.71 dB > D: alsa-source.c: Got hardware volume: 0: 45% 1: 45% > D: alsa-source.c: in dB: 0: -21.00 dB 1: -21.00 dB > D: alsa-source.c: Calculated software volume: 0: 101% 1: 101% > (accurate-enough=no) > D: alsa-source.c: in dB: 0: 0.29 dB 1: 0.29 dB > D: source.c: Volume going up to 29273 at 270475970821 > D: source.c: Volume change to 29273 at 270475970821 was written 34 usec late > D: alsa-source.c: Written HW volume did not match with the request: 0: 45% > 1: 45% (request) != 0: 42% 1: 42% > D: alsa-source.c:in dB: 0: -21.00 > dB 1: -21.00 dB (request) != 0: -22.50 dB 1: -22.50 dB > > Looking at the last line, the requested volume seems to hit exactly the > right step (-21.00dB), but for some reason Alsa decides to choose > something else. I'm pretty sure that this happens because of rounding > errors. In the first phase we ask Alsa what dB value we should set, and > it returns -21.00 dB. The value is given as a long int, but we convert > that to pa_cvolume. Then when we set the volume, we convert the > pa_cvolume value back to a long integer. At this point I believe it gets > converted to -2101. This is not visible in the debug message for some > reason - the rounding algorithm must be different from what was used > with the pa_cvolume -> long conversion. > > Here's a patch that should fix the problem: > > http://meego.gitorious.org/maemo-multimedia/pulseaudio/commit/a88c2b1d9f8c2088a430a257470a70efeaaf4b34 Thanks, I'll test this patch and see if it helps with my test case. Assuming that it does, is this something we should really push to alsa lib? When I first encountered the problem I did start hacking away at alsa lib looking for the problem. Perhaps a new direction argument -2 and +2 for "nearest below bias" and "nearest above bias" respectively? (the bias only coming into play when there you are exactly inbetween two possible values). 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/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
On Mon, 2011-05-16 at 21:22 +0100, Colin Guthrie wrote: > OK, here is the debug output from my run using this volume approach > > As you can see there are several points where the message: > > "Written HW volume did not match with the request" is shown. This is > shown quite clearly when writing the volumes to h/w > > I suspect this problem arrises due to the use of pa_cvolume_divide() > which is based around PA_VOLUME_NORM, when the second argument (the > software volume) is > PA_VOLUME_NORM. > > Any thoughts on this are welcome. I'll quote the log: D: protocol-native.c: Client pavucontrol changes volume of source alsa_input.pci-_00_1b.0.analog-stereo. D: alsa-source.c: Requested volume: 0: 45% 1: 45% D: alsa-source.c:in dB: 0: -20.71 dB 1: -20.71 dB D: alsa-source.c: Got hardware volume: 0: 45% 1: 45% D: alsa-source.c: in dB: 0: -21.00 dB 1: -21.00 dB D: alsa-source.c: Calculated software volume: 0: 101% 1: 101% (accurate-enough=no) D: alsa-source.c: in dB: 0: 0.29 dB 1: 0.29 dB D: source.c: Volume going up to 29273 at 270475970821 D: source.c: Volume change to 29273 at 270475970821 was written 34 usec late D: alsa-source.c: Written HW volume did not match with the request: 0: 45% 1: 45% (request) != 0: 42% 1: 42% D: alsa-source.c:in dB: 0: -21.00 dB 1: -21.00 dB (request) != 0: -22.50 dB 1: -22.50 dB Looking at the last line, the requested volume seems to hit exactly the right step (-21.00dB), but for some reason Alsa decides to choose something else. I'm pretty sure that this happens because of rounding errors. In the first phase we ask Alsa what dB value we should set, and it returns -21.00 dB. The value is given as a long int, but we convert that to pa_cvolume. Then when we set the volume, we convert the pa_cvolume value back to a long integer. At this point I believe it gets converted to -2101. This is not visible in the debug message for some reason - the rounding algorithm must be different from what was used with the pa_cvolume -> long conversion. Here's a patch that should fix the problem: http://meego.gitorious.org/maemo-multimedia/pulseaudio/commit/a88c2b1d9f8c2088a430a257470a70efeaaf4b34 -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
'Twas brillig, and David Henningsson at 16/05/11 07:23 did gyre and gimble:
> On 2011-05-15 16:45, Tanu Kaskinen wrote:
>> On Sun, 2011-05-15 at 17:43 +0300, Tanu Kaskinen wrote:
>>> This was discussed on the mailing list:
>>>
>>> https://tango.0pointer.de/pipermail/pulseaudio-discuss/2011-May/010091.html
>>>
>>> ---
>>> src/modules/alsa/alsa-mixer.c |2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/modules/alsa/alsa-mixer.c
>>> b/src/modules/alsa/alsa-mixer.c
>>> index f236da0..8375a2f 100644
>>> --- a/src/modules/alsa/alsa-mixer.c
>>> +++ b/src/modules/alsa/alsa-mixer.c
>>> @@ -893,7 +893,7 @@ static int element_set_volume(pa_alsa_element *e,
>>> snd_mixer_t *m, const pa_chann
>>>
>>> if (e->has_dB) {
>>> long value = to_alsa_dB(f);
>>> -int rounding = value> 0 ? -1 : +1;
>>> +int rounding = e->direction == PA_ALSA_DIRECTION_OUTPUT
>>> ? +1 : -1;
>>>
>>> if (e->volume_limit>= 0&& value> (e->max_dB * 100))
>>> value = e->max_dB * 100;
>>
>> David, are you happy with this change, or does this require more
>> discussion?
>>
>
> I think it's OK. I think your theory is at least as good as mine, so
> let's give it a try. For HDA Intel this does not make much of a
> difference as Playback almost always only goes up to 0 dB whereas
> Recording usually is above 0 dB (although not always).
Cool. I'll have to try this out in my tree with Source Output volumes as
I'm having a brain freeze as to whether or not this is the right way
round for what I observed. If it's not working I'll post back with some
numbers.
> Might be worth adding a comment referring to the discussion behind the
> reasoning though.
Yeah, I think a comment would be wise when this goes in.
I'll take care of it and report back sometime this week.
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/]
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
On 2011-05-15 16:45, Tanu Kaskinen wrote:
On Sun, 2011-05-15 at 17:43 +0300, Tanu Kaskinen wrote:
This was discussed on the mailing list:
https://tango.0pointer.de/pipermail/pulseaudio-discuss/2011-May/010091.html
---
src/modules/alsa/alsa-mixer.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index f236da0..8375a2f 100644
--- a/src/modules/alsa/alsa-mixer.c
+++ b/src/modules/alsa/alsa-mixer.c
@@ -893,7 +893,7 @@ static int element_set_volume(pa_alsa_element *e,
snd_mixer_t *m, const pa_chann
if (e->has_dB) {
long value = to_alsa_dB(f);
-int rounding = value> 0 ? -1 : +1;
+int rounding = e->direction == PA_ALSA_DIRECTION_OUTPUT ? +1 : -1;
if (e->volume_limit>= 0&& value> (e->max_dB * 100))
value = e->max_dB * 100;
David, are you happy with this change, or does this require more
discussion?
I think it's OK. I think your theory is at least as good as mine, so
let's give it a try. For HDA Intel this does not make much of a
difference as Playback almost always only goes up to 0 dB whereas
Recording usually is above 0 dB (although not always).
Might be worth adding a comment referring to the discussion behind the
reasoning though.
--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.
On Sun, 2011-05-15 at 17:43 +0300, Tanu Kaskinen wrote:
> This was discussed on the mailing list:
>
> https://tango.0pointer.de/pipermail/pulseaudio-discuss/2011-May/010091.html
> ---
> src/modules/alsa/alsa-mixer.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> index f236da0..8375a2f 100644
> --- a/src/modules/alsa/alsa-mixer.c
> +++ b/src/modules/alsa/alsa-mixer.c
> @@ -893,7 +893,7 @@ static int element_set_volume(pa_alsa_element *e,
> snd_mixer_t *m, const pa_chann
>
> if (e->has_dB) {
> long value = to_alsa_dB(f);
> -int rounding = value > 0 ? -1 : +1;
> +int rounding = e->direction == PA_ALSA_DIRECTION_OUTPUT ? +1 :
> -1;
>
> if (e->volume_limit >= 0 && value > (e->max_dB * 100))
> value = e->max_dB * 100;
David, are you happy with this change, or does this require more
discussion?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
