[pulseaudio-discuss] [PATCH v1 2/4] bluetooth: Do not switch to profile unless Playing

[Tanu Kaskinen]

On Fri, 2012-07-27 at 15:10 +0200, Mikel Astiz wrote:
> On Fri, Jul 27, 2012 at 10:54 AM, Luiz Augusto von Dentz
> > On Thu, Jul 26, 2012 at 3:32 PM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
> >> @@ -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.

I would too. The lines that are modified in these patches can be removed
altogether, if a replacement solution is provided (assuming that
something breaks if these lines are just dropped).

> 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.

I'll let Luiz answer before applying anything. I've pushed the first
three patches of the series, though.

-- 
Tanu