Skip to content

GitLab

T
tangled
Commit c29e41c1 authored by Sergei Datsenko's avatar Sergei Datsenko Committed by Chromium LUCI CQ
Browse files
Options

Revert "Consolidate interactions with |CrasAudioHandler|." 

This reverts commit ccba8bc4.

Reason for revert: Causes Chrome to crash on Chrome OS: crbug.com/1164258

Original change's description:
> Consolidate interactions with |CrasAudioHandler|.
>
> Previously 2 places in the code were interacting with |CrasAudioHandler|:
>    - |PlatformApiImpl| subscribes to device changes, and passes this
>      information to |AudioInputHost|.
>    - |AudioInputHost| posts the new hotword model to |CrasAudioHandler|.
>
> This made it hard/impossible to split |AudioInputHost| from
> |PlatformApiImpl|, which is needed to handle audio input in the
> Libassistant mojom service.
>
> To solve this I moved all interactions with |CrasAudioHandler| inside a
> new class |AudioDevices|, of which an instance is owned by
> |AudioInputHost|.
>
> This also allowed me to add a bunch of new unittests.
>
> deployed on real hardware.
>
> Bug: b/171748795
> Test: chromeos_unittests --gtest_filter="Assistant*:Audio*" and manually
> Change-Id: I8ec9cec6672236f6365345b292efbe828518be69
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2610953
> Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Auto-Submit: Jeroen Dhollander <jeroendh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#841155}

TBR=xiaohuic@chromium.org,meilinw@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,jeroendh@chromium.org

