Skip to content

Commit

Permalink
VC UI: Implement new id system for VC effects
Browse files Browse the repository at this point in the history 
Camera and audio effects are currently using their own id system, which
makes it hard to do metrics collection. This CL introduces a new id
class used by both camera and audio effects. Also, when creating an
effect, it is required to provide a new id to make sure that metrics are
collected for that new effect.

Bug: b:265197876
Change-Id: Id9d33315e65abe6cdfb3336c96425a9bdc2adb0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4347952
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Andre Le <leandre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118429}
  • Loading branch information
Andre Le authored and Chromium LUCI CQ committed Mar 16, 2023
1 parent 56f4512 commit 4d5aea6
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 192 deletions.
83 changes: 42 additions & 41 deletions ash/system/audio/audio_effects_controller.cc
View file
Expand Up @@ -33,65 +33,62 @@ AudioEffectsController::~AudioEffectsController() {
}
}

bool AudioEffectsController::IsEffectSupported(
AudioEffectId effect_id /*=AudioEffectId::kNone*/) {
bool AudioEffectsController::IsEffectSupported(VcEffectId effect_id) {
switch (effect_id) {
case AudioEffectId::kNoiseCancellation:
case VcEffectId::kNoiseCancellation:
return CrasAudioHandler::Get()->noise_cancellation_supported();
case AudioEffectId::kLiveCaption:
case VcEffectId::kLiveCaption:
return captions::IsLiveCaptionFeatureSupported();
case AudioEffectId::kNone:
return IsEffectSupported(AudioEffectId::kNoiseCancellation) ||
IsEffectSupported(AudioEffectId::kLiveCaption);
case VcEffectId::kBackgroundBlur:
case VcEffectId::kPortraitRelighting:
case VcEffectId::kTestEffect:
NOTREACHED();
return false;
}

NOTREACHED();
return false;
}

absl::optional<int> AudioEffectsController::GetEffectState(int effect_id) {
absl::optional<int> AudioEffectsController::GetEffectState(
VcEffectId effect_id) {
switch (effect_id) {
case AudioEffectId::kNoiseCancellation: {
case VcEffectId::kNoiseCancellation:
return CrasAudioHandler::Get()->GetNoiseCancellationState() ? 1 : 0;
}
case AudioEffectId::kLiveCaption: {
case VcEffectId::kLiveCaption:
return Shell::Get()->accessibility_controller()->live_caption().enabled()
? 1
: 0;
}
case AudioEffectId::kNone:
case VcEffectId::kBackgroundBlur:
case VcEffectId::kPortraitRelighting:
case VcEffectId::kTestEffect:
NOTREACHED();
return absl::nullopt;
}

NOTREACHED();
return absl::nullopt;
}

void AudioEffectsController::OnEffectControlActivated(
absl::optional<int> effect_id,
VcEffectId effect_id,
absl::optional<int> value) {
DCHECK(effect_id.has_value());
switch (effect_id.value()) {
case AudioEffectId::kNoiseCancellation: {
switch (effect_id) {
case VcEffectId::kNoiseCancellation: {
// Toggle noise cancellation.
CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
bool new_state = !audio_handler->GetNoiseCancellationState();
audio_handler->SetNoiseCancellationState(new_state);
return;
}
case AudioEffectId::kLiveCaption: {
case VcEffectId::kLiveCaption: {
// Toggle live caption.
AccessibilityControllerImpl* controller =
Shell::Get()->accessibility_controller();
controller->live_caption().SetEnabled(
!controller->live_caption().enabled());
return;
}
case AudioEffectId::kNone:
case VcEffectId::kBackgroundBlur:
case VcEffectId::kPortraitRelighting:
case VcEffectId::kTestEffect:
NOTREACHED();
return;
}

NOTREACHED();
}

void AudioEffectsController::OnActiveUserPrefServiceChanged(
Expand All @@ -105,26 +102,32 @@ void AudioEffectsController::OnActiveUserPrefServiceChanged(
return;
}

if (IsEffectSupported(AudioEffectId::kNoiseCancellation)) {
const bool noise_cancellation_supported =
IsEffectSupported(VcEffectId::kNoiseCancellation);
const bool live_caption_supported =
IsEffectSupported(VcEffectId::kLiveCaption);

if (noise_cancellation_supported) {
AddNoiseCancellationEffect();
}

if (IsEffectSupported(AudioEffectId::kLiveCaption)) {
if (live_caption_supported) {
AddLiveCaptionEffect();
}

if (IsEffectSupported()) {
if (noise_cancellation_supported || live_caption_supported) {
effects_manager.RegisterDelegate(this);
}
}

void AudioEffectsController::AddNoiseCancellationEffect() {
std::unique_ptr<VcHostedEffect> effect = std::make_unique<VcHostedEffect>(
VcEffectType::kToggle,
/*type=*/VcEffectType::kToggle,
/*get_state_callback=*/
base::BindRepeating(&AudioEffectsController::GetEffectState,
base::Unretained(this),
static_cast<int>(AudioEffectId::kNoiseCancellation)));
effect->set_id(static_cast<int>(AudioEffectId::kNoiseCancellation));
VcEffectId::kNoiseCancellation),
/*effect_id=*/VcEffectId::kNoiseCancellation);
effect->AddState(std::make_unique<VcEffectState>(
/*icon=*/&kVideoConferenceNoiseCancellationOnIcon,
/*label_text=*/
Expand All @@ -135,20 +138,19 @@ void AudioEffectsController::AddNoiseCancellationEffect() {
/*button_callback=*/
base::BindRepeating(&AudioEffectsController::OnEffectControlActivated,
weak_factory_.GetWeakPtr(),
/*effect_id=*/
static_cast<int>(AudioEffectId::kNoiseCancellation),
/*effect_id=*/VcEffectId::kNoiseCancellation,
/*value=*/0)));
effect->set_dependency_flags(VcHostedEffect::ResourceDependency::kMicrophone);
AddEffect(std::move(effect));
}

void AudioEffectsController::AddLiveCaptionEffect() {
std::unique_ptr<VcHostedEffect> effect = std::make_unique<VcHostedEffect>(
VcEffectType::kToggle,
/*type=*/VcEffectType::kToggle,
/*get_state_callback=*/
base::BindRepeating(&AudioEffectsController::GetEffectState,
base::Unretained(this),
static_cast<int>(AudioEffectId::kLiveCaption)));
effect->set_id(static_cast<int>(AudioEffectId::kLiveCaption));
base::Unretained(this), VcEffectId::kLiveCaption),
/*effect_id=*/VcEffectId::kLiveCaption);
effect->AddState(std::make_unique<VcEffectState>(
/*icon=*/&kVideoConferenceLiveCaptionOnIcon,
/*label_text=*/
Expand All @@ -158,8 +160,7 @@ void AudioEffectsController::AddLiveCaptionEffect() {
/*button_callback=*/
base::BindRepeating(&AudioEffectsController::OnEffectControlActivated,
weak_factory_.GetWeakPtr(),
/*effect_id=*/
static_cast<int>(AudioEffectId::kLiveCaption),
/*effect_id=*/VcEffectId::kLiveCaption,
/*value=*/0)));
AddEffect(std::move(effect));
}
Expand Down
18 changes: 6 additions & 12 deletions ash/system/audio/audio_effects_controller.h
View file
Expand Up @@ -15,30 +15,24 @@

namespace ash {

enum class VcEffectId;

class ASH_EXPORT AudioEffectsController : public VcEffectsDelegate,
public SessionObserver {
public:
enum AudioEffectId {
kNone = 0,
kNoiseCancellation = 1,
kLiveCaption = 2,
};

AudioEffectsController();

AudioEffectsController(const AudioEffectsController&) = delete;
AudioEffectsController& operator=(const AudioEffectsController&) = delete;

~AudioEffectsController() override;

// Returns whether `effect_id` is supported. If passed an `effect_id` of
// `AudioEffectId::kNone`, the function returns whether *any* effects are
// supported.
bool IsEffectSupported(AudioEffectId effect_id = AudioEffectId::kNone);
// Returns whether `effect_id` is supported.
bool IsEffectSupported(VcEffectId effect_id);

// VcEffectsDelegate:
absl::optional<int> GetEffectState(int effect_id) override;
void OnEffectControlActivated(absl::optional<int> effect_id,
absl::optional<int> GetEffectState(VcEffectId effect_id) override;
void OnEffectControlActivated(VcEffectId effect_id,
absl::optional<int> state) override;

// SessionObserver:
Expand Down
41 changes: 20 additions & 21 deletions ash/system/audio/audio_effects_controller_unittest.cc
View file
Expand Up @@ -89,7 +89,7 @@ TEST_F(AudioEffectsControllerTest, NoiseCancellationNotSupported) {

// `AudioEffectsController` reports noise that cancellation is not-supported.
EXPECT_FALSE(audio_effects_controller()->IsEffectSupported(
AudioEffectsController::AudioEffectId::kNoiseCancellation));
VcEffectId::kNoiseCancellation));
}

TEST_F(AudioEffectsControllerTest, NoiseCancellationSupported) {
Expand All @@ -101,12 +101,11 @@ TEST_F(AudioEffectsControllerTest, NoiseCancellationSupported) {

// `AudioEffectsController` reports that noise cancellation is supported.
EXPECT_TRUE(audio_effects_controller()->IsEffectSupported(
AudioEffectsController::AudioEffectId::kNoiseCancellation));
VcEffectId::kNoiseCancellation));

// Makes sure the dependency flag is set when the effect is supported.
auto* effect = audio_effects_controller()->GetEffect(0);
ASSERT_EQ(AudioEffectsController::AudioEffectId::kNoiseCancellation,
effect->id());
ASSERT_EQ(VcEffectId::kNoiseCancellation, effect->id());
EXPECT_EQ(VcHostedEffect::ResourceDependency::kMicrophone,
effect->dependency_flags());
}
Expand All @@ -123,13 +122,13 @@ TEST_F(AudioEffectsControllerTest, NoiseCancellationNotEnabled) {

// Noise cancellation effect state is disabled.
absl::optional<int> effect_state = audio_effects_controller()->GetEffectState(
AudioEffectsController::AudioEffectId::kNoiseCancellation);
VcEffectId::kNoiseCancellation);
EXPECT_TRUE(effect_state.has_value());
EXPECT_EQ(effect_state, 0);

cras_audio_handler()->SetNoiseCancellationState(true);
effect_state = audio_effects_controller()->GetEffectState(
AudioEffectsController::AudioEffectId::kNoiseCancellation);
VcEffectId::kNoiseCancellation);
EXPECT_TRUE(effect_state.has_value());
EXPECT_EQ(effect_state, 1);
}
Expand All @@ -146,7 +145,7 @@ TEST_F(AudioEffectsControllerTest, NoiseCancellationEnabled) {

// Noise cancellation effect state is disabled.
absl::optional<int> effect_state = audio_effects_controller()->GetEffectState(
AudioEffectsController::AudioEffectId::kNoiseCancellation);
VcEffectId::kNoiseCancellation);
EXPECT_TRUE(effect_state.has_value());
EXPECT_EQ(effect_state, 1);
}
Expand All @@ -166,7 +165,7 @@ TEST_F(AudioEffectsControllerTest, NoiseCancellationSetNotEnabled) {

// User pressed the noise cancellation toggle.
audio_effects_controller()->OnEffectControlActivated(
AudioEffectsController::AudioEffectId::kNoiseCancellation, absl::nullopt);
VcEffectId::kNoiseCancellation, absl::nullopt);

// State should now be disabled.
EXPECT_FALSE(cras_audio_handler()->GetNoiseCancellationState());
Expand All @@ -187,7 +186,7 @@ TEST_F(AudioEffectsControllerTest, NoiseCancellationSetEnabled) {

// User pressed the noise cancellation toggle.
audio_effects_controller()->OnEffectControlActivated(
AudioEffectsController::AudioEffectId::kNoiseCancellation, absl::nullopt);
VcEffectId::kNoiseCancellation, absl::nullopt);

// State should now be enabled.
EXPECT_TRUE(cras_audio_handler()->GetNoiseCancellationState());
Expand All @@ -198,8 +197,8 @@ TEST_F(AudioEffectsControllerTest, LiveCaptionNotSupported) {

// No live caption feature flags enabled, so `AudioEffectsController` reports
// that live caption is not supported.
EXPECT_FALSE(audio_effects_controller()->IsEffectSupported(
AudioEffectsController::AudioEffectId::kLiveCaption));
EXPECT_FALSE(
audio_effects_controller()->IsEffectSupported(VcEffectId::kLiveCaption));
}

TEST_F(AudioEffectsControllerTest, LiveCaptionSupported) {
Expand All @@ -213,8 +212,8 @@ TEST_F(AudioEffectsControllerTest, LiveCaptionSupported) {

// Live caption feature flags are enabled, so `AudioEffectsController` reports
// that live caption is supported.
EXPECT_TRUE(audio_effects_controller()->IsEffectSupported(
AudioEffectsController::AudioEffectId::kLiveCaption));
EXPECT_TRUE(
audio_effects_controller()->IsEffectSupported(VcEffectId::kLiveCaption));
}

TEST_F(AudioEffectsControllerTest, LiveCaptionNotEnabled) {
Expand All @@ -234,8 +233,8 @@ TEST_F(AudioEffectsControllerTest, LiveCaptionNotEnabled) {
EXPECT_FALSE(controller->live_caption().enabled());

// Live caption effect state is disabled.
absl::optional<int> state = audio_effects_controller()->GetEffectState(
AudioEffectsController::AudioEffectId::kLiveCaption);
absl::optional<int> state =
audio_effects_controller()->GetEffectState(VcEffectId::kLiveCaption);
EXPECT_TRUE(state.has_value());
EXPECT_FALSE(state.value());
}
Expand All @@ -257,8 +256,8 @@ TEST_F(AudioEffectsControllerTest, LiveCaptionEnabled) {
EXPECT_TRUE(controller->live_caption().enabled());

// Live caption effect state is enabled.
absl::optional<int> state = audio_effects_controller()->GetEffectState(
AudioEffectsController::AudioEffectId::kLiveCaption);
absl::optional<int> state =
audio_effects_controller()->GetEffectState(VcEffectId::kLiveCaption);
EXPECT_TRUE(state.has_value());
EXPECT_TRUE(state.value());
}
Expand All @@ -280,8 +279,8 @@ TEST_F(AudioEffectsControllerTest, LiveCaptionSetNotEnabled) {
EXPECT_TRUE(controller->live_caption().enabled());

// User pressed the live caption toggle.
audio_effects_controller()->OnEffectControlActivated(
AudioEffectsController::AudioEffectId::kLiveCaption, absl::nullopt);
audio_effects_controller()->OnEffectControlActivated(VcEffectId::kLiveCaption,
absl::nullopt);

// Live caption is now disabled.
EXPECT_FALSE(controller->live_caption().enabled());
Expand All @@ -304,8 +303,8 @@ TEST_F(AudioEffectsControllerTest, LiveCaptionSetEnabled) {
EXPECT_FALSE(controller->live_caption().enabled());

// User pressed the live caption toggle.
audio_effects_controller()->OnEffectControlActivated(
AudioEffectsController::AudioEffectId::kLiveCaption, absl::nullopt);
audio_effects_controller()->OnEffectControlActivated(VcEffectId::kLiveCaption,
absl::nullopt);

// Live caption is now enabled.
EXPECT_TRUE(controller->live_caption().enabled());
Expand Down

0 comments on commit 4d5aea6

Please sign in to comment.