OSDN Git Service

Hearing Aid: Remove std::promise from suspend / resume calls, and stop audio ticks...
authorCheney Ni <cheneyni@google.com>
Wed, 10 Jul 2019 08:24:31 +0000 (16:24 +0800)
committerCheney Ni <cheneyni@google.com>
Wed, 24 Jul 2019 03:30:44 +0000 (11:30 +0800)
The Hearing Aid uses the stack main thread to be the media task thread.
This thread handles events / feeds audio data sequentially, and won't
have a race condition within the single thread. The promise is a bit
redundant, but wasted time to cause a potential deadlock that the caller
waits for the task thread finished which needed the caller's resource.

This CL also stops the audio ticks when no devices available, and no
need to wait for next timer expired. Besides, move all timer controls
into the media task thread, so timer is started / stopped at the same
thread, too.

Bug: 134996542
Test: add big delay within functions, and check no abnormal manually
Change-Id: If0b279a191d86b30db41a682dc7f0ad9470cb10e

bta/hearing_aid/hearing_aid.cc
bta/hearing_aid/hearing_aid_audio_source.cc
bta/include/bta_hearing_aid_api.h

index 0767921..7463909 100644 (file)
@@ -1012,13 +1012,16 @@ class HearingAidImpl : public HearingAid {
     }
   }
 
-  void OnAudioSuspend() {
+  void OnAudioSuspend(const std::function<void()>& stop_audio_ticks) {
+    CHECK(stop_audio_ticks) << "stop_audio_ticks is empty";
+
     if (!audio_running) {
       LOG(WARNING) << __func__ << ": Unexpected audio suspend";
     } else {
       LOG(INFO) << __func__ << ": audio_running=" << audio_running;
     }
     audio_running = false;
+    stop_audio_ticks();
 
     std::vector<uint8_t> stop({CONTROL_POINT_OP_STOP});
     for (auto& device : hearingDevices.devices) {
@@ -1039,23 +1042,33 @@ class HearingAidImpl : public HearingAid {
     }
   }
 
-  void OnAudioResume() {
+  void OnAudioResume(const std::function<void()>& start_audio_ticks) {
+    CHECK(start_audio_ticks) << "start_audio_ticks is empty";
+
     if (audio_running) {
       LOG(ERROR) << __func__ << ": Unexpected Audio Resume";
     } else {
       LOG(INFO) << __func__ << ": audio_running=" << audio_running;
     }
-    audio_running = true;
+
+    for (auto& device : hearingDevices.devices) {
+      if (!device.accepting_audio) continue;
+      audio_running = true;
+      SendStart(&device);
+    }
+
+    if (!audio_running) {
+      LOG(INFO) << __func__ << ": No device (0/" << GetDeviceCount()
+                << ") ready to start";
+      return;
+    }
 
     // TODO: shall we also reset the encoder ?
     encoder_state_release();
     encoder_state_init();
     seq_counter = 0;
 
-    for (auto& device : hearingDevices.devices) {
-      if (!device.accepting_audio) continue;
-      SendStart(&device);
-    }
+    start_audio_ticks();
   }
 
   uint8_t GetOtherSideStreamStatus(HearingDevice* this_side_device) {
@@ -1147,10 +1160,9 @@ class HearingAidImpl : public HearingAid {
     }
 
     if (left == nullptr && right == nullptr) {
-      HearingAidAudioSource::Stop();
-      audio_running = false;
-      encoder_state_release();
-      current_volume = VOLUME_UNKNOWN;
+      LOG(WARNING) << __func__ << ": No more (0/" << GetDeviceCount()
+                   << ") devices ready";
+      DoDisconnectAudioStop();
       return;
     }
 
@@ -1463,8 +1475,17 @@ class HearingAidImpl : public HearingAid {
 
     hearingDevices.Remove(address);
 
-    if (connected)
-      callbacks->OnConnectionState(ConnectionState::DISCONNECTED, address);
+    if (!connected) {
+      return;
+    }
+
+    callbacks->OnConnectionState(ConnectionState::DISCONNECTED, address);
+    for (const auto& device : hearingDevices.devices) {
+      if (device.accepting_audio) return;
+    }
+    LOG(INFO) << __func__ << ": No more (0/" << GetDeviceCount()
+              << ") devices ready";
+    DoDisconnectAudioStop();
   }
 
   void OnGattDisconnected(tGATT_STATUS status, uint16_t conn_id,
@@ -1486,7 +1507,16 @@ class HearingAidImpl : public HearingAid {
 
     DoDisconnectCleanUp(hearingDevice);
 
+    // Keep this hearing aid in the list, and allow to reconnect back.
+
     callbacks->OnConnectionState(ConnectionState::DISCONNECTED, remote_bda);
+
+    for (const auto& device : hearingDevices.devices) {
+      if (device.accepting_audio) return;
+    }
+    LOG(INFO) << __func__ << ": No more (0/" << GetDeviceCount()
+              << ") devices ready";
+    DoDisconnectAudioStop();
   }
 
   void DoDisconnectCleanUp(HearingDevice* hearingDevice) {
@@ -1519,6 +1549,13 @@ class HearingAidImpl : public HearingAid {
     hearingDevice->command_acked = false;
   }
 
+  void DoDisconnectAudioStop() {
+    HearingAidAudioSource::Stop();
+    audio_running = false;
+    encoder_state_release();
+    current_volume = VOLUME_UNKNOWN;
+  }
+
   void SetVolume(int8_t volume) override {
     VLOG(2) << __func__ << ": " << +volume;
     current_volume = volume;
@@ -1730,14 +1767,11 @@ class HearingAidAudioReceiverImpl : public HearingAidAudioReceiver {
   void OnAudioDataReady(const std::vector<uint8_t>& data) override {
     if (instance) instance->OnAudioDataReady(data);
   }
-  void OnAudioSuspend(std::promise<void> do_suspend_promise) override {
-    if (instance) instance->OnAudioSuspend();
-    do_suspend_promise.set_value();
+  void OnAudioSuspend(const std::function<void()>& stop_audio_ticks) override {
+    if (instance) instance->OnAudioSuspend(stop_audio_ticks);
   }
-
-  void OnAudioResume(std::promise<void> do_resume_promise) override {
-    if (instance) instance->OnAudioResume();
-    do_resume_promise.set_value();
+  void OnAudioResume(const std::function<void()>& start_audio_ticks) override {
+    if (instance) instance->OnAudioResume(start_audio_ticks);
   }
 };
 
index 3b92d41..0896e2b 100644 (file)
@@ -97,12 +97,20 @@ void hearing_aid_send_ack(tHEARING_AID_CTRL_ACK status) {
 }
 
 void start_audio_ticks() {
+  if (data_interval_ms != HA_INTERVAL_10_MS &&
+      data_interval_ms != HA_INTERVAL_20_MS) {
+    LOG(FATAL) << " Unsupported data interval: " << data_interval_ms;
+  }
+
   wakelock_acquire();
-  audio_timer.SchedulePeriodic(get_main_thread()->GetWeakPtr(), FROM_HERE, base::Bind(&send_audio_data),
-                               base::TimeDelta::FromMilliseconds(data_interval_ms));
+  audio_timer.SchedulePeriodic(
+      get_main_thread()->GetWeakPtr(), FROM_HERE, base::Bind(&send_audio_data),
+      base::TimeDelta::FromMilliseconds(data_interval_ms));
+  LOG(INFO) << __func__ << ": running with data interval: " << data_interval_ms;
 }
 
 void stop_audio_ticks() {
+  LOG(INFO) << __func__ << ": stopped";
   audio_timer.CancelAndWait();
   wakelock_release();
 }
@@ -121,17 +129,12 @@ void hearing_aid_data_cb(tUIPC_CH_ID, tUIPC_EVENT event) {
       UIPC_Ioctl(*uipc_hearing_aid, UIPC_CH_ID_AV_AUDIO, UIPC_SET_READ_POLL_TMO,
                  reinterpret_cast<void*>(0));
 
-      if (data_interval_ms != HA_INTERVAL_10_MS &&
-          data_interval_ms != HA_INTERVAL_20_MS) {
-        LOG(FATAL) << " Unsupported data interval: " << data_interval_ms;
-      }
-
-      start_audio_ticks();
+      do_in_main_thread(FROM_HERE, base::BindOnce(start_audio_ticks));
       break;
     case UIPC_CLOSE_EVT:
       LOG(INFO) << __func__ << ": UIPC_CLOSE_EVT";
       hearing_aid_send_ack(HEARING_AID_CTRL_ACK_SUCCESS);
-      stop_audio_ticks();
+      do_in_main_thread(FROM_HERE, base::BindOnce(stop_audio_ticks));
       break;
     default:
       LOG(ERROR) << "Hearing Aid audio data event not recognized:" << event;
@@ -306,65 +309,52 @@ void hearing_aid_ctrl_cb(tUIPC_CH_ID, tUIPC_EVENT event) {
 }
 
 bool hearing_aid_on_resume_req(bool start_media_task) {
-  // hearing_aid_recv_ctrl_data(HEARING_AID_CTRL_CMD_START)
-  if (localAudioReceiver != 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(&HearingAidAudioReceiver::OnAudioResume,
-                                  base::Unretained(localAudioReceiver),
-                                  std::move(do_resume_promise)));
-    if (status == BT_STATUS_SUCCESS) {
-      do_resume_future.wait();
-    } else {
-      LOG(ERROR) << __func__
-                 << ": HEARING_AID_CTRL_CMD_START: do_in_main_thread err="
-                 << status;
-      return false;
-    }
-  } else {
+  if (localAudioReceiver == nullptr) {
     LOG(ERROR) << __func__
                << ": HEARING_AID_CTRL_CMD_START: audio receiver not started";
     return false;
   }
-
-  // hearing_aid_data_cb(UIPC_OPEN_EVT): start_media_task
+  bt_status_t status;
   if (start_media_task) {
-    if (data_interval_ms != HA_INTERVAL_10_MS &&
-        data_interval_ms != HA_INTERVAL_20_MS) {
-      LOG(FATAL) << " Unsupported data interval: " << data_interval_ms;
-      data_interval_ms = HA_INTERVAL_10_MS;
-    }
-    start_audio_ticks();
+    status = do_in_main_thread(
+        FROM_HERE, base::BindOnce(&HearingAidAudioReceiver::OnAudioResume,
+                                  base::Unretained(localAudioReceiver),
+                                  start_audio_ticks));
+  } else {
+    auto start_dummy_ticks = []() {
+      LOG(INFO) << "start_audio_ticks: waiting for data path opened";
+    };
+    status = do_in_main_thread(
+        FROM_HERE, base::BindOnce(&HearingAidAudioReceiver::OnAudioResume,
+                                  base::Unretained(localAudioReceiver),
+                                  start_dummy_ticks));
+  }
+  if (status != BT_STATUS_SUCCESS) {
+    LOG(ERROR) << __func__
+               << ": HEARING_AID_CTRL_CMD_START: do_in_main_thread err="
+               << status;
+    return false;
   }
   return true;
 }
 
 bool hearing_aid_on_suspend_req() {
-  // hearing_aid_recv_ctrl_data(HEARING_AID_CTRL_CMD_SUSPEND): stop_media_task
-  stop_audio_ticks();
-  if (localAudioReceiver != nullptr) {
-    // Call OnAudioSuspend and block till it returns.
-    std::promise<void> do_suspend_promise;
-    std::future<void> do_suspend_future = do_suspend_promise.get_future();
-    bt_status_t status = do_in_main_thread(
-        FROM_HERE, base::BindOnce(&HearingAidAudioReceiver::OnAudioSuspend,
-                                  base::Unretained(localAudioReceiver),
-                                  std::move(do_suspend_promise)));
-    if (status == BT_STATUS_SUCCESS) {
-      do_suspend_future.wait();
-      return true;
-    } else {
-      LOG(ERROR) << __func__
-                 << ": HEARING_AID_CTRL_CMD_SUSPEND: do_in_main_thread err="
-                 << status;
-    }
-  } else {
+  if (localAudioReceiver == nullptr) {
     LOG(ERROR) << __func__
                << ": HEARING_AID_CTRL_CMD_SUSPEND: audio receiver not started";
+    return false;
   }
-  return false;
+  bt_status_t status = do_in_main_thread(
+      FROM_HERE,
+      base::BindOnce(&HearingAidAudioReceiver::OnAudioSuspend,
+                     base::Unretained(localAudioReceiver), stop_audio_ticks));
+  if (status != BT_STATUS_SUCCESS) {
+    LOG(ERROR) << __func__
+               << ": HEARING_AID_CTRL_CMD_SUSPEND: do_in_main_thread err="
+               << status;
+    return false;
+  }
+  return true;
 }
 }  // namespace
 
index 9026262..6c4954b 100644 (file)
@@ -21,7 +21,6 @@
 #include <base/callback_forward.h>
 #include <hardware/bt_hearing_aid.h>
 #include <deque>
-#include <future>
 #include <vector>
 
 constexpr uint16_t HEARINGAID_MAX_NUM_UUIDS = 1;
@@ -39,8 +38,22 @@ class HearingAidAudioReceiver {
  public:
   virtual ~HearingAidAudioReceiver() = 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;
+
+  // API to stop our feeding timer, and notify hearing aid devices that the
+  // streaming would stop, too.
+  //
+  // @param stop_audio_ticks a callable function calls out to stop the media
+  // timer for reading data.
+  virtual void OnAudioSuspend(
+      const std::function<void()>& stop_audio_ticks) = 0;
+
+  // To notify hearing aid devices to be ready for streaming, and start the
+  // media timer to feed the audio data.
+  //
+  // @param start_audio_ticks a callable function calls out to start a periodic
+  // timer for feeding data from the audio HAL.
+  virtual void OnAudioResume(
+      const std::function<void()>& start_audio_ticks) = 0;
 };
 
 // Number of rssi reads to attempt when requested