From 7036662ec1070a7cadc59d163064fd3c1b3dc8b3 Mon Sep 17 00:00:00 2001 From: Cheney Ni Date: Tue, 26 Feb 2019 11:56:25 +0800 Subject: [PATCH] Reland Start the media encoder for a successful BTA_AV_START_EVT When phone is A2DP source and receives BTA_AV_START_EVT, we need to proceed to start the encoder of software or offload, and ack back to audio HAL. When issue happened, the remote as the AVDTP_START initiator triggered BTA_AV_START_EVT, and caused we did not start the encoder to stream the audio. This change simplified the handler of BTA_AV_START_EVT to always start the encoder for a successful event, and ack to audio HAL if we are AVDTP initiator. Bug: 126136429 Test: A2DP play and paused manually on Pixel 2 and Pixel 3 Change-Id: Ieb479fd6f42da1bf37f8f32af7794d86e04cac1b --- btif/include/btif_a2dp.h | 5 +-- btif/src/btif_a2dp.cc | 94 ++++++++++++++++++++---------------------------- btif/src/btif_av.cc | 21 +++-------- 3 files changed, 45 insertions(+), 75 deletions(-) diff --git a/btif/include/btif_a2dp.h b/btif/include/btif_a2dp.h index 7b7b2ca46..49e4f9c98 100644 --- a/btif/include/btif_a2dp.h +++ b/btif/include/btif_a2dp.h @@ -31,11 +31,8 @@ void btif_a2dp_on_idle(void); // streaming. // |peer_addr| is the peer address. // |p_av_start| is the data associated with the request - see |tBTA_AV_START|. -// |pending_start| should be set to true if the BTIF state machine is in -// 'pending start' state. // Returns true if an ACK for the local command was sent, otherwise false. -bool btif_a2dp_on_started(const RawAddress& peer_addr, - tBTA_AV_START* p_av_start, bool pending_start); +bool btif_a2dp_on_started(const RawAddress& peer_addr, tBTA_AV_START* p_av_start); // Process 'stop' request from the BTIF state machine to stop A2DP streaming. // |p_av_suspend| is the data associated with the request - see diff --git a/btif/src/btif_a2dp.cc b/btif/src/btif_a2dp.cc index 2efeec03a..e2d613274 100644 --- a/btif/src/btif_a2dp.cc +++ b/btif/src/btif_a2dp.cc @@ -46,71 +46,55 @@ void btif_a2dp_on_idle(void) { } } -bool btif_a2dp_on_started(const RawAddress& peer_addr, - tBTA_AV_START* p_av_start, bool pending_start) { - bool ack = false; - - LOG_INFO(LOG_TAG, - "%s: ## ON A2DP STARTED ## peer %s pending_start:%s p_av_start:%p", - __func__, peer_addr.ToString().c_str(), - logbool(pending_start).c_str(), p_av_start); +bool btif_a2dp_on_started(const RawAddress& peer_addr, tBTA_AV_START* p_av_start) { + LOG(INFO) << __func__ << ": ## ON A2DP STARTED ## peer " << peer_addr << " p_av_start:" << p_av_start; if (p_av_start == NULL) { - /* ack back a local start request */ - - if (!btif_av_is_a2dp_offload_enabled()) { - if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) { - bluetooth::audio::a2dp::ack_stream_started(A2DP_CTRL_ACK_SUCCESS); - } else { - btif_a2dp_command_ack(A2DP_CTRL_ACK_SUCCESS); - } - return true; - } else if (bluetooth::headset::IsCallIdle()) { - btif_av_stream_start_offload(); + tA2DP_CTRL_ACK status = A2DP_CTRL_ACK_SUCCESS; + if (!bluetooth::headset::IsCallIdle()) { + LOG(ERROR) << __func__ << ": peer " << peer_addr << " call in progress, do not start A2DP stream"; + status = A2DP_CTRL_ACK_INCALL_FAILURE; + } + /* just ack back a local start request, do not start the media encoder since + * this is not for BTA_AV_START_EVT. */ + if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) { + bluetooth::audio::a2dp::ack_stream_started(status); + } else if (btif_av_is_a2dp_offload_enabled()) { + btif_a2dp_audio_on_started(status); } else { - LOG_ERROR(LOG_TAG, "%s: peer %s call in progress, do not start offload", - __func__, peer_addr.ToString().c_str()); - btif_a2dp_audio_on_started(A2DP_CTRL_ACK_INCALL_FAILURE); + btif_a2dp_command_ack(status); } return true; } - LOG_INFO(LOG_TAG, - "%s: peer %s pending_start:%s status:%d suspending:%s initiator:%s", - __func__, peer_addr.ToString().c_str(), - logbool(pending_start).c_str(), p_av_start->status, - logbool(p_av_start->suspending).c_str(), - logbool(p_av_start->initiator).c_str()); + LOG(INFO) << __func__ << ": peer " << peer_addr << " status:" << +p_av_start->status + << " suspending:" << logbool(p_av_start->suspending) << " initiator:" << logbool(p_av_start->initiator); if (p_av_start->status == BTA_AV_SUCCESS) { - if (!p_av_start->suspending) { + if (p_av_start->suspending) { + LOG(WARNING) << __func__ << ": peer " << peer_addr << " A2DP is suspending and ignores the started event"; + return false; + } + if (btif_av_is_a2dp_offload_enabled()) { + btif_av_stream_start_offload(); + } else if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) { + if (btif_av_get_peer_sep() == AVDT_TSEP_SNK) { + /* Start the media encoder to do the SW audio stream */ + btif_a2dp_source_start_audio_req(); + } if (p_av_start->initiator) { - if (pending_start) { - if (btif_av_is_a2dp_offload_enabled()) { - btif_av_stream_start_offload(); - } else if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) { - bluetooth::audio::a2dp::ack_stream_started(A2DP_CTRL_ACK_SUCCESS); - if (btif_av_get_peer_sep() == AVDT_TSEP_SNK) { - /* Start the media task to do the SW encode audio */ - btif_a2dp_source_start_audio_req(); - } - } else { - btif_a2dp_command_ack(A2DP_CTRL_ACK_SUCCESS); - } - ack = true; - } - } else { - // We were started remotely - if (btif_av_is_a2dp_offload_enabled()) { - btif_av_stream_start_offload(); - } + bluetooth::audio::a2dp::ack_stream_started(A2DP_CTRL_ACK_SUCCESS); + return true; } - - /* media task is autostarted upon a2dp audiopath connection */ + } else { + if (p_av_start->initiator) { + btif_a2dp_command_ack(A2DP_CTRL_ACK_SUCCESS); + return true; + } + /* media task is auto-started upon UIPC connection of a2dp audiopath */ } - } else if (pending_start) { - LOG_ERROR(LOG_TAG, "%s: peer %s A2DP start request failed: status = %d", - __func__, peer_addr.ToString().c_str(), p_av_start->status); + } else if (p_av_start->initiator) { + LOG(ERROR) << __func__ << ": peer " << peer_addr << " A2DP start request failed: status = " << +p_av_start->status; if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) { bluetooth::audio::a2dp::ack_stream_started(A2DP_CTRL_ACK_FAILURE); } else if (btif_av_is_a2dp_offload_enabled()) { @@ -118,9 +102,9 @@ bool btif_a2dp_on_started(const RawAddress& peer_addr, } else { btif_a2dp_command_ack(A2DP_CTRL_ACK_FAILURE); } - ack = true; + return true; } - return ack; + return false; } void btif_a2dp_on_stopped(tBTA_AV_SUSPEND* p_av_suspend) { diff --git a/btif/src/btif_av.cc b/btif/src/btif_av.cc index fea37f8b7..3659534ee 100644 --- a/btif/src/btif_av.cc +++ b/btif/src/btif_av.cc @@ -1840,17 +1840,13 @@ bool BtifAvStateMachine::StateOpened::ProcessEvent(uint32_t event, bool should_suspend = false; if (peer_.IsSink() && !peer_.CheckFlags(BtifAvPeer::kFlagPendingStart | BtifAvPeer::kFlagRemoteSuspend)) { - BTIF_TRACE_WARNING("%s: Peer %s : trigger Suspend as remote initiated", - __PRETTY_FUNCTION__, - peer_.PeerAddress().ToString().c_str()); + LOG(WARNING) << __PRETTY_FUNCTION__ << ": Peer " << peer_.PeerAddress() + << " : trigger Suspend as remote initiated"; should_suspend = true; } - // If peer is A2DP Source, we do not want to ACK commands on UIPC - if (peer_.IsSink() && - btif_a2dp_on_started( - peer_.PeerAddress(), &p_av->start, - peer_.CheckFlags(BtifAvPeer::kFlagPendingStart))) { + // If peer is A2DP Source, do ACK commands to audio HAL and start media task + if (peer_.IsSink() && btif_a2dp_on_started(peer_.PeerAddress(), &p_av->start)) { // Only clear pending flag after acknowledgement peer_.ClearFlags(BtifAvPeer::kFlagPendingStart); } @@ -1863,12 +1859,6 @@ bool BtifAvStateMachine::StateOpened::ProcessEvent(uint32_t event, btif_a2dp_sink_set_rx_flush(false); } - // Change state to Started, send acknowledgement if start is pending - if (peer_.IsSink() && peer_.CheckFlags(BtifAvPeer::kFlagPendingStart)) { - btif_a2dp_on_started(peer_.PeerAddress(), nullptr, true); - // Pending start flag will be cleared when exit current state - } - if (should_suspend) { btif_av_source_dispatch_sm_event(peer_.PeerAddress(), BTIF_AV_SUSPEND_STREAM_REQ_EVT); @@ -2002,8 +1992,7 @@ bool BtifAvStateMachine::StateStarted::ProcessEvent(uint32_t event, BtifAvEvent::EventName(event).c_str(), peer_.FlagsToString().c_str()); // We were started remotely, just ACK back the local request - if (peer_.IsSink()) - btif_a2dp_on_started(peer_.PeerAddress(), nullptr, true); + if (peer_.IsSink()) btif_a2dp_on_started(peer_.PeerAddress(), nullptr); break; // FIXME -- use suspend = true always to work around issue with BTA AV -- 2.11.0