Change-Id: I1f11ed9e14063f91e7f734caf0696748b3d3b7de
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/171748795
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615734
Commit-Queue: Sergei Datsenko <dats@chromium.org>
Reviewed-by: default avatarSergei Datsenko <dats@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841458}
parent 2afeb1a6
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 275 additions and 807 deletions
chromeos/services/assistant/BUILD.gn
View file @ c29e41c1
......@@ -26,8 +26,6 @@ component("lib") {
"fake_assistant_manager_service_impl.h",
"fake_assistant_settings_impl.cc",
"fake_assistant_settings_impl.h",
"platform/audio_devices.cc",
"platform/audio_devices.h",
"service.cc",
"service.h",
"service_context.h",
......@@ -176,7 +174,6 @@ source_set("tests") {
]
sources = [
"platform/audio_devices_unittest.cc",
"service_unittest.cc",
"test_support/fully_initialized_assistant_state.cc",
"test_support/fully_initialized_assistant_state.h",
......
chromeos/services/assistant/platform/audio_devices.cc deleted 100644 → 0
View file @ 2afeb1a6
// Copyright 2020 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 "chromeos/services/assistant/platform/audio_devices.h"
#include "base/metrics/histogram_functions.h"
#include "base/optional.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/system/sys_info.h"
#include "chromeos/audio/audio_device.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/services/assistant/public/cpp/features.h"
namespace chromeos {
namespace assistant {
namespace {
constexpr const char kDefaultLocale[] = "en_us";
// Hotword model is expected to have <language>_<region> format with lower
// case, while the locale in pref is stored as <language>-<region> with region
// code in capital letters. So we need to convert the pref locale to the
// correct format.
// Examples:
// "fr" -> "fr_fr"
// "nl-BE" -> "nl_be"
base::Optional<std::string> ToHotwordModel(std::string pref_locale) {
std::vector<std::string> code_strings = base::SplitString(
pref_locale, "-", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
if (code_strings.size() == 0) {
// Note: I am not sure this happens during real operations, but it
// definitely happens during the ChromeOS performance tests.
return base::nullopt;
}
DCHECK_LT(code_strings.size(), 3u);
// For locales with language code "en", use "en_all" hotword model.
if (code_strings[0] == "en")
return "en_all";
// If the language code and country code happen to be the same, e.g.
// France (FR) and French (fr), the locale will be stored as "fr" instead
// of "fr-FR" in the profile on Chrome OS.
if (code_strings.size() == 1)
return code_strings[0] + "_" + code_strings[0];
return code_strings[0] + "_" + base::ToLowerASCII(code_strings[1]);
}
const chromeos::AudioDevice* GetHighestPriorityDevice(
const chromeos::AudioDevice* left,
const chromeos::AudioDevice* right) {
if (!left)
return right;
if (!right)
return left;
return left->priority < right->priority ? right : left;
}
base::Optional<uint64_t> IdToOptional(const AudioDevice* device) {
if (!device)
return base::nullopt;
return device->id;
}
base::Optional<uint64_t> GetHotwordDeviceId(
const chromeos::AudioDeviceList& devices) {
const chromeos::AudioDevice* result = nullptr;
for (const chromeos::AudioDevice& device : devices) {
if (!device.is_input)
continue;
switch (device.type) {
case chromeos::AUDIO_TYPE_HOTWORD:
result = GetHighestPriorityDevice(result, &device);
break;
default:
// ignore other devices
break;
}
}
return IdToOptional(result);
}
base::Optional<uint64_t> GetPreferredDeviceId(
const chromeos::AudioDeviceList& devices) {
const chromeos::AudioDevice* result = nullptr;
for (const chromeos::AudioDevice& device : devices) {
if (!device.is_input)
continue;
switch (device.type) {
case chromeos::AUDIO_TYPE_USB:
case chromeos::AUDIO_TYPE_HEADPHONE:
case chromeos::AUDIO_TYPE_INTERNAL_MIC:
case chromeos::AUDIO_TYPE_FRONT_MIC:
result = GetHighestPriorityDevice(result, &device);
break;
default:
// ignore other devices
break;
}
}
return IdToOptional(result);
}
base::Optional<std::string> ToString(base::Optional<uint64_t> int_value) {
if (!int_value)
return base::nullopt;
return base::NumberToString(int_value.value());
}
} // namespace
// Observer that will report all changes to the audio devices.
// It will automatically be triggered when it's created,
// and will unsubscribe from |CrasAudioHandler| in its destructor.
class AudioDevices::ScopedCrasAudioHandlerObserver
: private CrasAudioHandler::AudioObserver {
public:
ScopedCrasAudioHandlerObserver(CrasAudioHandler* cras_audio_handler,
AudioDevices* parent)
: parent_(parent), cras_audio_handler_(cras_audio_handler) {
scoped_observer_.Observe(cras_audio_handler);
FetchAudioNodes();
}
ScopedCrasAudioHandlerObserver(const ScopedCrasAudioHandlerObserver&) =
delete;
ScopedCrasAudioHandlerObserver& operator=(
const ScopedCrasAudioHandlerObserver&) = delete;
~ScopedCrasAudioHandlerObserver() override = default;
private:
// CrasAudioHandler::AudioObserver implementation:
void OnAudioNodesChanged() override { FetchAudioNodes(); }
void FetchAudioNodes() {
if (!base::SysInfo::IsRunningOnChromeOS())
return;
chromeos::AudioDeviceList audio_devices;
cras_audio_handler_->GetAudioDevices(&audio_devices);
parent_->SetAudioDevices(audio_devices);
}
AudioDevices* const parent_;
// Owned by |AssistantManagerServiceImpl|.
CrasAudioHandler* const cras_audio_handler_;
base::ScopedObservation<CrasAudioHandler,
CrasAudioHandler::AudioObserver,
&CrasAudioHandler::AddAudioObserver,
&CrasAudioHandler::RemoveAudioObserver>
scoped_observer_{this};
};
// Sends the new hotword model to |cras_audio_handler|. If that fails this class
// will attempt to set the hotword model to |kDefaultLocale|.
class AudioDevices::HotwordModelUpdater {
public:
HotwordModelUpdater(CrasAudioHandler* cras_audio_handler,
uint64_t hotword_device,
const std::string& locale)
: cras_audio_handler_(cras_audio_handler),
hotword_device_(hotword_device),
locale_(locale) {
SendUpdate();
}
HotwordModelUpdater(const HotwordModelUpdater&) = delete;
HotwordModelUpdater& operator=(const HotwordModelUpdater&) = delete;
~HotwordModelUpdater() = default;
private:
void SendUpdate() {
std::string hotword_model =
ToHotwordModel(locale_).value_or(kDefaultLocale);
cras_audio_handler_->SetHotwordModel(
hotword_device_, hotword_model,
base::BindOnce(&HotwordModelUpdater::SetDspHotwordLocaleCallback,
weak_factory_.GetWeakPtr(), hotword_model));
}
void SetDspHotwordLocaleCallback(std::string pref_locale, bool success) {
base::UmaHistogramBoolean("Assistant.SetDspHotwordLocale", success);
if (success) {
VLOG(2) << "Successfully changed audio hotword model";
return;
}
LOG(ERROR) << "Set " << pref_locale
<< " hotword model failed, fallback to default locale.";
// Reset the locale to the default value if we failed to sync it to the
// locale stored in user's pref.
cras_audio_handler_->SetHotwordModel(
hotword_device_, /* hotword_model */ kDefaultLocale,
base::BindOnce([](bool success) {
if (!success)
LOG(ERROR) << "Reset to default hotword model failed.";
}));
}
CrasAudioHandler* const cras_audio_handler_;
uint64_t hotword_device_;
std::string locale_;
base::WeakPtrFactory<HotwordModelUpdater> weak_factory_{this};
};
AudioDevices::AudioDevices(CrasAudioHandler* cras_audio_handler,
const std::string& locale)
: scoped_cras_audio_handler_observer_(
std::make_unique<ScopedCrasAudioHandlerObserver>(cras_audio_handler,
this)),
cras_audio_handler_(cras_audio_handler),
locale_(locale) {}
AudioDevices::~AudioDevices() = default;
void AudioDevices::AddAndFireObserver(Observer* observer) {
DCHECK(observer);
observers_.AddObserver(observer);
observer->SetHotwordDeviceId(ToString(hotword_device_id_));
observer->SetDeviceId(ToString(device_id_));
}
void AudioDevices::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
void AudioDevices::SetLocale(const std::string& locale) {
locale_ = locale;
UpdateHotwordModel();
}
void AudioDevices::SetAudioDevicesForTest(
const chromeos::AudioDeviceList& audio_devices) {
SetAudioDevices(audio_devices);
}
void AudioDevices::SetAudioDevices(const chromeos::AudioDeviceList& devices) {
UpdateHotwordDeviceId(devices);
UpdateDeviceId(devices);
UpdateHotwordModel();
}
void AudioDevices::UpdateHotwordDeviceId(
const chromeos::AudioDeviceList& devices) {
hotword_device_id_ = GetHotwordDeviceId(devices);
VLOG(2) << "Changed audio hotword input device to "
<< ToString(hotword_device_id_).value_or("<none>");
for (auto& observer : observers_)
observer.SetHotwordDeviceId(ToString(hotword_device_id_));
}
void AudioDevices::UpdateDeviceId(const chromeos::AudioDeviceList& devices) {
device_id_ = GetPreferredDeviceId(devices);
VLOG(2) << "Changed audio input device to "
<< ToString(device_id_).value_or("<none>");
for (auto& observer : observers_)
observer.SetDeviceId(ToString(device_id_));
}
void AudioDevices::UpdateHotwordModel() {
if (!hotword_device_id_)
return;
if (!features::IsDspHotwordEnabled())
return;
hotword_model_updater_ = std::make_unique<HotwordModelUpdater>(
cras_audio_handler_, hotword_device_id_.value(), locale_);
}
} // namespace assistant
} // namespace chromeos
chromeos/services/assistant/platform/audio_devices.h deleted 100644 → 0
View file @ 2afeb1a6
// Copyright 2020 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 CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_DEVICES_H_
#define CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_DEVICES_H_
#include <cstdint>
#include "base/component_export.h"
#include "base/observer_list.h"
#include "base/optional.h"
#include "base/scoped_observation.h"
#include "chromeos/audio/audio_device.h"
namespace chromeos {
class CrasAudioHandler;
} // namespace chromeos
namespace chromeos {
namespace assistant {
// This class will monitor the available audio devices (through
// |CrasAudioHandler|), and select the devices to use for audio input (both
// regular input and hotword detection).
// When the selected devices change, this class will:
// - Inform the observers.
// - Find the hotword model to use, and send it to
// CrasAudioHandler::SetHotwordModel().
class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioDevices {
public:
class Observer : public base::CheckedObserver {
public:
~Observer() override = default;
// Set the input device to use for audio capture.
virtual void SetDeviceId(const base::Optional<std::string>& device_id) = 0;
// Set the input device to use for hardware based hotword detection.
virtual void SetHotwordDeviceId(
const base::Optional<std::string>& device_id) = 0;
};
AudioDevices(CrasAudioHandler* cras_audio_handler, const std::string& locale);
AudioDevices(const AudioDevices&) = delete;
AudioDevices& operator=(const AudioDevices&) = delete;
~AudioDevices();
void AddAndFireObserver(Observer*);
void RemoveObserver(Observer*);
void SetLocale(const std::string& locale);
// Used during unittests to simulate an update to the list of available audio
// devices.
void SetAudioDevicesForTest(const chromeos::AudioDeviceList& audio_devices);
using ScopedObservation =
base::ScopedObservation<AudioDevices,
Observer,
&AudioDevices::AddAndFireObserver,
&AudioDevices::RemoveObserver>;
private:
class ScopedCrasAudioHandlerObserver;
class HotwordModelUpdater;
void SetAudioDevices(const chromeos::AudioDeviceList& audio_devices);
void UpdateHotwordDeviceId(const chromeos::AudioDeviceList& devices);
void UpdateDeviceId(const chromeos::AudioDeviceList& devices);
void UpdateHotwordModel();
// Observes changes to the available audio devices, and sends the list of
// devices to SetAudioDevices().
std::unique_ptr<ScopedCrasAudioHandlerObserver>
scoped_cras_audio_handler_observer_;
// Handles the asynchronous nature of sending a new hotword model to
// |cras_audio_handler_|.
std::unique_ptr<HotwordModelUpdater> hotword_model_updater_;
base::ObserverList<Observer> observers_;
// Owned by |AssistantManagerServiceImpl|.
CrasAudioHandler* const cras_audio_handler_;
std::string locale_;
base::Optional<uint64_t> hotword_device_id_;
base::Optional<uint64_t> device_id_;
};
} // namespace assistant
} // namespace chromeos
#endif // CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_DEVICES_H_
chromeos/services/assistant/platform/audio_input_host.cc
View file @ c29e41c1
......@@ -5,8 +5,11 @@
#include "chromeos/services/assistant/platform/audio_input_host.h"
#include "base/check.h"
#include "base/metrics/histogram_functions.h"
#include "base/optional.h"
#include "chromeos/services/assistant/platform/audio_devices.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/services/assistant/platform/audio_input_impl.h"
#include "chromeos/services/assistant/public/cpp/features.h"
......@@ -15,6 +18,8 @@ namespace assistant {
namespace {
constexpr const char kDefaultLocale[] = "en_us";
AudioInputImpl::LidState ConvertLidState(
chromeos::PowerManagerClient::LidState state) {
switch (state) {
......@@ -28,21 +33,52 @@ AudioInputImpl::LidState ConvertLidState(
}
}
// Hotword model is expected to have <language>_<region> format with lower
// case, while the locale in pref is stored as <language>-<region> with region
// code in capital letters. So we need to convert the pref locale to the
// correct format.
// Examples:
// "fr" -> "fr_fr"
// "nl-BE" -> "nl_be"
base::Optional<std::string> ToHotwordModel(std::string pref_locale) {
std::vector<std::string> code_strings = base::SplitString(
pref_locale, "-", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
if (code_strings.size() == 0) {
// Note: I am not sure this happens during real operations, but it
// definitely happens during the ChromeOS performance tests.
return base::nullopt;
}
DCHECK_LT(code_strings.size(), 3u);
// For locales with language code "en", use "en_all" hotword model.
if (code_strings[0] == "en")
return "en_all";
// If the language code and country code happen to be the same, e.g.
// France (FR) and French (fr), the locale will be stored as "fr" instead
// of "fr-FR" in the profile on Chrome OS.
if (code_strings.size() == 1)
return code_strings[0] + "_" + code_strings[0];
return code_strings[0] + "_" + base::ToLowerASCII(code_strings[1]);
}
} // namespace
chromeos::assistant::AudioInputHost::AudioInputHost(
AudioInputImpl* audio_input,
CrasAudioHandler* cras_audio_handler,
chromeos::PowerManagerClient* power_manager_client,
const std::string& locale)
chromeos::PowerManagerClient* power_manager_client)
: audio_input_(audio_input),
cras_audio_handler_(cras_audio_handler),
power_manager_client_(power_manager_client),
power_manager_client_observer_(this),
audio_devices_(cras_audio_handler, locale) {
power_manager_client_observer_(this) {
DCHECK(audio_input_);
DCHECK(cras_audio_handler_);
DCHECK(power_manager_client_);
audio_devices_observation_.Observe(&audio_devices_);
power_manager_client_observer_.Observe(power_manager_client);
power_manager_client->GetSwitchStates(base::BindOnce(
&AudioInputHost::OnInitialLidStateReceived, weak_factory_.GetWeakPtr()));
......@@ -54,8 +90,8 @@ void AudioInputHost::SetMicState(bool mic_open) {
audio_input_->SetMicState(mic_open);
}
void AudioInputHost::SetDeviceId(const base::Optional<std::string>& device_id) {
audio_input_->SetDeviceId(device_id.value_or(""));
void AudioInputHost::SetDeviceId(const std::string& device_id) {
audio_input_->SetDeviceId(device_id);
}
void AudioInputHost::OnConversationTurnStarted() {
......@@ -75,9 +111,48 @@ void AudioInputHost::OnHotwordEnabled(bool enable) {
audio_input_->OnHotwordEnabled(enable);
}
void AudioInputHost::SetHotwordDeviceId(
const base::Optional<std::string>& device_id) {
audio_input_->SetHotwordDeviceId(device_id.value_or(""));
void AudioInputHost::SetHotwordDeviceId(const std::string& device_id) {
hotword_device_id_ = device_id;
audio_input_->SetHotwordDeviceId(device_id);
}
void AudioInputHost::SetDspHotwordLocale(std::string pref_locale) {
if (!features::IsDspHotwordEnabled())
return;
std::string hotword_model =
ToHotwordModel(pref_locale).value_or(kDefaultLocale);
cras_audio_handler_->SetHotwordModel(
GetDspNodeId(), hotword_model,
base::BindOnce(&AudioInputHost::SetDspHotwordLocaleCallback,
weak_factory_.GetWeakPtr(), hotword_model));
}
void AudioInputHost::SetDspHotwordLocaleCallback(std::string pref_locale,
bool success) {
base::UmaHistogramBoolean("Assistant.SetDspHotwordLocale", success);
if (success)
return;
LOG(ERROR) << "Set " << pref_locale
<< " hotword model failed, fallback to default locale.";
// Reset the locale to the default value if we failed to sync it to the locale
// stored in user's pref.
cras_audio_handler_->SetHotwordModel(
GetDspNodeId(), /* hotword_model */ kDefaultLocale,
base::BindOnce([](bool success) {
if (!success)
LOG(ERROR) << "Reset to default hotword model failed.";
}));
}
uint64_t AudioInputHost::GetDspNodeId() const {
DCHECK(!hotword_device_id_.empty());
uint64_t result;
bool success = base::StringToUint64(hotword_device_id_, &result);
DCHECK(success) << "Invalid hotword device id '" << hotword_device_id_ << "'";
return result;
}
void AudioInputHost::LidEventReceived(
......
chromeos/services/assistant/platform/audio_input_host.h
View file @ c29e41c1
......@@ -11,7 +11,10 @@
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/services/assistant/platform/audio_devices.h"
namespace chromeos {
class CrasAudioHandler;
} // namespace chromeos
namespace chromeos {
namespace assistant {
......@@ -24,15 +27,11 @@ class AudioInputImpl;
// This will allow us to move it to the Libassistant mojom service (at which
// point this class will talk to the Libassistant mojom service).
class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputHost
: private chromeos::PowerManagerClient::Observer,
private AudioDevices::Observer
{
: public chromeos::PowerManagerClient::Observer {
public:
AudioInputHost(AudioInputImpl* audio_input,
CrasAudioHandler* cras_audio_handler,
chromeos::PowerManagerClient* power_manager_client,
const std::string& locale);
chromeos::PowerManagerClient* power_manager_client);
AudioInputHost(AudioInputHost&) = delete;
AudioInputHost& operator=(AudioInputHost&) = delete;
~AudioInputHost() override;
......@@ -40,18 +39,26 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputHost
// Called when the mic state associated with the interaction is changed.
void SetMicState(bool mic_open);
// Setting the input device to use for audio capture.
void SetDeviceId(const std::string& device_id);
// Called when hotword enabled status changed.
void OnHotwordEnabled(bool enable);
// Setting the hotword input device with hardware based hotword detection.
void SetHotwordDeviceId(const std::string& device_id);
// Setting the hotword locale for the input device with DSP support.
void SetDspHotwordLocale(std::string pref_locale);
void OnConversationTurnStarted();
void OnConversationTurnFinished();
// AudioDevices::Observer implementation:
void SetDeviceId(const base::Optional<std::string>& device_id) override;
void SetHotwordDeviceId(
const base::Optional<std::string>& device_id) override;
private:
void SetDspHotwordLocaleCallback(std::string pref_locale, bool success);
uint64_t GetDspNodeId() const;
// chromeos::PowerManagerClient::Observer overrides:
void LidEventReceived(chromeos::PowerManagerClient::LidState state,
base::TimeTicks timestamp) override;
......@@ -61,15 +68,14 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputHost
// Owned by |PlatformApiImpl| which also owns |this|.
AudioInputImpl* const audio_input_;
CrasAudioHandler* const cras_audio_handler_;
chromeos::PowerManagerClient* const power_manager_client_;
base::ScopedObservation<chromeos::PowerManagerClient,
chromeos::PowerManagerClient::Observer>
power_manager_client_observer_;
// Observes available audio devices and will set device-id/hotword-device-id
// accordingly.
AudioDevices audio_devices_;
AudioDevices::ScopedObservation audio_devices_observation_{this};
// Hotword input device used for hardware based hotword detection.
std::string hotword_device_id_;
base::WeakPtrFactory<AudioInputHost> weak_factory_{this};
};
......
chromeos/services/assistant/platform/audio_input_impl.cc
View file @ c29e41c1
......@@ -283,7 +283,6 @@ void AudioInputImpl::RemoveObserver(
DCHECK_CALLED_ON_VALID_SEQUENCE(observer_sequence_checker_);
if (open_audio_stream_)
VLOG(1) << open_audio_stream_->device_id() << " remove observer";
bool have_no_observer = false;
{
base::AutoLock lock(lock_);
......
chromeos/services/assistant/platform/audio_input_impl_unittest.cc
View file @ c29e41c1
......@@ -11,6 +11,7 @@
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/dbus/audio/fake_cras_audio_client.h"
#include "chromeos/dbus/power/fake_power_manager_client.h"
#include "chromeos/services/assistant/platform/audio_input_host.h"
#include "chromeos/services/assistant/platform/audio_stream_factory_delegate.h"
......@@ -47,14 +48,22 @@ class ScopedFakeAssistantClient : public ScopedAssistantClient {
DISALLOW_COPY_AND_ASSIGN(ScopedFakeAssistantClient);
};
class ScopedCrasAudioHandler {
// Mock for |CrosAudioClient|. This inherits from |FakeCrasAudioClient| so we
// only have to mock the methods we're interested in.
// It will automatically be installed as the global singleton in its
// constructor, and removed in the destructor.
class ScopedCrasAudioClientMock : public FakeCrasAudioClient {
public:
ScopedCrasAudioHandler() { CrasAudioHandler::InitializeForTesting(); }
ScopedCrasAudioHandler(const ScopedCrasAudioHandler&) = delete;
ScopedCrasAudioHandler& operator=(const ScopedCrasAudioHandler&) = delete;
~ScopedCrasAudioHandler() { CrasAudioHandler::Shutdown(); }
CrasAudioHandler* Get() { return CrasAudioHandler::Get(); }
ScopedCrasAudioClientMock() = default;
ScopedCrasAudioClientMock(ScopedCrasAudioClientMock&) = delete;
ScopedCrasAudioClientMock& operator=(ScopedCrasAudioClientMock&) = delete;
~ScopedCrasAudioClientMock() override = default;
MOCK_METHOD(void,
SetHotwordModel,
(uint64_t node_id,
const std::string& hotword_model,
VoidDBusMethodCallback callback));
};
} // namespace
......@@ -67,6 +76,7 @@ class AudioInputImplTest : public testing::Test,
scoped_feature_list_.InitAndEnableFeature(features::kEnableDspHotword);
PowerManagerClient::InitializeFake();
CrasAudioHandler::InitializeForTesting();
CreateNewAudioInputImpl();
}
......@@ -77,6 +87,7 @@ class AudioInputImplTest : public testing::Test,
// |audio_input_host_| uses the fake power manager client, so must be
// destroyed before the power manager client.
audio_input_host_.reset();
CrasAudioHandler::Shutdown();
chromeos::PowerManagerClient::Shutdown();
}
......@@ -102,9 +113,8 @@ class AudioInputImplTest : public testing::Test,
&audio_stream_factory_delegate_, "fake-device-id");
audio_input_host_ = std::make_unique<AudioInputHost>(
audio_input_impl_.get(), cras_audio_handler_.Get(),
FakePowerManagerClient::Get(), "initial-locale");
audio_input_host_->SetDeviceId("initial-device-id");
audio_input_impl_.get(), CrasAudioHandler::Get(),
FakePowerManagerClient::Get());
audio_input_impl_->AddObserver(this);
......@@ -122,6 +132,10 @@ class AudioInputImplTest : public testing::Test,
AudioInputHost& audio_input_host() { return *audio_input_host_; }
ScopedCrasAudioClientMock& cras_audio_client_mock() {
return cras_audio_client_mock_;
}
// assistant_client::AudioInput::Observer overrides:
void OnAudioBufferAvailable(const assistant_client::AudioBuffer& buffer,
int64_t timestamp) override {}
......@@ -150,7 +164,7 @@ class AudioInputImplTest : public testing::Test,
base::test::ScopedFeatureList scoped_feature_list_;
ScopedFakeAssistantClient fake_assistant_client_;
DefaultAudioStreamFactoryDelegate audio_stream_factory_delegate_;
ScopedCrasAudioHandler cras_audio_handler_;
::testing::NiceMock<ScopedCrasAudioClientMock> cras_audio_client_mock_;
std::unique_ptr<AudioInputImpl> audio_input_impl_;
std::unique_ptr<AudioInputHost> audio_input_host_;
......@@ -276,5 +290,81 @@ TEST_F(AudioInputImplTest,
EXPECT_FALSE(IsUsingDeadStreamDetection());
}
TEST_F(AudioInputImplTest, ShouldSendHotwordLocaleToCrasAudioClient) {
StopAudioRecording();
audio_input_host().SetHotwordDeviceId("111");
EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel);
audio_input_host().SetDspHotwordLocale("bla");
}
TEST_F(AudioInputImplTest,
ShouldFormatHotwordLocaleAndSendItToCrasAudioClient) {
StopAudioRecording();
audio_input_host().SetHotwordDeviceId("111");
// Normal case
EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(111, "nl_be", _));
audio_input_host().SetDspHotwordLocale("nl-BE");
// Handle the case where country code and language code are the same
EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(111, "fr_fr", _));
audio_input_host().SetDspHotwordLocale("fr");
// use "en_all" for all english locales
EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(111, "en_all", _));
audio_input_host().SetDspHotwordLocale("en-US");
}
TEST_F(AudioInputImplTest, ShouldUseDefaultLocaleIfUserPrefIsRejected) {
const std::string default_locale = "en_us";
StopAudioRecording();
audio_input_host().SetHotwordDeviceId("222");
EXPECT_CALL(cras_audio_client_mock(),
SetHotwordModel(222, "rejected_locale", _))
.WillOnce([](uint64_t node_id, const std::string&,
VoidDBusMethodCallback callback) {
// Report failure to change the locale
std::move(callback).Run(/*success=*/false);
});
EXPECT_CALL(cras_audio_client_mock(),
SetHotwordModel(222, default_locale, _));
audio_input_host().SetDspHotwordLocale("rejected-LOCALE");
}
TEST_F(AudioInputImplTest, ShouldUseDefaultLocaleIfUserPrefIsEmpty) {
const std::string default_locale = "en_us";
StopAudioRecording();
audio_input_host().SetHotwordDeviceId("222");
EXPECT_CALL(cras_audio_client_mock(),
SetHotwordModel(222, default_locale, _));
audio_input_host().SetDspHotwordLocale("");
}
TEST_F(AudioInputImplTest, ShouldDoNothingIfUserPrefIsAccepted) {
const std::string default_locale = "en_us";
StopAudioRecording();
audio_input_host().SetHotwordDeviceId("222");
EXPECT_CALL(cras_audio_client_mock(),
SetHotwordModel(222, "accepted_locale", _))
.WillOnce([](uint64_t node_id, const std::string&,
VoidDBusMethodCallback callback) {
// Accept the change to the locale.
std::move(callback).Run(/*success=*/true);
});
// Do not expect a second call if change of locale is accepted
EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(222, default_locale, _))
.Times(0);
audio_input_host().SetDspHotwordLocale("accepted-LOCALE");
}
} // namespace assistant
} // namespace chromeos
chromeos/services/assistant/platform_api_impl.cc
View file @ c29e41c1
......@@ -8,8 +8,9 @@
#include <utility>
#include <vector>
#include "base/system/sys_info.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/services/assistant/media_session/assistant_media_session.h"
#include "chromeos/services/assistant/platform/audio_devices.h"
#include "chromeos/services/assistant/platform/power_manager_provider_impl.h"
#include "chromeos/services/assistant/public/cpp/features.h"
#include "chromeos/services/assistant/utils.h"
......@@ -82,15 +83,16 @@ PlatformApiImpl::PlatformApiImpl(
mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor,
scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
const std::string& pref_locale)
std::string pref_locale)
: audio_input_provider_(),
audio_output_provider_(media_session,
background_task_runner,
media::AudioDeviceDescription::kDefaultDeviceId),
audio_input_host_(&audio_input_provider_.GetAudioInput(),
cras_audio_handler,
power_manager_client,
pref_locale) {
power_manager_client),
pref_locale_(pref_locale),
cras_audio_handler_(cras_audio_handler) {
// Only enable native power features if they are supported by the UI.
std::unique_ptr<PowerManagerProviderImpl> provider;
if (features::IsPowerManagerEnabled()) {
......@@ -99,9 +101,14 @@ PlatformApiImpl::PlatformApiImpl(
}
system_provider_ = std::make_unique<SystemProviderImpl>(
std::move(provider), std::move(battery_monitor));
cras_audio_handler_->AddAudioObserver(this);
OnAudioNodesChanged();
}
PlatformApiImpl::~PlatformApiImpl() = default;
PlatformApiImpl::~PlatformApiImpl() {
cras_audio_handler_->RemoveAudioObserver(this);
}
AudioInputProviderImpl& PlatformApiImpl::GetAudioInputProvider() {
return audio_input_provider_;
......@@ -127,6 +134,50 @@ SystemProvider& PlatformApiImpl::GetSystemProvider() {
return *system_provider_;
}
void PlatformApiImpl::OnAudioNodesChanged() {
if (!base::SysInfo::IsRunningOnChromeOS())
return;
chromeos::AudioDeviceList devices;
cras_audio_handler_->GetAudioDevices(&devices);
const chromeos::AudioDevice* input_device = nullptr;
const chromeos::AudioDevice* hotword_device = nullptr;
for (const chromeos::AudioDevice& device : devices) {
if (!device.is_input)
continue;
switch (device.type) {
case chromeos::AUDIO_TYPE_USB:
case chromeos::AUDIO_TYPE_HEADPHONE:
case chromeos::AUDIO_TYPE_INTERNAL_MIC:
case chromeos::AUDIO_TYPE_FRONT_MIC:
if (!input_device || input_device->priority < device.priority)
input_device = &device;
break;
case chromeos::AUDIO_TYPE_HOTWORD:
if (!hotword_device || hotword_device->priority < device.priority)
hotword_device = &device;
break;
default:
// ignore other devices
break;
}
}
audio_input_host_.SetDeviceId(
input_device ? base::NumberToString(input_device->id) : std::string());
if (hotword_device) {
audio_input_host_.SetHotwordDeviceId(
base::NumberToString(hotword_device->id));
audio_input_host_.SetDspHotwordLocale(pref_locale_);
} else {
audio_input_host_.SetHotwordDeviceId(std::string());
}
}
void PlatformApiImpl::SetMicState(bool mic_open) {
audio_input_host_.SetMicState(mic_open);
}
......
chromeos/services/assistant/platform_api_impl.h
View file @ c29e41c1
......@@ -10,6 +10,7 @@
#include <utility>
#include <vector>
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/services/assistant/platform/audio_input_host.h"
#include "chromeos/services/assistant/platform/audio_input_provider_impl.h"
#include "chromeos/services/assistant/platform/audio_output_provider_impl.h"
......@@ -23,7 +24,6 @@
#include "services/device/public/mojom/battery_monitor.mojom.h"
namespace chromeos {
class CrasAudioHandler;
class PowerManagerClient;
namespace assistant {
......@@ -31,7 +31,8 @@ namespace assistant {
class AssistantMediaSession;
// Platform API required by the voice assistant.
class PlatformApiImpl : public CrosPlatformApi {
class PlatformApiImpl : public CrosPlatformApi,
CrasAudioHandler::AudioObserver {
public:
PlatformApiImpl(
AssistantMediaSession* media_session,
......@@ -40,7 +41,7 @@ class PlatformApiImpl : public CrosPlatformApi {
mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor,
scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
const std::string& pref_locale);
std::string pref_locale);
~PlatformApiImpl() override;
// assistant_client::PlatformApi overrides
......@@ -51,6 +52,9 @@ class PlatformApiImpl : public CrosPlatformApi {
assistant_client::NetworkProvider& GetNetworkProvider() override;
assistant_client::SystemProvider& GetSystemProvider() override;
// chromeos::CrasAudioHandler::AudioObserver overrides
void OnAudioNodesChanged() override;
// Called when the mic state associated with the interaction is changed.
void SetMicState(bool mic_open) override;
......@@ -100,6 +104,9 @@ class PlatformApiImpl : public CrosPlatformApi {
NetworkProviderImpl network_provider_;
AudioInputHost audio_input_host_;
std::unique_ptr<SystemProviderImpl> system_provider_;
std::string pref_locale_;
CrasAudioHandler* const cras_audio_handler_;
DISALLOW_COPY_AND_ASSIGN(PlatformApiImpl);
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment