A2DP: Don't start or suspend the encoder for a non-active sink

While switching the A2DP active device, those AV_START or AV_SUSPEND
events might be from previous active device which was no longer active
now. We should keep it as suspended, but don't change the encoder and
audio provider state by these events.

This CL also simplify A2DP source suspend / stop callbacks.

Bug: 144670756
Test: Switch A2DP active device manually
Change-Id: I640e1e219306641beca1ff5ac758c2c95d925737

diff --git a/btif/src/btif_a2dp_source.cc b/btif/src/btif_a2dp_source.cc
index 6701888..72e3122 100644 (file)
--- a/btif/src/btif_a2dp_source.cc
+++ b/btif/src/btif_a2dp_source.cc
@@ -434,12 +434,10 @@ bool btif_a2dp_source_restart_session(const RawAddress& old_peer_address,
   }
 
   // Start the session.
-  // If audio was streaming before, start audio streaming as well.
   btif_a2dp_source_start_session(new_peer_address,
                                  std::move(peer_ready_promise));
-  if (is_streaming) {
-    btif_a2dp_source_start_audio_req();
-  }
+  // If audio was streaming before, DON'T start audio streaming, but leave the
+  // control to the audio HAL.
   return true;
 }
 
@@ -707,35 +705,33 @@ void btif_a2dp_source_on_stopped(tBTA_AV_SUSPEND* p_av_suspend) {
 
   if (btif_a2dp_source_cb.State() == BtifA2dpSource::kStateOff) return;
 
-  /* allow using this api for other than suspend */
-  if (p_av_suspend != nullptr) {
-    if (p_av_suspend->status != BTA_AV_SUCCESS) {
-      LOG_ERROR(LOG_TAG, "%s: A2DP stop request failed: status=%d", __func__,
-                p_av_suspend->status);
-      if (p_av_suspend->initiator) {
-        LOG_WARN(LOG_TAG, "%s: A2DP stop request failed: status=%d", __func__,
-                 p_av_suspend->status);
-        if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) {
-          bluetooth::audio::a2dp::ack_stream_suspended(A2DP_CTRL_ACK_FAILURE);
-        } else {
-          btif_a2dp_command_ack(A2DP_CTRL_ACK_FAILURE);
-        }
+  // allow using this API for other (acknowledgement and stopping media task)
+  // than suspend
+  if (p_av_suspend != nullptr && p_av_suspend->status != BTA_AV_SUCCESS) {
+    LOG_ERROR(LOG_TAG, "%s: A2DP stop failed: status=%d, initiator=%s",
+              __func__, p_av_suspend->status,
+              (p_av_suspend->initiator ? "true" : "false"));
+    if (p_av_suspend->initiator) {
+      if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) {
+        bluetooth::audio::a2dp::ack_stream_suspended(A2DP_CTRL_ACK_FAILURE);
+      } else {
+        btif_a2dp_command_ack(A2DP_CTRL_ACK_FAILURE);
       }
-      return;
     }
-  }
-  if (btif_av_is_a2dp_offload_enabled()) {
+  } else if (btif_av_is_a2dp_offload_enabled()) {
     bluetooth::audio::a2dp::ack_stream_suspended(A2DP_CTRL_ACK_SUCCESS);
     return;
   }
-  /* ensure tx frames are immediately suspended */
-  btif_a2dp_source_cb.tx_flush = true;
 
-  /* request to stop media task */
+  // ensure tx frames are immediately suspended
+  btif_a2dp_source_cb.tx_flush = true;
+  // ensure tx frames are immediately flushed
   btif_a2dp_source_audio_tx_flush_req();
+
+  // request to stop media task
   btif_a2dp_source_stop_audio_req();
 
-  /* once stream is fully stopped we will ack back */
+  // once software stream is fully stopped we will ack back
 }
 
 void btif_a2dp_source_on_suspended(tBTA_AV_SUSPEND* p_av_suspend) {
@@ -744,29 +740,32 @@ void btif_a2dp_source_on_suspended(tBTA_AV_SUSPEND* p_av_suspend) {
 
   if (btif_a2dp_source_cb.State() == BtifA2dpSource::kStateOff) return;
 
-  /* check for status failures */
+  CHECK(p_av_suspend != nullptr) << "Suspend result could not be nullptr";
+
+  // check for status failures
   if (p_av_suspend->status != BTA_AV_SUCCESS) {
+    LOG_WARN(LOG_TAG, "%s: A2DP suspend failed: status=%d, initiator=%s",
+             __func__, p_av_suspend->status,
+             (p_av_suspend->initiator ? "true" : "false"));
     if (p_av_suspend->initiator) {
-      LOG_WARN(LOG_TAG, "%s: A2DP suspend request failed: status=%d", __func__,
-               p_av_suspend->status);
       if (bluetooth::audio::a2dp::is_hal_2_0_enabled()) {
         bluetooth::audio::a2dp::ack_stream_suspended(A2DP_CTRL_ACK_FAILURE);
       } else {
         btif_a2dp_command_ack(A2DP_CTRL_ACK_FAILURE);
       }
     }
-  }
-  if (btif_av_is_a2dp_offload_enabled()) {
+  } else if (btif_av_is_a2dp_offload_enabled()) {
     bluetooth::audio::a2dp::ack_stream_suspended(A2DP_CTRL_ACK_SUCCESS);
     return;
   }
-  /* once stream is fully stopped we will ack back */
 
-  /* ensure tx frames are immediately flushed */
+  // ensure tx frames are immediately suspended
   btif_a2dp_source_cb.tx_flush = true;
 
-  /* stop timer tick */
+  // stop timer tick
   btif_a2dp_source_stop_audio_req();
+
+  // once software stream is fully stopped we will ack back
 }
 
 /* when true media task discards any tx frames */
@@ -853,7 +852,7 @@ static void btif_a2dp_source_audio_tx_stop_event(void) {
     UIPC_Close(*a2dp_uipc, UIPC_CH_ID_AV_AUDIO);
 
     /*
-     * Try to send acknowldegment once the media stream is
+     * Try to send acknowledgement once the media stream is
      * stopped. This will make sure that the A2DP HAL layer is
      * un-blocked on wait for acknowledgment for the sent command.
      * This resolves a corner cases AVDTP SUSPEND collision

diff --git a/btif/src/btif_av.cc b/btif/src/btif_av.cc
index 813cda3..b91bc42 100644 (file)
--- a/btif/src/btif_av.cc
+++ b/btif/src/btif_av.cc
@@ -1830,17 +1830,26 @@ bool BtifAvStateMachine::StateOpened::ProcessEvent(uint32_t event,
       // If remote tries to start A2DP when DUT is A2DP Source, then Suspend.
       // If A2DP is Sink and call is active, then disconnect the AVDTP channel.
       bool should_suspend = false;
-      if (peer_.IsSink() && !peer_.CheckFlags(BtifAvPeer::kFlagPendingStart |
-                                              BtifAvPeer::kFlagRemoteSuspend)) {
-        LOG(WARNING) << __PRETTY_FUNCTION__ << ": Peer " << peer_.PeerAddress()
-                     << " : trigger Suspend as remote initiated";
-        should_suspend = true;
-      }
+      if (peer_.IsSink()) {
+        if (!peer_.CheckFlags(BtifAvPeer::kFlagPendingStart |
+                              BtifAvPeer::kFlagRemoteSuspend)) {
+          LOG(WARNING) << __PRETTY_FUNCTION__ << ": Peer "
+                       << peer_.PeerAddress()
+                       << " : trigger Suspend as remote initiated";
+          should_suspend = true;
+        } else if (!peer_.IsActivePeer()) {
+          LOG(WARNING) << __PRETTY_FUNCTION__ << ": Peer "
+                       << peer_.PeerAddress()
+                       << " : trigger Suspend as non-active";
+          should_suspend = true;
+        }
 
-      // 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);
+        // If peer is A2DP Source, do ACK commands to audio HAL and start media
+        // task
+        if (btif_a2dp_on_started(peer_.PeerAddress(), &p_av->start)) {
+          // Only clear pending flag after acknowledgement
+          peer_.ClearFlags(BtifAvPeer::kFlagPendingStart);
+        }
       }
 
       // Remain in Open state if status failed
@@ -1875,17 +1884,20 @@ bool BtifAvStateMachine::StateOpened::ProcessEvent(uint32_t event,
 
     case BTA_AV_CLOSE_EVT:
       // AVDTP link is closed
-      if (peer_.IsActivePeer()) {
-        btif_a2dp_on_stopped(nullptr);
-      }
-
       // Change state to Idle, send acknowledgement if start is pending
       if (peer_.CheckFlags(BtifAvPeer::kFlagPendingStart)) {
         BTIF_TRACE_WARNING("%s: Peer %s : failed pending start request",
                            __PRETTY_FUNCTION__,
                            peer_.PeerAddress().ToString().c_str());
-        btif_a2dp_command_ack(A2DP_CTRL_ACK_FAILURE);
+        tBTA_AV_START av_start = {.chnl = p_av->close.chnl,
+                                  .hndl = p_av->close.hndl,
+                                  .status = BTA_AV_FAIL_STREAM,
+                                  .initiator = true,
+                                  .suspending = true};
+        btif_a2dp_on_started(peer_.PeerAddress(), &av_start);
         // Pending start flag will be cleared when exit current state
+      } else if (peer_.IsActivePeer()) {
+        btif_a2dp_on_stopped(nullptr);
       }
 
       // Inform the application that we are disconnected
@@ -2020,15 +2032,14 @@ bool BtifAvStateMachine::StateStarted::ProcessEvent(uint32_t event,
       // always overrides.
       peer_.ClearFlags(BtifAvPeer::kFlagRemoteSuspend);
 
-      if (peer_.IsSink()) {
+      if (peer_.IsSink() &&
+          (peer_.IsActivePeer() || !btif_av_stream_started_ready())) {
         // Immediately stop transmission of frames while suspend is pending
-        if (peer_.IsActivePeer()) {
-          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);
-          }
+        if (event == BTIF_AV_STOP_STREAM_REQ_EVT) {
+          btif_a2dp_on_stopped(nullptr);
+        } else {
+          // ensure tx frames are immediately suspended
+          btif_a2dp_source_set_tx_flush(true);
         }
       } else if (peer_.IsSource()) {
         btif_a2dp_on_stopped(nullptr);
@@ -2063,8 +2074,10 @@ bool BtifAvStateMachine::StateStarted::ProcessEvent(uint32_t event,
                BtifAvEvent::EventName(event).c_str(), p_av->suspend.status,
                p_av->suspend.initiator, peer_.FlagsToString().c_str());
 
-      // A2DP suspended, stop A2DP encoder/decoder until resumed
-      btif_a2dp_on_suspended(&p_av->suspend);
+      // A2DP suspended, stop A2DP encoder / decoder until resumed
+      if (peer_.IsActivePeer() || !btif_av_stream_started_ready()) {
+        btif_a2dp_on_suspended(&p_av->suspend);
+      }
 
       // If not successful, remain in current state
       if (p_av->suspend.status != BTA_AV_SUCCESS) {
@@ -2106,7 +2119,11 @@ bool BtifAvStateMachine::StateStarted::ProcessEvent(uint32_t event,
       peer_.SetFlags(BtifAvPeer::kFlagPendingStop);
       peer_.ClearFlags(BtifAvPeer::kFlagLocalSuspendPending);
 
-      btif_a2dp_on_stopped(&p_av->suspend);
+      // Don't change the encoder and audio provider state by a non-active peer
+      // since they are shared between peers
+      if (peer_.IsActivePeer() || !btif_av_stream_started_ready()) {
+        btif_a2dp_on_stopped(&p_av->suspend);
+      }
 
       btif_report_audio_state(peer_.PeerAddress(), BTAV_AUDIO_STATE_STOPPED);