[pulseaudio-discuss] [PATCH] sink/source: Fix restore of volume on devices without hw volume

[David Henningsson]

On 2014-08-27 12:09, Tanu Kaskinen wrote:
> On Tue, 2014-08-26 at 13:54 +0200, David Henningsson wrote:
>> Module-device-restore sets real_volume, but soft_volume remains at
>> max, so if a device only has soft_volume (i e no hw volume controls),
>> its volume was not restored correctly.
>
> module-device-restore deals with the sink reference volume only, so I
> think the wording of the commit message should be improved. Claiming
> that module-device-restore sets the real volume confused me a bit
> (making me think initially the patch was totally misguided). It's true
> that the real volume is always initialized directly from reference
> volume, however.

Right. It makes more sense to use reference_volume instead of 
real_volume here, so posting a v2 now.

>> Reported-by: Richardo Salveti de Araujo <ricardo.salveti at canonical.com>
>> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
>> ---
>>
>> Note: I haven't looked in detail what happens if the hw steps are coarse,
>> so we need to set both soft_volume and reference_volume from real_volume.
>> So it's possible this is just a bandaid.
>
> I don't know why the steps being coarse would have any effect on the
> logic - if there's hw volume, it will anyway be too coarse to be used
> without software compensation (assuming that there's dB information -
> otherwise soft_volume should always be at max anyway).
>
> I had a look, and if alsa provides dB information, alsa-sink.c will take
> care of initializing real_volume and soft_volume correctly (btw, I think
> it would be better if alsa-sink.c didn't directly manipulate real_volume
> and soft_volume, but that's another discussion). sink.c will then update
> reference_volume correctly in pa_sink_put(), so this case works fine. If
> alsa doesn't provide dB information, alsa-sink.c will initialize
> real_volume correctly and leave soft_volume alone (should be at max all
> the time when there's no dB information), so this works too.
>
> The patch looks good to me, except for the commit message.

Ok, thanks for the look and the review!

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic