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

Commit c7edf444 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

leaudio: Add reconfiguration guard

Android stack choose LeAudio configuration based on the local sink and
source metadata.

Local Source metadata change does immediate change if needed.
Local Sink metadata change just stores metadata and applies it when
Local Sink direction is resumed (started).

It has been observed that while BT Stack tries to reconfigure the
stream, Audio HAL keep changing Metadata on the Local Source which
breaks reconfiguration, especially when reconfiguration has been
triggered by Local Sink direction. This is because Audio HAL keeps
Suspending and Resuming Sink direction but keeps intention of previous
revonfiguration - which Android is not aware of.

With this change, when reconfiguration is scheduled, BT stack will not
allow for other reconfigurations unless

1. Reconfiguration timeout fires
2. All the directions for which reconfiguration has been scheduled, got
   resumed after Audio HAL was informed about reconfiguration being
   completed.

Bug: 375334939
Flag: Exempt, regression tests with unit tests
Test: atest bluetooth_le_audio_client_test
Change-Id: I041669332a5d7d465dd1dcfbbe8b353bf65d2883
parent cb111410
Loading
Loading
Loading
Loading
+146 −13
Original line number Diff line number Diff line
@@ -239,6 +239,7 @@ public:
    alarm_free(close_vbc_timeout_);
    alarm_free(disable_timer_);
    alarm_free(suspend_timeout_);
    alarm_free(reconfiguration_timeout_);
  }

  LeAudioClientImpl(bluetooth::le_audio::LeAudioClientCallbacks* callbacks,
@@ -263,6 +264,7 @@ public:
        le_audio_sink_hal_client_(nullptr),
        close_vbc_timeout_(alarm_new("LeAudioCloseVbcTimeout")),
        suspend_timeout_(alarm_new("LeAudioSuspendTimeout")),
        reconfiguration_timeout_(alarm_new("LeAudioReconfigurationTimeout")),
        disable_timer_(alarm_new("LeAudioDisableTimer")) {
    LeAudioGroupStateMachine::Initialize(state_machine_callbacks);
    groupStateMachine_ = LeAudioGroupStateMachine::Get();
@@ -364,6 +366,79 @@ public:
    }
  }

  bool IsReconfigurationTimeoutRunning(
          int group_id, uint8_t direction = bluetooth::le_audio::types::kLeAudioDirectionBoth) {
    if (alarm_is_scheduled(reconfiguration_timeout_)) {
      log::debug(" is {} group_id: {}, to check: {}, scheduled: {}",
                 group_id == reconfiguration_group_ ? "running" : " not running", group_id,
                 direction, reconfiguration_local_directions_);
      return group_id == reconfiguration_group_ && (direction & reconfiguration_local_directions_);
    }
    return false;
  }

  void StartReconfigurationTimeout(int group_id) {
    log::debug("group_id: {}", group_id);

    /* This is called when Reconfiguration has been completed. This function starts
     * timer which is a guard for unwanted reconfiguration which might happen when Audio HAL
     * is sending to Bluetooth stack multiple metadata updates and suspends/resume commands.
     * What we want to achieve with this timeout, that BT stack will resume the stream with
     * configuration picked up when ReconfigurationComplete command was sent out to Audio HAL.
     */

    if (alarm_is_scheduled(reconfiguration_timeout_)) {
      log::info("Is already running for group {}", reconfiguration_group_);
      return;
    }

    auto group = aseGroups_.FindById(group_id);
    if (group == nullptr) {
      log::warn("This shall not happen, group_id: {} is not available.", group_id);
      return;
    }

    if (IsDirectionAvailableForCurrentConfiguration(
                group, bluetooth::le_audio::types::kLeAudioDirectionSink)) {
      reconfiguration_local_directions_ |= bluetooth::le_audio::types::kLeAudioDirectionSource;
    }
    if (IsDirectionAvailableForCurrentConfiguration(
                group, bluetooth::le_audio::types::kLeAudioDirectionSource)) {
      reconfiguration_local_directions_ |= bluetooth::le_audio::types::kLeAudioDirectionSink;
    }

    log::debug("reconfiguration_local_directions_ : {}", reconfiguration_local_directions_);

    reconfiguration_group_ = group_id;
    alarm_set_on_mloop(
            reconfiguration_timeout_, kAudioReconfigurationTimeoutMs,
            [](void* data) {
              if (instance) {
                instance->StopReconfigurationTimeout(
                        PTR_TO_INT(data), bluetooth::le_audio::types::kLeAudioDirectionBoth);
              }
            },
            INT_TO_PTR(group_id));
  }

  void StopReconfigurationTimeout(int group_id, uint8_t local_direction) {
    log::debug("group_id: {}, local_direction {}, reconfiguration directions {}", group_id,
               local_direction, reconfiguration_local_directions_);

    reconfiguration_local_directions_ &= ~local_direction;

    if (reconfiguration_local_directions_ != 0) {
      log::debug("Wait for remaining directions: {} ", reconfiguration_local_directions_);
      return;
    }

    if (alarm_is_scheduled(reconfiguration_timeout_)) {
      log::debug("Canceling for group_id {}", reconfiguration_group_);
      alarm_cancel(reconfiguration_timeout_);
    }
    reconfiguration_group_ = bluetooth::groups::kGroupUnknown;
  }

  void StartSuspendTimeout(void) {
    StopSuspendTimeout();

@@ -619,6 +694,7 @@ public:
                                                      "s_state: " + ToString(audio_sender_state_));
      le_audio_sink_hal_client_->SuspendedForReconfiguration();
    }
    StartReconfigurationTimeout(active_group_id_);
  }

  void ReconfigurationComplete(uint8_t directions) {
@@ -1112,6 +1188,8 @@ public:
        return;
      }
      log::info("Call is coming, speed up reconfiguration for a call");
      local_metadata_context_types_.sink.clear();
      local_metadata_context_types_.source.clear();
      reconfigure = true;
    } else {
      if (configuration_context_type_ == LeAudioContextType::CONVERSATIONAL) {
@@ -4176,6 +4254,10 @@ public:
            /* Stream is not started. Try to do it.*/
            if (OnAudioResume(group, bluetooth::le_audio::types::kLeAudioDirectionSource)) {
              audio_sender_state_ = AudioState::READY_TO_START;
              if (IsReconfigurationTimeoutRunning(active_group_id_)) {
                StopReconfigurationTimeout(active_group_id_,
                                           bluetooth::le_audio::types::kLeAudioDirectionSource);
              }
            } else {
              CancelLocalAudioSourceStreamingRequest();
            }
@@ -4398,11 +4480,12 @@ public:
      return;
    }

    if (audio_receiver_state_ == AudioState::IDLE) {
      /* We need new configuration_context_type_ to be selected before we go any
       * further.
       */
    if (audio_receiver_state_ == AudioState::IDLE) {
      ReconfigureOrUpdateRemote(group, bluetooth::le_audio::types::kLeAudioDirectionSource);
      log::info("new_configuration_context = {}", ToString(configuration_context_type_));
    }

    /* Check if the device resume is allowed */
@@ -4437,6 +4520,10 @@ public:
          case AudioState::IDLE:
            if (OnAudioResume(group, bluetooth::le_audio::types::kLeAudioDirectionSink)) {
              audio_receiver_state_ = AudioState::READY_TO_START;
              if (IsReconfigurationTimeoutRunning(active_group_id_)) {
                StopReconfigurationTimeout(active_group_id_,
                                           bluetooth::le_audio::types::kLeAudioDirectionSink);
              }
            } else {
              CancelLocalAudioSinkStreamingRequest();
            }
@@ -4655,11 +4742,6 @@ public:
      return;
    }

    /* Stop the VBC close timeout timer, since we will reconfigure anyway if the
     * VBC was suspended.
     */
    StopVbcCloseTimeout();

    log::info(
            "group_id {} state={}, target_state={}, audio_receiver_state_: {}, "
            "audio_sender_state_: {}, dsa_mode: {}",
@@ -4667,6 +4749,16 @@ public:
            ToString(audio_receiver_state_), ToString(audio_sender_state_),
            static_cast<int>(dsa_mode));

    if (IsReconfigurationTimeoutRunning(group->group_id_)) {
      log::info("Skip it as group is reconfiguring");
      return;
    }

    /* Stop the VBC close timeout timer, since we will reconfigure anyway if the
     * VBC was suspended.
     */
    StopVbcCloseTimeout();

    group->dsa_.mode = dsa_mode;

    /* Set the remote sink metadata context from the playback tracks metadata */
@@ -4814,6 +4906,11 @@ public:
            group->group_id_, ToString(group->GetState()), ToString(group->GetTargetState()),
            ToString(audio_receiver_state_), ToString(audio_sender_state_));

    if (IsReconfigurationTimeoutRunning(group->group_id_)) {
      log::info("Skip it as group is reconfiguring");
      return;
    }

    /* Set remote source metadata context from the recording tracks metadata */
    local_metadata_context_types_.sink = GetAudioContextsFromSinkMetadata(sink_metadata);

