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?
