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

Commit 124fb75a authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

leaudio: Fix possible race on start stream request from HAL 

Bug: 322106376
Bug: 325181889
Test: mmm packages/modules/Bluetooth
Test: manual streaming regression tests

Change-Id: I079c9081dd165c77c7a1547c274f4c7c1d7bbf05
parent 9bc473a3

system/audio_hal_interface/aidl/le_audio_software_aidl.cc

+72 −0
Original line number Diff line number Diff line
@@ -108,6 +108,43 @@ BluetoothAudioCtrlAck LeAudioTransport::StartRequest(bool is_low_latency) { 
  return BluetoothAudioCtrlAck::FAILURE;

}



BluetoothAudioCtrlAck LeAudioTransport::StartRequestV2(bool is_low_latency) {

  // Check if operation is pending already

  if (GetStartRequestState() == StartRequestState::PENDING_AFTER_RESUME) {

    LOG_INFO("Start request is already pending. Ignore the request");

    return BluetoothAudioCtrlAck::PENDING;

  }



  SetStartRequestState(StartRequestState::PENDING_BEFORE_RESUME);

  if (stream_cb_.on_resume_(true)) {

    std::lock_guard<std::mutex> guard(start_request_state_mutex_);



    switch (start_request_state_) {

      case StartRequestState::CONFIRMED:

        LOG_INFO("Start completed.");

        SetStartRequestState(StartRequestState::IDLE);

        return BluetoothAudioCtrlAck::SUCCESS_FINISHED;

      case StartRequestState::CANCELED:

        LOG_INFO("Start request failed.");

        SetStartRequestState(StartRequestState::IDLE);

        return BluetoothAudioCtrlAck::FAILURE;

      case StartRequestState::PENDING_BEFORE_RESUME:

        LOG_INFO("Start pending.");

        SetStartRequestState(StartRequestState::PENDING_AFTER_RESUME);

        return BluetoothAudioCtrlAck::PENDING;

      default:

        SetStartRequestState(StartRequestState::IDLE);

        LOG_ERROR("Unexpected state %d",

                  static_cast<int>(start_request_state_.load()));

        return BluetoothAudioCtrlAck::FAILURE;

    }

  }



  SetStartRequestState(StartRequestState::IDLE);

  LOG_INFO("On resume failed.");

  return BluetoothAudioCtrlAck::FAILURE;

}