@@ -4849,11 +4946,21 @@ public:

    auto is_streaming_other_direction = (other_direction_hal == AudioState::STARTED) ||
                                        (other_direction_hal == AudioState::READY_TO_START);

    auto local_direction = bluetooth::le_audio::types::kLeAudioDirectionBoth & ~remote_direction;
    auto local_other_direction =
            bluetooth::le_audio::types::kLeAudioDirectionBoth & ~remote_other_direction;

    auto is_releasing_for_reconfiguration =
            (((audio_receiver_state_ == AudioState::RELEASING) ||
              (audio_sender_state_ == AudioState::RELEASING)) &&
             group->IsPendingConfiguration() &&
             IsDirectionAvailableForCurrentConfiguration(group, remote_other_direction));
             IsDirectionAvailableForCurrentConfiguration(group, remote_other_direction)) ||
            IsReconfigurationTimeoutRunning(active_group_id_, local_direction);

    auto is_releasing_for_reconfiguration_other_direction =
            is_releasing_for_reconfiguration &
            IsReconfigurationTimeoutRunning(active_group_id_, local_other_direction);

    // Inject conversational when ringtone is played - this is required for all
    // the VoIP applications which are not using the telecom API.
@@ -4915,11 +5022,13 @@ public:
    log::debug("is_streaming_other_direction= {}", is_streaming_other_direction ? "True" : "False");
    log::debug("is_releasing_for_reconfiguration= {}",
               is_releasing_for_reconfiguration ? "True" : "False");
    log::debug("is_releasing_for_reconfiguration_other_direction= {}",
               is_releasing_for_reconfiguration_other_direction ? "True" : "False");
    log::debug("is_ongoing_call_on_other_direction={}",
               is_ongoing_call_on_other_direction ? "True" : "False");

    if (remote_metadata.get(remote_other_direction).test_any(all_bidirectional_contexts) &&
        !is_streaming_other_direction) {
        !(is_streaming_other_direction || is_releasing_for_reconfiguration_other_direction)) {
      log::debug("The other direction is not streaming bidirectional, ignore that context.");
      remote_metadata.get(remote_other_direction).clear();
    }
