Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 92f52ac8 authored by Cheney Ni's avatar Cheney Ni Committed by Jack He
Browse files

Using std::promise and std::future to block till A2DP device activated 

There is a new interface to replace UIPC with Blueototh Audio Hal v2 and
synchronization issues between BT Stack and Audio Hal was found when
activating a new A2DP device. Because the API to activate an A2DP device
was non-blocking, it was possilbe that there was a race condition when
BT Stack starting A2DP session and Audio Hal was opening A2DP output.
There was a chance that the output was opened before session started and
causing A2DP to have no sound.

This CL uses std::promise and std::future are able to achieve the
serialize of starting session and opening output for A2DP.

Bug: 111519504
Bug: 122505783
Test: A2DP reconnection and switching

Change-Id: I88c42ea1eb5f8def2345dbfaab26c6d1a91c54cc
parent 69d75b47

system/btif/include/btif_a2dp_sink.h

+5 −2
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ 


#include <inttypes.h>

#include <stdbool.h>

#include <future>



#include "bt_types.h"

#include "bta_av_api.h"
@@ -52,7 +53,8 @@ bool btif_a2dp_sink_startup(void); 
// Start the A2DP Sink session.

// This function should be called by the BTIF state machine after

// btif_a2dp_sink_startup() to start the streaming session for |peer_address|.

bool btif_a2dp_sink_start_session(const RawAddress& peer_address);

bool btif_a2dp_sink_start_session(const RawAddress& peer_address,

                                  std::promise<void> peer_ready_promise);



// Restart the A2DP Sink session.

// This function should be called by the BTIF state machine after
@@ -62,7 +64,8 @@ bool btif_a2dp_sink_start_session(const RawAddress& peer_address); 
// |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);

                                    const RawAddress& new_peer_address,

                                    std::promise<void> peer_ready_promise);



// End the A2DP Sink session.

// This function should be called by the BTIF state machine to end the

system/btif/include/btif_a2dp_source.h

+5 −2
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#define BTIF_A2DP_SOURCE_H



#include <stdbool.h>

#include <future>



#include "bta_av_api.h"
@@ -37,7 +38,8 @@ bool btif_a2dp_source_startup(void); 
// Start the A2DP Source session.

// This function should be called by the BTIF state machine after

// btif_a2dp_source_startup() to start the streaming session for |peer_address|.

bool btif_a2dp_source_start_session(const RawAddress& peer_address);

bool btif_a2dp_source_start_session(const RawAddress& peer_address,

                                    std::promise<void> peer_ready_promise);



// Restart the A2DP Source session.

// This function should be called by the BTIF state machine after
@@ -47,7 +49,8 @@ bool btif_a2dp_source_start_session(const RawAddress& peer_address); 
// |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);

                                      const RawAddress& new_peer_address,

                                      std::promise<void> peer_ready_promise);



// End the A2DP Source session.

// This function should be called by the BTIF state machine to end the

system/btif/src/btif_a2dp_sink.cc

+28 −16
Original line number Diff line number Diff line
@@ -129,7 +129,8 @@ static std::atomic<int> btif_a2dp_sink_state{BTIF_A2DP_SINK_STATE_OFF}; 


static void btif_a2dp_sink_init_delayed();

static void btif_a2dp_sink_startup_delayed();

static void btif_a2dp_sink_start_session_delayed();

static void btif_a2dp_sink_start_session_delayed(

    std::promise<void> peer_ready_promise);

static void btif_a2dp_sink_end_session_delayed();

static void btif_a2dp_sink_shutdown_delayed();

static void btif_a2dp_sink_cleanup_delayed();
@@ -211,25 +212,34 @@ static void btif_a2dp_sink_startup_delayed() { 
  // Nothing to do

}



