[pulseaudio-discuss] [PATCH 2/3] bluetooth: Suspend the source/sink the HFP-oFono stream fd HUP

[Luiz Augusto von Dentz]

Hi Tanu,

On Tue, Sep 16, 2014 at 3:16 PM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Mon, 2014-09-15 at 13:02 +0300, Luiz Augusto von Dentz wrote:
>> From: João Paulo Rechi Vita <jprvita at openbossa.org>
>>
>> ---
>>  src/modules/bluetooth/module-bluez5-device.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
>> index fdf4078..d5d97c1 100644
>> --- a/src/modules/bluetooth/module-bluez5-device.c
>> +++ b/src/modules/bluetooth/module-bluez5-device.c
>> @@ -1980,6 +1980,7 @@ static pa_hook_result_t transport_state_changed_cb(pa_bluetooth_discovery *y, pa
>>  /* Run from main thread context */
>>  static int device_process_msg(pa_msgobject *obj, int code, void *data, int64_t offset, pa_memchunk *chunk) {
>>      struct bluetooth_msg *m = BLUETOOTH_MSG(obj);
>> +    struct userdata *u = m->card->userdata;
>>
>>      switch (code) {
>>          case BLUETOOTH_MESSAGE_IO_THREAD_FAILED:
>> @@ -1990,6 +1991,11 @@ static int device_process_msg(pa_msgobject *obj, int code, void *data, int64_t o
>>              pa_assert_se(pa_card_set_profile(m->card, pa_hashmap_get(m->card->profiles, "off"), false) >= 0);
>>              break;
>>          case BLUETOOTH_MESSAGE_STREAM_FD_HUP:
>> +            if (u->transport->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
>> +                pa_source_suspend(u->source, true, PA_SUSPEND_USER);
>> +                pa_sink_suspend(u->sink, true, PA_SUSPEND_USER);
>> +                pa_bluetooth_transport_put(u->transport);
>> +            }
>
> I'll quote what I wrote to João earlier:
>
> "Would it make more sense to set the transport state to IDLE, and let
> the normal transport state transition take care of suspending the sink
> and source? And is this really specific to the gateway profile? Why
> shouldn't we suspend the sink and source with any profile if the remote
> end hangs up the socket?"
>
> João's patch didn't have the pa_bluetooth_transport_put() call. I guess
> you added it to make the transport state IDLE. However, I meant that we
> can also remove the pa_sink_suspend() and pa_source_suspend() calls,
> because the sink and source will be suspended anyway as a reaction to
> the state change.
>
> Instead of pa_bluetooth_transport_put(), you should call
> pa_bluetooth_transport_set_state() directly. put() is supposed to be
> called only once during the object's lifetime.

Right I guess this got changed and I didn't notice.

> The question remains, why is this done only for the gateway profile and
> not the others?

Basically because it has to shutdown the socket to take effect but
does not cause any other event, in case of A2DP MediaTransport would
change state, thus we have to manually indicate this by switching to
IDLE. I guess we can do the same for other transports as well.


-- 
Luiz Augusto von Dentz