Re: [PATCH BlueZ bluez v3] shared/bap: Add stream state check in stream_disable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Jul 3, 2025 at 4:40 AM Yang Li <yang.li@xxxxxxxxxxx> wrote:
>
> Hi Luzi,
> > [ EXTERNAL EMAIL ]
> >
> > Hi,
> >
> > On Tue, Jul 1, 2025 at 9:19 PM Yang Li via B4 Relay
> > <devnull+yang.li.amlogic.com@xxxxxxxxxx> wrote:
> >> From: Yang Li <yang.li@xxxxxxxxxxx>
> >>
> >> Add a state check so that stream_disable() is a no-op when the stream
> >> is not in ENABLING or STREAMING state. This prevents unexpected state
> >> transitions or redundant operations during cleanup.
> > What is the issue here, do you have traces, debug logs, etc?
>
> Regarding the Unicast caching configuration you mentioned, there is
> currently an issue in the code flow. The relevant log is shown below:
>
> When music is paused on the pixel 9 phone, the CIS link gets
> disconnected. As the transport state changes from ACTIVE to IDLE, the
> stream state transitions from config to qos.
>
>  > HCI Event: Disconnect Complete (0x05) plen 4           #1425 [hci0]
> 49.572089
> Status: Success (0x00)
> Handle: 512 Address: 6A:AB:51:47:3B:80 (Resolvable)
> Identity type: Random (0x01)
> Identity: E8:D5:2B:59:57:A6 (Static)
> Reason: Remote User Terminated Connection (0x13)
> bluetoothd[2313]: src/shared/bap.c:stream_io_disconnected() stream
> 0x1f9fc20 io disconnected
> bluetoothd[2313]: src/shared/bap.c:bap_ucast_set_state() stream
> 0x1f9fc20 dir 0x01: releasing -> config
> bluetoothd[2313]: src/shared/bap.c:stream_notify() stream 0x1f9fc20
> state 1
> bluetoothd[2313]: profiles/audio/transport.c:bap_state_changed()
> stream 0x1f9fc20: releasing(6) -> config(1)
> bluetoothd[2313]:
> profiles/audio/transport.c:transport_update_playing()
> /org/bluez/hci0/dev_6A_AB_51_47_3B_80/fd1 State=TRANSPORT_STATE_ACTIVE
> Playing=0
> bluetoothd[2313]:
> profiles/audio/transport.c:media_transport_remove_owner() Transport
> /org/bluez/hci0/dev_6A_AB_51_47_3B_80/fd1 Owner :1.1
> bluetoothd[2313]: profiles/audio/transport.c:media_owner_free() Owner
> :1.1
> bluetoothd[2313]:
> profiles/audio/transport.c:media_transport_suspend() Transport
> /org/bluez/hci0/dev_6A_AB_51_47_3B_80/fd1 Owner
> bluetoothd[2313]: profiles/audio/transport.c:transport_set_state()
> State changed /org/bluez/hci0/dev_6A_AB_51_47_3B_80/fd1:
> TRANSPORT_STATE_ACTIVE -> TRANSPORT_STATE_IDLE
> bluetoothd[2313]: src/shared/bap.c:stream_disable() stream 0x1f9fc20
> bluetoothd[2313]: src/shared/bap.c:bap_ucast_set_state() stream
> 0x1f9fc20 dir 0x01: config -> qos

Ok, this indeed is not expected, that said we do have the following
checks already in place so we might as well update them:

    if (!stream || stream->ep->state == BT_BAP_STREAM_STATE_QOS ||
            stream->ep->state == BT_BAP_STREAM_STATE_IDLE)
        return 0;