bool btif_a2dp_sink_start_session(const RawAddress& peer_address) {

  LOG_INFO(LOG_TAG, "%s: peer_address=%s", __func__,

           peer_address.ToString().c_str());

  btif_a2dp_sink_cb.worker_thread.DoInThread(

      FROM_HERE, base::BindOnce(btif_a2dp_sink_start_session_delayed));

bool btif_a2dp_sink_start_session(const RawAddress& peer_address,

                                  std::promise<void> peer_ready_promise) {

  LOG(INFO) << __func__ << ": peer_address=" << peer_address;

  if (btif_a2dp_sink_cb.worker_thread.DoInThread(

          FROM_HERE, base::BindOnce(btif_a2dp_sink_start_session_delayed,

                                    std::move(peer_ready_promise)))) {

    return true;

  } else {

    // cannot set promise but triggers crash

    LOG(FATAL) << __func__ << ": peer_address=" << peer_address

               << " fails to context switch";

    return false;

  }

}



static void btif_a2dp_sink_start_session_delayed() {

  LOG_INFO(LOG_TAG, "%s", __func__);

static void btif_a2dp_sink_start_session_delayed(

    std::promise<void> peer_ready_promise) {

  LOG(INFO) << __func__;

  LockGuard lock(g_mutex);

  peer_ready_promise.set_value();

  // 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());

                                    const RawAddress& new_peer_address,

                                    std::promise<void> peer_ready_promise) {

  LOG(INFO) << __func__ << ": old_peer_address=" << old_peer_address

            << " new_peer_address=" << new_peer_address;



  CHECK(!new_peer_address.IsEmpty());
@@ -238,15 +248,17 @@ bool btif_a2dp_sink_restart_session(const RawAddress& 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());

    LOG(ERROR) << __func__

               << ": Cannot stream audio: cannot set active peer to "

               << new_peer_address;

    peer_ready_promise.set_value();

    return false;

  }



  if (old_peer_address.IsEmpty()) {

    btif_a2dp_sink_startup();

  }

  btif_a2dp_sink_start_session(new_peer_address);

  btif_a2dp_sink_start_session(new_peer_address, std::move(peer_ready_promise));



  return true;

}

system/btif/src/btif_a2dp_source.cc

+31 −22
Original line number Diff line number Diff line
@@ -229,7 +229,7 @@ static BtifA2dpSource btif_a2dp_source_cb; 
static void btif_a2dp_source_init_delayed(void);

static void btif_a2dp_source_startup_delayed(void);

static void btif_a2dp_source_start_session_delayed(

    const RawAddress& peer_address);

    const RawAddress& peer_address, std::promise<void> start_session_promise);

static void btif_a2dp_source_end_session_delayed(

    const RawAddress& peer_address);

static void btif_a2dp_source_shutdown_delayed(void);
@@ -359,24 +359,32 @@ static void btif_a2dp_source_startup_delayed() { 
  btif_a2dp_source_cb.SetState(BtifA2dpSource::kStateRunning);

}



