diff --git a/modules/audio_processing/aec3/render_delay_controller.cc b/modules/audio_processing/aec3/render_delay_controller.cc index 7fd2868c38..465e77fb7c 100644 --- a/modules/audio_processing/aec3/render_delay_controller.cc +++ b/modules/audio_processing/aec3/render_delay_controller.cc @@ -155,9 +155,11 @@ absl::optional RenderDelayControllerImpl::GetDelay( last_delay_estimate_quality_ = delay_samples_->quality; } - metrics_.Update(delay_samples_ ? absl::optional(delay_samples_->delay) - : absl::nullopt, - delay_ ? delay_->delay : 0, 0, delay_estimator_.Clockdrift()); + metrics_.Update( + delay_samples_ ? absl::optional(delay_samples_->delay) + : absl::nullopt, + delay_ ? absl::optional(delay_->delay) : absl::nullopt, + delay_estimator_.Clockdrift()); data_dumper_->DumpRaw("aec3_render_delay_controller_delay", delay_samples ? delay_samples->delay : 0); diff --git a/modules/audio_processing/aec3/render_delay_controller_metrics.cc b/modules/audio_processing/aec3/render_delay_controller_metrics.cc index 582e033482..1e0a0f443e 100644 --- a/modules/audio_processing/aec3/render_delay_controller_metrics.cc +++ b/modules/audio_processing/aec3/render_delay_controller_metrics.cc @@ -37,16 +37,13 @@ enum class DelayChangesCategory { kNumCategories }; -constexpr int kMaxSkewShiftCount = 20; - } // namespace RenderDelayControllerMetrics::RenderDelayControllerMetrics() = default; void RenderDelayControllerMetrics::Update( absl::optional delay_samples, - size_t buffer_delay_blocks, - absl::optional skew_shift_blocks, + absl::optional buffer_delay_blocks, ClockdriftDetector::Level clockdrift) { ++call_counter_; @@ -54,6 +51,8 @@ void RenderDelayControllerMetrics::Update( size_t delay_blocks; if (delay_samples) { ++reliable_delay_estimate_counter_; + // Add an offset by 1 (metric is halved before reporting) to reserve 0 for + // absent delay. delay_blocks = (*delay_samples) / kBlockSize + 2; } else { delay_blocks = 0; @@ -64,21 +63,21 @@ void RenderDelayControllerMetrics::Update( delay_blocks_ = delay_blocks; } - if (skew_shift_blocks) { - skew_shift_count_ = std::min(kMaxSkewShiftCount, skew_shift_count_); - } } else if (++initial_call_counter_ == 5 * kNumBlocksPerSecond) { initial_update = false; } if (call_counter_ == kMetricsReportingIntervalBlocks) { int value_to_report = static_cast(delay_blocks_); + // Divide by 2 to compress metric range. value_to_report = std::min(124, value_to_report >> 1); RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.EchoCanceller.EchoPathDelay", value_to_report, 0, 124, 125); - value_to_report = static_cast(buffer_delay_blocks + 2); - value_to_report = std::min(124, value_to_report >> 1); + // Divide by 2 to compress metric range. + // Offset by 1 to reserve 0 for absent delay. + value_to_report = buffer_delay_blocks ? (*buffer_delay_blocks + 2) >> 1 : 0; + value_to_report = std::min(124, value_to_report); RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.EchoCanceller.BufferDelay", value_to_report, 0, 124, 125); @@ -120,20 +119,8 @@ void RenderDelayControllerMetrics::Update( "WebRTC.Audio.EchoCanceller.Clockdrift", static_cast(clockdrift), static_cast(ClockdriftDetector::Level::kNumCategories)); - metrics_reported_ = true; call_counter_ = 0; ResetMetrics(); - } else { - metrics_reported_ = false; - } - - if (!initial_update && ++skew_report_timer_ == 60 * kNumBlocksPerSecond) { - RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.EchoCanceller.MaxSkewShiftCount", - skew_shift_count_, 0, kMaxSkewShiftCount, - kMaxSkewShiftCount + 1); - - skew_shift_count_ = 0; - skew_report_timer_ = 0; } } diff --git a/modules/audio_processing/aec3/render_delay_controller_metrics.h b/modules/audio_processing/aec3/render_delay_controller_metrics.h index 309122d80d..b81833b43f 100644 --- a/modules/audio_processing/aec3/render_delay_controller_metrics.h +++ b/modules/audio_processing/aec3/render_delay_controller_metrics.h @@ -29,13 +29,9 @@ class RenderDelayControllerMetrics { // Updates the metric with new data. void Update(absl::optional delay_samples, - size_t buffer_delay_blocks, - absl::optional skew_shift_blocks, + absl::optional buffer_delay_blocks, ClockdriftDetector::Level clockdrift); - // Returns true if the metrics have just been reported, otherwise false. - bool MetricsReported() { return metrics_reported_; } - private: // Resets the metrics. void ResetMetrics(); @@ -44,11 +40,8 @@ class RenderDelayControllerMetrics { int reliable_delay_estimate_counter_ = 0; int delay_change_counter_ = 0; int call_counter_ = 0; - int skew_report_timer_ = 0; int initial_call_counter_ = 0; - bool metrics_reported_ = false; bool initial_update = true; - int skew_shift_count_ = 0; }; } // namespace webrtc diff --git a/modules/audio_processing/aec3/render_delay_controller_metrics_unittest.cc b/modules/audio_processing/aec3/render_delay_controller_metrics_unittest.cc index e7d7703433..2d242336f2 100644 --- a/modules/audio_processing/aec3/render_delay_controller_metrics_unittest.cc +++ b/modules/audio_processing/aec3/render_delay_controller_metrics_unittest.cc @@ -12,6 +12,7 @@ #include "absl/types/optional.h" #include "modules/audio_processing/aec3/aec3_common.h" +#include "system_wrappers/include/metrics.h" #include "test/gtest.h" namespace webrtc { @@ -20,15 +21,49 @@ namespace webrtc { TEST(RenderDelayControllerMetrics, NormalUsage) { RenderDelayControllerMetrics metrics; + int expected_num_metric_reports = 0; + for (int j = 0; j < 3; ++j) { for (int k = 0; k < kMetricsReportingIntervalBlocks - 1; ++k) { - metrics.Update(absl::nullopt, 0, absl::nullopt, + metrics.Update(absl::nullopt, absl::nullopt, ClockdriftDetector::Level::kNone); - EXPECT_FALSE(metrics.MetricsReported()); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.EchoPathDelay"), + expected_num_metric_reports); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.BufferDelay"), + expected_num_metric_reports); + EXPECT_METRIC_EQ(metrics::NumSamples( + "WebRTC.Audio.EchoCanceller.ReliableDelayEstimates"), + expected_num_metric_reports); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.DelayChanges"), + expected_num_metric_reports); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.Clockdrift"), + expected_num_metric_reports); } - metrics.Update(absl::nullopt, 0, absl::nullopt, + + // We expect metric reports every kMetricsReportingIntervalBlocks blocks. + ++expected_num_metric_reports; + + metrics.Update(absl::nullopt, absl::nullopt, ClockdriftDetector::Level::kNone); - EXPECT_TRUE(metrics.MetricsReported()); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.EchoPathDelay"), + expected_num_metric_reports); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.BufferDelay"), + expected_num_metric_reports); + EXPECT_METRIC_EQ(metrics::NumSamples( + "WebRTC.Audio.EchoCanceller.ReliableDelayEstimates"), + expected_num_metric_reports); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.DelayChanges"), + expected_num_metric_reports); + EXPECT_METRIC_EQ( + metrics::NumSamples("WebRTC.Audio.EchoCanceller.Clockdrift"), + expected_num_metric_reports); } }