[pulseaudio-discuss] Reverting ade0a6f88464d8aecf83982d400ccfc402341920

[Colin Guthrie]

'Twas brillig, and Tanu Kaskinen at 13/05/11 08:49 did gyre and gimble:
> Hello,
> 
> Should this commit be reverted?
> 
> http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=ade0a6f88464d8aecf83982d400ccfc402341920
> 
> I don't know what problem that commit solves, but it introduces a new
> problem: if Pulseaudio requests a volume that is above 0dB in the alsa
> volume element scale, then it can easily happen that alsa will round the
> request down. That rounding is then compensated with software volume,
> which in this case is amplification. We don't want software
> amplification, or do we?

Wow spooky. Strangely enough I've already got this one reverted in my tree!

I found that when adding source output volumes (which I will publish
after Arun's passthrough work is merged), that this caused really
strange issues with volume.

This is the commit in my tree that solved that:



commit 8b595ac248d7ca48e28c3e4e84f1eaf4abf5439d
Author: Colin Guthrie <colin at mageia.org>
Date:   Sun May 8 12:44:50 2011 +0100

    alsa-mixer: Reverse rounding direction for sources.

    The previous logic in ade0a6f88464d8aecf83982d400ccfc402341920
    does not work with for input volumes.

    In the case of input 0dB is typically the point at which ALSA sliders
    are their minimum. This means that when the ALSA mixer is at it's
    maximum it could be applying e.g. +22.5dB.

    Considering a step size of ±1.5dB, if we request +22dB, the current
    system will ultimate set the h/w to +21dB (one step below +22.5dB)
    because it rounds towards 0. This means that software has to amplify the
    signal back up to +22dB which is obviously not ideal for the data
    integrity. So for inputs we should always go higher than we request if
    possible and then attenuate in software back down to the level
requested.

diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index f236da0..1d9a91a 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;

             if (e->volume_limit >= 0 && value > (e->max_dB * 100))
                 value = e->max_dB * 100;
@@ -903,6 +903,7 @@ static int element_set_volume(pa_alsa_element *e,
snd_mixer_t *m, const pa_chann
                  * if the channel is available, ALSA behaves very
                  * strangely and doesn't fail the call */
                 if (snd_mixer_selem_has_playback_channel(me, c)) {
+                    rounding = value > 0 ? -1 : +1;
                     if (e->db_fix) {
                         if (write_to_hw)
                             r = snd_mixer_selem_set_playback_volume(me,
c, decibel_fix_get_step(e->db_fix, &value, rounding));
@@ -925,6 +926,7 @@ static int element_set_volume(pa_alsa_element *e,
snd_mixer_t *m, const pa_chann
                     r = -1;
             } else {
                 if (snd_mixer_selem_has_capture_channel(me, c)) {
+                    rounding = value > 0 ? +1 : -1;
                     if (e->db_fix) {
                         if (write_to_hw)
                             r = snd_mixer_selem_set_capture_volume(me,
c, decibel_fix_get_step(e->db_fix, &value, rounding));


-- 

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/]