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