Reland: AEC3: clarify render delay controller metrics

This CL:
- makes it easier to understand the (nontrivial) metric interpretation
- corrects the computation of BufferDelay to use 0 for absent delay
- deletes metric MaxSkewShiftCount, unused since https://webrtc-review.googlesource.com/c/src/+/119701
- updates the unit test to directly test metric reporting

Corresponding update to histograms.xml:
https://crrev.com/c/3944909

Previous revert:
https://webrtc-review.googlesource.com/c/src/+/279040
This CL is identical to the original, except:
- the test is updated to spam fewer EXPECT_EQ failures on failure (EXPECT_EQs moved out of inner loop)
- the test not resets metrics (metrics::Reset()) at the beginning, like other histogram tests

Bug: webrtc:8671, chromium:1349051
Change-Id: Ie802e1f9d03a22ff7018f522a63b19e0b6eec2e8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279046
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38376}
This commit is contained in:
Sam Zackrisson
2022-10-12 17:14:31 +02:00
committed by WebRTC LUCI CQ
parent fbbe415dd4
commit 129f40718c
4 changed files with 55 additions and 36 deletions

View File

@ -155,9 +155,11 @@ absl::optional<DelayEstimate> RenderDelayControllerImpl::GetDelay(
last_delay_estimate_quality_ = delay_samples_->quality;
}
metrics_.Update(delay_samples_ ? absl::optional<size_t>(delay_samples_->delay)
: absl::nullopt,
delay_ ? delay_->delay : 0, 0, delay_estimator_.Clockdrift());
metrics_.Update(
delay_samples_ ? absl::optional<size_t>(delay_samples_->delay)
: absl::nullopt,
delay_ ? absl::optional<size_t>(delay_->delay) : absl::nullopt,
delay_estimator_.Clockdrift());
data_dumper_->DumpRaw("aec3_render_delay_controller_delay",
delay_samples ? delay_samples->delay : 0);

View File

@ -37,16 +37,13 @@ enum class DelayChangesCategory {
kNumCategories
};
constexpr int kMaxSkewShiftCount = 20;
} // namespace
RenderDelayControllerMetrics::RenderDelayControllerMetrics() = default;
void RenderDelayControllerMetrics::Update(
absl::optional<size_t> delay_samples,
size_t buffer_delay_blocks,
absl::optional<int> skew_shift_blocks,
absl::optional<size_t> 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<int>(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<int>(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<int>(clockdrift),
static_cast<int>(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;
}
}

View File

@ -29,13 +29,9 @@ class RenderDelayControllerMetrics {
// Updates the metric with new data.
void Update(absl::optional<size_t> delay_samples,
size_t buffer_delay_blocks,
absl::optional<int> skew_shift_blocks,
absl::optional<size_t> 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

View File

@ -12,23 +12,60 @@
#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 {
// Verify the general functionality of RenderDelayControllerMetrics.
TEST(RenderDelayControllerMetrics, NormalUsage) {
metrics::Reset();
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());
}
metrics.Update(absl::nullopt, 0, absl::nullopt,
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);
// 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);
}
}