[pulseaudio-discuss] [PATCH 2/4] backend-native: add support for the HSP Headset profile

[Georg Chini]

On 15.02.2017 15:10, Tanu Kaskinen wrote:
> On Tue, 2017-02-14 at 14:33 +0100, Georg Chini wrote:
>> From: Wim Taymans <wim.taymans at gmail.com>
>>
>> This is a rebase of Wim Taymans patch to support the HSP headset role that has
>> somehow been forgotten. Original patch can be found at
>> https://lists.freedesktop.org/archives/pulseaudio-discuss/2015-February/023242.html
>
>
>> @@ -898,6 +901,13 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
>>               return 0;
>>           }
>>   
>> +        case PA_SOURCE_MESSAGE_SET_SHARED_VOLUME: {
>> +            if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
>> +                source_set_volume_cb (u->source);
>> +            }
>> +            break;
>> +        }
>> +
> The message is processed in the IO thread, but source_set_volume_cb()
> is expected to be called from the main thread. Using the
> SET_SHARED_VOLUME is an unusual way to get volume change notifications
> anyway.
>
> I think we should set the set_volume() callback to get the volume
> change notifications. The documentation for set_volume() in sink.h says
> this (source.h seems to lack similarly detailed explanation):
>
>       * If PA_SINK_DEFERRED_VOLUME is not set, then this is called from the
>       * main thread. The callback implementation must set the hardware
>       * volume according to s->real_volume. If the driver can't set the
>       * hardware volume to the exact requested value, it has to update
>       * s->real_volume and/or s->soft_volume so that they together
>       * match the actual hardware volume that was set.
>
> I don't fully understand what that means (even though I think I wrote
> that myself). The last sentence looks a bit misleading.
>
> Here's how I think the volume should be handled: The callback should
> first read s->real_volume to get the target volume. Since HSP has only
> 17 volume levels, rounding should be applied like
> source_set_volume_cb() already does.

{sink,source}_set_volume_cb() both read s->real_volume and do the
same rounding, so that should be OK so far.

> When pulseaudio is in the headset
> role, there won't be any hardware volume applied, so s->soft_volume
> needs to set to the same value as s->real_volume.

Do I understand you right that I just need to do the same pa_cvolume_set()
call for s->soft_volume like for s->real_volume in that case?