Hi Luiz，
[ EXTERNAL EMAIL ] Hi Yang, On Mon, Apr 7, 2025 at 6:34 AM Yang Li via B4 Relay <devnull+yang.li.amlogic.com@xxxxxxxxxx> wrote:From: Yang Li <yang.li@xxxxxxxxxxx> When the ASE state changes from releasing(6) -> idle(0), the idle state needs to be notified to the Client. --- Signed-off-by: Yang Li <yang.li@xxxxxxxxxxx> --- src/shared/bap.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/shared/bap.c b/src/shared/bap.c index 650bea2f4..c40d6e051 100644 --- a/src/shared/bap.c +++ b/src/shared/bap.c @@ -1123,17 +1123,12 @@ static void stream_notify_metadata(struct bt_bap_stream *stream) free(status); } -static void stream_notify_release(struct bt_bap_stream *stream) +static void stream_notify_ase_state(struct bt_bap_stream *stream) { struct bt_bap_endpoint *ep = stream->ep; struct bt_ascs_ase_status status; - DBG(stream->bap, "stream %p", stream); - - - memset(&status, 0, sizeof(status)); status.id = ep->id; - ep->state = BT_BAP_STREAM_STATE_RELEASING;Not really sure why you are taking away releasing state, we actually depend on it for the new tests: https://patchwork.kernel.org/project/bluetooth/list/?series=950067
Well, I got it.
status.state = ep->state; gatt_db_attribute_notify(ep->attr, (void *)&status, sizeof(status), @@ -1713,6 +1708,7 @@ static void stream_notify(struct bt_bap_stream *stream, uint8_t state) switch (state) { case BT_ASCS_ASE_STATE_IDLE: + stream_notify_ase_state(stream);We need something like stream_notify_idle.
Well, I got it.
break; case BT_ASCS_ASE_STATE_CONFIG: stream_notify_config(stream); @@ -1726,7 +1722,7 @@ static void stream_notify(struct bt_bap_stream *stream, uint8_t state) stream_notify_metadata(stream); break; case BT_ASCS_ASE_STATE_RELEASING: - stream_notify_release(stream); + stream_notify_ase_state(stream);This is actually wrong, we need to notify the releasing state.break; } } @@ -6397,9 +6393,8 @@ static bool stream_io_disconnected(struct io *io, void *user_data) DBG(stream->bap, "stream %p io disconnected", stream); if (stream->ep->state == BT_ASCS_ASE_STATE_RELEASING) - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG); + stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);Ok, so this is sort of reverting from bap: Fix not generating releasing state? I was wondering how much testing you guys did to confirm this is the right behavior, I don't think it is and Im trying to figure out if there are any tests for the testing spec that do properly verify this behavior.
There are some misunderstandings that need to be clarified. Initially, patchset V1 https://lore.kernel.org/all/20250106-upstream-v1-1-a16879b78ffd@xxxxxxxxxxx/ was proposed to solve the issue https://github.com/bluez/bluez/issues/1053, but after discussion, I felt that the process of Streaming->Releasing->Idle was OK, so V1 was abandoned. Then I submitted patchset V2/V3 https://lore.kernel.org/all/20250213-ascs_bug-v3-1-d5594f0f20c6@xxxxxxxxxxx/. I tested K70 and Pixel phones on V3, and both can solve the above issues. So my original intention was to merge V3, but after the release of version 5.82, I checked the code and found that V1 was merged. So I submitted the modification again on the latest version.
I sorted out the process of ASE state switching when the media of different mobile phones is paused:
1. Redmi K70+DUT: Pause operation, ASE state process is Streaming--->Disabling->QoS 2. Pixel+DUT: Pause operation, DUT does not cache Codec, ASE state process is Streaming->Releasing->Idl 3. Pixel+RedmiBud5 earbuds: Pause operation, earbuds cache Codec, ASE state process is Streaming->Releasing->Codec If the DUT also caches Codec, the latest version of the process is Streaming->Releasing->Codec->QoS, but the QoS status will be abnormal on Pixel phones, causing LE disconnection, so the process without caching Codec is still adopted.
- bt_bap_stream_set_io(stream, -1); return false; } --- base-commit: 0efa20cbf3fb5693c7c2f14ba8cf67053ca029e5 change-id: 20250407-bap_aes_state-9306798ff95a Best regards, -- Yang Li <yang.li@xxxxxxxxxxx>-- Luiz Augusto von Dentz
