Re: [pulseaudio-discuss] [PATCH] alsa-mixer: When setting hw volume, round always up with playback and always down with capture.

2011-05-19 Thread Tanu Kaskinen 
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.

2011-05-19 Thread Tanu Kaskinen 
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.

2011-05-17 Thread Tanu Kaskinen 
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.

2011-05-17 Thread Colin Guthrie 
'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.

2011-05-17 Thread Colin Guthrie 
'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.

2011-05-16 Thread Colin Guthrie 
'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