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

Commit 4fdca873 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

leaudio: Fix handling OnAudioResume 

Blocking this request is not necessary as we always
reply with Pending to the audio framework.
In addition, Bluetooth can either Cancel or Confirm Request imidiatelly
and in Confirm case, there was a race condintion, which leads
to non started session.

Bug: 150670922
Tag: #feature
Sponsor: jpawlowski@
Test: atest --host bluetooth_le_audio_test

Change-Id: I58dcf18e93296fac73485093e33355472d54a0d9
parent 73d00b63

system/audio_hal_interface/le_audio_software.cc

+18 −37
Original line number Diff line number Diff line
@@ -66,17 +66,9 @@ class LeAudioTransport { 
  BluetoothAudioCtrlAck StartRequest() {

    LOG(INFO) << __func__;



    /*

     * Set successful state as initial, it may be overwritten with cancelation

     * during on_resume handling

     */

    pending_start_request_state_ = BluetoothAudioCtrlAck::PENDING;



    if (stream_cb_.on_resume_(true)) {

      if (pending_start_request_state_ == BluetoothAudioCtrlAck::PENDING)

      is_pending_start_request_ = true;



      return pending_start_request_state_;

      return BluetoothAudioCtrlAck::PENDING;

    }



    return BluetoothAudioCtrlAck::FAILURE;
@@ -158,14 +150,8 @@ class LeAudioTransport { 
    pcm_config_.dataIntervalUs = data_interval;

  }



  bool SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

      BluetoothAudioCtrlAck state) {

    bool ret = is_pending_start_request_;

    is_pending_start_request_ = false;

    pending_start_request_state_ = state;



    return ret;

  }

  bool IsPendingStartStream(void) { return is_pending_start_request_; }

  void ClearPendingStartStream(void) { is_pending_start_request_ = false; }



 private:

  void (*flush_)(void);
@@ -175,7 +161,6 @@ class LeAudioTransport { 
  timespec data_position_;

  PcmParameters pcm_config_;

  bool is_pending_start_request_;

  BluetoothAudioCtrlAck pending_start_request_state_;

};



static void flush_sink() {
@@ -245,11 +230,8 @@ class LeAudioSinkTransport 
                                               channel_mode, data_interval);

  }



  bool SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

      BluetoothAudioCtrlAck state = BluetoothAudioCtrlAck::PENDING) {

    return transport_->SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

        state);

  }

  bool IsPendingStartStream(void) { return transport_->IsPendingStartStream(); }

  void ClearPendingStartStream(void) { transport_->ClearPendingStartStream(); }



 private:

  LeAudioTransport* transport_;
@@ -321,11 +303,8 @@ class LeAudioSourceTransport 
                                               channel_mode, data_interval);

  }



  bool SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

      BluetoothAudioCtrlAck state = BluetoothAudioCtrlAck::PENDING) {

    return transport_->SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

        state);

  }

  bool IsPendingStartStream(void) { return transport_->IsPendingStartStream(); }

  void ClearPendingStartStream(void) { transport_->ClearPendingStartStream(); }



 private:

  LeAudioTransport* transport_;
