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

Commit caa0e32e authored by En-Shuo Hsu's avatar En-Shuo Hsu
Browse files

floss: Move the read buffer out of btm_route_sco_data 

We change the behavior to not drop PCM data we read when its length is
less than BTM_MSBC_CODE_SIZE bytes by moving the buffer out of the
function.

The new behaviors after this CL will be:
1. We don't drop PCM now if audio server send PCM less for one mSBC
   packet
2. Do not send zero frame to SCO if not enough

(1) is what we want to improve in this CL and we should have extra
attention for (2). (2) means that we intentionally let SCO Rx/Tx offset
by 1 frame when audio server underrun.
Since we choose to not add any mechanism to catch up. It now fully
relies on the controller to handle potential underrun in SCO.

With these new behaviors, we'd like to make the audio frames eventually
played continually without pop noises that was caused by the added zero
frames.

This also means that we now leave the underrun handling fully to the
audio stack. You may think this complicates audio server's
implementation. However, it actually simplifies audio server's
coordination with the Bluetooth stack as there is no longer implicit
data changes introduced by the Bluetooth stack from audio server's point
of view.

Also, this CL helps to simplify the code structure for future commits to
land.

Bug: 235901463
Tag: #floss
Test: gd/cert/run & Build and verify HFP works & cras_test_client \
--block_size 512 --playback_file 400.wav

Change-Id: I8323fdd7eb3af899722fd1aa3b6643d3f9a64bb6
parent f1347793

system/internal_include/bt_target.h

+2 −1
Original line number Diff line number Diff line
@@ -262,7 +262,8 @@ 
/**************************

 * Initial SCO TX credit

 ************************/

/* max TX SCO data packet size */

/* The size of buffer used for TX SCO data packets. The size should be divisible

 * by BTM_MSBC_CODE_SIZE(240) */

#ifndef BTM_SCO_DATA_SIZE_MAX

#define BTM_SCO_DATA_SIZE_MAX 240

#endif

system/stack/btm/btm_sco.cc

