[pulseaudio-discuss] [PATCH] Revert "bluetooth: Auto recover if profile is 'off'"

[Tanu Kaskinen]

On Mon, 2017-06-19 at 11:43 +0300, Luiz Augusto von Dentz wrote:
> Hi Tanu,
> 
> On Tue, Jun 13, 2017 at 9:39 PM, Georg Chini <georg at chini.tk> wrote:
> > On 13.06.2017 18:02, Tanu Kaskinen wrote:
> > > 
> > > This reverts commit 69c212f8c13794f70899d1fe6baec1b6e3c92032.
> > > 
> > > Reasons:
> > > 
> > > The original reason for the patch was to work around some issue
> > > regarding the profile not connecting immediately (sorry, I don't really
> > > know the details), but that issue was fixed later by commit 998dfdf4cc,
> > > so the original reason doesn't apply any more.
> > > 
> > > Automatically changing the profile when the transport state changes to
> > > PLAYING has traditionally been handled by module-bluetooth-policy, and
> > > as far as I can tell, there's no reason to change that.
> > > 
> > > The assertion is unsafe. It's not guaranteed that the profile change
> > > will always succeed (at least pa_thread_mq_init() can fail due to
> > > reaching the maximum file descriptor limit).
> > > ---
> > >   src/modules/bluetooth/module-bluez5-device.c | 8 +-------
> > >   1 file changed, 1 insertion(+), 7 deletions(-)
> > > 
> > > diff --git a/src/modules/bluetooth/module-bluez5-device.c
> > > b/src/modules/bluetooth/module-bluez5-device.c
> > > index 867def742..7e2b0d799 100644
> > > --- a/src/modules/bluetooth/module-bluez5-device.c
> > > +++ b/src/modules/bluetooth/module-bluez5-device.c
> > > @@ -2126,14 +2126,8 @@ static pa_hook_result_t
> > > transport_state_changed_cb(pa_bluetooth_discovery *y, pa
> > >       if (t == u->transport && t->state <=
> > > PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
> > >           pa_assert_se(pa_card_set_profile(u->card,
> > > pa_hashmap_get(u->card->profiles, "off"), false) >= 0);
> > >   -    if (t->device == u->device) {
> > > -        /* Auto recover from errors causing the profile to be set to off
> > > */
> > > -        if (u->profile == PA_BLUETOOTH_PROFILE_OFF && t->state ==
> > > PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
> > > -            pa_log_debug("Switching to profile %s",
> > > pa_bluetooth_profile_to_string(t->profile));
> > > -            pa_assert_se(pa_card_set_profile(u->card,
> > > pa_hashmap_get(u->card->profiles,
> > > pa_bluetooth_profile_to_string(t->profile)), false) >= 0);
> > > -        }
> 
> We could at very least put this logic into policy, since in most cases
> leaving the profile set to 'off' probably will cause bad user
> experience.

I'd rather not do that, unless you have a real-world use case that
requires it (so far it seems that you'd like to do that "just in
case").

-- 
Tanu

https://www.patreon.com/tanuk