Hi Daniel,
On Mon, Oct 28, 2024 at 1:11 PM Daniel Beer
<daniel.beer@xxxxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxxxx> 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.
Ok, then perhaps it is a good idea to have these applied, that said it
would have been great to have this type of test automation upstream in
the future so we can catch regressions if we ever change this logic
for some reason.
> 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@xxxxxxxxxxxxxxxxx or +64-27-420-8101
> Offices in Seattle, San Francisco, and Vancouver BC or (206) 494-3312
--
Luiz Augusto von Dentz