+107 −24
Original line number Diff line number Diff line
@@ -85,6 +85,16 @@ const bluetooth::legacy::hci::Interface& GetLegacyHciInterface() { 
  (ESCO_PKT_TYPES_MASK_NO_2_EV3 | ESCO_PKT_TYPES_MASK_NO_3_EV3 | \

   ESCO_PKT_TYPES_MASK_NO_2_EV5 | ESCO_PKT_TYPES_MASK_NO_3_EV5)



/* Buffer used for reading PCM data from audio server that will be encoded into

 * mSBC packet. The BTM_SCO_DATA_SIZE_MAX should be set to a number divisible by

 * BTM_MSBC_CODE_SIZE(240) */

static int16_t btm_pcm_buf[BTM_SCO_DATA_SIZE_MAX] = {0};



/* The read and write offset for btm_pcm_buf.

 * They are only used for WBS and the unit is byte. */

static size_t btm_pcm_buf_read_offset = 0;

static size_t btm_pcm_buf_write_offset = 0;



/* Per Bluetooth Core v5.0 and HFP 1.7 specification. */

#define BTM_MSBC_H2_HEADER_0 0x01

#define BTM_MSBC_H2_HEADER_LEN 2
@@ -214,6 +224,12 @@ static bool verify_h2_header_seq_num(const uint8_t num) { 
 * Function         btm_route_sco_data

 *

 * Description      Route received SCO data.

 *                  This function is triggered when we receive a packet of SCO

 *                  data. It regards the received SCO packet as a clock tick to

 *                  start the write and read to and from the audio server. It

 *                  also tries to balance the write/read data rate between the

 *                  Bluetooth and Audio stack by sending and receiving the same

 *                  amount of PCM data to and from the audio server.

 *

 * Returns          void

 *
@@ -245,6 +261,7 @@ void btm_route_sco_data(BT_HDR* p_msg) { 
  }



  const uint8_t* decoded = nullptr;

  size_t written = 0;

  if (active_sco->is_wbs()) {

    /* TODO(b/235901463): Support packet size != BTM_MSBC_PKT_LEN */

    if (data_len != BTM_MSBC_PKT_LEN) {
@@ -277,28 +294,65 @@ void btm_route_sco_data(BT_HDR* p_msg) { 
      decoded = btm_msbc_zero_frames;

    }



    bluetooth::audio::sco::write(decoded, BTM_MSBC_CODE_SIZE);

    written = bluetooth::audio::sco::write(decoded, BTM_MSBC_CODE_SIZE);

  } else {

    bluetooth::audio::sco::write(payload, data_len);

    written = bluetooth::audio::sco::write(payload, data_len);

  }

  osi_free(p_msg);



  uint8_t read_buf[BTM_SCO_DATA_SIZE_MAX];

  /* For Chrome OS, we send the outgoing data after receiving an incoming one */

  size_t read = bluetooth::audio::sco::read(read_buf, data_len);

  /* For Chrome OS, we send the outgoing data after receiving an incoming one.

   * server, so that we can keep the data read/write rate balanced */

  size_t read = 0, avail = 0;

  if (active_sco->is_wbs()) {

    while (written) {

      avail = BTM_SCO_DATA_SIZE_MAX - btm_pcm_buf_write_offset;

      if (avail) {

        data_len = written < avail ? written : avail;

        read = bluetooth::audio::sco::read(

            (uint8_t*)btm_pcm_buf + btm_pcm_buf_write_offset, data_len);

        if (read != data_len) {

          ASSERT_LOG(btm_pcm_buf_write_offset + read <= BTM_SCO_DATA_SIZE_MAX,

                     "Read more data (%lu) than available buffer (%lu) guarded "

                     "by read",

                     (unsigned long)read,

                     (unsigned long)(BTM_SCO_DATA_SIZE_MAX -

                                     btm_pcm_buf_write_offset));



          LOG_INFO(

              "Requested to read %lu bytes of data but got %lu bytes of PCM "

              "data from audio server: WriteOffset:%lu ReadOffset:%lu",

              (unsigned long)data_len, (unsigned long)read,

              (unsigned long)btm_pcm_buf_write_offset,

              (unsigned long)btm_pcm_buf_read_offset);

          if (read == 0) break;

        }



        written -= read;

      } else {

        /* We don't break here so that we can still decode the data in the

         * buffer to spare the buffer space when the buffer is full */

        LOG_WARN("Buffer is full when we try to read from audio server");

        ASSERT_LOG(btm_pcm_buf_write_offset - btm_pcm_buf_read_offset >=

                       BTM_MSBC_CODE_SIZE,

                   "PCM buffer is full but fails to encode a mSBC packet. "

                   "This is abnormal and can cause busy loop: "

                   "WriteOffset:%lu, ReadOffset:%lu, BufferSize:%lu",

                   (unsigned long)btm_pcm_buf_write_offset,

                   (unsigned long)btm_pcm_buf_read_offset,

                   (unsigned long)sizeof(btm_pcm_buf));

      }



      btm_pcm_buf_write_offset += read;

      if (btm_pcm_buf_write_offset - btm_pcm_buf_read_offset <

          BTM_MSBC_CODE_SIZE) {

        continue;

      }



      uint8_t encoded[BTM_MSBC_PKT_LEN] = {

          BTM_MSBC_H2_HEADER_0,

          btm_h2_header_frames_count[btm_msbc_num_out_frames % 4]};



    if (read != BTM_MSBC_CODE_SIZE) {

      LOG_WARN("Read partial data: %zu", read);

      std::copy(std::begin(btm_msbc_zero_packet),

                std::end(btm_msbc_zero_packet),

                &encoded[BTM_MSBC_H2_HEADER_LEN]);

    } else {

      uint32_t encoded_size;

      encoded_size = hfp_msbc_encode_frames((int16_t*)read_buf, encoded + 2);

      encoded_size = hfp_msbc_encode_frames(btm_pcm_buf, encoded + 2);

      if (encoded_size != BTM_MSBC_PKT_FRAME_LEN) {

        LOG_WARN("Encode invalid packet size: %lu",

                 (unsigned long)encoded_size);
@@ -306,17 +360,43 @@ void btm_route_sco_data(BT_HDR* p_msg) { 
                  std::end(btm_msbc_zero_packet),

                  &encoded[BTM_MSBC_H2_HEADER_LEN]);

      }

    }



      auto data = std::vector<uint8_t>(encoded, encoded + BTM_MSBC_PKT_LEN);

      btm_send_sco_packet(std::move(data));



      btm_msbc_num_out_frames++;



      /* The offsets should reset some time as the buffer length should always

       * divisible by BTM_MSBC_CODE_SIZE(240) */

      btm_pcm_buf_read_offset += BTM_MSBC_CODE_SIZE;

      if (btm_pcm_buf_write_offset == btm_pcm_buf_read_offset) {

        btm_pcm_buf_write_offset = 0;

        btm_pcm_buf_read_offset = 0;

      }

    }

  } else {

    auto data = std::vector<uint8_t>(read_buf, read_buf + read);

    while (written) {

      read = bluetooth::audio::sco::read(

          (uint8_t*)btm_pcm_buf,

          written < BTM_SCO_DATA_SIZE_MAX ? written : BTM_SCO_DATA_SIZE_MAX);

      if (read == 0) {

        LOG_INFO("Failed to read %lu bytes of PCM data from audio server",

                 (unsigned long)(written < BTM_SCO_DATA_SIZE_MAX

                                     ? written

                                     : BTM_SCO_DATA_SIZE_MAX));

        break;

      }

      written -= read;



      /* In narrow-band, the CVSD encode is offloaded to controller so we can

       * send PCM data directly to SCO.

       * We don't maintain buffer read/write offset for NB as we send all data

       * that we read from the audio server. */

      auto data = std::vector<uint8_t>((uint8_t*)btm_pcm_buf,

                                       (uint8_t*)btm_pcm_buf + read);

      btm_send_sco_packet(std::move(data));

    }

  }

}



void btm_send_sco_packet(std::vector<uint8_t> data) {

  auto* active_sco = btm_get_active_sco();
@@ -329,8 +409,8 @@ void btm_send_sco_packet(std::vector<uint8_t> data) { 


// Build a SCO packet from uint8

BT_HDR* btm_sco_make_packet(std::vector<uint8_t> data, uint16_t sco_handle) {

  ASSERT_LOG(data.size() <= BTM_SCO_DATA_SIZE_MAX, "Invalid SCO data size: %zu",

             data.size());

  ASSERT_LOG(data.size() <= BTM_SCO_DATA_SIZE_MAX, "Invalid SCO data size: %lu",

             (unsigned long)data.size());

  BT_HDR* p_buf = (BT_HDR*)osi_calloc(BT_SMALL_BUFFER_SIZE);

  p_buf->event = BT_EVT_TO_LM_HCI_SCO;

  // SCO header size is 3 per Core 5.2 Vol 4 Part E 5.4.3 figure 5.3
@@ -872,11 +952,14 @@ void btm_sco_connected(const RawAddress& bda, uint16_t hci_handle, 
      /* In-band (non-offload) data path */

      if (p->is_inband()) {

        if (p->is_wbs()) {

          btm_pcm_buf_read_offset = 0;

          btm_pcm_buf_write_offset = 0;

          btm_msbc_num_out_frames = 0;

          hfp_msbc_decoder_init();

          hfp_msbc_encoder_init();

        }



        std::fill(std::begin(btm_pcm_buf), std::end(btm_pcm_buf), 0);

        bluetooth::audio::sco::open();

      }

      return;

system/stack/btm/btm_sco.h

+2 −2
Original line number Diff line number Diff line
@@ -37,10 +37,10 @@ void open(); 
/* Clean up the socket when the SCO connection is done */

void cleanup();



/* Read from the socket (audio server) for SCO Tx */

/* Read PCM data from the socket (audio server) for SCO Tx */

size_t read(uint8_t* p_buf, uint32_t len);



/* Write to the socket from SCO Rx */

/* Write PCM data to the socket from SCO Rx */

size_t write(const uint8_t* buf, uint32_t len);

}  // namespace bluetooth::audio::sco

system/stack/btm/btm_sco_hci.cc

+1 −1
Original line number Diff line number Diff line
@@ -94,7 +94,7 @@ size_t write(const uint8_t* p_buf, uint32_t len) { 
    LOG_WARN("Write to uninitialized or closed UIPC");

    return 0;

  }

  return UIPC_Send(*sco_uipc, UIPC_CH_ID_AV_AUDIO, 0, p_buf, len);

  return UIPC_Send(*sco_uipc, UIPC_CH_ID_AV_AUDIO, 0, p_buf, len) ? len : 0;

}



}  // namespace sco