@@ -4946,7 +5055,7 @@ public:
        remote_metadata.get(remote_direction).unset_all(all_bidirectional_contexts);
        remote_metadata.get(remote_direction).set(LeAudioContextType::CONVERSATIONAL);
      } else {
        if (!is_streaming_other_direction) {
        if (!(is_streaming_other_direction || is_releasing_for_reconfiguration_other_direction)) {
          // Do not take the obsolete metadata
          remote_metadata.get(remote_other_direction).clear();
        } else {
@@ -4964,7 +5073,7 @@ public:
    log::debug("remote_metadata.sink= {}", ToString(remote_metadata.sink));

    if (is_releasing_for_reconfiguration || is_streaming_other_direction) {
      log::debug("Other direction is streaming. Taking its contexts {}",
      log::debug("Other direction is streaming or there is reconfiguration. Taking its contexts {}",
                 ToString(remote_metadata.get(remote_other_direction)));
      /* If current direction has no valid context or the other direction is
       * bidirectional scenario, take the other direction context as well
@@ -5008,7 +5117,7 @@ public:
    /* Choose the right configuration context */
    auto new_configuration_context = ChooseConfigurationContextType(override_contexts);

    log::debug("new_configuration_context= {}.", ToString(new_configuration_context));
    log::info("new_configuration_context= {}.", ToString(new_configuration_context));
    BidirectionalPair<AudioContexts> remote_contexts = {.sink = override_contexts,
                                                        .source = override_contexts};
    return GroupStream(active_group_id_, new_configuration_context, remote_contexts);
@@ -5612,6 +5721,12 @@ public:
          handleAsymmetricPhyForUnicast(group);
          UpdateLocationsAndContextsAvailability(group);
          if (group->IsPendingConfiguration()) {
            log::debug(
                    "Pending configuration for group_id: {} pre_configuration_context_type_ : {} "
                    "-> "
                    "configuration_context_type_ {}",
                    group->group_id_, ToString(pre_configuration_context_type_),
                    ToString(configuration_context_type_));
            auto remote_direction = kLeAudioContextAllRemoteSource.test(configuration_context_type_)
                                            ? bluetooth::le_audio::types::kLeAudioDirectionSource
                                            : bluetooth::le_audio::types::kLeAudioDirectionSink;
@@ -5624,6 +5739,11 @@ public:

            auto remote_contexts = DirectionalRealignMetadataAudioContexts(group, remote_direction);
            ApplyRemoteMetadataAudioContextPolicy(group, remote_contexts, remote_direction);
            log::verbose(
                    "Pending configuration 2 pre_configuration_context_type_ : {} -> "
                    "configuration_context_type_ {}",
                    ToString(pre_configuration_context_type_),
                    ToString(configuration_context_type_));
            if ((configuration_context_type_ != pre_configuration_context_type_) &&
                GroupStream(group->group_id_, configuration_context_type_, remote_contexts)) {
              /* If configuration succeed wait for new status. */
@@ -5780,8 +5900,21 @@ private:
  static constexpr uint64_t kAudioDisableTimeoutMs = 3000;
  static constexpr char kAudioSuspentKeepIsoAliveTimeoutMsProp[] =
          "persist.bluetooth.leaudio.audio.suspend.timeoutms";
  static constexpr uint64_t kAudioReconfigurationTimeoutMs = 1500;
  alarm_t* close_vbc_timeout_;
  alarm_t* suspend_timeout_;

  /* Reconfiguration guard to make sure reconfigration is not broken by unexpected Metadata change.
   * When Reconfiguration is scheduled then
   * 1. BT stack remembers local directions which should be resumed after reconfiguration
   * 2. Blocks another reconfiguration until:
   *      a) all the reconfigured directions has been resumed
   *      b) reconfiguration timeout fires
   */
  alarm_t* reconfiguration_timeout_;
  int reconfiguration_group_ = bluetooth::groups::kGroupUnknown;
  uint8_t reconfiguration_local_directions_ = 0;

  alarm_t* disable_timer_;
  static constexpr uint64_t kDeviceAttachDelayMs = 500;

+90 −10
Original line number Diff line number Diff line
@@ -859,13 +859,18 @@ protected:
                                          metadata_context_types,
                                  types::BidirectionalPair<std::vector<uint8_t>> ccid_lists,
                                  bool configure_qos) {
              auto group_state = group->GetState();
              bool isReconfiguration = group->IsPendingConfiguration();
              log::info(
                      "ConfigureStream: group_id {}, context_type {}, configure_qos {}, "
                      "ConfigureStream: group {} state {}, context type {} sink metadata_ctx {}, "
                      "source metadata_ctx {}, ccid_sink size {}, ccid_source_size {}, "
                      "isReconfiguration {}",
                      group->group_id_, bluetooth::common::ToString(context_type), configure_qos,
                      isReconfiguration);
                      group->group_id_, bluetooth::common::ToString(group_state),
                      bluetooth::common::ToString(context_type),
                      bluetooth::common::ToString(metadata_context_types.sink),
                      bluetooth::common::ToString(metadata_context_types.source),
                      ccid_lists.sink.size(), ccid_lists.source.size(), isReconfiguration);
              /* Do what ReleaseCisIds(group) does: start */
              LeAudioDevice* leAudioDevice = group->GetFirstDevice();
@@ -1030,11 +1035,12 @@ protected:
              auto group_state = group->GetState();
              log::info(
                      "StartStream: group {} state {}, context type {} sink metadata_ctx {}, "
                      "source metadata_ctx {}",
                      "source metadata_ctx {}, ccid_sink size {}, ccid_source_size {}",
                      group->group_id_, bluetooth::common::ToString(group_state),
                      bluetooth::common::ToString(context_type),
                      bluetooth::common::ToString(metadata_context_types.sink),
                      bluetooth::common::ToString(metadata_context_types.source));
                      bluetooth::common::ToString(metadata_context_types.source),
                      ccid_lists.sink.size(), ccid_lists.source.size());
              /* Do nothing if already streaming - the implementation would
               * probably update the metadata.
@@ -7535,11 +7541,11 @@ TEST_F(UnicastTest, TwoEarbudsSetPreferenceWhenMediaForBothMediaAndConv) {
                         is_using_set_before_media_codec_during_media,
                         is_using_set_while_media_codec_during_media, is_reconfig);
  // simulate suspend timeout passed, alarm executing
  log::info("simulate suspend timeout passed, alarm executing");
  fake_osi_alarm_set_on_mloop_.cb(fake_osi_alarm_set_on_mloop_.data);
  SyncOnMainLoop();
  // SetInCall is used by GTBS - and only then we can expect CCID to be set.
  log::info("SetInCall is used by GTBS - and only then we can expect CCID to be set.");
  LeAudioClient::Get()->SetInCall(true);
  bool set_before_conv = false;
@@ -7553,7 +7559,7 @@ TEST_F(UnicastTest, TwoEarbudsSetPreferenceWhenMediaForBothMediaAndConv) {
                         is_using_set_while_conv_codec_during_conv, is_reconfig);
  LeAudioClient::Get()->SetInCall(false);
  // should use preferred codec when switching back to media
  log::info("should use preferred codec when switching back to media");
  ASSERT_EQ(LeAudioClient::Get()->IsUsingPreferredCodecConfig(
                    group_id, static_cast<int>(types::LeAudioContextType::MEDIA)),
            true);
@@ -8310,7 +8316,7 @@ TEST_F(UnicastTest, TwoEarbudsStreamingContextSwitchReconfigure) {
  fake_osi_alarm_set_on_mloop_.cb(fake_osi_alarm_set_on_mloop_.data);
  Mock::VerifyAndClearExpectations(&mock_audio_hal_client_callbacks_);
  // SetInCall is used by GTBS - and only then we can expect CCID to be set.
  log::info("SetInCall is used by GTBS - and only then we can expect CCID to be set.");
  LeAudioClient::Get()->SetInCall(true);
  // Conversational is a bidirectional scenario so expect GTBS CCID
@@ -8331,11 +8337,12 @@ TEST_F(UnicastTest, TwoEarbudsStreamingContextSwitchReconfigure) {
  cis_count_in = 2;
  TestAudioDataTransfer(group_id, cis_count_out, cis_count_in, 1920, 40);
  log::info("End call");
  LeAudioClient::Get()->SetInCall(false);
  // Stop
  StopStreaming(group_id, true);
  // Switch back to MEDIA
  log::info("Switch back to MEDIA");
  ccids = {.sink = {gmcs_ccid}, .source = {}};
  types::BidirectionalPair<types::AudioContexts> contexts = {
          .sink = types::AudioContexts(types::LeAudioContextType::MEDIA),
@@ -10737,6 +10744,79 @@ TEST_F(UnicastTest, MusicDuringCallContextTypes) {
  SyncOnMainLoop();
}
TEST_F(UnicastTest, MetadataUpdateDuringReconfiguration) {
  com::android::bluetooth::flags::provider_->leaudio_speed_up_reconfiguration_between_call(true);
  const RawAddress test_address0 = GetTestAddress(0);
  int group_id = bluetooth::groups::kGroupUnknown;
  /* Scenario
   * 1. Start reconfiguration
   * 2. Send metadata update
   * 3. Make sure metadata updates are ignored
   */
  available_snk_context_types_ =
          (types::LeAudioContextType::CONVERSATIONAL | types::LeAudioContextType::RINGTONE |
           types::LeAudioContextType::GAME | types::LeAudioContextType::MEDIA |
           types::LeAudioContextType::LIVE | types::LeAudioContextType::NOTIFICATIONS)
                  .value();
  supported_snk_context_types_ =
          available_snk_context_types_ |
          types::AudioContexts(types::LeAudioContextType::UNSPECIFIED).value();
  available_src_context_types_ = available_snk_context_types_;
  supported_src_context_types_ =
          available_src_context_types_ |
          types::AudioContexts(types::LeAudioContextType::UNSPECIFIED).value();
  SetSampleDatabaseEarbudsValid(1, test_address0, codec_spec_conf::kLeAudioLocationAnyLeft,
                                codec_spec_conf::kLeAudioLocationStereo, default_channel_cnt,
                                default_channel_cnt, 0x0024, false /*add_csis*/, true /*add_cas*/,
                                true /*add_pacs*/, default_ase_cnt /*add_ascs_cnt*/, 1 /*set_size*/,
                                0 /*rank*/);
  EXPECT_CALL(mock_audio_hal_client_callbacks_,
              OnConnectionState(ConnectionState::CONNECTED, test_address0))
          .Times(1);
  EXPECT_CALL(mock_audio_hal_client_callbacks_,
              OnGroupNodeStatus(test_address0, _, GroupNodeStatus::ADDED))
          .WillOnce(DoAll(SaveArg<1>(&group_id)));
  ConnectLeAudio(test_address0);
  ASSERT_NE(group_id, bluetooth::groups::kGroupUnknown);
  // Audio sessions are started only when device gets active
  EXPECT_CALL(*mock_le_audio_source_hal_client_, Start(_, _, _)).Times(1);
  EXPECT_CALL(*mock_le_audio_sink_hal_client_, Start(_, _, _)).Times(1);
  LeAudioClient::Get()->GroupSetActive(group_id);
  SyncOnMainLoop();
  EXPECT_CALL(mock_state_machine_,
              StartStream(_, bluetooth::le_audio::types::LeAudioContextType::MEDIA, _, _))
          .Times(1);
  StartStreaming(AUDIO_USAGE_MEDIA, AUDIO_CONTENT_TYPE_MUSIC, group_id);
  SyncOnMainLoop();
  Mock::VerifyAndClearExpectations(&mock_state_machine_);
  auto group = streaming_groups.at(group_id);
  stay_at_qos_config_in_start_stream = true;
  log::info("Reconfigure to conversational and stay in Codec Config");
  EXPECT_CALL(mock_state_machine_, StopStream(_)).Times(1);
  EXPECT_CALL(mock_state_machine_, ConfigureStream(_, _, _, _, _)).Times(1);
  LeAudioClient::Get()->SetInCall(true);
  SyncOnMainLoop();
  ASSERT_TRUE(group->GetState() == types::AseState::BTA_LE_AUDIO_ASE_STATE_CODEC_CONFIGURED);
  log::info("Expect not action on metadata change");
  EXPECT_CALL(mock_state_machine_, StopStream(_)).Times(0);
  EXPECT_CALL(mock_state_machine_, ConfigureStream(_, _, _, _, _)).Times(0);
  UpdateLocalSourceMetadata(AUDIO_USAGE_MEDIA, AUDIO_CONTENT_TYPE_MUSIC);
  SyncOnMainLoop();
  Mock::VerifyAndClearExpectations(&mock_state_machine_);
}
TEST_F(UnicastTest, MusicDuringCallContextTypes_SpeedUpReconfigFlagEnabled) {
  com::android::bluetooth::flags::provider_->leaudio_speed_up_reconfiguration_between_call(true);