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

diff --git a/btif/include/btif_a2dp.h b/btif/include/btif_a2dp.h
index 7b7b2ca..49e4f9c 100644 (file)
--- 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 2efeec0..e2d6132 100644 (file)
--- 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 fea37f8..3659534 100644 (file)
--- 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