[pulseaudio-discuss] [PATCH] bluez5: Do not suspend on no -> unknown profile transitions
david.henningsson at canonical.com
Fri Dec 19 02:35:21 PST 2014
On 2014-12-19 10:55, Tanu Kaskinen wrote:
> 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.
Not necessarily. For HDMI, the capabilities can change when you
plug/unplug an HDMI devices. For some embedded devices, we can't change
some of the mixer controls when a stream is active.
So a port is no longer that simple.
> 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_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.
I admit it looks a bit ugly, but the alternative of creating some struct
that just contains a transport pointer and an old transport state (just
to use for the hook) seems equally ugly to me.
Ok to push together with your module-card-restore patch?
We could have a wider discussion on how we think about (and deal with)
unavailable profiles for PulseAudio v7 or so.
David Henningsson, Canonical Ltd.
More information about the pulseaudio-discuss