On Wed, Sep 25, 2013 at 7:18 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> 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.
>
Yes, I understood that. But when I said I didn't know exactly why we
were removing PA_SUSPEND it wasn't clear to me that you still wanted
me to add that 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.
>
Ok, I feel more comfortable with you do it, so I don't become
responsible for a comment that do not understand entirely.
--
Jo?o Paulo Rechi Vita
http://about.me/jprvita
