Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <bc567c4cb647d89e2e76568583716b4e44092519.1685284537.git.pav@iki.fi>
In-Reply-To: <bc567c4cb647d89e2e76568583716b4e44092519.1685284537.git.pav@iki.fi>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 30 May 2023 10:35:47 -0700
Message-ID: <CABBYNZLzc4HYaHnaTjb8nnBw6Xhd+_-qJu+U3W6zL5sQX7rE+w@mail.gmail.com>
Subject: Re: [PATCH BlueZ 1/2] shared/bap: detach io for source ASEs at QoS
 when disabling
To:     Pauli Virtanen <pav@iki.fi>
Cc:     linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Pauli,

On Sun, May 28, 2023 at 10:47=E2=80=AFAM Pauli Virtanen <pav@iki.fi> wrote:
>
> The Client may terminate a CIS when sink is in QOS and source in
> Disabling states (BAP v1.0.1 Sec 5.6.5).  It may also terminate it when
> Receiver Stop Ready has completed successfully (BAP v1.0.1 Sec 5.6.5.1).
> On successful Receiver Stop Ready the Server shall transition the ASE
> back to QoS state (ASCS v1.0 Sec 5.6).
>
> It appears Samsung Galaxy Buds2 Pro (R510XXUOAWA5) ignores the Receiver
> Stop Ready command if CIS is already disconnected, and then gets stuck
> in disabling state. It works if CIS is disconnected after Receiver Stop
> Ready.
>
> For better compatibility, disconnect CIS only after the source ASE is
> back in the QoS state. This is what we also do with sinks.
>
> Link: https://github.com/bluez/bluez/issues/516
> ---
>  src/shared/bap.c | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index f194f466f..16a9cec5b 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -1115,18 +1115,6 @@ static bool match_stream_io(const void *data, cons=
t void *user_data)
>         return stream->io =3D=3D io;
>  }
>
> -static void stream_stop_disabling(void *data, void *user_data)
> -{
> -       struct bt_bap_stream *stream =3D data;
> -
> -       if (stream->io || stream->ep->state !=3D BT_ASCS_ASE_STATE_DISABL=
ING)
> -               return;
> -
> -       DBG(stream->bap, "stream %p", stream);
> -
> -       bt_bap_stream_stop(stream, NULL, NULL);
> -}
> -
>  static bool bap_stream_io_detach(struct bt_bap_stream *stream)
>  {
>         struct bt_bap_stream *link;
> @@ -1145,9 +1133,6 @@ static bool bap_stream_io_detach(struct bt_bap_stre=
am *stream)
>                 /* Detach link if in QoS state */
>                 if (link->ep->state =3D=3D BT_ASCS_ASE_STATE_QOS)
>                         bap_stream_io_detach(link);
> -       } else {
> -               /* Links without IO on disabling state shall be stopped. =
*/
> -               queue_foreach(stream->links, stream_stop_disabling, NULL)=
;
>         }
>
>         stream_io_unref(io);
> @@ -1218,7 +1203,6 @@ static void bap_stream_state_changed(struct bt_bap_=
stream *stream)
>                 bap_stream_update_io_links(stream);
>                 break;
>         case BT_ASCS_ASE_STATE_DISABLING:
> -               bap_stream_io_detach(stream);
>                 break;
>         case BT_ASCS_ASE_STATE_QOS:
>                 if (stream->io && !stream->io->connecting)
> @@ -1252,8 +1236,8 @@ static void bap_stream_state_changed(struct bt_bap_=
stream *stream)
>                         bt_bap_stream_start(stream, NULL, NULL);
>                 break;
>         case BT_ASCS_ASE_STATE_DISABLING:
> -               if (!bt_bap_stream_get_io(stream))
> -                       bt_bap_stream_stop(stream, NULL, NULL);
> +               /* IO is detached when back in QOS */
> +               bt_bap_stream_stop(stream, NULL, NULL);
>                 break;

Note that doing this way makes our peripheral/server role detach by
itself because it will end up calling stream_stop, so perhaps we need
to add a check if we acting as a client or not, that said if we do it
late don't we risk the server not sending QOS until ISO is dropped? So
perhaps we also need to detect that somehow and drop the ISO socket if
the peripheral stays on DISABLING for too long (e.g. 2 sec) after
stop.

>         }
>
> --
> 2.40.1
>


--=20
Luiz Augusto von Dentz