Revert of New statistics interface for APM (patchset #11 id:200001 of https://codereview.webrtc.org/2433153003/ )
Reason for revert:
This CL breaks internal dependencies.
Original issue's description:
> New statistics interface for APM
>
> This adds functions to enable and retrieve statistics from APM. These functions are not yet called, which will happen in a follow-up CL.
>
> BUG=webrtc:6525
>
> Committed: https://crrev.com/8b8d3e4c30e8ea3846b58dfd36d1fd35a7799df4
> Cr-Commit-Position: refs/heads/master@{#14810}
TBR=peah@webrtc.org,solenberg@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6525
Review-Url: https://codereview.webrtc.org/2456333002
Cr-Commit-Position: refs/heads/master@{#14814}
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc
index b1a4193..acbc80c 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -440,10 +440,6 @@
num_proc_channels());
AllocateRenderQueue();
- int success = public_submodules_->echo_cancellation->enable_metrics(true);
- RTC_DCHECK_EQ(0, success);
- success = public_submodules_->echo_cancellation->enable_delay_logging(true);
- RTC_DCHECK_EQ(0, success);
public_submodules_->echo_control_mobile->Initialize(
proc_split_sample_rate_hz(), num_reverse_channels(),
num_output_channels());
@@ -1422,22 +1418,6 @@
#endif // WEBRTC_AUDIOPROC_DEBUG_DUMP
}
-AudioProcessing::AudioProcessingStatistics AudioProcessingImpl::GetStatistics()
- const {
- AudioProcessingStatistics stats;
- EchoCancellation::Metrics metrics;
- public_submodules_->echo_cancellation->GetMetrics(&metrics);
- stats.a_nlp.Set(metrics.a_nlp);
- stats.divergent_filter_fraction = metrics.divergent_filter_fraction;
- stats.echo_return_loss.Set(metrics.echo_return_loss);
- stats.echo_return_loss_enhancement.Set(metrics.echo_return_loss_enhancement);
- stats.residual_echo_return_loss.Set(metrics.residual_echo_return_loss);
- public_submodules_->echo_cancellation->GetDelayMetrics(
- &stats.delay_median, &stats.delay_standard_deviation,
- &stats.fraction_poor_delays);
- return stats;
-}
-
EchoCancellation* AudioProcessingImpl::echo_cancellation() const {
return public_submodules_->echo_cancellation.get();
}
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.h b/webrtc/modules/audio_processing/audio_processing_impl.h
index eb14308..aec4ed7 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.h
+++ b/webrtc/modules/audio_processing/audio_processing_impl.h
@@ -116,8 +116,6 @@
bool was_stream_delay_set() const override
EXCLUSIVE_LOCKS_REQUIRED(crit_capture_);
- AudioProcessingStatistics GetStatistics() const override;
-
// Methods returning pointers to APM submodules.
// No locks are aquired in those, as those locks
// would offer no protection (the submodules are
diff --git a/webrtc/modules/audio_processing/audio_processing_unittest.cc b/webrtc/modules/audio_processing/audio_processing_unittest.cc
index f865c87..d4faa81 100644
--- a/webrtc/modules/audio_processing/audio_processing_unittest.cc
+++ b/webrtc/modules/audio_processing/audio_processing_unittest.cc
@@ -929,9 +929,6 @@
EXPECT_EQ(apm_->kNotEnabledError,
apm_->echo_cancellation()->GetMetrics(&metrics));
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_enabled());
-
EXPECT_EQ(apm_->kNoError,
apm_->echo_cancellation()->enable_metrics(true));
EXPECT_TRUE(apm_->echo_cancellation()->are_metrics_enabled());
@@ -939,16 +936,6 @@
apm_->echo_cancellation()->enable_metrics(false));
EXPECT_FALSE(apm_->echo_cancellation()->are_metrics_enabled());
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_delay_logging(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_delay_logging_enabled());
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_delay_logging(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_delay_logging_enabled());
-
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_enabled());
-
int median = 0;
int std = 0;
float poor_fraction = 0;
@@ -956,6 +943,13 @@
apm_->echo_cancellation()->GetDelayMetrics(&median, &std,
&poor_fraction));
+ EXPECT_EQ(apm_->kNoError,
+ apm_->echo_cancellation()->enable_delay_logging(true));
+ EXPECT_TRUE(apm_->echo_cancellation()->is_delay_logging_enabled());
+ EXPECT_EQ(apm_->kNoError,
+ apm_->echo_cancellation()->enable_delay_logging(false));
+ EXPECT_FALSE(apm_->echo_cancellation()->is_delay_logging_enabled());
+
EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
EXPECT_TRUE(apm_->echo_cancellation()->is_enabled());
EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
diff --git a/webrtc/modules/audio_processing/echo_cancellation_impl.cc b/webrtc/modules/audio_processing/echo_cancellation_impl.cc
index 0b8fc14..e06b2fa 100644
--- a/webrtc/modules/audio_processing/echo_cancellation_impl.cc
+++ b/webrtc/modules/audio_processing/echo_cancellation_impl.cc
@@ -270,7 +270,7 @@
bool EchoCancellationImpl::are_metrics_enabled() const {
rtc::CritScope cs(crit_capture_);
- return enabled_ && metrics_enabled_;
+ return metrics_enabled_;
}
// TODO(ajm): we currently just use the metrics from the first AEC. Think more
@@ -333,7 +333,7 @@
bool EchoCancellationImpl::is_delay_logging_enabled() const {
rtc::CritScope cs(crit_capture_);
- return enabled_ && delay_logging_enabled_;
+ return delay_logging_enabled_;
}
bool EchoCancellationImpl::is_delay_agnostic_enabled() const {
diff --git a/webrtc/modules/audio_processing/echo_cancellation_impl.h b/webrtc/modules/audio_processing/echo_cancellation_impl.h
index 03be985..8a4ea71 100644
--- a/webrtc/modules/audio_processing/echo_cancellation_impl.h
+++ b/webrtc/modules/audio_processing/echo_cancellation_impl.h
@@ -58,20 +58,6 @@
static size_t NumCancellersRequired(size_t num_output_channels,
size_t num_reverse_channels);
- // Enable logging of various AEC statistics.
- int enable_metrics(bool enable) override;
-
- // Provides various statistics about the AEC.
- int GetMetrics(Metrics* metrics) override;
-
- // Enable logging of delay metrics.
- int enable_delay_logging(bool enable) override;
-
- // Provides delay metrics.
- int GetDelayMetrics(int* median,
- int* std,
- float* fraction_poor_delays) override;
-
private:
class Canceller;
struct StreamProperties;
@@ -81,10 +67,16 @@
int enable_drift_compensation(bool enable) override;
void set_stream_drift_samples(int drift) override;
int set_suppression_level(SuppressionLevel level) override;
+ int enable_metrics(bool enable) override;
bool are_metrics_enabled() const override;
bool stream_has_echo() const override;
+ int GetMetrics(Metrics* metrics) override;
+ int enable_delay_logging(bool enable) override;
bool is_delay_logging_enabled() const override;
int GetDelayMetrics(int* median, int* std) override;
+ int GetDelayMetrics(int* median,
+ int* std,
+ float* fraction_poor_delays) override;
struct AecCore* aec_core() const override;
diff --git a/webrtc/modules/audio_processing/include/audio_processing.h b/webrtc/modules/audio_processing/include/audio_processing.h
index d50c93d..0fdbfd2 100644
--- a/webrtc/modules/audio_processing/include/audio_processing.h
+++ b/webrtc/modules/audio_processing/include/audio_processing.h
@@ -469,74 +469,6 @@
// specific member variables are reset.
virtual void UpdateHistogramsOnCallEnd() = 0;
- // TODO(ivoc): Remove when the calling code no longer uses the old Statistics
- // API.
- struct Statistic {
- int instant = 0; // Instantaneous value.
- int average = 0; // Long-term average.
- int maximum = 0; // Long-term maximum.
- int minimum = 0; // Long-term minimum.
- };
-
- struct Stat {
- void Set(const Statistic& other) {
- Set(other.instant, other.average, other.maximum, other.minimum);
- }
- void Set(float instant, float average, float maximum, float minimum) {
- RTC_DCHECK_LE(instant, maximum);
- RTC_DCHECK_GE(instant, minimum);
- RTC_DCHECK_LE(average, maximum);
- RTC_DCHECK_GE(average, minimum);
- instant_ = instant;
- average_ = average;
- maximum_ = maximum;
- minimum_ = minimum;
- }
- float instant() const { return instant_; }
- float average() const { return average_; }
- float maximum() const { return maximum_; }
- float minimum() const { return minimum_; }
-
- private:
- float instant_ = 0.0f; // Instantaneous value.
- float average_ = 0.0f; // Long-term average.
- float maximum_ = 0.0f; // Long-term maximum.
- float minimum_ = 0.0f; // Long-term minimum.
- };
-
- struct AudioProcessingStatistics {
- // AEC Statistics.
- // RERL = ERL + ERLE
- Stat residual_echo_return_loss;
- // ERL = 10log_10(P_far / P_echo)
- Stat echo_return_loss;
- // ERLE = 10log_10(P_echo / P_out)
- Stat echo_return_loss_enhancement;
- // (Pre non-linear processing suppression) A_NLP = 10log_10(P_echo / P_a)
- Stat a_nlp;
- // Fraction of time that the AEC linear filter is divergent, in a 1-second
- // non-overlapped aggregation window.
- float divergent_filter_fraction = 0.0f;
-
- // The delay metrics consists of the delay median and standard deviation. It
- // also consists of the fraction of delay estimates that can make the echo
- // cancellation perform poorly. The values are aggregated until the first
- // call to |GetStatistics()| and afterwards aggregated and updated every
- // second. Note that if there are several clients pulling metrics from
- // |GetStatistics()| during a session the first call from any of them will
- // change to one second aggregation window for all.
- int delay_median = 0;
- int delay_standard_deviation = 0;
- float fraction_poor_delays = 0.0f;
-
- // Residual echo detector likelihood. This value is not yet calculated and
- // is currently always set to zero.
- // TODO(ivoc): Implement this stat.
- float residual_echo_likelihood = 0.0f;
- };
-
- virtual AudioProcessingStatistics GetStatistics() const = 0;
-
// These provide access to the component interfaces and should never return
// NULL. The pointers will be valid for the lifetime of the APM instance.
// The memory for these objects is entirely managed internally.
@@ -548,6 +480,13 @@
virtual NoiseSuppression* noise_suppression() const = 0;
virtual VoiceDetection* voice_detection() const = 0;
+ struct Statistic {
+ int instant; // Instantaneous value.
+ int average; // Long-term average.
+ int maximum; // Long-term maximum.
+ int minimum; // Long-term minimum.
+ };
+
enum Error {
// Fatal errors.
kNoError = 0,
@@ -769,7 +708,6 @@
float divergent_filter_fraction;
};
- // Deprecated. Use GetStatistics on the AudioProcessing interface instead.
// TODO(ajm): discuss the metrics update period.
virtual int GetMetrics(Metrics* metrics) = 0;
@@ -786,9 +724,8 @@
// Note that if there are several clients pulling metrics from
// |GetDelayMetrics()| during a session the first call from any of them will
// change to one second aggregation window for all.
- // Deprecated. Use GetStatistics on the AudioProcessing interface instead.
+ // TODO(bjornv): Deprecated, remove.
virtual int GetDelayMetrics(int* median, int* std) = 0;
- // Deprecated. Use GetStatistics on the AudioProcessing interface instead.
virtual int GetDelayMetrics(int* median, int* std,
float* fraction_poor_delays) = 0;
diff --git a/webrtc/modules/audio_processing/include/mock_audio_processing.h b/webrtc/modules/audio_processing/include/mock_audio_processing.h
index b5a1fa4..8acb83b 100644
--- a/webrtc/modules/audio_processing/include/mock_audio_processing.h
+++ b/webrtc/modules/audio_processing/include/mock_audio_processing.h
@@ -183,7 +183,6 @@
int(rtc::PlatformFile handle));
MOCK_METHOD0(StopDebugRecording, int());
MOCK_METHOD0(UpdateHistogramsOnCallEnd, void());
- MOCK_CONST_METHOD0(GetStatistics, AudioProcessingStatistics());
virtual MockEchoCancellation* echo_cancellation() const {
return echo_cancellation_.get();
}
diff --git a/webrtc/voice_engine/test/auto_test/extended/ec_metrics_test.cc b/webrtc/voice_engine/test/auto_test/extended/ec_metrics_test.cc
index 758a085..a202aea 100644
--- a/webrtc/voice_engine/test/auto_test/extended/ec_metrics_test.cc
+++ b/webrtc/voice_engine/test/auto_test/extended/ec_metrics_test.cc
@@ -13,19 +13,13 @@
class EcMetricsTest : public AfterStreamingFixture {
};
-TEST_F(EcMetricsTest, EcMetricsAreOnByDefault) {
- // AEC must be enabled fist.
- EXPECT_EQ(0, voe_apm_->SetEcStatus(true, webrtc::kEcAec));
-
- bool enabled = false;
+TEST_F(EcMetricsTest, EcMetricsAreOffByDefault) {
+ bool enabled = true;
EXPECT_EQ(0, voe_apm_->GetEcMetricsStatus(enabled));
- EXPECT_TRUE(enabled);
+ EXPECT_FALSE(enabled);
}
TEST_F(EcMetricsTest, CanEnableAndDisableEcMetrics) {
- // AEC must be enabled fist.
- EXPECT_EQ(0, voe_apm_->SetEcStatus(true, webrtc::kEcAec));
-
EXPECT_EQ(0, voe_apm_->SetEcMetricsStatus(true));
bool ec_on = false;
EXPECT_EQ(0, voe_apm_->GetEcMetricsStatus(ec_on));