[pulseaudio-discuss] [PATCH] bluetooth: When doing autoswitch call pa_card_set_profile with save=true

[Tanu Kaskinen]

On Thu, 2016-08-04 at 09:29 +0530, Arun Raghavan wrote:
> On Tue, 2 Aug 2016, at 12:50 AM, Pali Rohár wrote:
> > 
> > When pa_card_set_profile is called with save=false then
> > module-card-restore
> > start fight with module-bluetooth-policy and every second is profile
> > changing between hsp and a2dp.
> > 
> > So when pulseaudiu is changing profile in module-bluetooth-policy we need
> > to save it and tell module-card-restore to not try to restore old
> > profile.
> > 
> > > > Signed-off-by: Pali Rohár <pali.rohar at gmail.com>
> > ---
> >  src/modules/bluetooth/module-bluetooth-policy.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
> > b/src/modules/bluetooth/module-bluetooth-policy.c
> > index 68c8ab4..74ae898 100644
> > --- a/src/modules/bluetooth/module-bluetooth-policy.c
> > +++ b/src/modules/bluetooth/module-bluetooth-policy.c
> > @@ -159,7 +159,7 @@ static void card_set_profile(struct userdata *u,
> > pa_card *card, const char *to_p
> >  
> >          pa_log_debug("Setting card '%s' to profile '%s'", card->name,
> >          to_profile);
> >  
> > -        if (pa_card_set_profile(card, profile, false) != 0) {
> > +        if (pa_card_set_profile(card, profile, true) != 0) {
> >              pa_log_warn("Could not set profile '%s'", to_profile);
> >              continue;
> >          }
> > -- 
> 
> This is not actually what we want to do. Automatic switching should not
> make module-card-restore save the desired profile.
> 
> The short workaround for this is to register the hook in
> module-bluetooth-policy with priority later than module-card-restore.
> 
> That said, I'm not sure it's a good thing for hooks to be fighting to
> set the profile. I would rather have them select the profile and at the
> end of running hooks, if something provided a profile, we set it.
> 
> Tanu, since you added the original code -- wdyt?

module-card-restore should restore the profile only during the card
creation. If I understood correctly, the fighting continues also after
the card creation, so module-card-restore appears to be buggy. There
are multiple places in module-card-restore where pa_card_set_profile()
is called - which one causes the fighting?

I'd guess it's profile_available_changed_callback(). That code seems
bad - it will change the profile whenever the profile availability
changes, and in case of bluetooth the availability changes every time
we start or stop streaming audio to the device.

To understand why this code exists, see the commit message here:
https://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=1d3fd4f8627942b931ab6db4137186a567fe6939

I don't think it's a good idea to use profile availability changes as a
trigger in module-card-restore, even if the code is modified so that
only the first change is taken into account. When the code was added,
alsa cards never changed the profile availability, but now they do, and
it doesn't make sense for module-card-restore to switch to a profile
when an alsa profile becomes available due to the user plugging
something in.

I think the profile availability hook in module-card-restore should be
replaced with something is more specific to bluetooth. module-card-
restore should do something like this:

 * When a new card appears, check if it's a bluetooth card, and if it
is, save the list of profiles that are unavailable at that time.
 * When a profile changes, check if it's a bluetooth card, and if it
is, and the profile is in the list of initially-unavailable profiles,
remove the profile from that list and if the profile is the saved
profile, switch to that profile.

Another possibility would be to delay the card creation in the
bluetooth code until we know which profiles are available. If the delay
doesn't have to be big, this would be a better solution, because it's a
bit ugly to have bluetooth specific code in module-card-restore.

-- 
Tanu