> ATTbluetoothd[2313]: < ACL Data TX: H.. flags 0x00 dlen 51 #1426
> [hci0] 49.585656
> ATT: Handle Value Notification (0x1b) len 46
> Handle: 0x007b Type: Sink ASE (0x2bc4)
> Data[44]:
> 01010002050a00204e00409c00204e00409c0006000000001302010302020105030300000003042800020501
> ASE ID: 1
> State: Codec Configured (0x01)
> Framing: Unframed PDUs supported (0x00)
> PHY: 0x02
> LE 2M PHY preffered (0x02)
> RTN: 5
> Max Transport Latency: 10
> Presentation Delay Min: 20000 us
> ...
> bluetoothd[2313]: < ACL Data TX: H.. flags 0x00 dlen 24 #1427 [hci0]
> 49.585725
> ATT: Handle Value Notification (0x1b) len 19
> Handle: 0x007b Type: Sink ASE (0x2bc4)
> Data[17]: 0102010010270000025000050a00204e00
> ASE ID: 1
> State: QoS Configured (0x02)
> CIG ID: 0x01
> CIS ID: 0x00
> ...
>
> when playback resumes on the phone, it attempts to set the ASE state to
> Codec. However, since the stream has already transitioned from config to
> qos, the phone ends up disconnecting the connection.
>
> bluetoothd[2313]: < ACL Data TX: H.. flags 0x00 dlen 12  #1433 [hci0]
> 60.216004
> ATT: Handle Value Notification (0x1b) len 7
> Handle: 0x0087 Type: ASE Control Point (0x2bc6)
> Data[5]: 0101010000
> Opcode: Codec Configuration (0x01)
> Number of ASE(s): 1
> ASE: #0
> ASE ID: 0x01
> ASE Response Code: Success (0x00)
> ASE Response Reason: None (0x00)
> bluetoothd[2313]: < ACL Data TX: H.. flags 0x00 dlen 51 #1434 [hci0]
> 60.226086
> ATT: Handle Value Notification (0x1b) len 46
> Handle: 0x007b Type: Sink ASE (0x2bc4)
> Data[44]:
> 01010002050a00204e00409c00204e00409c0006000000001302010302020105030300000003042800020501
> ASE ID: 1
> State: Codec Configured (0x01)
> Framing: Unframed PDUs supported (0x00)
> PHY: 0x02
> LE 2M PHY preffered (0x02)
> RTN: 5
> Max Transport Latency: 10
> Presentation Delay Min: 20000 us
> Presentation Delay Max: 40000 us
> Preferred Presentation Delay Min: 20000 us
> Preferred Presentation Delay Max: 40000 us
> Codec: LC3 (0x06)
> Codec Specific Configuration: #0: len 0x02 type 0x01
> Sampling Frequency: 16 Khz (0x03)
> Codec Specific Configuration: #1: len 0x02 type 0x02
> Frame Duration: 10 ms (0x01)
> Codec Specific Configuration: #2: len 0x05 type 0x03
> Location: 0x00000003
> Front Left (0x00000001)
> Front Right (0x00000002)
> Codec Specific Configuration: #3: len 0x03 type 0x04
> Frame Length: 40 (0x0028)
> Codec Specific Configuration: #4: len 0x02 type 0x05
> Frame Blocks per SDU: 1 (0x01)
>
> ...
>
>  > HCI Event: Disconnect Complete (0x05) plen 4           #1445 [hci0]
> 63.651497
> Status: Success (0x00)
> Handle: 16 Address: 6A:AB:51:47:3B:80 (Resolvable)
> Identity type: Random (0x01)
> Identity: E8:D5:2B:59:57:A6 (Static)
> Reason: Remote User Terminated Connection (0x13)
>
> Here is Pauli Virtanen’s analysis and solution:
> https://lore.kernel.org/all/3ac16d0a7c5569bce0b28f18bc2245bef8ab64c2.camel@xxxxxx/
>
> AFAICS the bug appears to be:
>
> - bap.c:stream_disable() should do nothing if stream is
> not ENABLING or STREAMING
>
> since it's called from bt_bap_stream_disable() which is called on
> transport suspend which should be noop for BAP server if stream is
> already gone.
>
> Next, I will attach the relevant btmon trace to the commit message.
> >
> >> Signed-off-by: Yang Li <yang.li@xxxxxxxxxxx>
> >> ---
> >> Changes in v3:
> >> - Optimizing the code
> >> - Link to v2: https://patch.msgid.link/20250630-bap_for_big_sync_lost-v2-0-1491b608cda5@xxxxxxxxxxx
> >>
> >> bap for big sync lost
> >>
> >> To: Linux Bluetooth <linux-bluetooth@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Yang Li <yang.li@xxxxxxxxxxx>
> >>
> >> Changes in v2:
> >> - Add state check in stream_disable.
> >> - Add type check in stream_io_disconnected.
> >> - Link to v1: https://patch.msgid.link/20250624-bap_for_big_sync_lost-v1-1-0df90a0f55d0@xxxxxxxxxxx
> >> ---
> >>   src/shared/bap.c | 22 ++++++++++++++--------
> >>   1 file changed, 14 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/src/shared/bap.c b/src/shared/bap.c
> >> index 40e1c974b..1790b277b 100644
> >> --- a/src/shared/bap.c
> >> +++ b/src/shared/bap.c
> >> @@ -2131,14 +2131,20 @@ static uint8_t stream_disable(struct bt_bap_stream *stream, struct iovec *rsp)
> >>
> >>          ascs_ase_rsp_success(rsp, stream->ep->id);
> >>
> >> -       /* Sink can autonomously transit to QOS while source needs to go to
> >> -        * Disabling until BT_ASCS_STOP is received.
> >> -        */
> >> -       if (stream->ep->dir == BT_BAP_SINK)
> >> -               stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
> >> -
> >> -       if (stream->ep->dir == BT_BAP_SOURCE)
> >> -               stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
> >> +       switch (stream->ep->state) {
> >> +               case BT_ASCS_ASE_STATE_ENABLING:
> >> +               case BT_ASCS_ASE_STATE_STREAMING:
> >> +                       if (stream->ep->dir == BT_BAP_SINK)
> >> +                               stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
> >> +                       else if (stream->ep->dir == BT_BAP_SOURCE)
> >> +                               /* Sink can autonomously transit to QOS while source needs to go to
> >> +                               * Disabling until BT_ASCS_STOP is received.
> >> +                               */
> >> +                               stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
> >> +                       break;
> >> +               default:
> >> +                       break;
> >> +       }
> >>
> >>          return 0;
> >>   }
> >>
> >> ---
> >> base-commit: 55a6763cde8a2309fd23a96479ee4cf2fc23a442
> >> change-id: 20250624-bap_for_big_sync_lost-63476c679dbb
> >>
> >> Best regards,
> >> --
> >> Yang Li <yang.li@xxxxxxxxxxx>
> >>
> >>
> >>
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux