Reland "assistant: add audio ducking for internal media"

This is a reland of f7860793
Fix original build error and some additional refactoring.

Original change's description:
> assistant: add audio ducking for internal media
>
> Bug: b/135064564
> Test: locally build and run with multiple media playing
> Change-Id: I3814cfc3bc619077ca6781493fd369c27d50eb4e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899184
> Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#713200}

Bug: b/135064564
Change-Id: Ief51c008cb8dba3e6ae4757405c69fca01a94524
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903755
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739453}

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -59,9 +59,11 @@
     return;                                                                 \
   }
 
-using ActionModule = assistant_client::ActionModule;
+using assistant_client::ActionModule;
+using assistant_client::MediaStatus;
+using media_session::mojom::MediaSessionAction;
+using media_session::mojom::MediaSessionInfo;
 using Resolution = assistant_client::ConversationStateListener::Resolution;
-using MediaStatus = assistant_client::MediaStatus;
 using CommunicationErrorType =
     chromeos::assistant::AssistantManagerService::CommunicationErrorType;
 
@@ -156,6 +158,52 @@ std::vector<std::pair<std::string, std::string>> ReturnAuthTokensOrEmpty(
 
 }  // namespace
 
+class AssistantManagerServiceImpl::AssistantMediaSessionObserver
+    : public media_session::mojom::MediaSessionObserver {
+ public:
+  AssistantMediaSessionObserver(
+      AssistantManagerServiceImpl* assistant_manager_service,
+      AssistantMediaSession* media_session)
+      : service_(assistant_manager_service) {
+    media_session->AddObserver(session_receiver_.BindNewPipeAndPassRemote());
+  }
+  AssistantMediaSessionObserver(const AssistantMediaSessionObserver& observer) =
+      delete;
+  AssistantMediaSessionObserver operator=(
+      const AssistantMediaSessionObserver& observer) = delete;
+
+  // media_session::mojom::MediaSessionObserver overrides:
+  void MediaSessionInfoChanged(
+      media_session::mojom::MediaSessionInfoPtr info) override {
+    if (!info)
+      return;
+
+    if (info->state == MediaSessionInfo::SessionState::kSuspended)
+      service_->UpdateInternalMediaPlayerStatus(MediaSessionAction::kPause);
+    else if (info->state == MediaSessionInfo::SessionState::kActive)
+      service_->UpdateInternalMediaPlayerStatus(MediaSessionAction::kPlay);
+  }
+
+  void MediaSessionMetadataChanged(
+      const base::Optional<::media_session::MediaMetadata>& metadata) override {
+  }
+  void MediaSessionActionsChanged(
+      const std::vector<MediaSessionAction>& action) override {}
+  void MediaSessionImagesChanged(
+      const base::flat_map<media_session::mojom::MediaSessionImageType,
+                           std::vector<::media_session::MediaImage>>& images)
+      override {}
+  void MediaSessionPositionChanged(
+      const base::Optional<::media_session::MediaPosition>& position) override {
+  }
+
+ private:
+  AssistantManagerServiceImpl* service_;
+
+  mojo::Receiver<media_session::mojom::MediaSessionObserver> session_receiver_{
+      this};
+};
+
 AssistantManagerServiceImpl::AssistantManagerServiceImpl(
     mojom::Client* client,
     ServiceContext* context,
@@ -164,7 +212,10 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl(
         pending_url_loader_factory,
     base::Optional<std::string> s3_server_uri_override)
     : client_(client),
-      media_session_(std::make_unique<AssistantMediaSession>(client_, this)),
+      media_session_(std::make_unique<AssistantMediaSession>(client_)),
+      media_session_observer_(std::make_unique<AssistantMediaSessionObserver>(
+          this,
+          media_session_.get())),
       action_module_(std::make_unique<action::CrosActionModule>(
           this,
           assistant::features::IsAppSupportEnabled(),
@@ -291,6 +342,8 @@ void AssistantManagerServiceImpl::RegisterFallbackMediaHandler() {
 
 void AssistantManagerServiceImpl::UpdateInternalMediaPlayerStatus(
     media_session::mojom::MediaSessionAction action) {
+  if (!assistant_manager_)
+    return;
   auto* media_manager = assistant_manager_->GetMediaManager();
   if (!media_manager)
     return;
@@ -736,6 +789,11 @@ void AssistantManagerServiceImpl::OnOpenUrl(const std::string& url,
     it->OnOpenUrlResponse(gurl, is_background);
 }
 
+void AssistantManagerServiceImpl::OnPlaybackStateChange(
+    const MediaStatus& status) {
+  media_session_->NotifyMediaSessionMetadataChanged(status);
+}
+
 void AssistantManagerServiceImpl::OnShowNotification(
     const action::Notification& notification) {
   ENSURE_MAIN_THREAD(&AssistantManagerServiceImpl::OnShowNotification,
@@ -1365,12 +1423,6 @@ void AssistantManagerServiceImpl::MediaSessionMetadataChanged(
   UpdateMediaState();
 }
 
-void AssistantManagerServiceImpl::OnPlaybackStateChange(
-    const MediaStatus& status) {
-  if (media_session_)
-    media_session_->NotifyMediaSessionMetadataChanged(status);
-}
-
 void AssistantManagerServiceImpl::OnAlarmTimerStateChanged() {
   ENSURE_MAIN_THREAD(&AssistantManagerServiceImpl::OnAlarmTimerStateChanged);
   // Currently, we only handle ringing events here. After some AlarmTimerManager
@@ -1620,8 +1672,8 @@ void AssistantManagerServiceImpl::UpdateMediaState() {
   // media provider) will trigger media state change event. Only update the
   // external media status if the state changes is triggered by external
   // providers.
-  if (media_session_ && media_session_->internal_audio_focus_id() ==
-                            media_session_audio_focus_id_) {
+  if (media_session_->internal_audio_focus_id() ==
+      media_session_audio_focus_id_) {
     return;
   }
 

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -236,6 +236,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   void WaitUntilStartIsFinishedForTesting();
 
  private:
+  class AssistantMediaSessionObserver;
   void StartAssistantInternal(const base::Optional<std::string>& access_token);
   void PostInitAssistant();
 
@@ -316,6 +317,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   State state_ = State::STOPPED;
   std::unique_ptr<AssistantMediaSession> media_session_;
   std::unique_ptr<CrosPlatformApi> platform_api_;
+  std::unique_ptr<AssistantMediaSessionObserver> media_session_observer_;
   std::unique_ptr<action::CrosActionModule> action_module_;
   ChromiumApiDelegate chromium_api_delegate_;
   // NOTE: |display_connection_| is used by |assistant_manager_| and must be

chromeos/services/assistant/media_session/assistant_media_session.h

@@ -6,6 +6,9 @@
 #define CHROMEOS_SERVICES_ASSISTANT_MEDIA_SESSION_ASSISTANT_MEDIA_SESSION_H_
 
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/observer_list_threadsafe.h"
+#include "base/sequenced_task_runner.h"
 #include "base/timer/timer.h"
 #include "base/unguessable_token.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
@@ -23,18 +26,13 @@ struct MediaStatus;
 namespace chromeos {
 namespace assistant {
 
-class AssistantManagerServiceImpl;
-
 // MediaSession manages the media session and audio focus for Assistant.
 // MediaSession allows clients to observe its changes via MediaSessionObserver,
 // and allows clients to resume/suspend/stop the managed players.
-class AssistantMediaSession : public media_session::mojom::MediaSession {
+class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantMediaSession
+    : public media_session::mojom::MediaSession {
  public:
-  enum class State { ACTIVE, SUSPENDED, INACTIVE };
-
-  explicit AssistantMediaSession(
-      mojom::Client* client,
-      AssistantManagerServiceImpl* assistant_manager);
+  explicit AssistantMediaSession(mojom::Client* client);
   ~AssistantMediaSession() override;
 
   // media_session.mojom.MediaSession overrides:
@@ -49,8 +47,6 @@ class AssistantMediaSession : public media_session::mojom::MediaSession {
       override;
   void PreviousTrack() override {}
   void NextTrack() override {}
-  void NotifyMediaSessionMetadataChanged(
-      const assistant_client::MediaStatus& status);
   void SkipAd() override {}
   void Seek(base::TimeDelta seek_time) override {}
   void Stop(SuspendType suspend_type) override {}
@@ -67,6 +63,9 @@ class AssistantMediaSession : public media_session::mojom::MediaSession {
   void RequestAudioFocus(media_session::mojom::AudioFocusType audio_focus_type);
   void AbandonAudioFocusIfNeeded();
 
+  void NotifyMediaSessionMetadataChanged(
+      const assistant_client::MediaStatus& status);
+
   base::WeakPtr<AssistantMediaSession> GetWeakPtr();
 
   // Returns internal audio focus id.
@@ -83,29 +82,20 @@ class AssistantMediaSession : public media_session::mojom::MediaSession {
   void FinishInitialAudioFocusRequest(media_session::mojom::AudioFocusType type,
                                       const base::UnguessableToken& request_id);
 
-  // Returns information about |this|.
-  media_session::mojom::MediaSessionInfoPtr GetMediaSessionInfoInternal();
-
   // Sets |audio_focus_state_|, |audio_focus_type_| and notifies observers about
   // the state change.
-  void SetAudioFocusInfo(State audio_focus_state,
-                         media_session::mojom::AudioFocusType audio_focus_type);
+  void SetAudioFocusInfo(
+      media_session::mojom::MediaSessionInfo::SessionState audio_focus_state,
+      media_session::mojom::AudioFocusType audio_focus_type);
 
   // Notifies mojo observers that the MediaSessionInfo has changed.
   void NotifyMediaSessionInfoChanged();
 
-  // Returns if the session is currently active.
-  bool IsActive() const;
-
-  // Returns if the session is currently suspended.
-  bool IsSuspended() const;
-
   // The current metadata associated with the current media session.
   media_session::MediaMetadata metadata_;
 
-  AssistantManagerServiceImpl* const assistant_manager_service_;
   mojom::Client* const client_;
-
+  scoped_refptr<base::SequencedTaskRunner> main_task_runner_;
   // Binding for Mojo pointer to |this| held by AudioFocusManager.
   mojo::Receiver<media_session::mojom::MediaSession> receiver_{this};
 
@@ -114,22 +104,14 @@ class AssistantMediaSession : public media_session::mojom::MediaSession {
   mojo::RemoteSet<media_session::mojom::MediaSessionObserver> observers_;
 
   // Holds a pointer to the MediaSessionService.
-  mojo::Remote<media_session::mojom::AudioFocusManager> audio_focus_remote_;
-
-  // The ducking state of this media session. The initial value is |false|, and
-  // is set to |true| after StartDucking(), and will be set to |false| after
-  // StopDucking().
-  bool is_ducking_ = false;
+  mojo::Remote<media_session::mojom::AudioFocusManager> audio_focus_manager_;
 
   // If the media session has acquired audio focus then this will contain a
   // pointer to that requests AudioFocusRequestClient.
   mojo::Remote<media_session::mojom::AudioFocusRequestClient>
-      request_client_remote_;
-
-  // The last updated |MediaSessionInfo| that was sent to |observers_|.
-  media_session::mojom::MediaSessionInfoPtr session_info_;
+      audio_focus_request_client_;
 
-  State audio_focus_state_ = State::INACTIVE;
+  media_session::mojom::MediaSessionInfo session_info_;
 
   media_session::mojom::AudioFocusType audio_focus_type_;
 

chromeos/services/assistant/platform/audio_device_owner.cc

@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <utility>
 
+#include "chromeos/services/assistant/media_session/assistant_media_session.h"
 #include "media/audio/audio_device_description.h"
 #include "media/base/limits.h"
 #include "services/media_session/public/mojom/media_session.mojom.h"
@@ -80,6 +81,7 @@ AudioDeviceOwner::~AudioDeviceOwner() {
 }
 
 void AudioDeviceOwner::StartOnMainThread(
+    AssistantMediaSession* media_session,
     assistant_client::AudioOutput::Delegate* delegate,
     mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory,
     const assistant_client::OutputStreamFormat& format) {
@@ -115,26 +117,46 @@ void AudioDeviceOwner::StartOnMainThread(
     background_task_runner_->PostTask(
         FROM_HERE,
         base::BindOnce(&AudioDeviceOwner::StartDeviceOnBackgroundThread,
-                       base::Unretained(this), std::move(stream_factory)));
+                       base::Unretained(this), std::move(stream_factory),
+                       media_session));
   }
 }
 
 void AudioDeviceOwner::StopOnBackgroundThread() {
   DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
-  base::AutoLock lock(lock_);
   output_device_.reset();
-  if (delegate_) {
-    delegate_->OnStopped();
-    delegate_ = nullptr;
+  {
+    base::AutoLock lock(lock_);
+    if (delegate_) {
+      delegate_->OnStopped();
+      delegate_ = nullptr;
+    }
   }
 }
 
+void AudioDeviceOwner::MediaSessionInfoChanged(
+    media_session::mojom::MediaSessionInfoPtr info) {
+  // |output_device_| is only accessed on background thread.
+  DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
+
+  const bool is_ducking =
+      info->state ==
+      media_session::mojom::MediaSessionInfo::SessionState::kDucking;
+
+  constexpr double kDuckingVolume = 0.2;
+  if (output_device_)
+    output_device_->SetVolume(is_ducking ? kDuckingVolume : 1.0);
+}
+
 void AudioDeviceOwner::StartDeviceOnBackgroundThread(
-    mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory) {
+    mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory,
+    AssistantMediaSession* media_session) {
   DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
   output_device_ = std::make_unique<audio::OutputDevice>(
       std::move(stream_factory), audio_param_, this, device_id_);
   output_device_->Play();
+
+  media_session->AddObserver(session_receiver_.BindNewPipeAndPassRemote());
 }
 
 int AudioDeviceOwner::Render(base::TimeDelta delay,

chromeos/services/assistant/platform/audio_device_owner.h

@@ -11,6 +11,7 @@
 
 #include "base/component_export.h"
 #include "base/macros.h"
+#include "chromeos/services/assistant/media_session/assistant_media_session.h"
 #include "libassistant/shared/public/platform_audio_output.h"
 #include "media/base/audio_block_fifo.h"
 #include "media/base/audio_parameters.h"
@@ -23,7 +24,8 @@ namespace chromeos {
 namespace assistant {
 
 class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioDeviceOwner
-    : public media::AudioRendererSink::RenderCallback {
+    : public media::AudioRendererSink::RenderCallback,
+      media_session::mojom::MediaSessionObserver {
  public:
   AudioDeviceOwner(
       scoped_refptr<base::SequencedTaskRunner> task_runner,
@@ -32,12 +34,30 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioDeviceOwner
   ~AudioDeviceOwner() override;
 
   void StartOnMainThread(
+      AssistantMediaSession* media_session,
       assistant_client::AudioOutput::Delegate* delegate,
       mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory,
       const assistant_client::OutputStreamFormat& format);
 
   void StopOnBackgroundThread();
 
+  // media_session::mojom::MediaSessionObserver overrides:
+  void MediaSessionInfoChanged(
+      media_session::mojom::MediaSessionInfoPtr info) override;
+  void MediaSessionMetadataChanged(
+      const base::Optional<::media_session::MediaMetadata>& metadata) override {
+  }
+  void MediaSessionActionsChanged(
+      const std::vector<media_session::mojom::MediaSessionAction>& action)
+      override {}
+  void MediaSessionImagesChanged(
+      const base::flat_map<media_session::mojom::MediaSessionImageType,
+                           std::vector<::media_session::MediaImage>>& images)
+      override {}
+  void MediaSessionPositionChanged(
+      const base::Optional<::media_session::MediaPosition>& position) override {
+  }
+
   // media::AudioRenderSink::RenderCallback overrides:
   int Render(base::TimeDelta delay,
              base::TimeTicks delay_timestamp,
@@ -50,7 +70,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioDeviceOwner
 
  private:
   void StartDeviceOnBackgroundThread(
-      mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory);
+      mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory,
+      AssistantMediaSession* media_session);
 
   // Requests assistant to fill buffer with more data.
   void ScheduleFillLocked(const base::TimeTicks& time);
@@ -78,6 +99,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioDeviceOwner
   assistant_client::OutputStreamFormat format_;
   media::AudioParameters audio_param_;
 
+  mojo::Receiver<media_session::mojom::MediaSessionObserver> session_receiver_{
+      this};
+
   DISALLOW_COPY_AND_ASSIGN(AudioDeviceOwner);
 };
 

chromeos/services/assistant/platform/audio_output_provider_impl.cc

@@ -81,19 +81,20 @@ class AudioOutputImpl : public assistant_client::AudioOutput {
     if (IsEncodedFormat(format_)) {
       main_task_runner_->PostTask(
           FROM_HERE,
-          base::BindOnce(&AudioStreamHandler::StartAudioDecoder,
-                         base::Unretained(audio_stream_handler_.get()),
-                         audio_decoder_factory_, delegate,
-                         base::BindOnce(&AudioDeviceOwner::StartOnMainThread,
-                                        base::Unretained(device_owner_.get()),
-                                        audio_stream_handler_.get(),
-                                        std::move(stream_factory_))));
+          base::BindOnce(
+              &AudioStreamHandler::StartAudioDecoder,
+              base::Unretained(audio_stream_handler_.get()),
+              audio_decoder_factory_, delegate,
+              base::BindOnce(&AudioDeviceOwner::StartOnMainThread,
+                             base::Unretained(device_owner_.get()),
+                             media_session_, audio_stream_handler_.get(),
+                             std::move(stream_factory_))));
     } else {
       main_task_runner_->PostTask(
           FROM_HERE,
           base::BindOnce(&AudioDeviceOwner::StartOnMainThread,
-                         base::Unretained(device_owner_.get()), delegate,
-                         std::move(stream_factory_), format_));
+                         base::Unretained(device_owner_.get()), media_session_,
+                         delegate, std::move(stream_factory_), format_));
     }
   }
 

chromeos/services/assistant/platform/audio_output_provider_impl_unittest.cc

@@ -13,6 +13,9 @@
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "base/threading/thread.h"
+#include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
+#include "chromeos/services/assistant/media_session/assistant_media_session.h"
+#include "chromeos/services/assistant/test_support/fake_client.h"
 #include "libassistant/shared/public/platform_audio_output.h"
 #include "media/base/audio_bus.h"
 #include "media/base/bind_to_current_loop.h"
@@ -103,11 +106,16 @@ TEST_F(AudioDeviceOwnerTest, BufferFilling) {
 
   delegate.set_num_of_bytes_to_fill(200);
   delegate.Reset();
+  FakeClient client;
+  FakeAssistantManagerServiceImpl assistant_manager_service;
+  AssistantMediaSession media_session(&client);
+
   auto owner = std::make_unique<AudioDeviceOwner>(
       base::SequencedTaskRunnerHandle::Get(),
       base::SequencedTaskRunnerHandle::Get(), "test device");
   // Upon start, it will start to fill the buffer.
-  owner->StartOnMainThread(&delegate, mojo::NullRemote(), format);
+  owner->StartOnMainThread(&media_session, &delegate, mojo::NullRemote(),
+                           format);
   delegate.Wait();
 
   delegate.Reset();