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

Commit e4d072dd authored by Grzegorz Kołodziejczyk's avatar Grzegorz Kołodziejczyk Committed by Grzegorz Kolodziejczyk
Browse files

le_audio: Don't block main thread with suspend request call 

There is possibility that due to race in stream suspend Audio framework
may interrupt reporting stack about suspension with call to stack (Bluetooth
main thread).

Due to this race a Bluetooth Main Thread may remain waiting for streamSuspended
response from provider and not let suspend (stack part) Audio Framework.

Not waiting for finishing suspend handler (by not setting promise value
for future wait) allows Audio Framework caller schedule bind once on
Bluetooth Main Thread and be finished after stack report about
suspension would be finished. After such scenario stack will be in
proper state (audio_sender_state_ in state IDLE/RELEASING/SUSPENDING)
and call from Audio Framework will be just skipped.

Tag: #feature
Bug: 287890133
Test: atest bluetooth_le_audio_test bluetooth_le_audio_client_test
Change-Id: I12f7890dc03d0600378ce1e00e06c04fd214663c
parent dd8fa73a

system/bta/le_audio/audio_hal_client/audio_hal_client.h

+2 −2
Original line number Diff line number Diff line
@@ -102,7 +102,7 @@ class LeAudioSinkAudioHalClient { 
  class Callbacks {

   public:

    virtual ~Callbacks() = default;

    virtual void OnAudioSuspend(std::promise<void> do_suspend_promise) = 0;

    virtual void OnAudioSuspend(void) = 0;

    virtual void OnAudioResume(void) = 0;

    virtual void OnAudioMetadataUpdate(

        std::vector<struct record_track_metadata> sink_metadata) = 0;
@@ -139,7 +139,7 @@ class LeAudioSourceAudioHalClient { 
   public:

    virtual ~Callbacks() = default;

    virtual void OnAudioDataReady(const std::vector<uint8_t>& data) = 0;

    virtual void OnAudioSuspend(std::promise<void> do_suspend_promise) = 0;

    virtual void OnAudioSuspend(void) = 0;

    virtual void OnAudioResume(void) = 0;

    virtual void OnAudioMetadataUpdate(

        std::vector<struct playback_track_metadata> source_metadata) = 0;

system/bta/le_audio/audio_hal_client/audio_hal_client_test.cc

+4 −6
Original line number Diff line number Diff line
@@ -208,8 +208,7 @@ class MockLeAudioClientAudioSinkEventReceiver 
 public:

  MOCK_METHOD((void), OnAudioDataReady, (const std::vector<uint8_t>& data),

              (override));

  MOCK_METHOD((void), OnAudioSuspend, (std::promise<void> do_suspend_promise),

              (override));

  MOCK_METHOD((void), OnAudioSuspend, (), (override));

  MOCK_METHOD((void), OnAudioResume, (), (override));

  MOCK_METHOD((void), OnAudioMetadataUpdate,

              (std::vector<struct playback_track_metadata> source_metadata),
@@ -219,8 +218,7 @@ class MockLeAudioClientAudioSinkEventReceiver 
class MockAudioHalClientEventReceiver

    : public LeAudioSinkAudioHalClient::Callbacks {

 public:

  MOCK_METHOD((void), OnAudioSuspend, (std::promise<void> do_suspend_promise),

              (override));

  MOCK_METHOD((void), OnAudioSuspend, (), (override));

  MOCK_METHOD((void), OnAudioResume, (), (override));

  MOCK_METHOD((void), OnAudioMetadataUpdate,

              (std::vector<struct record_track_metadata> sink_metadata),
@@ -413,7 +411,7 @@ TEST_F(LeAudioClientAudioTest, testLeAudioClientAudioSinkSuspend) { 
  /* Expect LeAudio registered event listener to get called when HAL calls the

   * audio_hal_client's internal suspend callback.

   */

  EXPECT_CALL(mock_hal_source_event_receiver_, OnAudioSuspend(_)).Times(1);

  EXPECT_CALL(mock_hal_source_event_receiver_, OnAudioSuspend()).Times(1);

  ASSERT_TRUE(source_audio_hal_stream_cb.on_suspend_());

}
@@ -427,7 +425,7 @@ TEST_F(LeAudioClientAudioTest, testAudioHalClientSuspend) { 
  /* Expect LeAudio registered event listener to get called when HAL calls the

   * audio_hal_client's internal suspend callback.

   */

  EXPECT_CALL(mock_hal_sink_event_receiver_, OnAudioSuspend(_)).Times(1);

  EXPECT_CALL(mock_hal_sink_event_receiver_, OnAudioSuspend()).Times(1);

  ASSERT_TRUE(sink_audio_hal_stream_cb.on_suspend_());

}

system/bta/le_audio/audio_hal_client/audio_sink_hal_client.cc

+1 −6
Original line number Diff line number Diff line
@@ -143,16 +143,11 @@ bool SinkImpl::OnSuspendReq() { 
    return false;

  }



  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(&LeAudioSinkAudioHalClient::Callbacks::OnAudioSuspend,

                     base::Unretained(audioSinkCallbacks_),

                     std::move(do_suspend_promise)));

                     base::Unretained(audioSinkCallbacks_)));

  if (status == BT_STATUS_SUCCESS) {

    do_suspend_future.wait();

    return true;

  }

system/bta/le_audio/audio_hal_client/audio_source_hal_client.cc

+1 −6
Original line number Diff line number Diff line
@@ -252,16 +252,11 @@ bool SourceImpl::OnSuspendReq() { 
    return false;

  }



  // 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(&LeAudioSourceAudioHalClient::Callbacks::OnAudioSuspend,

                     base::Unretained(audioSourceCallbacks_),

                     std::move(do_suspend_promise)));

                     base::Unretained(audioSourceCallbacks_)));

  if (status == BT_STATUS_SUCCESS) {

    do_suspend_future.wait();

    return true;

  }

system/bta/le_audio/broadcaster/broadcaster.cc

+1 −3
Original line number Diff line number Diff line
@@ -1037,11 +1037,9 @@ class LeAudioBroadcasterImpl : public LeAudioBroadcaster, public BigCallbacks { 
      LOG_VERBOSE("All data sent.");

    }



    virtual void OnAudioSuspend(

        std::promise<void> do_suspend_promise) override {

    virtual void OnAudioSuspend(void) override {

      LOG_INFO();

      /* TODO: Should we suspend all broadcasts - remove BIGs? */

      do_suspend_promise.set_value();

      if (instance)

        instance->audio_data_path_state_ = AudioDataPathState::SUSPENDED;

    }