Cleanup in the code for the lag estimation in AEC3
Bug: webrtc:8379
Change-Id: Ic80ce86fc0f4ba42583bd43cb137c6e1c9e6513c
Reviewed-on: https://webrtc-review.googlesource.com/8560
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20252}diff --git a/modules/audio_processing/aec3/matched_filter_lag_aggregator.cc b/modules/audio_processing/aec3/matched_filter_lag_aggregator.cc
index 7c8ccb4..1d16dd6 100644
--- a/modules/audio_processing/aec3/matched_filter_lag_aggregator.cc
+++ b/modules/audio_processing/aec3/matched_filter_lag_aggregator.cc
@@ -27,7 +27,6 @@
histogram_.fill(0);
histogram_data_.fill(0);
histogram_data_index_ = 0;
- filled_histogram_ = false;
}
rtc::Optional<size_t> MatchedFilterLagAggregator::Aggregate(
@@ -62,7 +61,6 @@
histogram_data_index_ =
(histogram_data_index_ + 1) % histogram_data_.size();
- filled_histogram_ = filled_histogram_ || histogram_data_index_ == 0;
const int candidate =
std::distance(histogram_.begin(),
diff --git a/modules/audio_processing/aec3/matched_filter_lag_aggregator.h b/modules/audio_processing/aec3/matched_filter_lag_aggregator.h
index 5e6c7aa..76d711c 100644
--- a/modules/audio_processing/aec3/matched_filter_lag_aggregator.h
+++ b/modules/audio_processing/aec3/matched_filter_lag_aggregator.h
@@ -40,7 +40,6 @@
std::array<int, 1664> histogram_;
std::array<int, 250> histogram_data_;
int histogram_data_index_ = 0;
- bool filled_histogram_ = false;
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(MatchedFilterLagAggregator);
};
diff --git a/modules/audio_processing/aec3/matched_filter_lag_aggregator_unittest.cc b/modules/audio_processing/aec3/matched_filter_lag_aggregator_unittest.cc
index 9a6c838..9b0c2e1 100644
--- a/modules/audio_processing/aec3/matched_filter_lag_aggregator_unittest.cc
+++ b/modules/audio_processing/aec3/matched_filter_lag_aggregator_unittest.cc
@@ -22,152 +22,96 @@
namespace webrtc {
namespace {
-void VerifyNoAggregateOutputForRepeatedLagAggregation(
- size_t num_repetitions,
- rtc::ArrayView<const MatchedFilter::LagEstimate> lag_estimates,
- MatchedFilterLagAggregator* aggregator) {
- for (size_t k = 0; k < num_repetitions; ++k) {
- EXPECT_FALSE(aggregator->Aggregate(lag_estimates));
- }
-}
-
-constexpr size_t kThresholdForRequiredLagUpdatesInARow = 10;
-constexpr size_t kThresholdForRequiredIdenticalLagAggregates = 100;
+constexpr size_t kNumLagsBeforeDetection = 25;
} // namespace
// Verifies that the most accurate lag estimate is chosen.
-// TODO(peah): Modify and reenable according to new scheme.
-TEST(MatchedFilterLagAggregator, DISABLED_MostAccurateLagChosen) {
- constexpr size_t kArtificialLag1 = 5;
- constexpr size_t kArtificialLag2 = 10;
+TEST(MatchedFilterLagAggregator, MostAccurateLagChosen) {
+ constexpr size_t kLag1 = 5;
+ constexpr size_t kLag2 = 10;
ApmDataDumper data_dumper(0);
std::vector<MatchedFilter::LagEstimate> lag_estimates(2);
MatchedFilterLagAggregator aggregator(&data_dumper);
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag1, true);
- lag_estimates[1] =
- MatchedFilter::LagEstimate(0.5f, true, kArtificialLag2, true);
+ lag_estimates[0] = MatchedFilter::LagEstimate(1.f, true, kLag1, true);
+ lag_estimates[1] = MatchedFilter::LagEstimate(0.5f, true, kLag2, true);
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredLagUpdatesInARow +
- kThresholdForRequiredIdenticalLagAggregates,
- lag_estimates, &aggregator);
+ for (size_t k = 0; k < kNumLagsBeforeDetection; ++k) {
+ EXPECT_FALSE(aggregator.Aggregate(lag_estimates));
+ }
+
rtc::Optional<size_t> aggregated_lag = aggregator.Aggregate(lag_estimates);
EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag1, *aggregated_lag);
+ EXPECT_EQ(kLag1, *aggregated_lag);
- lag_estimates[0] =
- MatchedFilter::LagEstimate(0.5f, true, kArtificialLag1, true);
- lag_estimates[1] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag2, true);
+ lag_estimates[0] = MatchedFilter::LagEstimate(0.5f, true, kLag1, true);
+ lag_estimates[1] = MatchedFilter::LagEstimate(1.f, true, kLag2, true);
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredIdenticalLagAggregates, lag_estimates, &aggregator);
+ for (size_t k = 0; k < kNumLagsBeforeDetection; ++k) {
+ aggregated_lag = aggregator.Aggregate(lag_estimates);
+ EXPECT_TRUE(aggregated_lag);
+ EXPECT_EQ(kLag1, *aggregated_lag);
+ }
+
+ aggregated_lag = aggregator.Aggregate(lag_estimates);
aggregated_lag = aggregator.Aggregate(lag_estimates);
EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag2, *aggregated_lag);
+ EXPECT_EQ(kLag2, *aggregated_lag);
}
// Verifies that varying lag estimates causes lag estimates to not be deemed
// reliable.
-// TODO(peah): Modify and reenable according to new scheme.
TEST(MatchedFilterLagAggregator,
- DISABLED_LagEstimateInvarianceRequiredForAggregatedLag) {
- constexpr size_t kArtificialLag1 = 5;
- constexpr size_t kArtificialLag2 = 10;
+ LagEstimateInvarianceRequiredForAggregatedLag) {
ApmDataDumper data_dumper(0);
std::vector<MatchedFilter::LagEstimate> lag_estimates(1);
MatchedFilterLagAggregator aggregator(&data_dumper);
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag1, true);
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredLagUpdatesInARow +
- kThresholdForRequiredIdenticalLagAggregates,
- lag_estimates, &aggregator);
- rtc::Optional<size_t> aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag1, *aggregated_lag);
-
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag2, true);
-
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredIdenticalLagAggregates, lag_estimates, &aggregator);
- aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag2, *aggregated_lag);
+ for (size_t k = 0; k < kNumLagsBeforeDetection * 100; ++k) {
+ lag_estimates[0] = MatchedFilter::LagEstimate(1.f, true, k % 100, true);
+ rtc::Optional<size_t> aggregated_lag = aggregator.Aggregate(lag_estimates);
+ EXPECT_FALSE(aggregated_lag);
+ }
}
// Verifies that lag estimate updates are required to produce an updated lag
// aggregate.
-// TODO(peah): Modify and reenable according to new scheme.
TEST(MatchedFilterLagAggregator,
DISABLED_LagEstimateUpdatesRequiredForAggregatedLag) {
- constexpr size_t kArtificialLag1 = 5;
- constexpr size_t kArtificialLag2 = 10;
+ constexpr size_t kLag = 5;
ApmDataDumper data_dumper(0);
std::vector<MatchedFilter::LagEstimate> lag_estimates(1);
MatchedFilterLagAggregator aggregator(&data_dumper);
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag1, true);
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredLagUpdatesInARow +
- kThresholdForRequiredIdenticalLagAggregates,
- lag_estimates, &aggregator);
- rtc::Optional<size_t> aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag1, *aggregated_lag);
-
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag2, false);
-
- for (size_t k = 0; k < kThresholdForRequiredLagUpdatesInARow +
- kThresholdForRequiredIdenticalLagAggregates + 1;
- ++k) {
- aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag1, *aggregated_lag);
+ for (size_t k = 0; k < kNumLagsBeforeDetection * 10; ++k) {
+ lag_estimates[0] = MatchedFilter::LagEstimate(1.f, true, kLag, false);
+ rtc::Optional<size_t> aggregated_lag = aggregator.Aggregate(lag_estimates);
+ EXPECT_FALSE(aggregated_lag);
+ EXPECT_EQ(kLag, *aggregated_lag);
}
-
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag2, true);
- for (size_t k = 0; k < kThresholdForRequiredLagUpdatesInARow; ++k) {
- aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag1, *aggregated_lag);
- }
-
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredIdenticalLagAggregates, lag_estimates, &aggregator);
-
- aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag2, *aggregated_lag);
}
// Verifies that an aggregated lag is persistent if the lag estimates do not
// change and that an aggregated lag is not produced without gaining lag
// estimate confidence.
-// TODO(peah): Modify and reenable according to new scheme.
TEST(MatchedFilterLagAggregator, DISABLED_PersistentAggregatedLag) {
- constexpr size_t kArtificialLag = 5;
+ constexpr size_t kLag1 = 5;
+ constexpr size_t kLag2 = 10;
ApmDataDumper data_dumper(0);
std::vector<MatchedFilter::LagEstimate> lag_estimates(1);
MatchedFilterLagAggregator aggregator(&data_dumper);
- lag_estimates[0] =
- MatchedFilter::LagEstimate(1.f, true, kArtificialLag, true);
- VerifyNoAggregateOutputForRepeatedLagAggregation(
- kThresholdForRequiredLagUpdatesInARow +
- kThresholdForRequiredIdenticalLagAggregates,
- lag_estimates, &aggregator);
- rtc::Optional<size_t> aggregated_lag = aggregator.Aggregate(lag_estimates);
+ rtc::Optional<size_t> aggregated_lag;
+ for (size_t k = 0; k < kNumLagsBeforeDetection; ++k) {
+ lag_estimates[0] = MatchedFilter::LagEstimate(1.f, true, kLag1, true);
+ aggregated_lag = aggregator.Aggregate(lag_estimates);
+ }
EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag, *aggregated_lag);
+ EXPECT_EQ(kLag1, *aggregated_lag);
- aggregated_lag = aggregator.Aggregate(lag_estimates);
- EXPECT_TRUE(aggregated_lag);
- EXPECT_EQ(kArtificialLag, *aggregated_lag);
+ for (size_t k = 0; k < kNumLagsBeforeDetection * 40; ++k) {
+ lag_estimates[0] = MatchedFilter::LagEstimate(1.f, false, kLag2, true);
+ aggregated_lag = aggregator.Aggregate(lag_estimates);
+ EXPECT_TRUE(aggregated_lag);
+ EXPECT_EQ(kLag1, *aggregated_lag);
+ }
}
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
