[pulseaudio-discuss] [PATCH v4 36/41] bluetooth: Handle changes to BlueZ 5 transports state

[Tanu Kaskinen]

On Tue, 2013-09-24 at 16:02 -0300, João Paulo Rechi Vita wrote:
> On Sat, Sep 21, 2013 at 11:50 AM, Tanu Kaskinen
> <tanu.kaskinen at linux.intel.com> wrote:
> > On Wed, 2013-09-18 at 16:17 -0500, jprvita at gmail.com wrote:
> >> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> >>
> >> ---
> >>  src/modules/bluetooth/module-bluez5-device.c | 78 ++++++++++++++++++++++++++++
> >>  1 file changed, 78 insertions(+)
> >>
> >> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> >> index d9e1fc6..2eaeb77 100644
> >> --- a/src/modules/bluetooth/module-bluez5-device.c
> >> +++ b/src/modules/bluetooth/module-bluez5-device.c
> >> @@ -97,6 +97,7 @@ struct userdata {
> >>      pa_core *core;
> >>
> >>      pa_hook_slot *device_connection_changed_slot;
> >> +    pa_hook_slot *transport_state_changed_slot;
> >>
> >>      pa_bluetooth_discovery *discovery;
> >>      pa_bluetooth_device *device;
> >> @@ -1670,6 +1671,62 @@ static int add_card(struct userdata *u) {
> >>  }
> >>
> >>  /* Run from main thread */
> >> +static void handle_transport_state_change(struct userdata *u, struct pa_bluetooth_transport *t) {
> >> +    bool acquire = false;
> >> +    bool release = false;
> >> +    pa_card_profile *cp;
> >> +    pa_device_port *port;
> >> +
> >> +    pa_assert(u);
> >> +    pa_assert(t);
> >> +
> >> +    /* Update profile availability */
> >> +    if (!(cp = pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile))))
> >> +        return;
> >> +    pa_card_profile_set_available(cp, transport_state_to_availability(t->state));
> >> +
> >> +    /* Update port availability */
> >> +    pa_assert_se(port = pa_hashmap_get(u->card->ports, u->output_port_name));
> >> +    pa_device_port_set_available(port, get_port_availability(u, PA_DIRECTION_OUTPUT));
> >> +    pa_assert_se(port = pa_hashmap_get(u->card->ports, u->input_port_name));
> >> +    pa_device_port_set_available(port, get_port_availability(u, PA_DIRECTION_INPUT));
> >> +
> >> +    /* 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);
> >> +
> >> +    if (acquire && transport_acquire(u, true) >= 0) {
> >> +        if (u->source) {
> >> +            pa_log_debug("Resuming source %s because its transport state changed to playing", u->source->name);
> >> +            pa_source_suspend(u->source, false, PA_SUSPEND_IDLE|PA_SUSPEND_USER);
> >
> > This is now the third time I ask you to add the following comment:
> >
> > /* We remove the IDLE suspend cause, because otherwise module-loopback
> > doesn't uncork its streams. FIXME: Messing with the IDLE suspend cause
> > here is wrong, the correct way to handle this would probably be to
> > uncork the loopback streams not only when the other end is unsuspended,
> > but also when the other end's suspend cause changes to IDLE only
> > (currently there's no notification mechanism for suspend cause changes,
> > though). */
> >
> 
> I've answered you before, this line was added by Mikel at some point
> to the BlueZ 4 support. This whole code series is a refactor, I think
> we shouldn't block this series because of issues like this one, which
> are from the original code and can be very well addressed later on.

I was not asking you to fix the issue, I was only asking you to add the
comment.

> And actually, for the matter of adding this comment it makes more
> sense that you add it in a separate commit, since the comment is
> coming from you, not from me.

Ok, I'll add it.

-- 
Tanu