assistant: update platform audio to use audio service.

Bug: b:77916222
Test: Manual
Change-Id: I8d12eb12c6026a862c4546456f38ec943bd2e30d
Reviewed-on: https://chromium-review.googlesource.com/1056298
Commit-Queue: Muyuan Li <muyuanli@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569102}

chrome/browser/ui/BUILD.gn

@@ -3741,16 +3741,12 @@ split_static_library("ui") {
     sources += [
       "ash/assistant/assistant_client.cc",
       "ash/assistant/assistant_client.h",
-      "ash/assistant/assistant_context.cc",
-      "ash/assistant/assistant_context.h",
       "ash/assistant/assistant_context_util.cc",
       "ash/assistant/assistant_context_util.h",
       "ash/assistant/assistant_image_downloader.cc",
       "ash/assistant/assistant_image_downloader.h",
       "ash/assistant/assistant_setup.cc",
       "ash/assistant/assistant_setup.h",
-      "ash/assistant/platform_audio_input_host.cc",
-      "ash/assistant/platform_audio_input_host.h",
       "ash/assistant/web_contents_manager.cc",
       "ash/assistant/web_contents_manager.h",
     ]

chrome/browser/ui/ash/assistant/assistant_client.cc

@@ -8,6 +8,7 @@
 
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h"
+#include "chrome/browser/ui/ash/assistant/assistant_context_util.h"
 #include "chrome/browser/ui/ash/assistant/assistant_image_downloader.h"
 #include "chrome/browser/ui/ash/assistant/assistant_setup.h"
 #include "chrome/browser/ui/ash/assistant/web_contents_manager.h"
@@ -25,10 +26,7 @@ AssistantClient* AssistantClient::Get() {
   return g_instance;
 }
 
-AssistantClient::AssistantClient()
-    : client_binding_(this),
-      audio_input_binding_(&audio_input_),
-      context_binding_(&context_) {
+AssistantClient::AssistantClient() : client_binding_(this) {
   DCHECK_EQ(nullptr, g_instance);
   g_instance = this;
 }
@@ -36,7 +34,6 @@ AssistantClient::AssistantClient()
 AssistantClient::~AssistantClient() {
   DCHECK(g_instance);
   g_instance = nullptr;
-  context_binding_.Close();
 }
 
 void AssistantClient::MaybeInit(service_manager::Connector* connector) {
@@ -46,17 +43,10 @@ void AssistantClient::MaybeInit(service_manager::Connector* connector) {
   initialized_ = true;
   connector->BindInterface(chromeos::assistant::mojom::kServiceName,
                            &assistant_connection_);
-  chromeos::assistant::mojom::AudioInputPtr audio_input_ptr;
-  audio_input_binding_.Bind(mojo::MakeRequest(&audio_input_ptr));
 
   chromeos::assistant::mojom::ClientPtr client_ptr;
   client_binding_.Bind(mojo::MakeRequest(&client_ptr));
-
-  chromeos::assistant::mojom::ContextPtr context_ptr;
-  context_binding_.Bind(mojo::MakeRequest(&context_ptr));
-
-  assistant_connection_->Init(std::move(client_ptr), std::move(context_ptr),
-                              std::move(audio_input_ptr));
+  assistant_connection_->Init(std::move(client_ptr));
 
   assistant_image_downloader_ =
       std::make_unique<AssistantImageDownloader>(connector);
@@ -69,3 +59,8 @@ void AssistantClient::OnAssistantStatusChanged(bool running) {
       running ? ash::mojom::VoiceInteractionState::RUNNING
               : ash::mojom::VoiceInteractionState::STOPPED);
 }
+
+void AssistantClient::RequestAssistantStructure(
+    RequestAssistantStructureCallback callback) {
+  RequestAssistantStructureForActiveBrowserWindow(std::move(callback));
+}

chrome/browser/ui/ash/assistant/assistant_client.h

@@ -8,8 +8,6 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "chrome/browser/ui/ash/assistant/assistant_context.h"
-#include "chrome/browser/ui/ash/assistant/platform_audio_input_host.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -33,18 +31,12 @@ class AssistantClient : chromeos::assistant::mojom::Client {
 
   // assistant::mojom::Client overrides:
   void OnAssistantStatusChanged(bool running) override;
+  void RequestAssistantStructure(
+      RequestAssistantStructureCallback callback) override;
 
  private:
   mojo::Binding<chromeos::assistant::mojom::Client> client_binding_;
-
   chromeos::assistant::mojom::AssistantPlatformPtr assistant_connection_;
-  mojo::Binding<chromeos::assistant::mojom::AudioInput> audio_input_binding_;
-
-  mojo::Binding<chromeos::assistant::mojom::Context> context_binding_;
-
-  PlatformAudioInputHost audio_input_;
-
-  AssistantContext context_;
 
   std::unique_ptr<AssistantImageDownloader> assistant_image_downloader_;
   std::unique_ptr<AssistantSetup> assistant_setup_;

chrome/browser/ui/ash/assistant/assistant_context.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/ui/ash/assistant/assistant_context.h"
-
-#include "chrome/browser/ui/ash/assistant/assistant_context_util.h"
-
-AssistantContext::AssistantContext() = default;
-AssistantContext::~AssistantContext() = default;
-
-void AssistantContext::RequestAssistantStructure(
-    RequestAssistantStructureCallback callback) {
-  RequestAssistantStructureForActiveBrowserWindow(std::move(callback));
-}

chrome/browser/ui/ash/assistant/assistant_context.h

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_CONTEXT_H_
-#define CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_CONTEXT_H_
-
-#include "base/macros.h"
-#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
-
-// Class for Assistant to retrieve view hierarchy information from browser
-// windows.
-class AssistantContext : public chromeos::assistant::mojom::Context {
- public:
-  AssistantContext();
-  ~AssistantContext() override;
-
-  // mojom::Context overrides
-  void RequestAssistantStructure(
-      RequestAssistantStructureCallback callback) override;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(AssistantContext);
-};
-
-#endif  // CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_CONTEXT_H_

chrome/browser/ui/ash/assistant/assistant_context_browsertest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/ash/assistant/assistant_context.h"
+#include "chrome/browser/ui/ash/assistant/assistant_client.h"
 
 #include <string>
 #include <vector>

chrome/browser/ui/ash/assistant/platform_audio_input_host.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/ui/ash/assistant/platform_audio_input_host.h"
-
-#include <utility>
-
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "base/logging.h"
-#include "base/single_thread_task_runner.h"
-#include "media/audio/audio_input_controller.h"
-#include "media/audio/audio_manager.h"
-#include "media/base/audio_bus.h"
-#include "media/base/audio_parameters.h"
-#include "media/base/audio_sample_types.h"
-#include "media/base/channel_layout.h"
-
-namespace {
-
-void NotifyDataAvailable(const base::WeakPtr<PlatformAudioInputHost>& host,
-                         const std::vector<int32_t>& data,
-                         int32_t frames,
-                         base::TimeTicks capture_time) {
-  if (host)
-    host->NotifyDataAvailable(std::move(data), frames, capture_time);
-}
-
-void NotifyAudioClosed(const base::WeakPtr<PlatformAudioInputHost>& host) {
-  if (host)
-    host->NotifyAudioClosed();
-}
-
-}  // namespace
-
-class PlatformAudioInputHost::Writer
-    : public media::AudioInputController::SyncWriter {
- public:
-  Writer(base::WeakPtr<PlatformAudioInputHost> host,
-         scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-      : host_(std::move(host)), task_runner_(task_runner) {}
-  ~Writer() override = default;
-
-  // media::AudioInputController::SyncWriter overrides:
-  void Write(const media::AudioBus* data,
-             double volume,
-             bool key_pressed,
-             base::TimeTicks capture_time) override {
-    // 2 channels * # of frames.
-    std::vector<int32_t> buffer(2 * data->frames());
-    data->ToInterleaved<media::SignedInt32SampleTypeTraits>(data->frames(),
-                                                            buffer.data());
-
-    task_runner_->PostTask(
-        FROM_HERE,
-        base::BindOnce(&::NotifyDataAvailable, host_, std::move(buffer),
-                       data->frames(), capture_time));
-  }
-
-  void Close() override {
-    task_runner_->PostTask(FROM_HERE,
-                           base::BindOnce(&::NotifyAudioClosed, host_));
-  }
-
- private:
-  base::WeakPtr<PlatformAudioInputHost> host_;
-
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-
-  DISALLOW_COPY_AND_ASSIGN(Writer);
-};
-
-class PlatformAudioInputHost::EventHandler
-    : public media::AudioInputController::EventHandler {
- public:
-  EventHandler() = default;
-  ~EventHandler() override = default;
-
-  // media::AudioInputController::EventHandler overrides:
-  void OnCreated(bool initially_muted) override {}
-  void OnError(media::AudioInputController::ErrorCode error_code) override {}
-  void OnLog(base::StringPiece log) override {}
-  void OnMuted(bool is_muted) override {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(EventHandler);
-};
-
-PlatformAudioInputHost::PlatformAudioInputHost() : weak_factory_(this) {
-  sync_writer_ = std::make_unique<Writer>(weak_factory_.GetWeakPtr(),
-                                          base::ThreadTaskRunnerHandle::Get());
-  event_handler_ = std::make_unique<EventHandler>();
-  audio_input_controller_ = media::AudioInputController::Create(
-      media::AudioManager::Get(), event_handler_.get(), sync_writer_.get(),
-      nullptr,
-      media::AudioParameters(media::AudioParameters::AUDIO_PCM_LINEAR,
-                             media::CHANNEL_LAYOUT_STEREO,
-                             16000 /* 16000 frames per second */,
-                             1600 /* 1600 (16000 / 10) frames per buffer */),
-      "default" /* device_id */, false /* agc_is_enabled */);
-}
-
-PlatformAudioInputHost::~PlatformAudioInputHost() {
-  // Bind |sync_writer_| and |event_handler_| to the callback closure to ensure
-  // they live longer than the |Close| call, which is async.
-  audio_input_controller_->Close(
-      base::BindOnce([](std::unique_ptr<EventHandler> event_handler,
-                        std::unique_ptr<Writer> writer) {},
-                     std::move(event_handler_), std::move(sync_writer_)));
-}
-
-void PlatformAudioInputHost::AddObserver(
-    chromeos::assistant::mojom::AudioInputObserverPtr observer) {
-  observers_.AddPtr(std::move(observer));
-  if (!recording_) {
-    audio_input_controller_->Record();
-    recording_ = true;
-  }
-}
-
-void PlatformAudioInputHost::NotifyDataAvailable(
-    const std::vector<int32_t>& data,
-    int32_t frames,
-    base::TimeTicks capture_time) {
-  observers_.ForAllPtrs([&data, frames, capture_time](auto* observer) {
-    observer->OnAudioInputFramesAvailable(data, frames, capture_time);
-  });
-
-  if (observers_.empty() && recording_) {
-    recording_ = false;
-    audio_input_controller_->Close(base::DoNothing());
-  }
-}
-
-void PlatformAudioInputHost::NotifyAudioClosed() {
-  recording_ = false;
-  observers_.ForAllPtrs([](auto* observer) { observer->OnAudioInputClosed(); });
-  observers_.CloseAll();
-}

chrome/browser/ui/ash/assistant/platform_audio_input_host.h

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_UI_ASH_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_
-#define CHROME_BROWSER_UI_ASH_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_
-
-#include <memory>
-#include <vector>
-
-#include "base/memory/scoped_refptr.h"
-#include "base/memory/weak_ptr.h"
-#include "base/strings/string_piece_forward.h"
-#include "base/time/time.h"
-#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
-#include "mojo/public/cpp/bindings/interface_ptr_set.h"
-
-namespace media {
-class AudioInputController;
-}  // namespace media
-
-// Interacts with AudioController and forwards audio input stream to assistant.
-class PlatformAudioInputHost : public chromeos::assistant::mojom::AudioInput {
- public:
-  PlatformAudioInputHost();
-  ~PlatformAudioInputHost() override;
-
-  // mojom::AudioInput overrides:
-  void AddObserver(
-      chromeos::assistant::mojom::AudioInputObserverPtr observer) override;
-
-  void NotifyDataAvailable(const std::vector<int32_t>& data,
-                           int32_t frames,
-                           base::TimeTicks capture_time);
-  void NotifyAudioClosed();
-
- private:
-  class Writer;
-  class EventHandler;
-
-  std::unique_ptr<Writer> sync_writer_;
-  std::unique_ptr<EventHandler> event_handler_;
-  scoped_refptr<media::AudioInputController> audio_input_controller_;
-  mojo::InterfacePtrSet<chromeos::assistant::mojom::AudioInputObserver>
-      observers_;
-
-  bool recording_ = false;
-
-  base::WeakPtrFactory<PlatformAudioInputHost> weak_factory_;
-
-  DISALLOW_COPY_AND_ASSIGN(PlatformAudioInputHost);
-};
-
-#endif  // CHROME_BROWSER_UI_ASH_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_

chromeos/services/assistant/BUILD.gn

@@ -36,6 +36,7 @@ source_set("lib") {
   public_deps = [
     "//ash/public/cpp:cpp",
     "//mojo/public/cpp/bindings:bindings",
+    "//services/audio/public/cpp:cpp",
     "//services/service_manager/public/cpp:cpp",
   ]
 

chromeos/services/assistant/DEPS

@@ -3,9 +3,12 @@ include_rules = [
   "+chromeos/assistant",
   "+google_apis/gaia",
   "+libassistant",
+  "+media/base",
+  "+media/audio",
   "+mojo/public",
   "+services/device/public",
   "+services/identity/public",
   "+services/service_manager/public",
   "+ui/base",
+  "+services/audio/public",
 ]

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -32,10 +32,10 @@ const char kWiFiDeviceSettingId[] = "WIFI";
 const char kBluetoothDeviceSettingId[] = "BLUETOOTH";
 
 AssistantManagerServiceImpl::AssistantManagerServiceImpl(
-    mojom::AudioInputPtr audio_input,
+    service_manager::Connector* connector,
     device::mojom::BatteryMonitorPtr battery_monitor)
     : platform_api_(CreateLibAssistantConfig(),
-                    std::move(audio_input),
+                    connector,
                     std::move(battery_monitor)),
       action_module_(std::make_unique<action::CrosActionModule>(this)),
       display_connection_(std::make_unique<CrosDisplayConnection>(this)),

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -28,6 +28,10 @@ class AssistantManager;
 class AssistantManagerInternal;
 }  // namespace assistant_client
 
+namespace service_manager {
+class Connector;
+}  // namespace service_manager
+
 namespace chromeos {
 namespace assistant {
 
@@ -43,9 +47,8 @@ class AssistantManagerServiceImpl
       public assistant_client::ConversationStateListener,
       public assistant_client::AssistantManagerDelegate {
  public:
-  explicit AssistantManagerServiceImpl(
-      mojom::AudioInputPtr audio_input,
-      device::mojom::BatteryMonitorPtr battery_monitor);
+  AssistantManagerServiceImpl(service_manager::Connector* connector,
+                              device::mojom::BatteryMonitorPtr battery_monitor);
   ~AssistantManagerServiceImpl() override;
 
   // assistant::AssistantManagerService overrides

chromeos/services/assistant/manifest.json

@@ -6,7 +6,8 @@
       "requires": {
         "ash": [ "system_ui" ],
         "identity": [ "identity_manager" ],
-        "device": [ "device:battery_monitor" ]
+        "device": [ "device:battery_monitor" ],
+        "audio": [ "stream_factory" ]
       },
       "provides": {
         "assistant": [

chromeos/services/assistant/platform/audio_input_provider_impl.cc

@@ -7,6 +7,12 @@
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "libassistant/shared/public/platform_audio_buffer.h"
+#include "media/audio/audio_device_description.h"
+#include "media/base/audio_parameters.h"
+#include "media/base/audio_sample_types.h"
+#include "media/base/channel_layout.h"
+#include "services/audio/public/cpp/device_factory.h"
+#include "services/service_manager/public/cpp/connector.h"
 
 namespace chromeos {
 namespace assistant {
@@ -15,78 +21,136 @@ namespace {
 
 // This format should match //c/b/c/assistant/platform_audio_input_host.cc.
 constexpr assistant_client::BufferFormat kFormat{
-    16000 /* sample_rate */, assistant_client::INTERLEAVED_S32, 2 /* channels */
+    16000 /* sample_rate */, assistant_client::INTERLEAVED_S32, 1 /* channels */
 };
 
 }  // namespace
 
-class AudioInputBufferImpl : public assistant_client::AudioBuffer {
- public:
-  AudioInputBufferImpl(const void* data, uint32_t frame_count)
-      : data_(data), frame_count_(frame_count) {}
-  ~AudioInputBufferImpl() override = default;
-
-  // assistant_client::AudioBuffer overrides:
-  assistant_client::BufferFormat GetFormat() const override { return kFormat; }
-  const void* GetData() const override { return data_; }
-  void* GetWritableData() override {
-    NOTREACHED();
-    return nullptr;
-  }
-  int GetFrameCount() const override { return frame_count_; }
+AudioInputBufferImpl::AudioInputBufferImpl(const void* data,
+                                           uint32_t frame_count)
+    : data_(data), frame_count_(frame_count) {}
 
- private:
-  const void* data_;
-  int frame_count_;
-  DISALLOW_COPY_AND_ASSIGN(AudioInputBufferImpl);
-};
+AudioInputBufferImpl::~AudioInputBufferImpl() = default;
+
+assistant_client::BufferFormat AudioInputBufferImpl::GetFormat() const {
+  return kFormat;
+}
+
+const void* AudioInputBufferImpl::GetData() const {
+  return data_;
+}
+
+void* AudioInputBufferImpl::GetWritableData() {
+  NOTREACHED();
+  return nullptr;
+}
 
-AudioInputImpl::AudioInputImpl(mojom::AudioInputPtr audio_input)
-    : binding_(this) {
-  mojom::AudioInputObserverPtr observer;
-  binding_.Bind(mojo::MakeRequest(&observer));
-  audio_input->AddObserver(std::move(observer));
+int AudioInputBufferImpl::GetFrameCount() const {
+  return frame_count_;
 }
 
-AudioInputImpl::~AudioInputImpl() = default;
+AudioInputImpl::AudioInputImpl(
+    std::unique_ptr<service_manager::Connector> connector)
+    : source_(audio::CreateInputDevice(
+          std::move(connector),
+          media::AudioDeviceDescription::kDefaultDeviceId)),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      weak_factory_(this) {
+  DETACH_FROM_SEQUENCE(observer_sequence_checker_);
+  // AUDIO_PCM_LINEAR and AUDIO_PCM_LOW_LATENCY are the same on CRAS.
+  source_->Initialize(
+      media::AudioParameters(
+          media::AudioParameters::AUDIO_PCM_LOW_LATENCY,
+          media::CHANNEL_LAYOUT_MONO, kFormat.sample_rate,
+          kFormat.sample_rate / 10 /* buffer size for 100 ms */),
+      this);
+}
+
+AudioInputImpl::~AudioInputImpl() {
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  source_->Stop();
+}
 
-void AudioInputImpl::OnAudioInputFramesAvailable(
-    const std::vector<int32_t>& buffer,
-    uint32_t frame_count,
-    base::TimeTicks timestamp) {
-  AudioInputBufferImpl input_buffer(buffer.data(), frame_count);
-  int64_t time = timestamp.since_origin().InMilliseconds();
+void AudioInputImpl::Capture(const media::AudioBus* audio_source,
+                             int audio_delay_milliseconds,
+                             double volume,
+                             bool key_pressed) {
+  DCHECK_EQ(kFormat.num_channels, audio_source->channels());
+  std::vector<int32_t> buffer(kFormat.num_channels * audio_source->frames());
+  audio_source->ToInterleaved<media::SignedInt32SampleTypeTraits>(
+      audio_source->frames(), buffer.data());
+  int64_t time = base::TimeTicks::Now().since_origin().InMilliseconds() -
+                 audio_delay_milliseconds;
+  AudioInputBufferImpl input_buffer(buffer.data(), audio_source->frames());
   {
-    base::AutoLock auto_lock(lock_);
+    base::AutoLock lock(lock_);
     for (auto* observer : observers_)
       observer->OnBufferAvailable(input_buffer, time);
   }
 }
 
-void AudioInputImpl::OnAudioInputClosed() {
-  base::AutoLock auto_lock(lock_);
+void AudioInputImpl::OnCaptureError(const std::string& message) {
+  DLOG(ERROR) << "Capture error " << message;
+  base::AutoLock lock(lock_);
   for (auto* observer : observers_)
-    observer->OnStopped();
+    observer->OnError(AudioInput::Error::FATAL_ERROR);
 }
 
+void AudioInputImpl::OnCaptureMuted(bool is_muted) {}
+
 assistant_client::BufferFormat AudioInputImpl::GetFormat() const {
   return kFormat;
 }
 
 void AudioInputImpl::AddObserver(
     assistant_client::AudioInput::Observer* observer) {
-  base::AutoLock auto_lock(lock_);
-  observers_.push_back(observer);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(observer_sequence_checker_);
+  bool should_start = false;
+  {
+    base::AutoLock lock(lock_);
+    observers_.push_back(observer);
+    should_start = observers_.size() == 1;
+  }
+
+  if (should_start) {
+    // Post to main thread runner to start audio recording. Assistant thread
+    // does not have thread context defined in //base and will fail sequence
+    // check in AudioCapturerSource::Start().
+    task_runner_->PostTask(FROM_HERE,
+                           base::BindOnce(&AudioInputImpl::StartRecording,
+                                          weak_factory_.GetWeakPtr()));
+  }
 }
 
 void AudioInputImpl::RemoveObserver(
     assistant_client::AudioInput::Observer* observer) {
-  base::AutoLock auto_lock(lock_);
-  base::Erase(observers_, observer);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(observer_sequence_checker_);
+  bool should_stop = false;
+  {
+    base::AutoLock lock(lock_);
+    base::Erase(observers_, observer);
+    should_stop = observers_.empty();
+  }
+  if (should_stop) {
+    task_runner_->PostTask(FROM_HERE,
+                           base::BindOnce(&AudioInputImpl::StopRecording,
+                                          weak_factory_.GetWeakPtr()));
+  }
+}
+
+void AudioInputImpl::StartRecording() {
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  source_->Start();
+}
+
+void AudioInputImpl::StopRecording() {
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  source_->Stop();
 }
 
-AudioInputProviderImpl::AudioInputProviderImpl(mojom::AudioInputPtr audio_input)
-    : audio_input_(std::move(audio_input)) {}
+AudioInputProviderImpl::AudioInputProviderImpl(
+    service_manager::Connector* connector)
+    : audio_input_(connector->Clone()) {}
 
 AudioInputProviderImpl::~AudioInputProviderImpl() = default;
 

chromeos/services/assistant/platform/audio_input_provider_impl.h

@@ -9,45 +9,90 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/observer_list.h"
+#include "base/sequence_checker.h"
 #include "base/synchronization/lock.h"
 #include "base/time/time.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "libassistant/shared/public/platform_audio_input.h"
+#include "media/base/audio_capturer_source.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
+namespace service_manager {
+class Connector;
+}  // namespace service_manager
+
+namespace media {
+class AudioBus;
+}  // namespace media
+
 namespace chromeos {
 namespace assistant {
 
+class AudioInputBufferImpl : public assistant_client::AudioBuffer {
+ public:
+  AudioInputBufferImpl(const void* data, uint32_t frame_count);
+  ~AudioInputBufferImpl() override;
+
+  // assistant_client::AudioBuffer overrides:
+  assistant_client::BufferFormat GetFormat() const override;
+  const void* GetData() const override;
+  void* GetWritableData() override;
+  int GetFrameCount() const override;
+
+ private:
+  const void* data_;
+  int frame_count_;
+  DISALLOW_COPY_AND_ASSIGN(AudioInputBufferImpl);
+};
+
 class AudioInputImpl : public assistant_client::AudioInput,
-                       public mojom::AudioInputObserver {
+                       public media::AudioCapturerSource::CaptureCallback {
  public:
-  explicit AudioInputImpl(mojom::AudioInputPtr audio_input);
+  explicit AudioInputImpl(
+      std::unique_ptr<service_manager::Connector> connector);
   ~AudioInputImpl() override;
 
-  // mojom::AudioInputObserver overrides:
-  void OnAudioInputFramesAvailable(const std::vector<int32_t>& buffer,
-                                   uint32_t frame_count,
-                                   base::TimeTicks timestamp) override;
-  void OnAudioInputClosed() override;
+  // media::AudioCapturerSource::CaptureCallback overrides:
+  void Capture(const media::AudioBus* audio_source,
+               int audio_delay_milliseconds,
+               double volume,
+               bool key_pressed) override;
+  void OnCaptureError(const std::string& message) override;
+  void OnCaptureMuted(bool is_muted) override;
 
-  // assistant_client::AudioInput overrides:
+  // assistant_client::AudioInput overrides. These function are called by
+  // assistant from assistant thread, for which we should not assume any
+  // //base related thread context to be in place.
   assistant_client::BufferFormat GetFormat() const override;
   void AddObserver(assistant_client::AudioInput::Observer* observer) override;
   void RemoveObserver(
       assistant_client::AudioInput::Observer* observer) override;
 
  private:
-  // Protects |observers_| access becase AssistantManager calls
-  // Add/RemoveObserver on its own thread.
+  void StartRecording();
+  void StopRecording();
+
+  scoped_refptr<media::AudioCapturerSource> source_;
+
+  // Guards observers_;
   base::Lock lock_;
   std::vector<assistant_client::AudioInput::Observer*> observers_;
-  mojo::Binding<mojom::AudioInputObserver> binding_;
+
+  // To be initialized on assistant thread the first call to AddObserver.
+  // It ensures that AddObserver / RemoveObserver are called on the same
+  // sequence.
+  SEQUENCE_CHECKER(observer_sequence_checker_);
+
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  base::WeakPtrFactory<AudioInputImpl> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(AudioInputImpl);
 };
 
 class AudioInputProviderImpl : public assistant_client::AudioInputProvider {
  public:
-  explicit AudioInputProviderImpl(mojom::AudioInputPtr audio_input);
+  explicit AudioInputProviderImpl(service_manager::Connector* connector);
   ~AudioInputProviderImpl() override;
 
   // assistant_client::AudioInputProvider overrides:

chromeos/services/assistant/platform_api_impl.cc

@@ -74,9 +74,9 @@ void PlatformApiImpl::DummyAuthProvider::Reset() {}
 
 PlatformApiImpl::PlatformApiImpl(
     const std::string& config,
-    mojom::AudioInputPtr audio_input,
+    service_manager::Connector* connector,
     device::mojom::BatteryMonitorPtr battery_monitor)
-    : audio_input_provider_(std::move(audio_input)),
+    : audio_input_provider_(connector),
       audio_output_provider_(config, this),
       system_provider_(std::move(battery_monitor)) {}
 

chromeos/services/assistant/platform_api_impl.h

@@ -24,6 +24,10 @@
 #include "libassistant/shared/public/platform_auth.h"
 #include "services/device/public/mojom/battery_monitor.mojom.h"
 
+namespace service_manager {
+class Connector;
+}  // namespace service_manager
+
 namespace chromeos {
 namespace assistant {
 
@@ -31,7 +35,7 @@ namespace assistant {
 class PlatformApiImpl : public assistant_client::PlatformApi {
  public:
   PlatformApiImpl(const std::string& config,
-                  mojom::AudioInputPtr audio_input,
+                  service_manager::Connector* connector,
                   device::mojom::BatteryMonitorPtr battery_monitor);
   ~PlatformApiImpl() override;
 

chromeos/services/assistant/public/mojom/assistant.mojom

@@ -67,22 +67,18 @@ interface AssistantEventSubscriber {
   OnSpeechLevelUpdated(float speech_level);
 };
 
-// Platform connection to assistant.
+// Interface for browser to bind and start assistant.
 interface AssistantPlatform {
   // Initiates assistant and provides interfaces for assistant to call into the
   // browser.
-  Init(Client assistant_client_interface, Context context_interface,
-       AudioInput audio_input_interface);
+  Init(Client assistant_client_interface);
 };
 
 // Interface for assistant to call into client.
 interface Client {
   // Notifies assistant client that assistant running status has changed.
   OnAssistantStatusChanged(bool running);
-};
 
-// Interface for assistant to request view hierarchy for current focused window.
-interface Context {
   // Request context of current window from browser.
   RequestAssistantStructure() => (
     ax.mojom.AssistantExtra? extra,

chromeos/services/assistant/service.cc

@@ -117,31 +117,6 @@ void Service::SuspendDone(const base::TimeDelta& sleep_duration) {
   }
 }
 
-void Service::Init(mojom::ClientPtr client,
-                   mojom::ContextPtr assistant_context,
-                   mojom::AudioInputPtr audio_input) {
-  client_ = std::move(client);
-
-  // unit tests may set it early, so we don't need to create one.
-  if (!assistant_manager_service_) {
-#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
-    device::mojom::BatteryMonitorPtr battery_monitor;
-    context()->connector()->BindInterface(device::mojom::kServiceName,
-                                          mojo::MakeRequest(&battery_monitor));
-
-    assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>(
-        std::move(audio_input), std::move(battery_monitor));
-#else
-    assistant_manager_service_ =
-        std::make_unique<FakeAssistantManagerServiceImpl>();
-#endif
-  }
-
-  // This will eventually trigger the actual start of assistant services because
-  // they all depend on it.
-  RequestAccessToken();
-}
-
 void Service::OnSessionActivated(bool activated) {
   DCHECK(client_);
   session_active_ = activated;
@@ -185,6 +160,24 @@ void Service::RetryRefreshToken() {
                               &Service::RequestAccessToken);
 }
 
+void Service::Init(mojom::ClientPtr client) {
+  client_ = std::move(client);
+#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
+  device::mojom::BatteryMonitorPtr battery_monitor;
+  context()->connector()->BindInterface(device::mojom::kServiceName,
+                                        mojo::MakeRequest(&battery_monitor));
+  assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>(
+      context()->connector(), std::move(battery_monitor));
+#else
+  assistant_manager_service_ =
+      std::make_unique<FakeAssistantManagerServiceImpl>();
+#endif
+
+  // This will eventually trigger the actual start of assistant services because
+  // they all depend on it.
+  RequestAccessToken();
+}
+
 void Service::GetPrimaryAccountInfoCallback(
     const base::Optional<AccountInfo>& account_info,
     const identity::AccountState& account_state) {

chromeos/services/assistant/service.h

@@ -67,9 +67,7 @@ class Service : public service_manager::Service,
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 
   // mojom::AssistantPlatform overrides:
-  void Init(mojom::ClientPtr client,
-            mojom::ContextPtr assistant_context,
-            mojom::AudioInputPtr audio_input) override;
+  void Init(mojom::ClientPtr client) override;
 
   // ash::mojom::SessionActivationObserver overrides:
   void OnSessionActivated(bool activated) override;

chromeos/services/assistant/service_unittest.cc

@@ -115,33 +115,12 @@ class FakeAssistantClient : mojom::Client {
  private:
   // mojom::Client:
   void OnAssistantStatusChanged(bool running) override {}
-
-  mojo::Binding<mojom::Client> binding_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeAssistantClient);
-};
-
-class FakeAssistantContext : mojom::Context {
- public:
-  FakeAssistantContext() : binding_(this) {}
-
-  mojom::ContextPtr CreateInterfacePtrAndBind() {
-    mojom::ContextPtr ptr;
-    binding_.Bind(mojo::MakeRequest(&ptr));
-    return ptr;
-  }
-
- private:
-  // mojom::Context:
-  using RequestAssistantStructureCallback =
-      base::OnceCallback<void(::ax::mojom::AssistantExtraPtr,
-                              std::unique_ptr<ui::AssistantTree>)>;
   void RequestAssistantStructure(
       RequestAssistantStructureCallback callback) override {}
 
-  mojo::Binding<mojom::Context> binding_;
+  mojo::Binding<mojom::Client> binding_;
 
-  DISALLOW_COPY_AND_ASSIGN(FakeAssistantContext);
+  DISALLOW_COPY_AND_ASSIGN(FakeAssistantClient);
 };
 
 class FakeAudioInput : mojom::AudioInput {
@@ -232,9 +211,7 @@ class ServiceTest : public service_manager::test::ServiceTest {
   void SetUp() override {
     service_manager::test::ServiceTest::SetUp();
 
-    GetService()->Init(fake_assistant_client_->CreateInterfacePtrAndBind(),
-                       fake_assistant_context_->CreateInterfacePtrAndBind(),
-                       fake_audio_input_->CreateInterfacePtrAndBind());
+    GetService()->Init(fake_assistant_client_->CreateInterfacePtrAndBind());
     platform_service_.FlushForTesting();
     base::RunLoop().RunUntilIdle();
   }
@@ -245,7 +222,6 @@ class ServiceTest : public service_manager::test::ServiceTest {
   std::unique_ptr<service_manager::Service> CreateService() override {
     fake_identity_manager_ = std::make_unique<FakeIdentityManager>();
     fake_assistant_client_ = std::make_unique<FakeAssistantClient>();
-    fake_assistant_context_ = std::make_unique<FakeAssistantContext>();
     fake_audio_input_ = std::make_unique<FakeAudioInput>();
     fake_assistant_manager_ptr_ = new FakeAssistantManagerServiceImpl();
 
@@ -294,7 +270,6 @@ class ServiceTest : public service_manager::test::ServiceTest {
 
   std::unique_ptr<FakeIdentityManager> fake_identity_manager_;
   std::unique_ptr<FakeAssistantClient> fake_assistant_client_;
-  std::unique_ptr<FakeAssistantContext> fake_assistant_context_;
   std::unique_ptr<FakeAudioInput> fake_audio_input_;
 
   FakeAssistantManagerServiceImpl* fake_assistant_manager_ptr_;