Re: [PATCH BlueZ 2/2] source: clean up outstanding AVDTP requests if the stream goes away.

[Daniel Beer <daniel.beer@igorinstitute.com>]

On Mon, Oct 28, 2024 at 11:04:20AM -0400, Luiz Augusto von Dentz wrote:
> Hi Daniel,
> 
> On Fri, Oct 25, 2024 at 4:30 PM Daniel Beer
> <daniel.beer@igorinstitute.com> wrote:
> >
> > If the stream goes IDLE while we have an outstanding request, connect_id
> > stays non-zero and is never cleared via a completion callback. As a
> > consequence, the profile on this device will never be connected
> > successfully again until BlueZ restarts.
> > ---
> >  profiles/audio/source.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/profiles/audio/source.c b/profiles/audio/source.c
> > index 9fac352c8..db777e86d 100644
> > --- a/profiles/audio/source.c
> > +++ b/profiles/audio/source.c
> > @@ -134,6 +134,11 @@ static void stream_state_changed(struct avdtp_stream *stream,
> >         case AVDTP_STATE_IDLE:
> >                 btd_service_disconnecting_complete(source->service, 0);
> >
> > +               if (source->connect_id > 0) {
> > +                       a2dp_cancel(source->connect_id);
> > +                       source->connect_id = 0;
> > +               }
> > +
> 
> Is this really happening or is more of a fix based on disconnect_id?
> If that really is happening then we need to fix the sink as well since
> it appears to be doing the same, that said connect_id may be set with
> a2dp_discover which can happen in AVDTP_STATE_IDLE so in theory that
> can still be ongoing, anyway this code hasn't change in very long time
> so I wonder if this is sort of workaround to specific model or
> use-case we haven't tried before?

Hi Luiz,

Yes, it is really happening, and yes, the same problem occurs in sink.c
(see patch 1/1). We have a client who can reproduce the issue in
automated testing with Bluetooth A2DP devices, and who has tested the
fix above.

The symptom we notice is that PulseAudio complains about a timeout
connecting the A2DP profile, which is never attempted because
{sink,source}_setup_stream() never appears to do anything. It returns
immediately on the first line test because connect_id is non-zero and
stays that way forever.

Immediately before the sink/source code stops working we see a
communication failure in which the connection is dropped while
a2dp_discover is ongoing.

Cheers,
Daniel

> >                 if (source->disconnect_id > 0) {
> >                         a2dp_cancel(source->disconnect_id);
> >                         source->disconnect_id = 0;
> > --
> > 2.43.0
> >
> >
> 
> 
> -- 
> Luiz Augusto von Dentz

-- 
Daniel Beer
Director of Firmware Engineering at Igor Institute
daniel.beer@igorinstitute.com or +64-27-420-8101
Offices in Seattle, San Francisco, and Vancouver BC or (206) 494-3312