From 6558fa5d64c4d913f4b3ac863c2670e865dbeda6 Mon Sep 17 00:00:00 2001 From: Sam Zackrisson Date: Mon, 26 Aug 2019 10:12:41 +0200 Subject: [PATCH] Reintroduce command line controlled reference data updating for ApmTest.Process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces a hardcoded bool in a test with command line flag. The current hardcoding of the bool is a little bit hacky. And the tests will pass automatically, so it is possible to accidentally commit the flipped bool in a CL, like here: https://webrtc-review.googlesource.com/c/src/+/150221 I am fairly sure this resolves the vague issue referred to in the attached bug. The bug is introduced with a TODO here: https://webrtc-codereview.appspot.com/1728005/diff/4001/webrtc/modules/audio_processing/test/unit_test.cc Another TODO was added later that refers to the first TODO: https://webrtc-codereview.appspot.com/6879004/diff/150001/webrtc/modules/audio_processing/test/audio_processing_unittest.cc Bug: webrtc:1981 Change-Id: I066f41add602c791a5f2ba18829c4306da7dac15 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150334 Reviewed-by: Per Åhgren Commit-Queue: Sam Zackrisson Cr-Commit-Position: refs/heads/master@{#28955} --- modules/audio_processing/BUILD.gn | 1 + .../audio_processing_unittest.cc | 22 +++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index c8a9dbc383..376dae2217 100644 --- a/modules/audio_processing/BUILD.gn +++ b/modules/audio_processing/BUILD.gn @@ -529,6 +529,7 @@ if (rtc_include_tests) { "../../rtc_base:rtc_task_queue", "aec_dump", "aec_dump:aec_dump_unittests", + "//third_party/abseil-cpp/absl/flags:flag", ] sources += [ "audio_processing_impl_locking_unittest.cc", diff --git a/modules/audio_processing/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc index 23657b8e45..2556f67d4e 100644 --- a/modules/audio_processing/audio_processing_unittest.cc +++ b/modules/audio_processing/audio_processing_unittest.cc @@ -18,6 +18,7 @@ #include #include +#include "absl/flags/flag.h" #include "common_audio/include/audio_util.h" #include "common_audio/resampler/include/push_resampler.h" #include "common_audio/resampler/push_sinc_resampler.h" @@ -53,19 +54,18 @@ RTC_PUSH_IGNORING_WUNDEF() #endif RTC_POP_IGNORING_WUNDEF() +ABSL_FLAG(bool, + write_apm_ref_data, + false, + "Write ApmTest.Process results to file, instead of comparing results " + "to the existing reference data file."); + namespace webrtc { namespace { // TODO(ekmeyerson): Switch to using StreamConfig and ProcessingConfig where // applicable. -// TODO(bjornv): This is not feasible until the functionality has been -// re-implemented; see comment at the bottom of this file. For now, the user has -// to hard code the |write_ref_data| value. -// When false, this will compare the output data with the results stored to -// file. This is the typical case. When the file should be updated, it can -// be set to true with the command-line switch --write_ref_data. -bool write_ref_data = false; const int32_t kChannels[] = {1, 2}; const int kSampleRates[] = {8000, 16000, 32000, 48000}; @@ -1569,7 +1569,7 @@ TEST_F(ApmTest, Process) { GOOGLE_PROTOBUF_VERIFY_VERSION; audioproc::OutputData ref_data; - if (!write_ref_data) { + if (!absl::GetFlag(FLAGS_write_apm_ref_data)) { OpenFileAndReadMessage(ref_filename_, &ref_data); } else { // Write the desired tests to the protobuf reference file. @@ -1689,7 +1689,7 @@ TEST_F(ApmTest, Process) { const float residual_echo_likelihood_recent_max = stats.residual_echo_likelihood_recent_max.value_or(-1.0f); - if (!write_ref_data) { + if (!absl::GetFlag(FLAGS_write_apm_ref_data)) { const audioproc::Test::EchoMetrics& reference = test->echo_metrics(stats_index); constexpr float kEpsilon = 0.01; @@ -1719,7 +1719,7 @@ TEST_F(ApmTest, Process) { ns_speech_prob_average /= frame_count; rms_dbfs_average /= frame_count; - if (!write_ref_data) { + if (!absl::GetFlag(FLAGS_write_apm_ref_data)) { const int kIntNear = 1; // When running the test on a N7 we get a {2, 6} difference of // |has_voice_count| and |max_output_average| is up to 18 higher. @@ -1771,7 +1771,7 @@ TEST_F(ApmTest, Process) { rewind(near_file_); } - if (write_ref_data) { + if (absl::GetFlag(FLAGS_write_apm_ref_data)) { OpenFileAndWriteMessage(ref_filename_, ref_data); } }