BluetoothAudioCtrlAck LeAudioTransport::SuspendRequest() {

  LOG(INFO) << __func__;

  if (stream_cb_.on_suspend_()) {
@@ -245,6 +282,23 @@ LeAudioTransport::LeAudioGetBroadcastConfig() { 
  return broadcast_config_;

}



bool LeAudioTransport::IsRequestCompletedAfterUpdate(

    const std::function<std::pair<StartRequestState, bool>(StartRequestState)>&

        lambda) {

  std::lock_guard<std::mutex> guard(start_request_state_mutex_);

  auto result = lambda(start_request_state_);

  auto new_state = std::get<0>(result);

  if (new_state != start_request_state_) {

    start_request_state_ = new_state;

  }



  auto ret = std::get<1>(result);

  LOG_VERBOSE(" new state: %d, return %s", (int)(start_request_state_.load()),

              ret ? "true" : "false");



  return ret;

}



StartRequestState LeAudioTransport::GetStartRequestState(void) {

  return start_request_state_;

}
@@ -290,6 +344,9 @@ LeAudioSinkTransport::LeAudioSinkTransport(SessionType session_type, 
LeAudioSinkTransport::~LeAudioSinkTransport() { delete transport_; }



BluetoothAudioCtrlAck LeAudioSinkTransport::StartRequest(bool is_low_latency) {

  if (IS_FLAG_ENABLED(leaudio_start_stream_race_fix)) {

    return transport_->StartRequestV2(is_low_latency);

  }

  return transport_->StartRequest(is_low_latency);

}
@@ -353,6 +410,12 @@ LeAudioSinkTransport::LeAudioGetBroadcastConfig() { 
  return transport_->LeAudioGetBroadcastConfig();

}



bool LeAudioSinkTransport::IsRequestCompletedAfterUpdate(

    const std::function<std::pair<StartRequestState, bool>(StartRequestState)>&

        lambda) {

  return transport_->IsRequestCompletedAfterUpdate(lambda);

}



StartRequestState LeAudioSinkTransport::GetStartRequestState(void) {

  return transport_->GetStartRequestState();

}
@@ -380,6 +443,9 @@ LeAudioSourceTransport::~LeAudioSourceTransport() { delete transport_; } 


BluetoothAudioCtrlAck LeAudioSourceTransport::StartRequest(

    bool is_low_latency) {

  if (IS_FLAG_ENABLED(leaudio_start_stream_race_fix)) {

    return transport_->StartRequestV2(is_low_latency);

  }

  return transport_->StartRequest(is_low_latency);

}
@@ -434,6 +500,12 @@ void LeAudioSourceTransport::LeAudioSetSelectedHalPcmConfig( 
                                             channels_count, data_interval);

}



bool LeAudioSourceTransport::IsRequestCompletedAfterUpdate(

    const std::function<std::pair<StartRequestState, bool>(StartRequestState)>&

        lambda) {

  return transport_->IsRequestCompletedAfterUpdate(lambda);

}



StartRequestState LeAudioSourceTransport::GetStartRequestState(void) {

  return transport_->GetStartRequestState();

}

system/audio_hal_interface/aidl/le_audio_software_aidl.h

+14 −0
Original line number Diff line number Diff line
@@ -71,6 +71,7 @@ class LeAudioTransport { 
                   PcmConfiguration pcm_config);



  BluetoothAudioCtrlAck StartRequest(bool is_low_latency);

  BluetoothAudioCtrlAck StartRequestV2(bool is_low_latency);



  BluetoothAudioCtrlAck SuspendRequest();
@@ -103,6 +104,9 @@ class LeAudioTransport { 


  const LeAudioBroadcastConfiguration& LeAudioGetBroadcastConfig();



  bool IsRequestCompletedAfterUpdate(

      const std::function<

          std::pair<StartRequestState, bool>(StartRequestState)>& lambda);

  StartRequestState GetStartRequestState(void);

  void ClearStartRequestState(void);

  void SetStartRequestState(StartRequestState state);
@@ -115,6 +119,7 @@ class LeAudioTransport { 
  timespec data_position_;

  PcmConfiguration pcm_config_;

  LeAudioBroadcastConfiguration broadcast_config_;

  mutable std::mutex start_request_state_mutex_;

  std::atomic<StartRequestState> start_request_state_;

  DsaMode dsa_mode_;

};
@@ -128,6 +133,7 @@ class LeAudioSinkTransport 
  ~LeAudioSinkTransport();



  BluetoothAudioCtrlAck StartRequest(bool is_low_latency) override;

  BluetoothAudioCtrlAck StartRequestV2(bool is_low_latency);



  BluetoothAudioCtrlAck SuspendRequest() override;
@@ -161,6 +167,9 @@ class LeAudioSinkTransport 


  const LeAudioBroadcastConfiguration& LeAudioGetBroadcastConfig();



  bool IsRequestCompletedAfterUpdate(

      const std::function<

          std::pair<StartRequestState, bool>(StartRequestState)>& lambda);

  StartRequestState GetStartRequestState(void);

  void ClearStartRequestState(void);

  void SetStartRequestState(StartRequestState state);
@@ -184,6 +193,7 @@ class LeAudioSourceTransport 
  ~LeAudioSourceTransport();



  BluetoothAudioCtrlAck StartRequest(bool is_low_latency) override;

  BluetoothAudioCtrlAck StartRequestV2(bool is_low_latency);



  BluetoothAudioCtrlAck SuspendRequest() override;
@@ -212,6 +222,10 @@ class LeAudioSourceTransport 
                                      uint8_t channels_count,

                                      uint32_t data_interval);



  bool IsRequestCompletedAfterUpdate(

      const std::function<

          std::pair<StartRequestState, bool>(StartRequestState)>& lambda);



  StartRequestState GetStartRequestState(void);

  void ClearStartRequestState(void);

  void SetStartRequestState(StartRequestState state);

system/audio_hal_interface/hidl/le_audio_software_hidl.cc

+59 −0
Original line number Diff line number Diff line
@@ -134,6 +134,32 @@ BluetoothAudioCtrlAck LeAudioTransport::StartRequest() { 
  return BluetoothAudioCtrlAck::FAILURE;

}



BluetoothAudioCtrlAck LeAudioTransport::StartRequestV2() {

  SetStartRequestState(StartRequestState::PENDING_BEFORE_RESUME);

  if (stream_cb_.on_resume_(true)) {

    std::lock_guard<std::mutex> guard(start_request_state_mutex_);

    if (start_request_state_ == StartRequestState::CONFIRMED) {

      LOG_INFO("Start completed.");

      SetStartRequestState(StartRequestState::IDLE);

      return BluetoothAudioCtrlAck::SUCCESS_FINISHED;

    }



    if (start_request_state_ == StartRequestState::CANCELED) {

      LOG_INFO("Start request failed.");

      SetStartRequestState(StartRequestState::IDLE);

      return BluetoothAudioCtrlAck::FAILURE;

    }



    LOG_INFO("Start pending.");

    SetStartRequestState(StartRequestState::PENDING_AFTER_RESUME);

    return BluetoothAudioCtrlAck::PENDING;

  }



  LOG_ERROR("Start request failed.");

  SetStartRequestState(StartRequestState::IDLE);

  return BluetoothAudioCtrlAck::FAILURE;

}



BluetoothAudioCtrlAck LeAudioTransport::SuspendRequest() {

  LOG(INFO) << __func__;

  if (stream_cb_.on_suspend_()) {
@@ -227,6 +253,22 @@ void LeAudioTransport::LeAudioSetSelectedHalPcmConfig(uint32_t sample_rate_hz, 
  pcm_config_.dataIntervalUs = data_interval;

}



bool LeAudioTransport::IsRequestCompletedAfterUpdate(

    const std::function<std::pair<StartRequestState, bool>(StartRequestState)>&

        lambda) {

  std::lock_guard<std::mutex> guard(start_request_state_mutex_);

  auto result = lambda(start_request_state_);

  auto new_state = std::get<0>(result);

  if (new_state != start_request_state_) {

    start_request_state_ = new_state;

  }



  auto ret = std::get<1>(result);

  LOG_VERBOSE("new state: %d, return: %s",

              static_cast<int>(start_request_state_.load()), ret);

  return ret;

}



StartRequestState LeAudioTransport::GetStartRequestState(void) {

  return start_request_state_;

}
@@ -255,6 +297,9 @@ LeAudioSinkTransport::LeAudioSinkTransport(SessionType_2_1 session_type, 
LeAudioSinkTransport::~LeAudioSinkTransport() { delete transport_; }



BluetoothAudioCtrlAck LeAudioSinkTransport::StartRequest() {

  if (IS_FLAG_ENABLED(leaudio_start_stream_race_fix)) {

    return transport_->StartRequestV2();

  }

  return transport_->StartRequest();

}
@@ -299,6 +344,12 @@ void LeAudioSinkTransport::LeAudioSetSelectedHalPcmConfig( 
                                             channels_count, data_interval);

}



bool LeAudioSinkTransport::IsRequestCompletedAfterUpdate(

    const std::function<std::pair<StartRequestState, bool>(StartRequestState)>&

        lambda) {

  return transport_->IsRequestCompletedAfterUpdate(lambda);

}



StartRequestState LeAudioSinkTransport::GetStartRequestState(void) {

  return transport_->GetStartRequestState();

}
@@ -327,6 +378,9 @@ LeAudioSourceTransport::LeAudioSourceTransport(SessionType_2_1 session_type, 
LeAudioSourceTransport::~LeAudioSourceTransport() { delete transport_; }



BluetoothAudioCtrlAck LeAudioSourceTransport::StartRequest() {

  if (IS_FLAG_ENABLED(leaudio_start_stream_race_fix)) {

    return transport_->StartRequestV2();

  }

  return transport_->StartRequest();

}
@@ -371,6 +425,11 @@ void LeAudioSourceTransport::LeAudioSetSelectedHalPcmConfig( 
                                             channels_count, data_interval);

}



bool LeAudioSourceTransport::IsRequestCompletedAfterUpdate(

    const std::function<std::pair<StartRequestState, bool>(StartRequestState)>&

        lambda) {

  return transport_->IsRequestCompletedAfterUpdate(lambda);

}

StartRequestState LeAudioSourceTransport::GetStartRequestState(void) {

  return transport_->GetStartRequestState();

}

system/audio_hal_interface/hidl/le_audio_software_hidl.h

+14 −0
Original line number Diff line number Diff line
@@ -60,6 +60,7 @@ class LeAudioTransport { 
                   PcmParameters pcm_config);



  BluetoothAudioCtrlAck StartRequest();

  BluetoothAudioCtrlAck StartRequestV2();



  BluetoothAudioCtrlAck SuspendRequest();
@@ -83,6 +84,10 @@ class LeAudioTransport { 
                                      uint8_t channels_count,

                                      uint32_t data_interval);



  bool IsRequestCompletedAfterUpdate(

      const std::function<

          std::pair<StartRequestState, bool>(StartRequestState)>& lambda);



  StartRequestState GetStartRequestState(void);

  void ClearStartRequestState(void);

  void SetStartRequestState(StartRequestState state);
@@ -94,6 +99,7 @@ class LeAudioTransport { 
  uint64_t total_bytes_processed_;

  timespec data_position_;

  PcmParameters pcm_config_;

  mutable std::mutex start_request_state_mutex_;

  std::atomic<StartRequestState> start_request_state_;

};
@@ -106,6 +112,7 @@ class LeAudioSinkTransport 
  ~LeAudioSinkTransport();



  BluetoothAudioCtrlAck StartRequest() override;

  BluetoothAudioCtrlAck StartRequestV2();



  BluetoothAudioCtrlAck SuspendRequest() override;
@@ -129,6 +136,10 @@ class LeAudioSinkTransport 
                                      uint8_t channels_count,

                                      uint32_t data_interval);



  bool IsRequestCompletedAfterUpdate(

      const std::function<

          std::pair<StartRequestState, bool>(StartRequestState)>& lambda);



  StartRequestState GetStartRequestState(void);

  void ClearStartRequestState(void);

  void SetStartRequestState(StartRequestState state);
@@ -172,6 +183,9 @@ class LeAudioSourceTransport 
                                      uint8_t channels_count,

                                      uint32_t data_interval);



  bool IsRequestCompletedAfterUpdate(

      const std::function<

          std::pair<StartRequestState, bool>(StartRequestState)>& lambda);

  StartRequestState GetStartRequestState(void);

  void ClearStartRequestState(void);

  void SetStartRequestState(StartRequestState state);

system/audio_hal_interface/le_audio_software.cc

+142 −0
Original line number Diff line number Diff line
@@ -262,6 +262,44 @@ void LeAudioClientInterface::Sink::ConfirmStreamingRequest() { 
  }

}



void LeAudioClientInterface::Sink::ConfirmStreamingRequestV2() {

  auto lambda = [&](StartRequestState currect_start_request_state)

      -> std::pair<StartRequestState, bool> {

    switch (currect_start_request_state) {

      case StartRequestState::IDLE:

        LOG_WARN(", no pending start stream request");

        return std::make_pair(StartRequestState::IDLE, false);

      case StartRequestState::PENDING_BEFORE_RESUME:

        LOG_INFO("Response before sending PENDING to audio HAL");

        return std::make_pair(StartRequestState::CONFIRMED, false);

      case StartRequestState::PENDING_AFTER_RESUME:

        LOG_INFO("Response after sending PENDING to audio HAL");

        return std::make_pair(StartRequestState::IDLE, true);

      case StartRequestState::CONFIRMED:

      case StartRequestState::CANCELED:

        LOG_ERROR("Invalid state, start stream already confirmed");

        return std::make_pair(currect_start_request_state, false);

    }

  };



  if (HalVersionManager::GetHalTransport() ==

      BluetoothAudioHalTransport::HIDL) {

    auto hidl_instance = hidl::le_audio::LeAudioSinkTransport::instance;

    if (hidl_instance->IsRequestCompletedAfterUpdate(lambda)) {

      hidl::le_audio::LeAudioSinkTransport::interface->StreamStarted(

          hidl::BluetoothAudioCtrlAck::SUCCESS_FINISHED);

    }



    return;

  }



  auto aidl_instance = get_aidl_transport_instance(is_broadcaster_);

  if (aidl_instance->IsRequestCompletedAfterUpdate(lambda)) {

    get_aidl_client_interface(is_broadcaster_)

        ->StreamStarted(aidl::BluetoothAudioCtrlAck::SUCCESS_FINISHED);

  }

}



void LeAudioClientInterface::Sink::CancelStreamingRequest() {

  if (HalVersionManager::GetHalTransport() ==

      BluetoothAudioHalTransport::HIDL) {
@@ -287,6 +325,33 @@ void LeAudioClientInterface::Sink::CancelStreamingRequest() { 
        break;

    }

  }

}



void LeAudioClientInterface::Sink::CancelStreamingRequestV2() {

  if (HalVersionManager::GetHalTransport() ==

      BluetoothAudioHalTransport::HIDL) {

    auto hidl_instance = hidl::le_audio::LeAudioSinkTransport::instance;

    auto start_request_state = hidl_instance->GetStartRequestState();

    switch (start_request_state) {

      case StartRequestState::IDLE:

        LOG_WARN(", no pending start stream request");

        return;

      case StartRequestState::PENDING_BEFORE_RESUME:

        LOG_INFO("Response before sending PENDING to audio HAL");

        hidl_instance->SetStartRequestState(StartRequestState::CANCELED);

        return;

      case StartRequestState::PENDING_AFTER_RESUME:

        LOG_INFO("Response after sending PENDING to audio HAL");

        hidl_instance->ClearStartRequestState();

        hidl::le_audio::LeAudioSinkTransport::interface->StreamStarted(

            hidl::BluetoothAudioCtrlAck::FAILURE);

        return;

      case StartRequestState::CONFIRMED:

      case StartRequestState::CANCELED:

        LOG_ERROR("Invalid state, start stream already confirmed");

        break;

    }

  }



  auto aidl_instance = get_aidl_transport_instance(is_broadcaster_);

  auto start_request_state = aidl_instance->GetStartRequestState();
@@ -549,6 +614,46 @@ void LeAudioClientInterface::Source::ConfirmStreamingRequest() { 
  }

}



void LeAudioClientInterface::Source::ConfirmStreamingRequestV2() {

  LOG_INFO("Rymek source");



  auto lambda = [&](StartRequestState currect_start_request_state)

      -> std::pair<StartRequestState, bool> {

    switch (currect_start_request_state) {

      case StartRequestState::IDLE:

        LOG_WARN(", no pending start stream request");

        return std::make_pair(StartRequestState::IDLE, false);

      case StartRequestState::PENDING_BEFORE_RESUME:

        LOG_INFO("Response before sending PENDING to audio HAL");

        return std::make_pair(StartRequestState::CONFIRMED, false);

      case StartRequestState::PENDING_AFTER_RESUME:

        LOG_INFO("Response after sending PENDING to audio HAL");

        return std::make_pair(StartRequestState::IDLE, true);

      case StartRequestState::CONFIRMED:

      case StartRequestState::CANCELED:

        LOG_ERROR("Invalid state, start stream already confirmed");

        return std::make_pair(currect_start_request_state, false);

    }

  };



  if (HalVersionManager::GetHalTransport() ==

      BluetoothAudioHalTransport::HIDL) {

    auto hidl_instance = hidl::le_audio::LeAudioSourceTransport::instance;



    if (hidl_instance->IsRequestCompletedAfterUpdate(lambda)) {

      hidl::le_audio::LeAudioSourceTransport::interface->StreamStarted(

          hidl::BluetoothAudioCtrlAck::SUCCESS_FINISHED);

    }

    return;

  }



  auto aidl_instance = aidl::le_audio::LeAudioSourceTransport::instance;

  if (aidl_instance->IsRequestCompletedAfterUpdate(lambda)) {

    aidl::le_audio::LeAudioSourceTransport::interface->StreamStarted(

        aidl::BluetoothAudioCtrlAck::SUCCESS_FINISHED);

  }

}



void LeAudioClientInterface::Source::CancelStreamingRequest() {

  if (HalVersionManager::GetHalTransport() ==

      BluetoothAudioHalTransport::HIDL) {
@@ -598,6 +703,43 @@ void LeAudioClientInterface::Source::CancelStreamingRequest() { 
  }

}



void LeAudioClientInterface::Source::CancelStreamingRequestV2() {

  auto lambda = [&](StartRequestState currect_start_request_state)

      -> std::pair<StartRequestState, bool> {

    switch (currect_start_request_state) {

      case StartRequestState::IDLE:

        LOG_WARN(", no pending start stream request");

        return std::make_pair(StartRequestState::IDLE, false);

      case StartRequestState::PENDING_BEFORE_RESUME:

        LOG_INFO("Response before sending PENDING to audio HAL");

        return std::make_pair(StartRequestState::CANCELED, false);

      case StartRequestState::PENDING_AFTER_RESUME:

        LOG_INFO("Response after sending PENDING to audio HAL");

        return std::make_pair(StartRequestState::IDLE, true);

      case StartRequestState::CONFIRMED:

      case StartRequestState::CANCELED:

        LOG_ERROR("Invalid state, start stream already confirmed");

        return std::make_pair(currect_start_request_state, false);

    }

  };



  if (HalVersionManager::GetHalTransport() ==

      BluetoothAudioHalTransport::HIDL) {

    auto hidl_instance = hidl::le_audio::LeAudioSourceTransport::instance;

    if (hidl_instance->IsRequestCompletedAfterUpdate(lambda)) {

      hidl::le_audio::LeAudioSourceTransport::interface->StreamStarted(

          hidl::BluetoothAudioCtrlAck::FAILURE);

    }

    return;

  }



  auto aidl_instance = aidl::le_audio::LeAudioSourceTransport::instance;

  if (aidl_instance->IsRequestCompletedAfterUpdate(lambda)) {

    aidl::le_audio::LeAudioSourceTransport::interface->StreamStarted(

        aidl::BluetoothAudioCtrlAck::FAILURE);

  }

}



void LeAudioClientInterface::Source::StopSession() {

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

  if (HalVersionManager::GetHalTransport() ==