Skip to content

GitLab

T
tangled
Commit 4848c486 authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Chromium LUCI CQ
Browse files
Options

Revert "Remove external dependencies from |AudioInputImpl|" 

This reverts commit 340ff5b3.

Reason for revert: causes crash crbug.com/1158611

Original change's description:
> Remove external dependencies from |AudioInputImpl|
>
> This will allow us to easier move this class to the Libassistant mojm
> service.
>
> All dependencies have been moved to the new class |AudioInputHost|.
> This class currently talks directly to |AudioInputImpl|, but later
> it will instead talk to the mojom APIs.
>
> Some notes:
>    - |AudioInputImpl| still depends on
>      |AssistantClient::Get()->RequestAudioStreamFactory()|, because the
>      Libassistant mojom service will also use an audio stream factory to
>      open/close its audio input. In a follow up CL I will introduce a
>      delegate to sever this dependency.
>    - There is one behavior change:
>      |power_manager_client_->NotifyWakeNotification()| was previously
>      only called when DSP was enabled, and it is now always called (when
>      Libassistant starts a conversation). I can think of no reason why
>      this would be an issue.
>
> Bug: b/171748795
> Test: chromeos_unittests, deployed on real hardware with and without DSP.
> Change-Id: I119521c7a72bf5361019d70240c02ba47a23d6ad
> Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582537
> Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#836710}

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

