[pulseaudio-discuss] [PATCH] bluez5: Do not suspend on no -> unknown profile transitions

[Tanu Kaskinen]

On Fri, 2014-12-19 at 10:24 +0100, David Henningsson wrote:
> In case a transport is currently disconnected and transitions to
> idle, that should not count as a "remote hang up" event.

Great, thanks for debugging this!

> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
> 
> When used together with Tanu's module-card-restore patch, then the a2dp profile could
> successfully be restored when PA starts up.
> 
> That said, I somewhat prefer my own version which never sets the profile to off. Just like
> an available port does not cause the backend to switch away from it, (this logic is up to
> module-switch-on-port-available), unavailable profiles should work the same way IMO.

I think that's comparing apples to oranges. When an alsa port is
unavailable, we are still able to write audio to the device just fine.
An apples to apples comparison would be the case where the alsa PCM
device disappears (which is currently handled by unloading the whole
card module - not a good model to emulate in bluetooth).

>  src/modules/bluetooth/module-bluez5-device.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index e6a8071..995e550 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -1968,11 +1968,13 @@ static void handle_transport_state_change(struct userdata *u, struct pa_bluetoot
>      bool release = false;
>      pa_card_profile *cp;
>      pa_device_port *port;
> +    pa_available_t oldavail;
>  
>      pa_assert(u);
>      pa_assert(t);
>      pa_assert_se(cp = pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)));
>  
> +    oldavail = cp->available;
>      pa_card_profile_set_available(cp, transport_state_to_availability(t->state));
>  
>      /* Update port availability */
> @@ -1983,7 +1985,7 @@ static void handle_transport_state_change(struct userdata *u, struct pa_bluetoot
>  
>      /* Acquire or release transport as needed */
>      acquire = (t->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING && u->profile == t->profile);
> -    release = (t->state != PA_BLUETOOTH_TRANSPORT_STATE_PLAYING && u->profile == t->profile);
> +    release = (oldavail != PA_AVAILABLE_NO && t->state != PA_BLUETOOTH_TRANSPORT_STATE_PLAYING && u->profile == t->profile);

The profile availability and the transport state have 1:1 mapping, but
it's nevertheless (or perhaps precisely for that reason) ugly to mix the
two. I'd fix this by making the old transport state available in the
state change hook.

If you disagree, I don't care enough to block the patch on this issue.

-- 
Tanu