bool btif_a2dp_source_start_session(const RawAddress& peer_address) {

  LOG_INFO(LOG_TAG, "%s: peer_address=%s state=%s", __func__,

           peer_address.ToString().c_str(),

           btif_a2dp_source_cb.StateStr().c_str());

bool btif_a2dp_source_start_session(const RawAddress& peer_address,

                                    std::promise<void> peer_ready_promise) {

  LOG(INFO) << __func__ << ": peer_address=" << peer_address

            << " state=" << btif_a2dp_source_cb.StateStr();

  btif_a2dp_source_setup_codec(peer_address);

  btif_a2dp_source_thread.DoInThread(

  if (btif_a2dp_source_thread.DoInThread(

          FROM_HERE,

      base::Bind(&btif_a2dp_source_start_session_delayed, peer_address));

          base::BindOnce(&btif_a2dp_source_start_session_delayed, peer_address,

                         std::move(peer_ready_promise)))) {

    return true;

  } else {

    // cannot set promise but triggers crash

    LOG(FATAL) << __func__ << ": peer_address=" << peer_address

               << " state=" << btif_a2dp_source_cb.StateStr()

               << " fails to context switch";

    return false;

  }

}



static void btif_a2dp_source_start_session_delayed(

    const RawAddress& peer_address) {

  LOG_INFO(LOG_TAG, "%s: peer_address=%s state=%s", __func__,

           peer_address.ToString().c_str(),

           btif_a2dp_source_cb.StateStr().c_str());

    const RawAddress& peer_address, std::promise<void> peer_ready_promise) {

  LOG(INFO) << __func__ << ": peer_address=" << peer_address

            << " state=" << btif_a2dp_source_cb.StateStr();

  if (btif_a2dp_source_cb.State() != BtifA2dpSource::kStateRunning) {

    LOG_ERROR(LOG_TAG, "%s: A2DP Source media task is not running", __func__);

    LOG(ERROR) << __func__ << ": A2DP Source media task is not running";

    peer_ready_promise.set_value();

    return;

  }

  if (btif_av_is_a2dp_offload_enabled()) {
@@ -385,17 +393,17 @@ static void btif_a2dp_source_start_session_delayed( 
    BluetoothMetricsLogger::GetInstance()->LogBluetoothSessionStart(

        bluetooth::common::CONNECTION_TECHNOLOGY_TYPE_BREDR, 0);

  }

  peer_ready_promise.set_value();

}



bool btif_a2dp_source_restart_session(const RawAddress& old_peer_address,

                                      const RawAddress& new_peer_address) {

                                      const RawAddress& new_peer_address,

                                      std::promise<void> peer_ready_promise) {

  bool is_streaming = btif_a2dp_source_cb.media_alarm.IsScheduled();

  LOG_INFO(LOG_TAG,

           "%s: old_peer_address=%s new_peer_address=%s is_streaming=%s "

           "state=%s",

           __func__, old_peer_address.ToString().c_str(),

           new_peer_address.ToString().c_str(), logbool(is_streaming).c_str(),

           btif_a2dp_source_cb.StateStr().c_str());

  LOG(INFO) << __func__ << ": old_peer_address=" << old_peer_address

            << " new_peer_address=" << new_peer_address

            << " is_streaming=" << logbool(is_streaming)

            << " state=" << btif_a2dp_source_cb.StateStr();



  CHECK(!new_peer_address.IsEmpty());
@@ -414,7 +422,8 @@ 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);

  btif_a2dp_source_start_session(new_peer_address,

                                 std::move(peer_ready_promise));

  if (is_streaming) {

    btif_a2dp_source_start_audio_req();

  }

system/btif/src/btif_av.cc

+88 −39
Original line number Diff line number Diff line
@@ -443,32 +443,39 @@ class BtifAvSource { 
   * reset the active peer

   * @return true on success, otherwise false

   */

  bool SetActivePeer(const RawAddress& peer_address) {

    LOG_INFO(LOG_TAG, "%s: peer: %s", __PRETTY_FUNCTION__,

             peer_address.ToString().c_str());

  bool SetActivePeer(const RawAddress& peer_address,

                     std::promise<void> peer_ready_promise) {

    LOG(INFO) << __PRETTY_FUNCTION__ << ": peer: " << peer_address;



    if (active_peer_ == peer_address) return true;  // Nothing has changed

    if (active_peer_ == peer_address) {

      peer_ready_promise.set_value();

      return true;  // Nothing has changed

    }

    if (peer_address.IsEmpty()) {

      BTIF_TRACE_EVENT("%s: peer address is empty, shutdown the Audio source",

                       __func__);

      if (!bta_av_co_set_active_peer(peer_address)) {

        BTIF_TRACE_WARNING("%s: unable to set active peer to empty in BtaAvCo",

                           __func__);

        LOG(WARNING) << __func__

                     << ": unable to set active peer to empty in BtaAvCo";

      }

      btif_a2dp_source_end_session(active_peer_);

      btif_a2dp_source_shutdown();

      active_peer_ = peer_address;

      peer_ready_promise.set_value();

      return true;

    }



    BtifAvPeer* peer = FindPeer(peer_address);

    if (peer != nullptr && !peer->IsConnected()) {

      BTIF_TRACE_ERROR("%s: Error setting %s as active Source peer", __func__,

                       peer->PeerAddress().ToString().c_str());

      LOG(ERROR) << __func__ << ": Error setting " << peer->PeerAddress()

                 << " as active Source peer";

      peer_ready_promise.set_value();

      return false;

    }



    if (!btif_a2dp_source_restart_session(active_peer_, peer_address)) {

    if (!btif_a2dp_source_restart_session(active_peer_, peer_address,

                                          std::move(peer_ready_promise))) {

      // cannot set promise but need to be handled within restart_session

      return false;

    }

    active_peer_ = peer_address;
@@ -483,7 +490,8 @@ class BtifAvSource { 
   */

  void UpdateCodecConfig(

      const RawAddress& peer_address,

      const std::vector<btav_a2dp_codec_config_t>& codec_preferences) {

      const std::vector<btav_a2dp_codec_config_t>& codec_preferences,

      std::promise<void> peer_ready_promise) {

    // Restart the session if the codec for the active peer is updated

    bool restart_session =

        ((active_peer_ == peer_address) && !active_peer_.IsEmpty());
@@ -497,7 +505,10 @@ class BtifAvSource { 
      btif_a2dp_source_encoder_user_config_update_req(peer_address, cp);

    }

    if (restart_session) {

      btif_a2dp_source_start_session(active_peer_);

      btif_a2dp_source_start_session(active_peer_,

                                     std::move(peer_ready_promise));

    } else {

      peer_ready_promise.set_value();

    }

  }
@@ -581,32 +592,39 @@ class BtifAvSink { 
   * reset the active peer

   * @return true on success, otherwise false

   */

  bool SetActivePeer(const RawAddress& peer_address) {

    LOG_INFO(LOG_TAG, "%s: peer: %s", __PRETTY_FUNCTION__,

             peer_address.ToString().c_str());

  bool SetActivePeer(const RawAddress& peer_address,

                     std::promise<void> peer_ready_promise) {

    LOG(INFO) << __PRETTY_FUNCTION__ << ": peer: " << peer_address;



    if (active_peer_ == peer_address) return true;  // Nothing has changed

    if (active_peer_ == peer_address) {

      peer_ready_promise.set_value();

      return true;  // Nothing has changed

    }

    if (peer_address.IsEmpty()) {

      BTIF_TRACE_EVENT("%s: peer address is empty, shutdown the Audio sink",

                       __func__);

      if (!bta_av_co_set_active_peer(peer_address)) {

        BTIF_TRACE_WARNING("%s: unable to set active peer to empty in BtaAvCo",

                           __func__);

        LOG(WARNING) << __func__

                     << ": unable to set active peer to empty in BtaAvCo";

      }

      btif_a2dp_sink_end_session(active_peer_);

      btif_a2dp_sink_shutdown();

      active_peer_ = peer_address;

      peer_ready_promise.set_value();

      return true;

    }



    BtifAvPeer* peer = FindPeer(peer_address);

    if (peer != nullptr && !peer->IsConnected()) {

      BTIF_TRACE_ERROR("%s: Error setting %s as active Sink peer", __func__,

                       peer->PeerAddress().ToString().c_str());

      LOG(ERROR) << __func__ << ": Error setting " << peer->PeerAddress()

                 << " as active Sink peer";

      peer_ready_promise.set_value();

      return false;

    }



    if (!btif_a2dp_sink_restart_session(active_peer_, peer_address)) {

    if (!btif_a2dp_sink_restart_session(active_peer_, peer_address,

                                        std::move(peer_ready_promise))) {

      // cannot set promise but need to be handled within restart_session

      return false;

    }

    active_peer_ = peer_address;
@@ -961,10 +979,12 @@ void BtifAvSource::Cleanup() { 


  btif_queue_cleanup(UUID_SERVCLASS_AUDIO_SOURCE);



  std::promise<void> peer_ready_promise;

  do_in_main_thread(

      FROM_HERE,

      base::Bind(base::IgnoreResult(&BtifAvSource::SetActivePeer),

                 base::Unretained(&btif_av_source), RawAddress::kEmpty));

      base::BindOnce(base::IgnoreResult(&BtifAvSource::SetActivePeer),

                     base::Unretained(&btif_av_source), RawAddress::kEmpty,

                     std::move(peer_ready_promise)));

  do_in_main_thread(FROM_HERE, base::Bind(&btif_a2dp_source_cleanup));



  btif_disable_service(BTA_A2DP_SOURCE_SERVICE_ID);
@@ -1146,10 +1166,12 @@ void BtifAvSink::Cleanup() { 


  btif_queue_cleanup(UUID_SERVCLASS_AUDIO_SINK);



  std::promise<void> peer_ready_promise;

  do_in_main_thread(

      FROM_HERE,

      base::Bind(base::IgnoreResult(&BtifAvSink::SetActivePeer),

                 base::Unretained(&btif_av_sink), RawAddress::kEmpty));

      base::BindOnce(base::IgnoreResult(&BtifAvSink::SetActivePeer),

                     base::Unretained(&btif_av_sink), RawAddress::kEmpty,

                     std::move(peer_ready_promise)));

  do_in_main_thread(FROM_HERE, base::Bind(&btif_a2dp_sink_cleanup));



  btif_disable_service(BTA_A2DP_SINK_SERVICE_ID);
@@ -1319,10 +1341,13 @@ void BtifAvStateMachine::StateIdle::OnEnter() { 
  // Reset the active peer if this was the active peer and

  // the Idle state was reentered

  if (peer_.IsActivePeer() && peer_.CanBeDeleted()) {

    std::promise<void> peer_ready_promise;

    if (peer_.IsSink()) {

      btif_av_source.SetActivePeer(RawAddress::kEmpty);

      btif_av_source.SetActivePeer(RawAddress::kEmpty,

                                   std::move(peer_ready_promise));

    } else if (peer_.IsSource()) {

      btif_av_sink.SetActivePeer(RawAddress::kEmpty);

      btif_av_sink.SetActivePeer(RawAddress::kEmpty,

                                 std::move(peer_ready_promise));

    }

  }
@@ -1747,7 +1772,9 @@ void BtifAvStateMachine::StateOpened::OnEnter() { 
  // For A2DP Source, the setting of the Active device is done by the

  // ActiveDeviceManager in Java.

  if (peer_.IsSource() && btif_av_sink.ActivePeer().IsEmpty()) {

    if (!btif_av_sink.SetActivePeer(peer_.PeerAddress())) {

    std::promise<void> peer_ready_promise;

    if (!btif_av_sink.SetActivePeer(peer_.PeerAddress(),

                                    std::move(peer_ready_promise))) {

      BTIF_TRACE_ERROR("%s: Error setting %s as active Source peer", __func__,

                       peer_.PeerAddress().ToString().c_str());

    }
@@ -2652,20 +2679,23 @@ static void set_source_silence_peer_int(const RawAddress& peer_address, 


// Set the active peer

static void set_active_peer_int(uint8_t peer_sep,

                                const RawAddress& peer_address) {

                                const RawAddress& peer_address,

                                std::promise<void> peer_ready_promise) {

  BTIF_TRACE_EVENT("%s: peer_sep=%s (%d) peer_address=%s", __func__,

                   (peer_sep == AVDT_TSEP_SRC) ? "Source" : "Sink", peer_sep,

                   peer_address.ToString().c_str());

  BtifAvPeer* peer = nullptr;

  if (peer_sep == AVDT_TSEP_SNK) {

    if (!btif_av_source.SetActivePeer(peer_address)) {

    if (!btif_av_source.SetActivePeer(peer_address,

                                      std::move(peer_ready_promise))) {

      BTIF_TRACE_ERROR("%s: Error setting %s as active Sink peer", __func__,

                       peer_address.ToString().c_str());

    }

    return;

  }

  if (peer_sep == AVDT_TSEP_SRC) {

    if (!btif_av_sink.SetActivePeer(peer_address)) {

    if (!btif_av_sink.SetActivePeer(peer_address,

                                    std::move(peer_ready_promise))) {

      BTIF_TRACE_ERROR("%s: Error setting %s as active Source peer", __func__,

                       peer_address.ToString().c_str());

    }
@@ -2676,6 +2706,7 @@ static void set_active_peer_int(uint8_t peer_sep, 
                   (peer_sep == AVDT_TSEP_SRC) ? "Source" : "Sink",

                   peer_address.ToString().c_str(),

                   (peer == nullptr) ? "found" : "connected");

  peer_ready_promise.set_value();

}



static bt_status_t src_connect_sink(const RawAddress& peer_address) {
@@ -2752,13 +2783,22 @@ static bt_status_t src_set_active_sink(const RawAddress& peer_address) { 
  BTIF_TRACE_EVENT("%s: Peer %s", __func__, peer_address.ToString().c_str());



  if (!btif_av_source.Enabled()) {

    BTIF_TRACE_WARNING("%s: BTIF AV Source is not enabled", __func__);

    LOG(WARNING) << __func__ << ": BTIF AV Source is not enabled";

    return BT_STATUS_NOT_READY;

  }



  return do_in_main_thread(FROM_HERE, base::Bind(&set_active_peer_int,

  std::promise<void> peer_ready_promise;

  std::future<void> peer_ready_future = peer_ready_promise.get_future();

  bt_status_t status = do_in_main_thread(

      FROM_HERE, base::BindOnce(&set_active_peer_int,

                                AVDT_TSEP_SNK,  // peer_sep

                                                 peer_address));

                                peer_address, std::move(peer_ready_promise)));

  if (status == BT_STATUS_SUCCESS) {

    peer_ready_future.wait();

  } else {

    LOG(WARNING) << __func__ << ": BTIF AV Source fails to change peer";

  }

  return status;

}



static bt_status_t codec_config_src(
@@ -2767,14 +2807,23 @@ static bt_status_t codec_config_src( 
  BTIF_TRACE_EVENT("%s", __func__);



  if (!btif_av_source.Enabled()) {

    BTIF_TRACE_WARNING("%s: BTIF AV Source is not enabled", __func__);

    LOG(WARNING) << __func__ << ": BTIF AV Source is not enabled";

    return BT_STATUS_NOT_READY;

  }



  return do_in_main_thread(

      FROM_HERE, base::Bind(&BtifAvSource::UpdateCodecConfig,

  std::promise<void> peer_ready_promise;

  std::future<void> peer_ready_future = peer_ready_promise.get_future();

  bt_status_t status = do_in_main_thread(

      FROM_HERE,

      base::BindOnce(&BtifAvSource::UpdateCodecConfig,

                     base::Unretained(&btif_av_source), peer_address,

                            codec_preferences));

                     codec_preferences, std::move(peer_ready_promise)));

  if (status == BT_STATUS_SUCCESS) {

    peer_ready_future.wait();

  } else {

    LOG(WARNING) << __func__ << ": BTIF AV Source fails to config codec";

  }

  return status;

}



static void cleanup_src(void) {