Change-Id: I2525fce21492c0417d0411db99fbb8463583cb02
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/171748795
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2591771Reviewed-by: default avatarJeroen Dhollander <jeroendh@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836891}
parent e441cf2a
Hide whitespace changes
Inline Side-by-side
Showing with 212 additions and 445 deletions
chromeos/services/assistant/BUILD.gn
View file @ 4848c486
......@@ -78,8 +78,6 @@ component("lib") {
"media_session/assistant_media_session.h",
"platform/audio_device_owner.cc",
"platform/audio_device_owner.h",
"platform/audio_input_host.cc",
"platform/audio_input_host.h",
"platform/audio_input_impl.cc",
"platform/audio_input_impl.h",
"platform/audio_input_provider_impl.cc",
......@@ -152,7 +150,6 @@ source_set("tests") {
"//chromeos/audio",
"//chromeos/constants",
"//chromeos/dbus:test_support",
"//chromeos/dbus/audio",
"//chromeos/dbus/power",
"//chromeos/services/assistant/public/cpp",
"//chromeos/services/assistant/public/mojom",
......
chromeos/services/assistant/platform/audio_input_host.cc deleted 100644 → 0
View file @ e441cf2a
// 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_input_host.h"
#include "base/check.h"
#include "base/metrics/histogram_functions.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"
namespace chromeos {
namespace assistant {
namespace {
AudioInputImpl::LidState ConvertLidState(
chromeos::PowerManagerClient::LidState state) {
switch (state) {
case chromeos::PowerManagerClient::LidState::CLOSED:
return AudioInputImpl::LidState::kClosed;
case chromeos::PowerManagerClient::LidState::OPEN:
return AudioInputImpl::LidState::kOpen;
case chromeos::PowerManagerClient::LidState::NOT_PRESENT:
// If there is no lid, it can't be closed.
return AudioInputImpl::LidState::kOpen;
}
}
// 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"
std::string ToHotwordModel(std::string pref_locale) {
std::vector<std::string> code_strings = base::SplitString(
pref_locale, "-", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
DCHECK_GT(code_strings.size(), 0u);
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)
: audio_input_(audio_input),
cras_audio_handler_(cras_audio_handler),
power_manager_client_(power_manager_client),
power_manager_client_observer_(this) {
DCHECK(audio_input_);
DCHECK(cras_audio_handler_);
DCHECK(power_manager_client_);
power_manager_client_observer_.Observe(power_manager_client);
power_manager_client->GetSwitchStates(base::BindOnce(
&AudioInputHost::OnInitialLidStateReceived, weak_factory_.GetWeakPtr()));
}
AudioInputHost::~AudioInputHost() = default;
void AudioInputHost::SetMicState(bool mic_open) {
audio_input_->SetMicState(mic_open);
}
void AudioInputHost::SetDeviceId(const std::string& device_id) {
audio_input_->SetDeviceId(device_id);
}
void AudioInputHost::OnConversationTurnStarted() {
audio_input_->OnConversationTurnStarted();
// Inform power manager of a wake notification when Libassistant
// recognized hotword and started a conversation. We intentionally
// avoid using |NotifyUserActivity| because it is not suitable for
// this case according to the Platform team.
power_manager_client_->NotifyWakeNotification();
}
void AudioInputHost::OnConversationTurnFinished() {
audio_input_->OnConversationTurnFinished();
}
void AudioInputHost::OnHotwordEnabled(bool enable) {
audio_input_->OnHotwordEnabled(enable);
}
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);
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 "en_us" if we failed to sync it to
// the locale stored in user's pref.
cras_audio_handler_->SetHotwordModel(
GetDspNodeId(), /* hotword_model */ "en_us",
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::PowerManagerClient::LidState state,
base::TimeTicks timestamp) {
// Lid switch event still gets fired during system suspend, which enables
// us to stop DSP recording correctly when user closes lid after the device
// goes to sleep.
audio_input_->OnLidStateChanged(ConvertLidState(state));
}
void AudioInputHost::OnInitialLidStateReceived(
base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states) {
if (switch_states.has_value())
audio_input_->OnLidStateChanged(ConvertLidState(switch_states->lid_state));
}
} // namespace assistant
} // namespace chromeos
chromeos/services/assistant/platform/audio_input_host.h deleted 100644 → 0
View file @ e441cf2a
// 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_INPUT_HOST_H_
#define CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_
#include <string>
#include "base/component_export.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "chromeos/dbus/power/power_manager_client.h"
namespace chromeos {
class CrasAudioHandler;
} // namespace chromeos
namespace chromeos {
namespace assistant {
class AudioInputImpl;
// Class that provides the bridge between the ChromeOS UI thread and the
// Libassistant audio input class.
// The goal is that |AudioInputImpl| no longer depends on any external events.
// 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
: public chromeos::PowerManagerClient::Observer {
public:
AudioInputHost(AudioInputImpl* audio_input,
CrasAudioHandler* cras_audio_handler,
chromeos::PowerManagerClient* power_manager_client);
AudioInputHost(AudioInputHost&) = delete;
AudioInputHost& operator=(AudioInputHost&) = delete;
~AudioInputHost() override;
// 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();
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;
void OnInitialLidStateReceived(
base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states);
// 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_;
// Hotword input device used for hardware based hotword detection.
std::string hotword_device_id_;
base::WeakPtrFactory<AudioInputHost> weak_factory_{this};
};
} // namespace assistant
} // namespace chromeos
#endif // CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_
chromeos/services/assistant/platform/audio_input_impl.cc
View file @ 4848c486
......@@ -12,6 +12,8 @@
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "base/timer/timer.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/services/assistant/buildflags.h"
#include "chromeos/services/assistant/public/cpp/assistant_client.h"
#include "chromeos/services/assistant/public/cpp/features.h"
......@@ -59,9 +61,13 @@ media::ChannelLayout GetChannelLayout(
class DspHotwordStateManager : public AudioInputImpl::HotwordStateManager {
public:
DspHotwordStateManager(AudioInputImpl* input,
scoped_refptr<base::SequencedTaskRunner> task_runner)
: AudioInputImpl::HotwordStateManager(input), task_runner_(task_runner) {
scoped_refptr<base::SequencedTaskRunner> task_runner,
chromeos::PowerManagerClient* power_manager_client)
: AudioInputImpl::HotwordStateManager(input),
task_runner_(task_runner),
power_manager_client_(power_manager_client) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
DCHECK(power_manager_client_);
}
// HotwordStateManager overrides:
......@@ -76,6 +82,12 @@ class DspHotwordStateManager : public AudioInputImpl::HotwordStateManager {
input_->RecreateAudioInputStream(false /* use_dsp */);
}
stream_state_ = StreamState::NORMAL;
// Inform power manager of a wake notification when Libassistant
// recognized hotword and started a conversation. We intentionally
// avoid using |NotifyUserActivity| because it is not suitable for
// this case according to the Platform team.
power_manager_client_->NotifyWakeNotification();
}
// Runs on main thread.
......@@ -143,6 +155,7 @@ class DspHotwordStateManager : public AudioInputImpl::HotwordStateManager {
}
scoped_refptr<base::SequencedTaskRunner> task_runner_;
chromeos::PowerManagerClient* power_manager_client_;
StreamState stream_state_ = StreamState::HOTWORD;
base::OneShotTimer second_phase_timer_;
base::WeakPtrFactory<DspHotwordStateManager> weak_factory_{this};
......@@ -183,12 +196,22 @@ void AudioInputImpl::HotwordStateManager::RecreateAudioInputStream() {
input_->RecreateAudioInputStream(/*use_dsp=*/false);
}
AudioInputImpl::AudioInputImpl(const std::string& device_id)
: task_runner_(base::SequencedTaskRunnerHandle::Get()),
AudioInputImpl::AudioInputImpl(PowerManagerClient* power_manager_client,
CrasAudioHandler* cras_audio_handler,
const std::string& device_id)
: power_manager_client_(power_manager_client),
power_manager_client_observer_(this),
cras_audio_handler_(cras_audio_handler),
task_runner_(base::SequencedTaskRunnerHandle::Get()),
preferred_device_id_(device_id),
weak_factory_(this) {
DETACH_FROM_SEQUENCE(observer_sequence_checker_);
DCHECK(power_manager_client);
power_manager_client_observer_.Add(power_manager_client);
power_manager_client->GetSwitchStates(base::BindOnce(
&AudioInputImpl::OnSwitchStatesReceived, weak_factory_.GetWeakPtr()));
RecreateStateManager();
if (features::IsStereoAudioInputEnabled())
g_current_format = kFormatStereo;
......@@ -203,8 +226,8 @@ AudioInputImpl::~AudioInputImpl() {
void AudioInputImpl::RecreateStateManager() {
if (IsHotwordAvailable()) {
state_manager_ =
std::make_unique<DspHotwordStateManager>(this, task_runner_);
state_manager_ = std::make_unique<DspHotwordStateManager>(
this, task_runner_, power_manager_client_);
} else {
state_manager_ = std::make_unique<HotwordStateManager>(this);
}
......@@ -308,6 +331,19 @@ void AudioInputImpl::RemoveObserver(
}
}
void AudioInputImpl::LidEventReceived(
chromeos::PowerManagerClient::LidState state,
base::TimeTicks timestamp) {
// Lid switch event still gets fired during system suspend, which enables
// us to stop DSP recording correctly when user closes lid after the device
// goes to sleep.
DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (state != lid_state_) {
lid_state_ = state;
UpdateRecordingState();
}
}
void AudioInputImpl::SetMicState(bool mic_open) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (mic_open_ == mic_open)
......@@ -358,14 +394,58 @@ void AudioInputImpl::SetHotwordDeviceId(const std::string& device_id) {
state_manager_->RecreateAudioInputStream();
}
void AudioInputImpl::OnLidStateChanged(LidState new_state) {
// Lid switch event still gets fired during system suspend, which enables
// us to stop DSP recording correctly when user closes lid after the device
// goes to sleep.
if (new_state != lid_state_) {
lid_state_ = new_state;
UpdateRecordingState();
void AudioInputImpl::SetDspHotwordLocale(std::string pref_locale) {
DCHECK(!hotword_device_id_.empty());
// SetHotwordModel will fail if hotword streaming is running.
DCHECK(!source_);
if (!features::IsDspHotwordEnabled())
return;
// 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.
if (!base::ReplaceChars(pref_locale, "-", "_", &pref_locale)) {
// 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.
std::string region_code = pref_locale;
pref_locale.append("_").append(region_code);
}
// For locales with language code "en", use "en_all" hotword model.
std::vector<std::string> code_strings = base::SplitString(
pref_locale, "_", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
if (code_strings.size() > 0 && code_strings[0] == "en")
pref_locale = "en_all";
uint64_t dsp_node_id;
base::StringToUint64(hotword_device_id_, &dsp_node_id);
cras_audio_handler_->SetHotwordModel(
dsp_node_id, /* hotword_model */ base::ToLowerASCII(pref_locale),
base::BindOnce(&AudioInputImpl::SetDspHotwordLocaleCallback,
weak_factory_.GetWeakPtr(), pref_locale));
}
void AudioInputImpl::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 "en_us" if we failed to sync it to
// the locale stored in user's pref.
uint64_t dsp_node_id;
base::StringToUint64(hotword_device_id_, &dsp_node_id);
cras_audio_handler_->SetHotwordModel(
dsp_node_id, /* hotword_model */ "en_us",
base::BindOnce([](bool success) {
if (!success)
LOG(ERROR) << "Reset to default hotword model failed.";
}));
}
void AudioInputImpl::RecreateAudioInputStream(bool use_dsp) {
......@@ -440,6 +520,15 @@ void AudioInputImpl::StopRecording() {
}
}
void AudioInputImpl::OnSwitchStatesReceived(
base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (switch_states.has_value()) {
lid_state_ = switch_states->lid_state;
UpdateRecordingState();
}
}
void AudioInputImpl::UpdateRecordingState() {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
......@@ -449,7 +538,8 @@ void AudioInputImpl::UpdateRecordingState() {
has_observers = observers_.size() > 0;
}
bool is_lid_closed = (lid_state_ == LidState::kClosed);
bool is_lid_closed =
lid_state_ == chromeos::PowerManagerClient::LidState::CLOSED;
bool should_enable_hotword =
hotword_enabled_ && (!preferred_device_id_.empty());
bool should_start =
......
chromeos/services/assistant/platform/audio_input_impl.h
View file @ 4848c486
......@@ -12,26 +12,28 @@
#include "base/component_export.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/scoped_observer.h"
#include "base/sequence_checker.h"
#include "base/synchronization/lock.h"
#include "base/time/time.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/services/assistant/public/cpp/assistant_service.h"
#include "libassistant/shared/public/platform_audio_input.h"
#include "media/base/audio_capturer_source.h"
namespace chromeos {
class CrasAudioHandler;
namespace assistant {
class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
: public assistant_client::AudioInput,
public media::AudioCapturerSource::CaptureCallback {
public media::AudioCapturerSource::CaptureCallback,
public chromeos::PowerManagerClient::Observer {
public:
enum class LidState {
kOpen,
kClosed,
};
explicit AudioInputImpl(const std::string& device_id);
AudioInputImpl(PowerManagerClient* power_manager_client,
CrasAudioHandler* cras_audio_handler,
const std::string& device_id);
~AudioInputImpl() override;
class HotwordStateManager {
......@@ -68,6 +70,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
void RemoveObserver(
assistant_client::AudioInput::Observer* observer) override;
// chromeos::PowerManagerClient::Observer overrides:
void LidEventReceived(chromeos::PowerManagerClient::LidState state,
base::TimeTicks timestamp) override;
// Called when the mic state associated with the interaction is changed.
void SetMicState(bool mic_open);
void OnConversationTurnStarted();
......@@ -78,9 +84,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
void SetDeviceId(const std::string& device_id);
void SetHotwordDeviceId(const std::string& device_id);
// Called when the user opens/closes the lid.
void OnLidStateChanged(LidState new_state);
void SetDspHotwordLocale(std::string pref_locale);
void SetDspHotwordLocaleCallback(std::string pref_locale, bool success);
void RecreateAudioInputStream(bool use_dsp);
......@@ -96,6 +101,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
void StopRecording();
void UpdateRecordingState();
// Updates lid state from received switch states.
void OnSwitchStatesReceived(
base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states);
scoped_refptr<media::AudioCapturerSource> source_;
// User explicitly requested to open microphone.
......@@ -119,6 +128,13 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
// sequence.
SEQUENCE_CHECKER(observer_sequence_checker_);
chromeos::PowerManagerClient* power_manager_client_;
ScopedObserver<chromeos::PowerManagerClient,
chromeos::PowerManagerClient::Observer>
power_manager_client_observer_;
CrasAudioHandler* const cras_audio_handler_;
scoped_refptr<base::SequencedTaskRunner> task_runner_;
std::unique_ptr<HotwordStateManager> state_manager_;
......@@ -130,9 +146,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
// Device currently being used for recording.
std::string device_id_;
// Start with lidstate |kClosed| so we do not open the microphone before we
// know if the lid is open or closed.
LidState lid_state_ = LidState::kClosed;
chromeos::PowerManagerClient::LidState lid_state_ =
chromeos::PowerManagerClient::LidState::NOT_PRESENT;
base::WeakPtrFactory<AudioInputImpl> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(AudioInputImpl);
......
chromeos/services/assistant/platform/audio_input_impl_unittest.cc
View file @ 4848c486
......@@ -11,13 +11,10 @@
#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/public/cpp/features.h"
#include "chromeos/services/assistant/test_support/scoped_assistant_client.h"
#include "services/audio/public/cpp/fake_stream_factory.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace chromeos {
......@@ -25,9 +22,6 @@ namespace assistant {
namespace {
using LidState = chromeos::PowerManagerClient::LidState;
using ::testing::_;
class ScopedFakeAssistantClient : public ScopedAssistantClient {
public:
ScopedFakeAssistantClient() = default;
......@@ -46,24 +40,6 @@ class ScopedFakeAssistantClient : public ScopedAssistantClient {
DISALLOW_COPY_AND_ASSIGN(ScopedFakeAssistantClient);
};
// 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:
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
class AudioInputImplTest : public testing::Test,
......@@ -76,15 +52,16 @@ class AudioInputImplTest : public testing::Test,
PowerManagerClient::InitializeFake();
CrasAudioHandler::InitializeForTesting();
CreateNewAudioInputImpl();
audio_input_impl_ = std::make_unique<AudioInputImpl>(
FakePowerManagerClient::Get(), CrasAudioHandler::Get(),
"fake-device-id");
audio_input_impl_->AddObserver(this);
}
~AudioInputImplTest() override {
audio_input_impl_->RemoveObserver(this);
audio_input_impl_.reset();
// |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();
}
......@@ -97,33 +74,8 @@ class AudioInputImplTest : public testing::Test,
return audio_input_impl_->IsUsingHotwordDeviceForTesting();
}
void CreateNewAudioInputImpl() {
audio_input_impl_ = std::make_unique<AudioInputImpl>("fake-device-id");
audio_input_host_ = std::make_unique<AudioInputHost>(
audio_input_impl_.get(), CrasAudioHandler::Get(),
FakePowerManagerClient::Get());
audio_input_impl_->AddObserver(this);
// Allow the asynchronous triggered events to run.
base::RunLoop().RunUntilIdle();
}
void StopAudioRecording() {
SetLidState(LidState::CLOSED);
// Allow the asynchronous triggered events to run.
base::RunLoop().RunUntilIdle();
}
AudioInputImpl* audio_input_impl() { return audio_input_impl_.get(); }
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 {}
......@@ -131,46 +83,37 @@ class AudioInputImplTest : public testing::Test,
void OnAudioStopped() override {}
protected:
void ReportLidEvent(LidState state) {
void ReportLidEvent(chromeos::PowerManagerClient::LidState state) {
FakePowerManagerClient::Get()->SetLidState(state,
base::TimeTicks::UnixEpoch());
}
void SetLidState(LidState state) { ReportLidEvent(state); }
private:
base::test::TaskEnvironment task_environment_;
base::test::ScopedFeatureList scoped_feature_list_;
ScopedFakeAssistantClient fake_assistant_client_;
::testing::NiceMock<ScopedCrasAudioClientMock> cras_audio_client_mock_;
std::unique_ptr<AudioInputImpl> audio_input_impl_;
std::unique_ptr<AudioInputHost> audio_input_host_;
DISALLOW_COPY_AND_ASSIGN(AudioInputImplTest);
};
TEST_F(AudioInputImplTest, StopRecordingWhenLidClosed) {
// Trigger a lid open event.
ReportLidEvent(LidState::OPEN);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
// Trigger a lid closed event.
ReportLidEvent(LidState::CLOSED);
ReportLidEvent(chromeos::PowerManagerClient::LidState::CLOSED);
EXPECT_FALSE(GetRecordingStatus());
// Trigger a lid open event again.
ReportLidEvent(LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
}
TEST_F(AudioInputImplTest, StartRecordingWhenThereIsNoLid) {
ReportLidEvent(LidState::NOT_PRESENT);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
}
TEST_F(AudioInputImplTest, StopRecordingWithNoPreferredDevice) {
// Start as recording.
ReportLidEvent(LidState::OPEN);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
// Preferred input device is lost.
......@@ -184,7 +127,7 @@ TEST_F(AudioInputImplTest, StopRecordingWithNoPreferredDevice) {
TEST_F(AudioInputImplTest, StopRecordingWhenDisableHotword) {
// Start as recording.
ReportLidEvent(LidState::OPEN);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
// Hotword disabled should stop recording.
......@@ -198,7 +141,7 @@ TEST_F(AudioInputImplTest, StopRecordingWhenDisableHotword) {
TEST_F(AudioInputImplTest, StartRecordingWhenDisableHotwordAndForceOpenMic) {
// Start as recording.
ReportLidEvent(LidState::OPEN);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
// Hotword disabled should stop recording.
......@@ -214,19 +157,9 @@ TEST_F(AudioInputImplTest, StartRecordingWhenDisableHotwordAndForceOpenMic) {
EXPECT_FALSE(GetRecordingStatus());
}
TEST_F(AudioInputImplTest, ShouldReadCurrentLidStateWhenLaunching) {
SetLidState(LidState::OPEN);
CreateNewAudioInputImpl();
EXPECT_TRUE(GetRecordingStatus());
SetLidState(LidState::CLOSED);
CreateNewAudioInputImpl();
EXPECT_FALSE(GetRecordingStatus());
}
TEST_F(AudioInputImplTest, SettingHotwordDeviceDoesNotAffectRecordingState) {
// Start as recording.
ReportLidEvent(LidState::OPEN);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
// Hotword device does not change recording state.
......@@ -239,7 +172,7 @@ TEST_F(AudioInputImplTest, SettingHotwordDeviceDoesNotAffectRecordingState) {
TEST_F(AudioInputImplTest, SettingHotwordDeviceUsesHotwordDeviceForRecording) {
// Start as recording.
ReportLidEvent(LidState::OPEN);
ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
EXPECT_TRUE(GetRecordingStatus());
// Hotword device does not change recording state.
......@@ -252,70 +185,5 @@ TEST_F(AudioInputImplTest, SettingHotwordDeviceUsesHotwordDeviceForRecording) {
EXPECT_TRUE(IsUsingHotwordDevice());
}
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, 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/audio_input_provider_impl.cc
View file @ 4848c486
......@@ -9,8 +9,12 @@
namespace chromeos {
namespace assistant {
AudioInputProviderImpl::AudioInputProviderImpl()
: audio_input_(/*input_device_id=*/std::string()) {}
AudioInputProviderImpl::AudioInputProviderImpl(
PowerManagerClient* power_manager_client,
CrasAudioHandler* cras_audio_handler)
: audio_input_(power_manager_client,
cras_audio_handler,
/*input_device_id=*/std::string()) {}
AudioInputProviderImpl::~AudioInputProviderImpl() = default;
......@@ -25,5 +29,25 @@ int64_t AudioInputProviderImpl::GetCurrentAudioTime() {
return 0;
}
void AudioInputProviderImpl::SetMicState(bool mic_open) {
audio_input_.SetMicState(mic_open);
}
void AudioInputProviderImpl::OnHotwordEnabled(bool enable) {
audio_input_.OnHotwordEnabled(enable);
}
void AudioInputProviderImpl::SetDeviceId(const std::string& device_id) {
audio_input_.SetDeviceId(device_id);
}
void AudioInputProviderImpl::SetHotwordDeviceId(const std::string& device_id) {
audio_input_.SetHotwordDeviceId(device_id);
}
void AudioInputProviderImpl::SetDspHotwordLocale(std::string pref_locale) {
audio_input_.SetDspHotwordLocale(pref_locale);
}
} // namespace assistant
} // namespace chromeos
chromeos/services/assistant/platform/audio_input_provider_impl.h
View file @ 4848c486
......@@ -14,18 +14,36 @@
#include "libassistant/shared/public/platform_audio_input.h"
namespace chromeos {
class CrasAudioHandler;
class PowerManagerClient;
namespace assistant {
class AudioInputProviderImpl : public assistant_client::AudioInputProvider {
public:
AudioInputProviderImpl();
AudioInputProviderImpl(PowerManagerClient* power_manager_client,
CrasAudioHandler* cras_audio_handler);
~AudioInputProviderImpl() override;
// assistant_client::AudioInputProvider overrides:
AudioInputImpl& GetAudioInput() override;
int64_t GetCurrentAudioTime() override;
// Called when the mic state associated with the interaction is changed.
void SetMicState(bool mic_open);
// Called when hotword enabled status changed.
void OnHotwordEnabled(bool enable);
// Setting the input device to use for audio capture.
void SetDeviceId(const std::string& device_id);
// 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);
private:
AudioInputImpl audio_input_;
......
chromeos/services/assistant/platform/audio_output_provider_impl.cc
View file @ 4848c486
......@@ -142,10 +142,14 @@ class AudioOutputImpl : public assistant_client::AudioOutput {
} // namespace
AudioOutputProviderImpl::AudioOutputProviderImpl(
PowerManagerClient* power_manager_client,
CrasAudioHandler* cras_audio_handler,
AssistantMediaSession* media_session,
scoped_refptr<base::SequencedTaskRunner> background_task_runner,
const std::string& device_id)
: loop_back_input_(media::AudioDeviceDescription::kLoopbackInputDeviceId),
: loop_back_input_(power_manager_client,
cras_audio_handler,
media::AudioDeviceDescription::kLoopbackInputDeviceId),
volume_control_impl_(media_session),
main_task_runner_(base::SequencedTaskRunnerHandle::Get()),
background_task_runner_(background_task_runner),
......
chromeos/services/assistant/platform/audio_output_provider_impl.h
View file @ 4848c486
......@@ -24,6 +24,8 @@
#include "services/audio/public/mojom/stream_factory.mojom.h"
namespace chromeos {
class CrasAudioHandler;
class PowerManagerClient;
namespace assistant {
......@@ -32,6 +34,8 @@ class AssistantMediaSession;
class AudioOutputProviderImpl : public assistant_client::AudioOutputProvider {
public:
AudioOutputProviderImpl(
PowerManagerClient* power_manager_client,
CrasAudioHandler* cras_audio_handler,
AssistantMediaSession* media_session,
scoped_refptr<base::SequencedTaskRunner> background_task_runner,
const std::string& device_id);
......
chromeos/services/assistant/platform_api_impl.cc
View file @ 4848c486
......@@ -84,13 +84,12 @@ PlatformApiImpl::PlatformApiImpl(
scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
std::string pref_locale)
: audio_input_provider_(),
audio_output_provider_(media_session,
: audio_input_provider_(power_manager_client, cras_audio_handler),
audio_output_provider_(power_manager_client,
cras_audio_handler,
media_session,
background_task_runner,
media::AudioDeviceDescription::kDefaultDeviceId),
audio_input_host_(&audio_input_provider_.GetAudioInput(),
cras_audio_handler,
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.
......@@ -166,32 +165,32 @@ void PlatformApiImpl::OnAudioNodesChanged() {
}
}
audio_input_host_.SetDeviceId(
audio_input_provider_.SetDeviceId(
input_device ? base::NumberToString(input_device->id) : std::string());
if (hotword_device) {
audio_input_host_.SetHotwordDeviceId(
audio_input_provider_.SetHotwordDeviceId(
base::NumberToString(hotword_device->id));
audio_input_host_.SetDspHotwordLocale(pref_locale_);
audio_input_provider_.SetDspHotwordLocale(pref_locale_);
} else {
audio_input_host_.SetHotwordDeviceId(std::string());
audio_input_provider_.SetHotwordDeviceId(std::string());
}
}
void PlatformApiImpl::SetMicState(bool mic_open) {
audio_input_host_.SetMicState(mic_open);
audio_input_provider_.SetMicState(mic_open);
}
void PlatformApiImpl::OnConversationTurnStarted() {
audio_input_host_.OnConversationTurnStarted();
audio_input_provider_.GetAudioInput().OnConversationTurnStarted();
}
void PlatformApiImpl::OnConversationTurnFinished() {
audio_input_host_.OnConversationTurnFinished();
audio_input_provider_.GetAudioInput().OnConversationTurnFinished();
}
void PlatformApiImpl::OnHotwordEnabled(bool enable) {
audio_input_host_.OnHotwordEnabled(enable);
audio_input_provider_.OnHotwordEnabled(enable);
}
} // namespace assistant
......
chromeos/services/assistant/platform_api_impl.h
View file @ 4848c486
......@@ -11,7 +11,6 @@
#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"
#include "chromeos/services/assistant/platform/file_provider_impl.h"
......@@ -102,7 +101,6 @@ class PlatformApiImpl : public CrosPlatformApi,
FakeAuthProvider auth_provider_;
FileProviderImpl file_provider_;
NetworkProviderImpl network_provider_;
AudioInputHost audio_input_host_;
std::unique_ptr<SystemProviderImpl> system_provider_;
std::string pref_locale_;
......
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