@@ -450,30 +429,31 @@ void LeAudioClientInterface::Sink::StartSession() { 


void LeAudioClientInterface::Sink::ConfirmStreamingRequest() {

  LOG(INFO) << __func__;

  if (!le_audio_sink->SetAudioCtrlAckStateAndResetPendingStartStreamFlag()) {

  if (!le_audio_sink->IsPendingStartStream()) {

    LOG(WARNING) << ", no pending start stream request";

    return;

  }



  le_audio_sink->ClearPendingStartStream();

  le_audio_sink_hal_clientinterface->StreamStarted(

      BluetoothAudioCtrlAck::SUCCESS_FINISHED);

}



void LeAudioClientInterface::Sink::CancelStreamingRequest() {

  LOG(INFO) << __func__;

  if (!le_audio_sink->SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

          BluetoothAudioCtrlAck::FAILURE)) {

  if (!le_audio_sink->IsPendingStartStream()) {

    LOG(WARNING) << ", no pending start stream request";

    return;

  }



  le_audio_sink->ClearPendingStartStream();

  le_audio_sink_hal_clientinterface->StreamStarted(

      BluetoothAudioCtrlAck::FAILURE);

}



void LeAudioClientInterface::Sink::StopSession() {

  LOG(INFO) << __func__ << " sink";

  le_audio_sink->SetAudioCtrlAckStateAndResetPendingStartStreamFlag();

  le_audio_sink->ClearPendingStartStream();

  le_audio_sink_hal_clientinterface->EndSession();

}
@@ -533,30 +513,31 @@ void LeAudioClientInterface::Source::StartSession() { 


void LeAudioClientInterface::Source::ConfirmStreamingRequest() {

  LOG(INFO) << __func__;

  if (!le_audio_source->SetAudioCtrlAckStateAndResetPendingStartStreamFlag()) {

  if (!le_audio_source->IsPendingStartStream()) {

    LOG(WARNING) << ", no pending start stream request";

    return;

  }



  le_audio_source->ClearPendingStartStream();

  le_audio_source_hal_clientinterface->StreamStarted(

      BluetoothAudioCtrlAck::SUCCESS_FINISHED);

}



void LeAudioClientInterface::Source::CancelStreamingRequest() {

  LOG(INFO) << __func__;

  if (!le_audio_source->SetAudioCtrlAckStateAndResetPendingStartStreamFlag(

          BluetoothAudioCtrlAck::FAILURE)) {

  if (!le_audio_source->IsPendingStartStream()) {

    LOG(WARNING) << ", no pending start stream request";

    return;

  }



  le_audio_source->ClearPendingStartStream();

  le_audio_source_hal_clientinterface->StreamStarted(

      BluetoothAudioCtrlAck::FAILURE);

}



void LeAudioClientInterface::Source::StopSession() {

  LOG(INFO) << __func__ << " source";

  le_audio_source->SetAudioCtrlAckStateAndResetPendingStartStreamFlag();

  le_audio_source->ClearPendingStartStream();

  le_audio_source_hal_clientinterface->EndSession();

}

system/bta/le_audio/client.cc

+2 −4
Original line number Diff line number Diff line
@@ -2920,9 +2920,8 @@ class LeAudioClientAudioSinkReceiverImpl 
    do_suspend_promise.set_value();

  }



  void OnAudioResume(std::promise<void> do_resume_promise) override {

  void OnAudioResume(void) override {

    if (instance) instance->OnAudioSinkResume();

    do_resume_promise.set_value();

  }



  void OnAudioMetadataUpdate(
@@ -2940,9 +2939,8 @@ class LeAudioClientAudioSourceReceiverImpl 
    if (instance) instance->OnAudioSourceSuspend();

    do_suspend_promise.set_value();

  }

  void OnAudioResume(std::promise<void> do_resume_promise) override {

  void OnAudioResume(void) override {

    if (instance) instance->OnAudioSourceResume();

    do_resume_promise.set_value();

  }

};

system/bta/le_audio/client_audio.cc

+21 −40
Original line number Diff line number Diff line
@@ -107,26 +107,16 @@ void stop_audio_ticks() { 
}



bool le_audio_sink_on_resume_req(bool start_media_task) {

  if (localAudioSinkReceiver != nullptr) {

    // Call OnAudioResume and block till it returns.

    std::promise<void> do_resume_promise;

    std::future<void> do_resume_future = do_resume_promise.get_future();

    bt_status_t status = do_in_main_thread(

        FROM_HERE,

        base::BindOnce(&LeAudioClientAudioSinkReceiver::OnAudioResume,

                       base::Unretained(localAudioSinkReceiver),

                       std::move(do_resume_promise)));

    if (status == BT_STATUS_SUCCESS) {

      do_resume_future.wait();

    } else {

      LOG(ERROR) << __func__

                 << ": LE_AUDIO_CTRL_CMD_START: do_in_main_thread err="

                 << status;

  if (localAudioSinkReceiver == nullptr) {

    LOG(ERROR) << __func__ << ": localAudioSinkReceiver is nullptr";

    return false;

  }

  } else {

  bt_status_t status = do_in_main_thread(

      FROM_HERE, base::BindOnce(&LeAudioClientAudioSinkReceiver::OnAudioResume,

                                base::Unretained(localAudioSinkReceiver)));

  if (status != BT_STATUS_SUCCESS) {

    LOG(ERROR) << __func__

               << ": LE_AUDIO_CTRL_CMD_START: audio sink receiver not started";

               << ": LE_AUDIO_CTRL_CMD_START: do_in_main_thread err=" << status;

    return false;

  }
@@ -134,27 +124,18 @@ bool le_audio_sink_on_resume_req(bool start_media_task) { 
}



bool le_audio_source_on_resume_req(bool start_media_task) {

  if (localAudioSourceReceiver != nullptr) {

    // Call OnAudioResume and block till it returns.

    std::promise<void> do_resume_promise;

    std::future<void> do_resume_future = do_resume_promise.get_future();

  if (localAudioSourceReceiver == nullptr) {

    LOG(ERROR) << __func__ << ": localAudioSourceReceiver is nullptr";

    return false;

  }



  bt_status_t status = do_in_main_thread(

      FROM_HERE,

      base::BindOnce(&LeAudioClientAudioSourceReceiver::OnAudioResume,

                       base::Unretained(localAudioSourceReceiver),

                       std::move(do_resume_promise)));

    if (status == BT_STATUS_SUCCESS) {

      do_resume_future.wait();

    } else {

                     base::Unretained(localAudioSourceReceiver)));

  if (status != BT_STATUS_SUCCESS) {

    LOG(ERROR) << __func__

                 << ": LE_AUDIO_CTRL_CMD_START: do_in_main_thread err="

                 << status;

      return false;

    }

  } else {

    LOG(ERROR)

        << __func__

        << ": LE_AUDIO_CTRL_CMD_START: audio source receiver not started";

               << ": LE_AUDIO_CTRL_CMD_START: do_in_main_thread err=" << status;

    return false;

  }

system/bta/le_audio/client_audio.h

+2 −2
Original line number Diff line number Diff line
@@ -26,7 +26,7 @@ class LeAudioClientAudioSinkReceiver { 
  virtual ~LeAudioClientAudioSinkReceiver() = default;

  virtual void OnAudioDataReady(const std::vector<uint8_t>& data) = 0;

  virtual void OnAudioSuspend(std::promise<void> do_suspend_promise) = 0;

  virtual void OnAudioResume(std::promise<void> do_resume_promise) = 0;

  virtual void OnAudioResume(void) = 0;

  virtual void OnAudioMetadataUpdate(

      std::promise<void> do_update_metadata_promise,

      const source_metadata_t& source_metadata) = 0;
@@ -35,7 +35,7 @@ class LeAudioClientAudioSourceReceiver { 
 public:

  virtual ~LeAudioClientAudioSourceReceiver() = default;

  virtual void OnAudioSuspend(std::promise<void> do_suspend_promise) = 0;

  virtual void OnAudioResume(std::promise<void> do_resume_promise) = 0;

  virtual void OnAudioResume(void) = 0;

};



/* Represents configuration of audio codec, as exchanged between le audio and

system/bta/le_audio/client_audio_test.cc

+5 −7
Original line number Diff line number Diff line
@@ -185,8 +185,7 @@ class MockLeAudioClientAudioSinkEventReceiver 
              (override));

  MOCK_METHOD((void), OnAudioSuspend, (std::promise<void> do_suspend_promise),

              (override));

  MOCK_METHOD((void), OnAudioResume, (std::promise<void> do_resume_promise),

              (override));

  MOCK_METHOD((void), OnAudioResume, (), (override));

  MOCK_METHOD((void), OnAudioMetadataUpdate,

              (std::promise<void> do_update_metadata_promise,

               const source_metadata_t& source_metadata),
@@ -198,8 +197,7 @@ class MockLeAudioClientAudioSourceEventReceiver 
 public:

  MOCK_METHOD((void), OnAudioSuspend, (std::promise<void> do_suspend_promise),

              (override));

  MOCK_METHOD((void), OnAudioResume, (std::promise<void> do_resume_promise),

              (override));

  MOCK_METHOD((void), OnAudioResume, (), (override));

};



class LeAudioClientAudioTest : public ::testing::Test {
@@ -419,7 +417,7 @@ TEST_F(LeAudioClientAudioTest, testLeAudioClientAudioSinkResume) { 
  /* Expect LeAudio registered event listener to get called when HAL calls the

   * client_audio's internal resume callback.

   */

  EXPECT_CALL(mock_hal_source_event_receiver_, OnAudioResume(_)).Times(1);

  EXPECT_CALL(mock_hal_source_event_receiver_, OnAudioResume()).Times(1);

  bool start_media_task = false;

  ASSERT_TRUE(hal_source_stream_cb.on_resume_(start_media_task));

}
@@ -473,7 +471,7 @@ TEST_F(LeAudioClientAudioTest, 
   * client_audio's internal resume callback.

   */

  ASSERT_NE(hal_sink_stream_cb.on_resume_, nullptr);

  EXPECT_CALL(mock_hal_sink_event_receiver_, OnAudioResume(_)).Times(1);

  EXPECT_CALL(mock_hal_sink_event_receiver_, OnAudioResume()).Times(1);

  resumed_ts = std::chrono::system_clock::now();

  bool start_media_task = true;

  ASSERT_TRUE(hal_sink_stream_cb.on_resume_(start_media_task));
@@ -513,7 +511,7 @@ TEST_F(LeAudioClientAudioTest, testLeAudioClientAudioSourceResume) { 
  /* Expect LeAudio registered event listener to get called when HAL calls the

   * client_audio's internal resume callback.

   */

  EXPECT_CALL(mock_hal_sink_event_receiver_, OnAudioResume(_)).Times(1);

  EXPECT_CALL(mock_hal_sink_event_receiver_, OnAudioResume()).Times(1);

  bool start_media_task = false;

  ASSERT_TRUE(hal_sink_stream_cb.on_resume_(start_media_task));

}