Hi Luiz,
On Fri, Jul 27, 2012 at 10:54 AM, Luiz Augusto von Dentz
<luiz.dentz at gmail.com> wrote:
> Hi Mikel,
>
> On Thu, Jul 26, 2012 at 3:32 PM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
>> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
>>
>> If no audio stream exists to the remote device during discovery,
>> setting the profile to hfgw or a2dp_source would request it. This is
>> something that should not be done automatically.
>> ---
>> src/modules/bluetooth/module-bluetooth-discover.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
>> index e96a4f3..6fe1334 100644
>> --- a/src/modules/bluetooth/module-bluetooth-discover.c
>> +++ b/src/modules/bluetooth/module-bluetooth-discover.c
>> @@ -107,10 +107,10 @@ static pa_hook_result_t load_module_for_device(pa_bluetooth_discovery *y, const
>> args = tmp;
>> }
>>
>> - if (d->audio_source_state >= PA_BT_AUDIO_STATE_CONNECTED)
>> + if (d->audio_source_state >= PA_BT_AUDIO_STATE_PLAYING)
>> args = pa_sprintf_malloc("%s profile=\"a2dp_source\" auto_connect=no", args);
>>
>> - if (d->hfgw_state >= PA_BT_AUDIO_STATE_CONNECTED)
>> + if (d->hfgw_state >= PA_BT_AUDIO_STATE_PLAYING)
>> args = pa_sprintf_malloc("%s profile=\"hfgw\"", args);
>>
>> pa_log_debug("Loading module-bluetooth-device %s", args);
>
> Im afraid this will cause more warm than good, the module being loaded
> when connected means that we are able to control it before any stream
> is created, this include setting up loopback and switching profiles.
> Now regarding the problem of requesting the stream I though this would
> be fixed by Acquire with '?', or this is a different matter?
The Acquire with "?" is kind-of related but is not the same issue.
This is no race condition, it's much worse, at least for HFP :-)
(Of course we could use the "?" always in module-bluetooth-device,
this is not going to work because then you would never be able to
initiate an SCO connection)
Without this patch, every time you connect a phone with HFP (PA/BlueZ
have the handsfree role), we would try to connect SCO. This is not
only a bad thing (because many phones would reject it and thus the
module load fails) but it's also inconsistent with how the state
PropertyChanged is handled inside module-bluetooth-device (where we
change the profile only if the new state is "Playing").
On the other hand: this triggers a very interesting discussion: how
important is that the card profile can be set even while the stream is
not available (and thus BlueZ state would be "Connected" instead of
"Playing")? Is this maybe interesting to implement the A2DP source
role?
For example, module-bluetooth-policy waits until the sink/sources are
created, so I believe it wouldn't make any difference.
If we do apply this patch, there would probably be one step missing
for A2DP: we need to track the PropertyChanged in order to switch the
profile automatically (I will send a patch for this too). This would
still be a transitional solution since I think we need to rethink all
this. As I already pointed out in another thread: I would remove all
these policies from module-bt-discover and module-bt-device.
In a nutshell, this patch is at least useful for HFP. I also added
A2DP here for consistency, but we can leave it out if you prefer.
Cheers,
Mikel
