From c7a5e6038a1c01de1a2219659f999e91003af598 Mon Sep 17 00:00:00 2001 From: Pavlin Radoslavov Date: Thu, 10 May 2018 09:52:58 -0700 Subject: [PATCH] Explicitly restart audio processing when switching active device * Add internal APIs to restart A2DP session and use them when switching the A2DP active device: btif_a2dp_source_restart_session() and btif_a2dp_sink_restart_session() . * Removed internal A2DP Source APIs that shouldn't be APIs * Call immediately btif_a2dp_on_stopped() when processing BTIF_AV_STOP_STREAM_REQ_EVT event to stop sooner the audio datapath processing and report back to the Audio HAL call. This eliminates the need for waiting for the BTA_AV_STOP_EVT upcall event from the remote device. * Added extra log information when processing events in the BtifAv state machine. Bug: 78360113 Test: Manual: streaming A2DP, change active device, change codec setup. Change-Id: Icb356587af9cfaee7aea9f3f9bc95a0fe000fc52 --- btif/include/btif_a2dp_sink.h | 14 +++++--- btif/include/btif_a2dp_source.h | 19 ++++++----- btif/src/btif_a2dp_sink.cc | 26 ++++++++++++++ btif/src/btif_a2dp_source.cc | 43 +++++++++++++++++++++-- btif/src/btif_av.cc | 76 ++++++++++++++++------------------------- 5 files changed, 117 insertions(+), 61 deletions(-) diff --git a/btif/include/btif_a2dp_sink.h b/btif/include/btif_a2dp_sink.h index 8a3c10ca2..25e65f410 100644 --- a/btif/include/btif_a2dp_sink.h +++ b/btif/include/btif_a2dp_sink.h @@ -54,6 +54,16 @@ bool btif_a2dp_sink_startup(void); // btif_a2dp_sink_startup() to start the streaming session for |peer_address|. bool btif_a2dp_sink_start_session(const RawAddress& peer_address); +// Restart the A2DP Sink session. +// This function should be called by the BTIF state machine after +// btif_a2dp_sink_startup() to restart the streaming session. +// |old_peer_address| is the peer address of the old session. This address +// can be empty. +// |new_peer_address| is the peer address of the new session. This address +// cannot be empty. +bool btif_a2dp_sink_restart_session(const RawAddress& old_peer_address, + const RawAddress& new_peer_address); + // End the A2DP Sink session. // This function should be called by the BTIF state machine to end the // streaming session for |peer_address|. @@ -109,10 +119,6 @@ uint8_t btif_a2dp_sink_enqueue_buf(BT_HDR* p_buf); // information. void btif_a2dp_sink_debug_dump(int fd); -// Update the A2DP Sink related metrics. -// This function should be called before collecting the metrics. -void btif_a2dp_sink_update_metrics(void); - // Create a request to set the audio focus state for the audio track. // |state| is the new state value - see |btif_a2dp_sink_focus_state_t| // for valid values. diff --git a/btif/include/btif_a2dp_source.h b/btif/include/btif_a2dp_source.h index 1fc7d9a56..bb96e1ace 100644 --- a/btif/include/btif_a2dp_source.h +++ b/btif/include/btif_a2dp_source.h @@ -39,6 +39,16 @@ bool btif_a2dp_source_startup(void); // btif_a2dp_source_startup() to start the streaming session for |peer_address|. bool btif_a2dp_source_start_session(const RawAddress& peer_address); +// Restart the A2DP Source session. +// This function should be called by the BTIF state machine after +// btif_a2dp_source_startup() to restart the streaming session. +// |old_peer_address| is the peer address of the old session. This address +// can be empty. +// |new_peer_address| is the peer address of the new session. This address +// cannot be empty. +bool btif_a2dp_source_restart_session(const RawAddress& old_peer_address, + const RawAddress& new_peer_address); + // End the A2DP Source session. // This function should be called by the BTIF state machine to end the // streaming session for |peer_address|. @@ -64,11 +74,6 @@ bool btif_a2dp_source_media_task_is_shutting_down(void); // Return true if the A2DP Source module is streaming. bool btif_a2dp_source_is_streaming(void); -// Setup the A2DP Source codec, and prepare the encoder. -// The peer address is |peer_addr|. -// This function should be called prior to starting A2DP streaming. -void btif_a2dp_source_setup_codec(const RawAddress& peer_addr); - // Process a request to start the A2DP audio encoding task. void btif_a2dp_source_start_audio_req(void); @@ -117,8 +122,4 @@ BT_HDR* btif_a2dp_source_audio_readbuf(void); // information. void btif_a2dp_source_debug_dump(int fd); -// Update the A2DP Source related metrics. -// This function should be called before collecting the metrics. -void btif_a2dp_source_update_metrics(void); - #endif /* BTIF_A2DP_SOURCE_H */ diff --git a/btif/src/btif_a2dp_sink.cc b/btif/src/btif_a2dp_sink.cc index b80cdc740..c17b33a48 100644 --- a/btif/src/btif_a2dp_sink.cc +++ b/btif/src/btif_a2dp_sink.cc @@ -197,6 +197,32 @@ static void btif_a2dp_sink_start_session_delayed(UNUSED_ATTR void* context) { // Nothing to do } +bool btif_a2dp_sink_restart_session(const RawAddress& old_peer_address, + const RawAddress& new_peer_address) { + LOG_INFO(LOG_TAG, "%s: old_peer_address=%s new_peer_address=%s", __func__, + old_peer_address.ToString().c_str(), + new_peer_address.ToString().c_str()); + + CHECK(!new_peer_address.IsEmpty()); + + if (!old_peer_address.IsEmpty()) { + btif_a2dp_sink_end_session(old_peer_address); + } + + if (!bta_av_co_set_active_peer(new_peer_address)) { + LOG_ERROR(LOG_TAG, "%s: Cannot stream audio: cannot set active peer to %s", + __func__, new_peer_address.ToString().c_str()); + return false; + } + + if (old_peer_address.IsEmpty()) { + btif_a2dp_sink_startup(); + } + btif_a2dp_sink_start_session(new_peer_address); + + return true; +} + bool btif_a2dp_sink_end_session(const RawAddress& peer_address) { LOG_INFO(LOG_TAG, "%s: peer_address=%s", __func__, peer_address.ToString().c_str()); diff --git a/btif/src/btif_a2dp_source.cc b/btif/src/btif_a2dp_source.cc index f81834cbf..adaa04422 100644 --- a/btif/src/btif_a2dp_source.cc +++ b/btif/src/btif_a2dp_source.cc @@ -310,6 +310,10 @@ static void btif_a2dp_source_cleanup_delayed(void); static void btif_a2dp_source_audio_tx_start_event(void); static void btif_a2dp_source_audio_tx_stop_event(void); static void btif_a2dp_source_audio_tx_flush_event(void); +// Set up the A2DP Source codec, and prepare the encoder. +// The peer address is |peer_addr|. +// This function should be called prior to starting A2DP streaming. +static void btif_a2dp_source_setup_codec(const RawAddress& peer_addr); static void btif_a2dp_source_setup_codec_delayed( const RawAddress& peer_address); static void btif_a2dp_source_encoder_user_config_update_event( @@ -326,6 +330,9 @@ static bool btif_a2dp_source_enqueue_callback(BT_HDR* p_buf, size_t frames_n, static void log_tstamps_us(const char* comment, uint64_t timestamp_us); static void update_scheduling_stats(SchedulingStats* stats, uint64_t now_us, uint64_t expected_delta); +// Update the A2DP Source related metrics. +// This function should be called before collecting the metrics. +static void btif_a2dp_source_update_metrics(void); static void btm_read_rssi_cb(void* data); static void btm_read_failed_contact_counter_cb(void* data); static void btm_read_automatic_flush_timeout_cb(void* data); @@ -447,6 +454,38 @@ static void btif_a2dp_source_start_session_delayed( } } +bool btif_a2dp_source_restart_session(const RawAddress& old_peer_address, + const RawAddress& new_peer_address) { + bool is_streaming = alarm_is_scheduled(btif_a2dp_source_cb.media_alarm); + LOG_INFO(LOG_TAG, + "%s: old_peer_address=%s new_peer_address=%s is_streaming=%s", + __func__, old_peer_address.ToString().c_str(), + new_peer_address.ToString().c_str(), logbool(is_streaming).c_str()); + + CHECK(!new_peer_address.IsEmpty()); + + // Must stop first the audio streaming + if (is_streaming) { + btif_a2dp_source_stop_audio_req(); + } + + // If the old active peer was valid, end the old session. + // Otherwise, time to startup the A2DP Source processing. + if (!old_peer_address.IsEmpty()) { + btif_a2dp_source_end_session(old_peer_address); + } else { + btif_a2dp_source_startup(); + } + + // Start the session. + // If audio was streaming before, start audio streaming as well. + btif_a2dp_source_start_session(new_peer_address); + if (is_streaming) { + btif_a2dp_source_start_audio_req(); + } + return true; +} + bool btif_a2dp_source_end_session(const RawAddress& peer_address) { LOG_INFO(LOG_TAG, "%s: peer_address=%s", __func__, peer_address.ToString().c_str()); @@ -532,7 +571,7 @@ bool btif_a2dp_source_is_streaming(void) { return alarm_is_scheduled(btif_a2dp_source_cb.media_alarm); } -void btif_a2dp_source_setup_codec(const RawAddress& peer_address) { +static void btif_a2dp_source_setup_codec(const RawAddress& peer_address) { LOG_INFO(LOG_TAG, "%s: peer_address=%s", __func__, peer_address.ToString().c_str()); @@ -1173,7 +1212,7 @@ void btif_a2dp_source_debug_dump(int fd) { (unsigned long long)ave_time_us / 1000); } -void btif_a2dp_source_update_metrics(void) { +static void btif_a2dp_source_update_metrics(void) { const BtifMediaStats& stats = btif_a2dp_source_cb.stats; const SchedulingStats& enqueue_stats = stats.tx_queue_enqueue_stats; A2dpSessionMetrics metrics; diff --git a/btif/src/btif_av.cc b/btif/src/btif_av.cc index 5cd135d93..e4e6bac59 100644 --- a/btif/src/btif_av.cc +++ b/btif/src/btif_av.cc @@ -417,24 +417,11 @@ class BtifAvSource { peer->PeerAddress().ToString().c_str()); return false; } - if (!bta_av_co_set_active_peer(peer_address)) { - BTIF_TRACE_ERROR("%s: unable to set active peer to %s in BtaAvCo", - __func__, peer_address.ToString().c_str()); - return false; - } - // Start/restart the session - if (!active_peer_.IsEmpty()) { - btif_a2dp_source_end_session(active_peer_); + if (!btif_a2dp_source_restart_session(active_peer_, peer_address)) { + return false; } - bool should_startup = active_peer_.IsEmpty(); active_peer_ = peer_address; - if (should_startup) { - BTIF_TRACE_EVENT("%s: active peer is empty, startup the Audio source", - __func__); - btif_a2dp_source_startup(); - } - btif_a2dp_source_start_session(peer_address); return true; } @@ -567,24 +554,11 @@ class BtifAvSink { peer->PeerAddress().ToString().c_str()); return false; } - if (!bta_av_co_set_active_peer(peer_address)) { - BTIF_TRACE_ERROR("%s: unable to set active peer to %s in BtaAvCo", - __func__, peer_address.ToString().c_str()); - return false; - } - // Start/restart the session - if (!active_peer_.IsEmpty()) { - btif_a2dp_sink_end_session(active_peer_); + if (!btif_a2dp_sink_restart_session(active_peer_, peer_address)) { + return false; } - bool should_startup = active_peer_.IsEmpty(); active_peer_ = peer_address; - if (should_startup) { - BTIF_TRACE_EVENT("%s: active peer is empty, startup the Audio sink", - __func__); - btif_a2dp_sink_startup(); - } - btif_a2dp_sink_start_session(peer_address); return true; } @@ -1302,10 +1276,11 @@ void BtifAvStateMachine::StateIdle::OnExit() { } bool BtifAvStateMachine::StateIdle::ProcessEvent(uint32_t event, void* p_data) { - BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s", __PRETTY_FUNCTION__, - peer_.PeerAddress().ToString().c_str(), + BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s active_peer=%s", + __PRETTY_FUNCTION__, peer_.PeerAddress().ToString().c_str(), BtifAvEvent::EventName(event).c_str(), - peer_.FlagsToString().c_str()); + peer_.FlagsToString().c_str(), + logbool(peer_.IsActivePeer()).c_str()); switch (event) { case BTA_AV_ENABLE_EVT: @@ -1528,10 +1503,11 @@ void BtifAvStateMachine::StateOpening::OnExit() { bool BtifAvStateMachine::StateOpening::ProcessEvent(uint32_t event, void* p_data) { - BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s", __PRETTY_FUNCTION__, - peer_.PeerAddress().ToString().c_str(), + BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s active_peer=%s", + __PRETTY_FUNCTION__, peer_.PeerAddress().ToString().c_str(), BtifAvEvent::EventName(event).c_str(), - peer_.FlagsToString().c_str()); + peer_.FlagsToString().c_str(), + logbool(peer_.IsActivePeer()).c_str()); switch (event) { case BTIF_AV_STOP_STREAM_REQ_EVT: @@ -1724,10 +1700,11 @@ bool BtifAvStateMachine::StateOpened::ProcessEvent(uint32_t event, void* p_data) { tBTA_AV* p_av = (tBTA_AV*)p_data; - BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s", __PRETTY_FUNCTION__, - peer_.PeerAddress().ToString().c_str(), + BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s active_peer=%s", + __PRETTY_FUNCTION__, peer_.PeerAddress().ToString().c_str(), BtifAvEvent::EventName(event).c_str(), - peer_.FlagsToString().c_str()); + peer_.FlagsToString().c_str(), + logbool(peer_.IsActivePeer()).c_str()); if ((event == BTA_AV_REMOTE_CMD_EVT) && peer_.CheckFlags(BtifAvPeer::kFlagRemoteSuspend) && @@ -1910,10 +1887,11 @@ bool BtifAvStateMachine::StateStarted::ProcessEvent(uint32_t event, void* p_data) { tBTA_AV* p_av = (tBTA_AV*)p_data; - BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s", __PRETTY_FUNCTION__, - peer_.PeerAddress().ToString().c_str(), + BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s active_peer=%s", + __PRETTY_FUNCTION__, peer_.PeerAddress().ToString().c_str(), BtifAvEvent::EventName(event).c_str(), - peer_.FlagsToString().c_str()); + peer_.FlagsToString().c_str(), + logbool(peer_.IsActivePeer()).c_str()); switch (event) { case BTIF_AV_ACL_DISCONNECTED: @@ -1947,7 +1925,12 @@ bool BtifAvStateMachine::StateStarted::ProcessEvent(uint32_t event, if (peer_.IsSink()) { // Immediately stop transmission of frames while suspend is pending if (peer_.IsActivePeer()) { - btif_a2dp_source_set_tx_flush(true); + if (event == BTIF_AV_STOP_STREAM_REQ_EVT) { + btif_a2dp_on_stopped(nullptr); + } else { + // (event == BTIF_AV_SUSPEND_STREAM_REQ_EVT) + btif_a2dp_source_set_tx_flush(true); + } } } else if (peer_.IsSource()) { btif_a2dp_on_stopped(nullptr); @@ -2096,10 +2079,11 @@ void BtifAvStateMachine::StateClosing::OnExit() { bool BtifAvStateMachine::StateClosing::ProcessEvent(uint32_t event, void* p_data) { - BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s", __PRETTY_FUNCTION__, - peer_.PeerAddress().ToString().c_str(), + BTIF_TRACE_DEBUG("%s: Peer %s : event=%s flags=%s active_peer=%s", + __PRETTY_FUNCTION__, peer_.PeerAddress().ToString().c_str(), BtifAvEvent::EventName(event).c_str(), - peer_.FlagsToString().c_str()); + peer_.FlagsToString().c_str(), + logbool(peer_.IsActivePeer()).c_str()); switch (event) { case BTIF_AV_SUSPEND_STREAM_REQ_EVT: -- 2.11.0