Failure to start playback on A2DP sink after connection
This is what happen:
after Headset is connected, we call start_audio_datapath which will send AVDTP_Start command to Headset,
Headset reject it with bad state. Bluedroid stack will ack failure to start_audio_datapath.
The next time we write audio data to bluetooth, we will call start_audio_datapath again to send AVDTP_Start command to Headset
Headset reject it with bad state again. Bluedroid stack will ack failure to start_audio_datapath.
When the third time we call start_audio_datapath, right at that time we receive AVDTP_Start command from Headset.
Handle AVDTP_Start command and Handle start_audio_datapath are in two different threads.
Handle AVDTP_Start command is in btu_task thread.
Handle start_audio_datapath() is in btif_task thread.
We have race condition in this case
Because when btif_task processed BTIF_AV_START_STREAM_REQ_EVT(triggered by start_audio_datapath),
it don't know we receive AVDTP_Start command which is processed in btu_task.
btif_task will send a message BTA_AV_API_START_EVT to btu_task, which will be handled by bta_av_do_start.
AVDTP_start command from headset is handled by bta_av_start_ok.
bta_av_start_ok will send BTA_AV_START_EVT with suspending true to btif_task and send AVDTP_Suspend command
to headset to suspend the AVDTP for reconfiguration purpose.
in bta_av_do_start, we will check whether the AVDTP is already started, we will know the AVDTP is already start at this time because
bta_av_do_start is also running in btu_task. We will send BTA_AV_START_EVT with success to btif_task.
In the btif_task, BTA_AV_START_EVT will be processed by btif_av_state_opened_handler:
For the first BTA_AV_START_EVT with suspending true sent by bta_av_start_ok, it will ignore it:
if ((p_av->start.status == BTA_SUCCESS) && (p_av->start.suspending == TRUE))
return TRUE;
For the second BTA_AV_START_EVT with success sent by bta_av_do_start , it will ack success to start_audio_datapath, and change to
BTIF_AV_STATE_STARTED/BTAV_AUDIO_STATE_STARTED, after receive success ack from bluedroid stack, we will start send Audio data to bluetooth.
At last we received AVDTP_Suspend response accept from Headset, we will send BTA_AV_SUSPEND_EVT to btif_task, which will be
handled by btif_av_state_started_handler. It will call btif_a2dp_on_suspended and call audio_state_cb with new audio state
BTAV_AUDIO_STATE_STOPPED.
so The state between bluedroid stack and audio data path is out of sync.
The fix is to send failure message if we know we suspend AVDTP in bta_av_do_start,
also make sure we won't miss acknowledgement for pending start if we exit opened state,
to avoid audio data path dead lock.
bug:10953908
Change-Id: I1704839977324b7c4e234eb843cddf3719e10d2c
diff --git a/bta/av/bta_av_aact.c b/bta/av/bta_av_aact.c
index 9a38689..a50fb80 100644
--- a/bta/av/bta_av_aact.c
+++ b/bta/av/bta_av_aact.c
@@ -255,6 +255,28 @@
/*******************************************************************************
**
+** Function notify_start_failed
+**
+** Description notify up-layer AV start failed
+**
+**
+** Returns void
+**
+*******************************************************************************/
+static void notify_start_failed(tBTA_AV_SCB *p_scb)
+{
+ tBTA_AV_START start;
+ /* if start failed, clear role */
+ p_scb->role &= ~BTA_AV_ROLE_START_INT;
+ start.chnl = p_scb->chnl;
+ start.status = BTA_AV_FAIL;
+ start.initiator = TRUE;
+ start.hndl = p_scb->hndl;
+ (*bta_av_cb.p_cback)(BTA_AV_START_EVT, (tBTA_AV *) &start);
+}
+
+/*******************************************************************************
+**
** Function bta_av_st_rc_timer
**
** Description start the AVRC timer if no RC connection & CT is supported &
@@ -1782,8 +1804,13 @@
else if (p_scb->started)
{
p_scb->role |= BTA_AV_ROLE_START_INT;
- if ( p_scb->wait == 0 )
- bta_av_start_ok(p_scb, NULL);
+ if ( p_scb->wait == 0 ) {
+ if (p_scb->role & BTA_AV_ROLE_SUSPEND) {
+ notify_start_failed(p_scb);
+ } else {
+ bta_av_start_ok(p_scb, NULL);
+ }
+ }
}
APPL_TRACE_DEBUG2("started %d role:x%x", p_scb->started, p_scb->role);
}
@@ -2233,19 +2260,10 @@
*******************************************************************************/
void bta_av_start_failed (tBTA_AV_SCB *p_scb, tBTA_AV_DATA *p_data)
{
- tBTA_AV_START start;
-
if(p_scb->started == FALSE && p_scb->co_started == FALSE)
{
- /* if start failed, clear role */
- p_scb->role &= ~BTA_AV_ROLE_START_INT;
-
bta_sys_idle(BTA_ID_AV, bta_av_cb.audio_open_cnt, p_scb->peer_addr);
- start.chnl = p_scb->chnl;
- start.status = BTA_AV_FAIL;
- start.initiator = TRUE;
- start.hndl = p_scb->hndl;
- (*bta_av_cb.p_cback)(BTA_AV_START_EVT, (tBTA_AV *) &start);
+ notify_start_failed(p_scb);
}
bta_sys_set_policy(BTA_ID_AV, (HCI_ENABLE_SNIFF_MODE|HCI_ENABLE_MASTER_SLAVE_SWITCH), p_scb->peer_addr);
diff --git a/btif/include/btif_media.h b/btif/include/btif_media.h
index 9e20e21..4cdbb8c 100755
--- a/btif/include/btif_media.h
+++ b/btif/include/btif_media.h
@@ -236,7 +236,7 @@
void btif_a2dp_setup_codec(void);
void btif_a2dp_on_idle(void);
void btif_a2dp_on_open(void);
-void btif_a2dp_on_started(tBTA_AV_START *p_av);
+BOOLEAN btif_a2dp_on_started(tBTA_AV_START *p_av, BOOLEAN pending_start);
void btif_a2dp_ack_fail(void);
void btif_a2dp_on_stop_req(void);
void btif_a2dp_on_stopped(tBTA_AV_SUSPEND *p_av);
diff --git a/btif/src/btif_av.c b/btif/src/btif_av.c
index 748b204..f173f6a 100755
--- a/btif/src/btif_av.c
+++ b/btif/src/btif_av.c
@@ -491,14 +491,21 @@
if ((p_av->start.status == BTA_SUCCESS) && (p_av->start.suspending == TRUE))
return TRUE;
- btif_av_cb.flags &= ~BTIF_AV_FLAG_PENDING_START;
- btif_a2dp_on_started(&p_av->start);
+ if (btif_a2dp_on_started(&p_av->start,
+ ((btif_av_cb.flags & BTIF_AV_FLAG_PENDING_START) != 0))) {
+ /* only clear pending flag after acknowledgement */
+ btif_av_cb.flags &= ~BTIF_AV_FLAG_PENDING_START;
+ }
/* remain in open state if status failed */
if (p_av->start.status != BTA_AV_SUCCESS)
return FALSE;
- /* change state to started */
+ /* change state to started, send acknowledgement if start is pending */
+ if (btif_av_cb.flags & BTIF_AV_FLAG_PENDING_START) {
+ btif_a2dp_on_started(NULL, TRUE);
+ /* pending start flag will be cleared when exit current state */
+ }
btif_sm_change_state(btif_av_cb.sm_handle, BTIF_AV_STATE_STARTED);
} break;
@@ -520,6 +527,11 @@
HAL_CBACK(bt_av_callbacks, connection_state_cb,
BTAV_CONNECTION_STATE_DISCONNECTED, &(btif_av_cb.peer_bda));
+ /* change state to idle, send acknowledgement if start is pending */
+ if (btif_av_cb.flags & BTIF_AV_FLAG_PENDING_START) {
+ btif_a2dp_ack_fail();
+ /* pending start flag will be cleared when exit current state */
+ }
btif_sm_change_state(btif_av_cb.sm_handle, BTIF_AV_STATE_IDLE);
break;
@@ -581,7 +593,7 @@
case BTIF_AV_START_STREAM_REQ_EVT:
/* we were remotely started, just ack back the local request */
- btif_a2dp_on_started(NULL);
+ btif_a2dp_on_started(NULL, TRUE);
break;
/* fixme -- use suspend = true always to work around issue with BTA AV */
diff --git a/btif/src/btif_media_task.c b/btif/src/btif_media_task.c
index 7fe04fd..ff9f92c 100755
--- a/btif/src/btif_media_task.c
+++ b/btif/src/btif_media_task.c
@@ -844,9 +844,10 @@
**
*******************************************************************************/
-void btif_a2dp_on_started(tBTA_AV_START *p_av)
+BOOLEAN btif_a2dp_on_started(tBTA_AV_START *p_av, BOOLEAN pending_start)
{
tBTIF_STATUS status;
+ BOOLEAN ack = FALSE;
APPL_TRACE_EVENT0("## ON A2DP STARTED ##");
@@ -854,7 +855,7 @@
{
/* ack back a local start request */
a2dp_cmd_acknowledge(A2DP_CTRL_ACK_SUCCESS);
- return;
+ return TRUE;
}
if (p_av->status == BTA_AV_SUCCESS)
@@ -863,7 +864,10 @@
{
if (p_av->initiator)
{
- a2dp_cmd_acknowledge(A2DP_CTRL_ACK_SUCCESS);
+ if (pending_start) {
+ a2dp_cmd_acknowledge(A2DP_CTRL_ACK_SUCCESS);
+ ack = TRUE;
+ }
}
else
{
@@ -875,10 +879,12 @@
/* media task is autostarted upon a2dp audiopath connection */
}
}
- else
+ else if (pending_start)
{
a2dp_cmd_acknowledge(A2DP_CTRL_ACK_FAILURE);
+ ack = TRUE;
}
+ return ack;
}
