[pulseaudio-discuss] [PATCH] alsa-sink: take base volume into account when applying hw volume
Currently if sink base volume differs from 0dB and sync-volume is used, wrong volume values are written to hw. This patch fixes that. Signed-off-by: Juho Hämäläinen --- src/modules/alsa/alsa-sink.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c index 2249f66..53c14b8 100644 --- a/src/modules/alsa/alsa-sink.c +++ b/src/modules/alsa/alsa-sink.c @@ -1263,12 +1263,18 @@ static void sink_write_volume_cb(pa_sink *s) { pa_assert(u->mixer_handle); pa_assert(s->flags & PA_SINK_SYNC_VOLUME); +/* Shift up by the base volume */ +pa_sw_cvolume_divide_scalar(&hw_vol, &hw_vol, s->base_volume); + if (pa_alsa_path_set_volume(u->mixer_path, u->mixer_handle, &s->channel_map, &hw_vol, TRUE) < 0) pa_log_error("Writing HW volume failed"); else { pa_cvolume tmp_vol; pa_bool_t accurate_enough; +/* Shift down by the base volume, so that 0dB becomes maximum volume */ +pa_sw_cvolume_multiply_scalar(&hw_vol, &hw_vol, s->base_volume); + pa_sw_cvolume_divide(&tmp_vol, &hw_vol, &s->thread_info.current_hw_volume); accurate_enough = (pa_cvolume_min(&tmp_vol) >= (PA_VOLUME_NORM - VOLUME_ACCURACY)) && @@ -1278,10 +1284,10 @@ static void sink_write_volume_cb(pa_sink *s) { char vol_str_pcnt[PA_CVOLUME_SNPRINT_MAX]; char vol_str_db[PA_SW_CVOLUME_SNPRINT_DB_MAX]; -pa_log_debug("Written HW volume did not match with the request %s != %s", +pa_log_debug("Written HW volume did not match with the request: %s (request) != %s", pa_cvolume_snprint(vol_str_pcnt, sizeof(vol_str_pcnt), &s->thread_info.current_hw_volume), pa_cvolume_snprint(vol_str_pcnt, sizeof(vol_str_pcnt), &hw_vol)); -pa_log_debug(" in dB: %s != %s", +pa_log_debug(" in dB: %s (request) != %s", pa_sw_cvolume_snprint_dB(vol_str_db, sizeof(vol_str_db), &s->thread_info.current_hw_volume), pa_sw_cvolume_snprint_dB(vol_str_db, sizeof(vol_str_db), &hw_vol)); } ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-sink: take base volume into account when applying hw volume
Any comments on this one Jyri? It seems add code to sink_write_volume_cb() to mirror the normal method sink_set_volume_cb() so looks OK to me but not followed the path through to see if this is supposed to be factored in already in some other way... Your thoughts would be appreciated. Col 'Twas brillig, and Juho Hämäläinen at 25/11/10 13:15 did gyre and gimble: > Currently if sink base volume differs from 0dB and sync-volume is used, > wrong volume values are written to hw. This patch fixes that. > > > Signed-off-by: Juho Hämäläinen > --- > src/modules/alsa/alsa-sink.c | 10 -- > 1 files changed, 8 insertions(+), 2 deletions(-) > > > > ___ > pulseaudio-discuss mailing list > pulseaudio-discuss@mail.0pointer.de > https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss -- 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-sink: take base volume into account when applying hw volume
On Tue, 30 Nov 2010, Colin Guthrie wrote: Any comments on this one Jyri? It seems add code to sink_write_volume_cb() to mirror the normal method sink_set_volume_cb() so looks OK to me but not followed the path through to see if this is supposed to be factored in already in some other way... Your thoughts would be appreciated. Yes, The base volume stuff appeared some time between the first version of sync volume and my upstream rebase. AFAIK, the fix is correct. However, there is still the double use of string bug (originally coming from me, but it looks quite different now). The bug is not serious, the print just is not very informative. Am currently working on a problem caused by safety-margin not taken into account when rewinding. When I get this fixed I'll add a fix for double use bug too. Cheers, Jyri Col 'Twas brillig, and Juho Hämäläinen at 25/11/10 13:15 did gyre and gimble: Currently if sink base volume differs from 0dB and sync-volume is used, wrong volume values are written to hw. This patch fixes that. Signed-off-by: Juho Hämäläinen --- src/modules/alsa/alsa-sink.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss -- 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 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-sink: take base volume into account when applying hw volume
'Twas brillig, and Jyri Sarha at 01/12/10 10:31 did gyre and gimble: > > On Tue, 30 Nov 2010, Colin Guthrie wrote: > >> Any comments on this one Jyri? >> >> It seems add code to sink_write_volume_cb() to mirror the normal method >> sink_set_volume_cb() so looks OK to me but not followed the path through >> to see if this is supposed to be factored in already in some other way... >> >> Your thoughts would be appreciated. >> > > > Yes, > The base volume stuff appeared some time between the first version of sync > volume and my upstream rebase. AFAIK, the fix is correct. However, there > is still the double use of string bug (originally coming from me, but it > looks quite different now). The bug is not serious, the print just is > not very informative. > > Am currently working on a problem caused by safety-margin not > taken into account when rewinding. When I get this fixed I'll add a fix > for double use bug too. Great, I'll push this fix for now then and look forward to your next batch of fixes too :) Cheers 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-sink: take base volume into account when applying hw volume
On 2010-12-01 11:31, Jyri Sarha wrote: Am currently working on a problem caused by safety-margin not taken into account when rewinding. When I get this fixed I'll add a fix for double use bug too. Hmm, for tsched or non-tsched? I think we merged a patch related to this from pl bossart a while ago. -- 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-sink: take base volume into account when applying hw volume
On Wed, 1 Dec 2010, David Henningsson wrote: On 2010-12-01 11:31, Jyri Sarha wrote: Am currently working on a problem caused by safety-margin not taken into account when rewinding. When I get this fixed I'll add a fix for double use bug too. Hmm, for tsched or non-tsched? I think we merged a patch related to this from pl bossart a while ago. Sorry, should have been more specific. I was talking about sync-volume-safety-margin, not rewind_safeguard related to tsched mode rewinding. However, the two things are closely related in this problem case. When DMA buffer is rewound the sync-volume events should rewound at the same time. This works already now. However the volume event rewinding is not modifying the new expiry time with sync-volume-safety-margin depending on whether we are changing the volume up or down. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss