diff mbox series

[BlueZ,bluez,v3] bap: fixed issue of muting music silent after pause and resume.

Message ID 20250213-ascs_bug-v3-1-d5594f0f20c6@amlogic.com (mailing list archive)
State New
Headers show
Series [BlueZ,bluez,v3] bap: fixed issue of muting music silent after pause and resume. | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch warning CheckSparse WARNING src/shared/bap.c:296:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:296:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:296:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Yang Li via B4 Relay Feb. 13, 2025, 2:57 a.m. UTC 
From: Yang Li <yang.li@amlogic.com>

After the ASE state changes (streaming->releasing->idle),
the Client needs to be notified. The process as follows:

...(Sink ASE: ID=1, State=Streaming)
ATT Write Command Packet (ASE Control Point: Op=Release)
ATT Notification Packet (Sink ASE: ID=1, State=Releasing)
ATT Notification Packet (Sink ASE: ID=1, State=Idle)

Signed-off-by: Yang Li <yang.li@amlogic.com>
---
Changes in v3:
- Solve the compilation error reported by test.bot
- Link to v2: https://patch.msgid.link/20250208-ascs_bug-v2-1-b7e062d6604d@amlogic.com
---
 src/shared/bap.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)


---
base-commit: 2ee08ffd4d469781dc627fa50b4a015d9ad68007
change-id: 20250208-ascs_bug-e7c5715d3d8c

Best regards,

Comments

bluez.test.bot@gmail.com Feb. 13, 2025, 4:12 a.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=933440

---Test result---

Test Summary:
CheckPatch                    PENDING   0.23 seconds
GitLint                       PENDING   0.16 seconds
BuildEll                      PASS      20.20 seconds
BluezMake                     PASS      1452.79 seconds
MakeCheck                     PASS      13.57 seconds
MakeDistcheck                 PASS      156.87 seconds
CheckValgrind                 PASS      211.58 seconds
CheckSmatch                   WARNING   282.02 seconds
bluezmakeextell               PASS      97.73 seconds
IncrementalBuild              PENDING   0.23 seconds
ScanBuild                     PASS      858.21 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:296:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:296:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:296:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Feb. 13, 2025, 4:34 p.m. UTC | #2 
Hi Yang,

On Wed, Feb 12, 2025 at 9:58 PM Yang Li via B4 Relay
<devnull+yang.li.amlogic.com@kernel.org> wrote:
>
> From: Yang Li <yang.li@amlogic.com>
>
> After the ASE state changes (streaming->releasing->idle),
> the Client needs to be notified. The process as follows:
>
> ...(Sink ASE: ID=1, State=Streaming)
> ATT Write Command Packet (ASE Control Point: Op=Release)
> ATT Notification Packet (Sink ASE: ID=1, State=Releasing)
> ATT Notification Packet (Sink ASE: ID=1, State=Idle)
>
> Signed-off-by: Yang Li <yang.li@amlogic.com>
> ---
> Changes in v3:
> - Solve the compilation error reported by test.bot
> - Link to v2: https://patch.msgid.link/20250208-ascs_bug-v2-1-b7e062d6604d@amlogic.com
> ---
>  src/shared/bap.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 167501282..079f7ede0 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -930,6 +930,18 @@ static void ascs_ase_rsp_success(struct iovec *iov, uint8_t id)
>                                         BT_ASCS_REASON_NONE);
>  }
>
> +static void stream_notify_ase_state(struct bt_bap_stream *stream)
> +{
> +       struct bt_bap_endpoint *ep = stream->ep;
> +       struct bt_ascs_ase_status status;
> +
> +       status.id = ep->id;
> +       status.state = ep->state;
> +
> +       gatt_db_attribute_notify(ep->attr, (void *)&status, sizeof(status),
> +                                       bt_bap_get_att(stream->bap));
> +}
> +
>  static void stream_notify_config(struct bt_bap_stream *stream)
>  {
>         struct bt_bap_endpoint *ep = stream->ep;
> @@ -1634,7 +1646,9 @@ static bool stream_notify_state(void *data)
>         struct bt_bap_stream *stream = data;
>         struct bt_bap_endpoint *ep = stream->ep;
>
> -       DBG(stream->bap, "stream %p", stream);
> +       DBG(stream->bap, "stream %p state %s",
> +                       stream,
> +                       bt_bap_stream_statestr(ep->state));
>
>         if (stream->state_id) {
>                 timeout_remove(stream->state_id);
> @@ -1643,6 +1657,7 @@ static bool stream_notify_state(void *data)
>
>         switch (ep->state) {
>         case BT_ASCS_ASE_STATE_IDLE:
> +               stream_notify_ase_state(stream);
>                 break;
>         case BT_ASCS_ASE_STATE_CONFIG:
>                 stream_notify_config(stream);
> @@ -1655,6 +1670,9 @@ static bool stream_notify_state(void *data)
>         case BT_ASCS_ASE_STATE_DISABLING:
>                 stream_notify_metadata(stream);
>                 break;
> +       case BT_ASCS_ASE_STATE_RELEASING:
> +               stream_notify_ase_state(stream);
> +               break;
>         }
>
>         return false;
> @@ -2068,17 +2086,11 @@ static unsigned int bap_ucast_metadata(struct bt_bap_stream *stream,
>
>  static uint8_t stream_release(struct bt_bap_stream *stream, struct iovec *rsp)
>  {
> -       struct bt_bap_pac *pac;
> -
>         DBG(stream->bap, "stream %p", stream);
>
>         ascs_ase_rsp_success(rsp, stream->ep->id);
>
> -       pac = stream->lpac;
> -       if (pac->ops && pac->ops->clear)
> -               pac->ops->clear(stream, pac->user_data);

This part I don't really understand, why are you removing the call to
clear? Or are you relying on bap_stream_clear_cfm? That is only called
on detach/disconnect so I don't think we should be removing the code
above since it is still possible to reconfigure after releasing.

> -       stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
> +       stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
>
>         return 0;
>  }
> @@ -6172,7 +6184,8 @@ static bool stream_io_disconnected(struct io *io, void *user_data)
>
>         DBG(stream->bap, "stream %p io disconnected", stream);
>
> -       bt_bap_stream_set_io(stream, -1);
> +       if (stream->ep->state == BT_BAP_STREAM_STATE_RELEASING)
> +               stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
>
>         return false;
>  }
>
> ---
> base-commit: 2ee08ffd4d469781dc627fa50b4a015d9ad68007
> change-id: 20250208-ascs_bug-e7c5715d3d8c
>
> Best regards,
> --
> Yang Li <yang.li@amlogic.com>
>
>
>
Yang Li Feb. 17, 2025, 8:20 a.m. UTC | #3 
Hi Luiz
> [ EXTERNAL EMAIL ]
>
> Hi Yang,
>
> On Wed, Feb 12, 2025 at 9:58 PM Yang Li via B4 Relay
> <devnull+yang.li.amlogic.com@kernel.org> wrote:
>> From: Yang Li <yang.li@amlogic.com>
>>
>> After the ASE state changes (streaming->releasing->idle),
>> the Client needs to be notified. The process as follows:
>>
>> ...(Sink ASE: ID=1, State=Streaming)
>> ATT Write Command Packet (ASE Control Point: Op=Release)
>> ATT Notification Packet (Sink ASE: ID=1, State=Releasing)
>> ATT Notification Packet (Sink ASE: ID=1, State=Idle)
>>
>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>> ---
>> Changes in v3:
>> - Solve the compilation error reported by test.bot
>> - Link to v2: https://patch.msgid.link/20250208-ascs_bug-v2-1-b7e062d6604d@amlogic.com
>> ---
>>   src/shared/bap.c | 31 ++++++++++++++++++++++---------
>>   1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/shared/bap.c b/src/shared/bap.c
>> index 167501282..079f7ede0 100644
>> --- a/src/shared/bap.c
>> +++ b/src/shared/bap.c
>> @@ -930,6 +930,18 @@ static void ascs_ase_rsp_success(struct iovec *iov, uint8_t id)
>>                                          BT_ASCS_REASON_NONE);
>>   }
>>
>> +static void stream_notify_ase_state(struct bt_bap_stream *stream)
>> +{
>> +       struct bt_bap_endpoint *ep = stream->ep;
>> +       struct bt_ascs_ase_status status;
>> +
>> +       status.id = ep->id;
>> +       status.state = ep->state;
>> +
>> +       gatt_db_attribute_notify(ep->attr, (void *)&status, sizeof(status),
>> +                                       bt_bap_get_att(stream->bap));
>> +}
>> +
>>   static void stream_notify_config(struct bt_bap_stream *stream)
>>   {
>>          struct bt_bap_endpoint *ep = stream->ep;
>> @@ -1634,7 +1646,9 @@ static bool stream_notify_state(void *data)
>>          struct bt_bap_stream *stream = data;
>>          struct bt_bap_endpoint *ep = stream->ep;
>>
>> -       DBG(stream->bap, "stream %p", stream);
>> +       DBG(stream->bap, "stream %p state %s",
>> +                       stream,
>> +                       bt_bap_stream_statestr(ep->state));
>>
>>          if (stream->state_id) {
>>                  timeout_remove(stream->state_id);
>> @@ -1643,6 +1657,7 @@ static bool stream_notify_state(void *data)
>>
>>          switch (ep->state) {
>>          case BT_ASCS_ASE_STATE_IDLE:
>> +               stream_notify_ase_state(stream);
>>                  break;
>>          case BT_ASCS_ASE_STATE_CONFIG:
>>                  stream_notify_config(stream);
>> @@ -1655,6 +1670,9 @@ static bool stream_notify_state(void *data)
>>          case BT_ASCS_ASE_STATE_DISABLING:
>>                  stream_notify_metadata(stream);
>>                  break;
>> +       case BT_ASCS_ASE_STATE_RELEASING:
>> +               stream_notify_ase_state(stream);
>> +               break;
>>          }
>>
>>          return false;
>> @@ -2068,17 +2086,11 @@ static unsigned int bap_ucast_metadata(struct bt_bap_stream *stream,
>>
>>   static uint8_t stream_release(struct bt_bap_stream *stream, struct iovec *rsp)
>>   {
>> -       struct bt_bap_pac *pac;
>> -
>>          DBG(stream->bap, "stream %p", stream);
>>
>>          ascs_ase_rsp_success(rsp, stream->ep->id);
>>
>> -       pac = stream->lpac;
>> -       if (pac->ops && pac->ops->clear)
>> -               pac->ops->clear(stream, pac->user_data);
> This part I don't really understand, why are you removing the call to
> clear? Or are you relying on bap_stream_clear_cfm? That is only called
> on detach/disconnect so I don't think we should be removing the code
> above since it is still possible to reconfigure after releasing.
I removed the code to allow the mobile phone to disconnect CIS iso and 
switch the ASE status to IDLE in function ofstream_io_disconnected().

If pac->ops->clear() is executed, CIS iso disconnects actively, and 
stream_disable() transitions the status to QoS. bluetoothd.log as below:

bluetoothd[2223]: src/shared/bap.c:stream_release() stream 0x1ed5ae8 
bluetoothd[2223]: profiles/audio/media.c:pac_clear() endpoint 0x1ec8780 
stream 0x1ed5ae8 bluetoothd[2223]: src/shared/bap.c:stream_disable() 
stream 0x1ed5ae8 bluetoothd[2223]: 
src/shared/bap.c:bap_ucast_set_state() stream 0x1ed5ae8 dir 0x01: 
streaming -> qos bluetoothd[2223]: 
src/shared/bap.c:bap_stream_io_detach() stream 0x1ed5ae8 
bluetoothd[2223]: src/shared/bap.c:stream_io_free() fd 18 
bluetoothd[2223]: profiles/audio/bap.c:bap_state() stream 0x1ed5ae8: 
streaming(4) -> qos(2)
Could you please guide me on where to switch the ASE status to IDLE?
>> -       stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
>> +       stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
>>
>>          return 0;
>>   }
>> @@ -6172,7 +6184,8 @@ static bool stream_io_disconnected(struct io *io, void *user_data)
>>
>>          DBG(stream->bap, "stream %p io disconnected", stream);
>>
>> -       bt_bap_stream_set_io(stream, -1);
>> +       if (stream->ep->state == BT_BAP_STREAM_STATE_RELEASING)
>> +               stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
>>
>>          return false;
>>   }
>>
>> ---
>> base-commit: 2ee08ffd4d469781dc627fa50b4a015d9ad68007
>> change-id: 20250208-ascs_bug-e7c5715d3d8c
>>
>> Best regards,
>> --
>> Yang Li <yang.li@amlogic.com>
>>
>>
>>
>
> --
> Luiz Augusto von Dentz
diff mbox series

Patch

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 167501282..079f7ede0 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -930,6 +930,18 @@  static void ascs_ase_rsp_success(struct iovec *iov, uint8_t id)
 					BT_ASCS_REASON_NONE);
 }
 
+static void stream_notify_ase_state(struct bt_bap_stream *stream)
+{
+	struct bt_bap_endpoint *ep = stream->ep;
+	struct bt_ascs_ase_status status;
+
+	status.id = ep->id;
+	status.state = ep->state;
+
+	gatt_db_attribute_notify(ep->attr, (void *)&status, sizeof(status),
+					bt_bap_get_att(stream->bap));
+}
+
 static void stream_notify_config(struct bt_bap_stream *stream)
 {
 	struct bt_bap_endpoint *ep = stream->ep;
@@ -1634,7 +1646,9 @@  static bool stream_notify_state(void *data)
 	struct bt_bap_stream *stream = data;
 	struct bt_bap_endpoint *ep = stream->ep;
 
-	DBG(stream->bap, "stream %p", stream);
+	DBG(stream->bap, "stream %p state %s",
+			stream,
+			bt_bap_stream_statestr(ep->state));
 
 	if (stream->state_id) {
 		timeout_remove(stream->state_id);
@@ -1643,6 +1657,7 @@  static bool stream_notify_state(void *data)
 
 	switch (ep->state) {
 	case BT_ASCS_ASE_STATE_IDLE:
+		stream_notify_ase_state(stream);
 		break;
 	case BT_ASCS_ASE_STATE_CONFIG:
 		stream_notify_config(stream);
@@ -1655,6 +1670,9 @@  static bool stream_notify_state(void *data)
 	case BT_ASCS_ASE_STATE_DISABLING:
 		stream_notify_metadata(stream);
 		break;
+	case BT_ASCS_ASE_STATE_RELEASING:
+		stream_notify_ase_state(stream);
+		break;
 	}
 
 	return false;
@@ -2068,17 +2086,11 @@  static unsigned int bap_ucast_metadata(struct bt_bap_stream *stream,
 
 static uint8_t stream_release(struct bt_bap_stream *stream, struct iovec *rsp)
 {
-	struct bt_bap_pac *pac;
-
 	DBG(stream->bap, "stream %p", stream);
 
 	ascs_ase_rsp_success(rsp, stream->ep->id);
 
-	pac = stream->lpac;
-	if (pac->ops && pac->ops->clear)
-		pac->ops->clear(stream, pac->user_data);
-
-	stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
+	stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
 
 	return 0;
 }
@@ -6172,7 +6184,8 @@  static bool stream_io_disconnected(struct io *io, void *user_data)
 
 	DBG(stream->bap, "stream %p io disconnected", stream);
 
-	bt_bap_stream_set_io(stream, -1);
+	if (stream->ep->state == BT_BAP_STREAM_STATE_RELEASING)
+		stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
 
 	return false;
 }