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

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

leaudio: Fix invalid CIG parameters 

This patch fixes inproper zeroing ase.max_trasmit_latency during
ConfigureAse() call when ase is in Streaming state.
This could leads to set improper qos params and later to creating CIG
with invalid parameters.

Also this patchs adds more loggin features including printing a whole
group state on error.

Bug: 263561700
Test: atest bluetooth_le_audio_test
Test: atest BluetoothInstrumentationTests
Tag: #feature
Change-Id: I5c14c5ace731eec0e7e9c2929940427893f6f237
parent 32b81cf9

system/bta/le_audio/devices.cc

+128 −31
Original line number Diff line number Diff line
@@ -1496,6 +1496,12 @@ bool LeAudioDevice::ConfigureAses( 
    return false;

  }



  /* The number_of_already_active_group_ase keeps all the active ases

   * in other devices in the group.

   * This function counts active ases only for this device, and we count here

   * new active ases and already active ases which we want to reuse in the

   * scenario

   */

  uint8_t active_ases = *number_of_already_active_group_ase;

  uint8_t max_required_ase_per_dev =

      ent.ase_cnt / ent.device_cnt + (ent.ase_cnt % ent.device_cnt);
@@ -1523,6 +1529,12 @@ bool LeAudioDevice::ConfigureAses( 
    ase->configured_for_context_type = context_type;

    active_ases++;



    /* In case of late connect, we could be here for STREAMING ase.

     * in such case, it is needed to mark ase as known active ase which

     * is important to validate scenario and is done already few lines above.

     * Nothing more to do is needed here.

     */

    if (ase->state != AseState::BTA_LE_AUDIO_ASE_STATE_STREAMING) {

      if (ase->state == AseState::BTA_LE_AUDIO_ASE_STATE_CODEC_CONFIGURED)

        ase->reconfigure = true;
@@ -1535,7 +1547,8 @@ bool LeAudioDevice::ConfigureAses( 
      ase->codec_config.audio_channel_allocation =

          PickAudioLocation(strategy, audio_locations, group_audio_locations);



    /* Get default value if no requirement for specific frame blocks per sdu */

      /* Get default value if no requirement for specific frame blocks per sdu

       */

      if (!ase->codec_config.codec_frames_blocks_per_sdu) {

        ase->codec_config.codec_frames_blocks_per_sdu =

            GetMaxCodecFramesPerSduFromPac(pac);
@@ -1558,6 +1571,7 @@ bool LeAudioDevice::ConfigureAses( 
            GetMetadata(AudioContexts(LeAudioContextType::UNSPECIFIED),

                        std::vector<uint8_t>());

      }

    }



    LOG_DEBUG(

        "device=%s, activated ASE id=%d, direction=%s, max_sdu_size=%d, "
@@ -1961,6 +1975,61 @@ bool LeAudioDeviceGroup::Configure(LeAudioContextType context_type, 
}



LeAudioDeviceGroup::~LeAudioDeviceGroup(void) { this->Cleanup(); }



void LeAudioDeviceGroup::PrintDebugState(void) {

  auto* active_conf = GetActiveConfiguration();

  std::stringstream debug_str;



  debug_str << "\n Groupd id: " << group_id_

            << ", state: " << bluetooth::common::ToString(GetState())

            << ", target state: "

            << bluetooth::common::ToString(GetTargetState())

            << ", cig state: " << bluetooth::common::ToString(cig_state_)

            << ", \n group available contexts: "

            << bluetooth::common::ToString(GetAvailableContexts())

            << ", \n configuration context type: "

            << bluetooth::common::ToString(GetConfigurationContextType())

            << ", \n active configuration name: "

            << (active_conf ? active_conf->name : " not set");



  if (cises_.size() > 0) {

    LOG_INFO("\n Allocated CISes: %d", static_cast<int>(cises_.size()));

    for (auto cis : cises_) {

      LOG_INFO("\n cis id: %d, type: %d, conn_handle %d, addr: %s", cis.id,

               cis.type, cis.conn_handle, cis.addr.ToString().c_str());

    }

  }



  if (GetFirstActiveDevice() != nullptr) {

    uint32_t sink_delay = 0;

    uint32_t source_delay = 0;

    GetPresentationDelay(&sink_delay, le_audio::types::kLeAudioDirectionSink);

    GetPresentationDelay(&source_delay,

                         le_audio::types::kLeAudioDirectionSource);

    auto phy_mtos = GetPhyBitmask(le_audio::types::kLeAudioDirectionSink);

    auto phy_stom = GetPhyBitmask(le_audio::types::kLeAudioDirectionSource);

    auto max_transport_latency_mtos = GetMaxTransportLatencyMtos();

    auto max_transport_latency_stom = GetMaxTransportLatencyStom();

    auto sdu_mts = GetSduInterval(le_audio::types::kLeAudioDirectionSink);

    auto sdu_stom = GetSduInterval(le_audio::types::kLeAudioDirectionSource);



    debug_str << "\n resentation_delay for sink (speaker): " << +sink_delay

              << " us, presentation_delay for source (microphone): "

              << +source_delay << "us, \n MtoS transport latency:  "

              << +max_transport_latency_mtos

              << ", StoM transport latency: " << +max_transport_latency_stom

              << ", \n MtoS Phy: " << loghex(phy_mtos)

              << ", MtoS sdu: " << loghex(phy_stom)

              << " \n MtoS sdu: " << +sdu_mts << ", StoM sdu: " << +sdu_stom;

  }



  LOG_INFO("%s", debug_str.str().c_str());



  for (const auto& device_iter : leAudioDevices_) {

    device_iter.lock()->PrintDebugState();

  }

}



void LeAudioDeviceGroup::Dump(int fd, int active_group_id) {

  bool is_active = (group_id_ == active_group_id);

  std::stringstream stream;
@@ -2455,6 +2524,34 @@ void LeAudioDevice::SetSupportedContexts(AudioContexts snk_contexts, 
  supp_contexts_.source = src_contexts;

}



void LeAudioDevice::PrintDebugState(void) {

  std::stringstream debug_str;



  debug_str << " address: " << address_ << ", "

            << bluetooth::common::ToString(connection_state_)

            << ", conn_id: " << +conn_id_ << ", mtu: " << +mtu_

            << ", num_of_ase: " << static_cast<int>(ases_.size());



  if (ases_.size() > 0) {

    debug_str << "\n  == ASEs == ";

    for (auto& ase : ases_) {

      debug_str << "\n  id: " << +ase.id << ", active: " << ase.active

                << ", dir: "

                << (ase.direction == types::kLeAudioDirectionSink ? "sink"

                                                                  : "source")

                << ", cis_id: " << +ase.cis_id

                << ", cis_handle: " << +ase.cis_conn_hdl << ", state: "

                << bluetooth::common::ToString(ase.data_path_state)

                << "\n ase max_latency: " << +ase.max_transport_latency

                << ", rtn: " << +ase.retrans_nb

                << ", max_sdu: " << +ase.max_sdu_size

                << ", target latency: " << +ase.target_latency;

    }

  }



  LOG_INFO("%s", debug_str.str().c_str());

}



void LeAudioDevice::Dump(int fd) {

  uint16_t acl_handle = BTM_GetHCIConnHandle(address_, BT_TRANSPORT_LE);

system/bta/le_audio/devices.h

+5 −0
Original line number Diff line number Diff line
@@ -179,7 +179,10 @@ class LeAudioDevice { 
                                            types::AudioContexts src_cont_val);

  void DeactivateAllAses(void);

  bool ActivateConfiguredAses(types::LeAudioContextType context_type);



  void PrintDebugState(void);

  void Dump(int fd);



  void DisconnectAcl(void);

  std::vector<uint8_t> GetMetadata(types::AudioContexts context_type,

                                   const std::vector<uint8_t>& ccid_list);
@@ -370,6 +373,8 @@ class LeAudioDeviceGroup { 


  bool IsInTransition(void);

  bool IsReleasingOrIdle(void);



  void PrintDebugState(void);

  void Dump(int fd, int active_group_id);



 private:

system/bta/le_audio/le_audio_types.h

+7 −0
Original line number Diff line number Diff line
@@ -627,6 +627,13 @@ struct ase { 
        data_path_state(AudioStreamDataPathState::IDLE),

        configured_for_context_type(LeAudioContextType::UNINITIALIZED),

        preferred_phy(0),

        max_sdu_size(0),

        retrans_nb(0),

        max_transport_latency(0),

        pres_delay_min(0),

        pres_delay_max(0),

        preferred_pres_delay_min(0),

        preferred_pres_delay_max(0),

        state(AseState::BTA_LE_AUDIO_ASE_STATE_IDLE) {}



  struct hdl_pair hdls;

system/bta/le_audio/state_machine.cc

+32 −5
Original line number Diff line number Diff line
@@ -1276,6 +1276,15 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
      }

    }



    if ((sdu_interval_mtos == 0 && sdu_interval_stom == 0) ||

        (max_trans_lat_mtos == le_audio::types::kMaxTransportLatencyMin &&

         max_trans_lat_stom == le_audio::types::kMaxTransportLatencyMin) ||

        (max_sdu_size_mtos == 0 && max_sdu_size_stom == 0)) {

      LOG_ERROR(" Trying to create invalid group");

      group->PrintDebugState();

      return false;

    }



    bluetooth::hci::iso_manager::cig_create_params param = {

        .sdu_itv_mtos = sdu_interval_mtos,

        .sdu_itv_stom = sdu_interval_stom,
@@ -1973,6 +1982,9 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
                               LeAudioDevice* leAudioDevice) {

    std::vector<struct le_audio::client_parser::ascs::ctp_qos_conf> confs;



    bool validate_transport_latency = false;

    bool validate_max_sdu_size = false;



    for (struct ase* ase = leAudioDevice->GetFirstActiveAse(); ase != nullptr;

         ase = leAudioDevice->GetNextActiveAse(ase)) {

      LOG_DEBUG("device: %s, ase_id: %d, cis_id: %d, ase state: %s",
@@ -1989,14 +2001,16 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
      conf.max_sdu = ase->max_sdu_size;

      conf.retrans_nb = ase->retrans_nb;

      if (!group->GetPresentationDelay(&conf.pres_delay, ase->direction)) {

        LOG(ERROR) << __func__ << ", inconsistent presentation delay for group";

        LOG_ERROR("inconsistent presentation delay for group");

        group->PrintDebugState();

        StopStream(group);

        return;

      }



      conf.sdu_interval = group->GetSduInterval(ase->direction);

      if (!conf.sdu_interval) {

        LOG(ERROR) << __func__ << ", unsupported SDU interval for group";

        LOG_ERROR("unsupported SDU interval for group");

        group->PrintDebugState();

        StopStream(group);

        return;

      }
@@ -2006,15 +2020,28 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
      } else {

        conf.max_transport_latency = group->GetMaxTransportLatencyStom();

      }



      if (conf.max_transport_latency >

          le_audio::types::kMaxTransportLatencyMin) {

        validate_transport_latency = true;

      }



      if (conf.max_sdu > 0) {

        validate_max_sdu_size = true;

      }

      confs.push_back(conf);

    }



    LOG_ASSERT(confs.size() > 0)

        << __func__ << " shouldn't be called without an active ASE";

    if (confs.size() == 0 || !validate_transport_latency ||

        !validate_max_sdu_size) {

      LOG_ERROR("Invalid configuration or latency or sdu size");

      group->PrintDebugState();

      StopStream(group);

      return;

    }



    std::vector<uint8_t> value;

    le_audio::client_parser::ascs::PrepareAseCtpConfigQos(confs, value);



    BtaGattQueue::WriteCharacteristic(leAudioDevice->conn_id_,

                                      leAudioDevice->ctp_hdls_.val_hdl, value,

                                      GATT_WRITE_NO_RSP, NULL, NULL);

system/bta/le_audio/state_machine_test.cc

+7 −1
Original line number Diff line number Diff line
@@ -865,7 +865,7 @@ class StateMachineTest : public Test { 
            codec_configured_state_params.framing =

                ascs::kAseParamFramingUnframedSupported;

            codec_configured_state_params.preferred_retrans_nb = 0x04;

            codec_configured_state_params.max_transport_latency = 0x0005;

            codec_configured_state_params.max_transport_latency = 0x0010;

            codec_configured_state_params.pres_delay_min = 0xABABAB;

            codec_configured_state_params.pres_delay_max = 0xCDCDCD;

            codec_configured_state_params.preferred_pres_delay_min =
@@ -2918,6 +2918,7 @@ TEST_F(StateMachineTest, testAttachDeviceToTheStream) { 


  auto* leAudioDevice = group->GetFirstDevice();

  LeAudioDevice* lastDevice;

  LeAudioDevice* fistDevice = leAudioDevice;



  auto expected_devices_written = 0;

  while (leAudioDevice) {
@@ -2997,6 +2998,11 @@ TEST_F(StateMachineTest, testAttachDeviceToTheStream) { 
  auto ccids = ltv.Find(le_audio::types::kLeAudioMetadataTypeCcidList);

  ASSERT_TRUE(ccids.has_value());

  ASSERT_NE(std::find(ccids->begin(), ccids->end(), media_ccid), ccids->end());



  /* Verify that ASE of first device are still good*/

  auto ase = fistDevice->GetFirstActiveAse();

  ASSERT_NE(ase->max_transport_latency, 0);

  ASSERT_NE(ase->retrans_nb, 0);

}



TEST_F(StateMachineTest, StartStreamAfterConfigure) {