Changed the aggregation of AEC3 matched filter delay estimates

This CL changes the aggregation of the matched filter delay
estimates in AEC3 to using a histogram approach.

Bug: chromium:773541,webrtc:8379
Change-Id: I5322c65858188599397ef5716fecdebc34852e6a
Reviewed-on: https://webrtc-review.googlesource.com/8261
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20234}

modules/audio_processing/aec3/echo_path_delay_estimator.cc

@@ -34,8 +34,7 @@ EchoPathDelayEstimator::EchoPathDelayEstimator(
                       kNumMatchedFilters,
                       kMatchedFilterAlignmentShiftSizeSubBlocks,
                       config.param.render_levels.poor_excitation_render_limit),
-      matched_filter_lag_aggregator_(data_dumper_,
+      matched_filter_lag_aggregator_(data_dumper_) {
-                                     matched_filter_.NumLagEstimates()) {
   RTC_DCHECK(data_dumper);
 }
 

modules/audio_processing/aec3/matched_filter.cc

@@ -368,7 +368,7 @@ void MatchedFilter::Update(const DownsampledRenderBuffer& render_buffer,
             [](float a, float b) -> bool { return a * a < b * b; }));
 
     // Update the lag estimates for the matched filter.
-    const float kMatchingFilterThreshold = 0.1f;
+    const float kMatchingFilterThreshold = 0.2f;
     lag_estimates_[n] = LagEstimate(
         error_sum_anchor - error_sum,
         (lag_estimate > 2 && lag_estimate < (filters_[n].size() - 10) &&

modules/audio_processing/aec3/matched_filter_lag_aggregator.cc

@@ -14,40 +14,33 @@
 namespace webrtc {
 
 MatchedFilterLagAggregator::MatchedFilterLagAggregator(
-    ApmDataDumper* data_dumper,
+    ApmDataDumper* data_dumper)
-    size_t num_lag_estimates)
+    : data_dumper_(data_dumper) {
-    : data_dumper_(data_dumper), lag_updates_in_a_row_(num_lag_estimates, 0) {
   RTC_DCHECK(data_dumper);
-  RTC_DCHECK_LT(0, num_lag_estimates);
+  histogram_.fill(0);
+  histogram_data_.fill(0);
 }
 
 MatchedFilterLagAggregator::~MatchedFilterLagAggregator() = default;
 
 void MatchedFilterLagAggregator::Reset() {
-  candidate_ = 0;
+  histogram_.fill(0);
-  candidate_counter_ = 0;
+  histogram_data_.fill(0);
-  std::fill(lag_updates_in_a_row_.begin(), lag_updates_in_a_row_.end(), 0.f);
+  histogram_data_index_ = 0;
+  filled_histogram_ = false;
 }
 
 rtc::Optional<size_t> MatchedFilterLagAggregator::Aggregate(
     rtc::ArrayView<const MatchedFilter::LagEstimate> lag_estimates) {
-  RTC_DCHECK_EQ(lag_updates_in_a_row_.size(), lag_estimates.size());
+  // hoose the strongest lag estimate as the best one.
-
+  float best_accuracy = 0.f;
-  // Count the number of lag updates in a row to ensure that only stable lags
-  // are taken into account.
-  for (size_t k = 0; k < lag_estimates.size(); ++k) {
-    lag_updates_in_a_row_[k] =
-        lag_estimates[k].updated ? lag_updates_in_a_row_[k] + 1 : 0;
-  }
-
-  // If available, choose the strongest lag estimate as the best one.
   int best_lag_estimate_index = -1;
   for (size_t k = 0; k < lag_estimates.size(); ++k) {
-    if (lag_updates_in_a_row_[k] > 10 && lag_estimates[k].reliable &&
+    if (lag_estimates[k].updated && lag_estimates[k].reliable) {
-        (best_lag_estimate_index == -1 ||
+      if (lag_estimates[k].accuracy > best_accuracy) {
-         lag_estimates[k].accuracy >
+        best_accuracy = std::max(0.f, lag_estimates[k].accuracy);
-             lag_estimates[best_lag_estimate_index].accuracy)) {
+        best_lag_estimate_index = static_cast<int>(k);
-      best_lag_estimate_index = k;
+      }
     }
   }
 
@@ -55,18 +48,31 @@ rtc::Optional<size_t> MatchedFilterLagAggregator::Aggregate(
   data_dumper_->DumpRaw("aec3_echo_path_delay_estimator_best_index",
                         best_lag_estimate_index);
 
-  // Require the same lag to be detected 10 times in a row before considering
+  if (best_lag_estimate_index != -1) {
-  // it reliable.
+    RTC_DCHECK_GT(histogram_.size(), histogram_data_[histogram_data_index_]);
-  if (best_lag_estimate_index >= 0) {
+    RTC_DCHECK_LE(0, histogram_data_[histogram_data_index_]);
-    candidate_counter_ =
+    --histogram_[histogram_data_[histogram_data_index_]];
-        (candidate_ == lag_estimates[best_lag_estimate_index].lag)
-            ? candidate_counter_ + 1
-            : 0;
-    candidate_ = lag_estimates[best_lag_estimate_index].lag;
-  }
 
-  return candidate_counter_ >= 15 ? rtc::Optional<size_t>(candidate_)
+    histogram_data_[histogram_data_index_] =
-                                  : rtc::Optional<size_t>();
+        lag_estimates[best_lag_estimate_index].lag;
+
+    RTC_DCHECK_GT(histogram_.size(), histogram_data_[histogram_data_index_]);
+    RTC_DCHECK_LE(0, histogram_data_[histogram_data_index_]);
+    ++histogram_[histogram_data_[histogram_data_index_]];
+
+    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(),
+                      std::max_element(histogram_.begin(), histogram_.end()));
+
+    if (histogram_[candidate] > 25) {
+      return rtc::Optional<size_t>(candidate);
+    }
+  }
+  return rtc::Optional<size_t>();
 }
 
 }  // namespace webrtc

modules/audio_processing/aec3/matched_filter_lag_aggregator.h

@@ -25,8 +25,7 @@ class ApmDataDumper;
 // reliable combined lag estimate.
 class MatchedFilterLagAggregator {
  public:
-  MatchedFilterLagAggregator(ApmDataDumper* data_dumper,
+  explicit MatchedFilterLagAggregator(ApmDataDumper* data_dumper);
-                             size_t num_lag_estimates);
   ~MatchedFilterLagAggregator();
 
   // Resets the aggregator.
@@ -38,9 +37,10 @@ class MatchedFilterLagAggregator {
 
  private:
   ApmDataDumper* const data_dumper_;
-  std::vector<size_t> lag_updates_in_a_row_;
+  std::array<int, 1664> histogram_;
-  size_t candidate_ = 0;
+  std::array<int, 250> histogram_data_;
-  size_t candidate_counter_ = 0;
+  int histogram_data_index_ = 0;
+  bool filled_histogram_ = false;
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(MatchedFilterLagAggregator);
 };

modules/audio_processing/aec3/matched_filter_lag_aggregator_unittest.cc

@@ -32,17 +32,18 @@ void VerifyNoAggregateOutputForRepeatedLagAggregation(
 }
 
 constexpr size_t kThresholdForRequiredLagUpdatesInARow = 10;
-constexpr size_t kThresholdForRequiredIdenticalLagAggregates = 15;
+constexpr size_t kThresholdForRequiredIdenticalLagAggregates = 100;
 
 }  // namespace
 
 // Verifies that the most accurate lag estimate is chosen.
-TEST(MatchedFilterLagAggregator, MostAccurateLagChosen) {
+// TODO(peah): Modify and reenable according to new scheme.
+TEST(MatchedFilterLagAggregator, DISABLED_MostAccurateLagChosen) {
   constexpr size_t kArtificialLag1 = 5;
   constexpr size_t kArtificialLag2 = 10;
   ApmDataDumper data_dumper(0);
   std::vector<MatchedFilter::LagEstimate> lag_estimates(2);
-  MatchedFilterLagAggregator aggregator(&data_dumper, lag_estimates.size());
+  MatchedFilterLagAggregator aggregator(&data_dumper);
   lag_estimates[0] =
       MatchedFilter::LagEstimate(1.f, true, kArtificialLag1, true);
   lag_estimates[1] =
@@ -70,13 +71,14 @@ TEST(MatchedFilterLagAggregator, MostAccurateLagChosen) {
 
 // Verifies that varying lag estimates causes lag estimates to not be deemed
 // reliable.
+// TODO(peah): Modify and reenable according to new scheme.
 TEST(MatchedFilterLagAggregator,
-     LagEstimateInvarianceRequiredForAggregatedLag) {
+     DISABLED_LagEstimateInvarianceRequiredForAggregatedLag) {
   constexpr size_t kArtificialLag1 = 5;
   constexpr size_t kArtificialLag2 = 10;
   ApmDataDumper data_dumper(0);
   std::vector<MatchedFilter::LagEstimate> lag_estimates(1);
-  MatchedFilterLagAggregator aggregator(&data_dumper, lag_estimates.size());
+  MatchedFilterLagAggregator aggregator(&data_dumper);
   lag_estimates[0] =
       MatchedFilter::LagEstimate(1.f, true, kArtificialLag1, true);
   VerifyNoAggregateOutputForRepeatedLagAggregation(
@@ -99,12 +101,14 @@ TEST(MatchedFilterLagAggregator,
 
 // Verifies that lag estimate updates are required to produce an updated lag
 // aggregate.
-TEST(MatchedFilterLagAggregator, LagEstimateUpdatesRequiredForAggregatedLag) {
+// TODO(peah): Modify and reenable according to new scheme.
+TEST(MatchedFilterLagAggregator,
+     DISABLED_LagEstimateUpdatesRequiredForAggregatedLag) {
   constexpr size_t kArtificialLag1 = 5;
   constexpr size_t kArtificialLag2 = 10;
   ApmDataDumper data_dumper(0);
   std::vector<MatchedFilter::LagEstimate> lag_estimates(1);
-  MatchedFilterLagAggregator aggregator(&data_dumper, lag_estimates.size());
+  MatchedFilterLagAggregator aggregator(&data_dumper);
   lag_estimates[0] =
       MatchedFilter::LagEstimate(1.f, true, kArtificialLag1, true);
   VerifyNoAggregateOutputForRepeatedLagAggregation(
@@ -145,11 +149,12 @@ TEST(MatchedFilterLagAggregator, LagEstimateUpdatesRequiredForAggregatedLag) {
 // 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.
-TEST(MatchedFilterLagAggregator, PersistentAggregatedLag) {
+// TODO(peah): Modify and reenable according to new scheme.
+TEST(MatchedFilterLagAggregator, DISABLED_PersistentAggregatedLag) {
   constexpr size_t kArtificialLag = 5;
   ApmDataDumper data_dumper(0);
   std::vector<MatchedFilter::LagEstimate> lag_estimates(1);
-  MatchedFilterLagAggregator aggregator(&data_dumper, lag_estimates.size());
+  MatchedFilterLagAggregator aggregator(&data_dumper);
   lag_estimates[0] =
       MatchedFilter::LagEstimate(1.f, true, kArtificialLag, true);
   VerifyNoAggregateOutputForRepeatedLagAggregation(
@@ -167,24 +172,9 @@ TEST(MatchedFilterLagAggregator, PersistentAggregatedLag) {
 
 #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
 
-// Verifies the check for correct number of lag estimates.
-TEST(MatchedFilterLagAggregator, IncorrectNumberOfLagEstimates) {
-  ApmDataDumper data_dumper(0);
-  MatchedFilterLagAggregator aggregator(&data_dumper, 1);
-  std::vector<MatchedFilter::LagEstimate> lag_estimates(2);
-
-  EXPECT_DEATH(aggregator.Aggregate(lag_estimates), "");
-}
-
-// Verifies the check for non-zero number of lag estimates.
-TEST(MatchedFilterLagAggregator, NonZeroLagEstimates) {
-  ApmDataDumper data_dumper(0);
-  EXPECT_DEATH(MatchedFilterLagAggregator(&data_dumper, 0), "");
-}
-
 // Verifies the check for non-null data dumper.
 TEST(MatchedFilterLagAggregator, NullDataDumper) {
-  EXPECT_DEATH(MatchedFilterLagAggregator(nullptr, 1), "");
+  EXPECT_DEATH(MatchedFilterLagAggregator(nullptr), "");
 }
 
 #endif