From 4876cb21c82f3886d67888f75bee2ab522e55a94 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Tue, 9 Jul 2019 20:08:55 +0000 Subject: [PATCH] Revert "Fix collection of audio metrics from PC test framework for audio test" This reverts commit 2d0880b56954f57548deea51dfa678b80dbf618f. Reason for revert: Breaks perf tests (e.g. https://ci.chromium.org/p/webrtc/builders/perf/Perf%20Android32%20(L%20Nexus4)/1470). Original change's description: > Fix collection of audio metrics from PC test framework for audio test > > Bug: webrtc:10138 > Change-Id: I18a8509a0cdc4ed1db6894c7540d5c0a155d6233 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144784 > Reviewed-by: Oleh Prypin > Reviewed-by: Oskar Sundbom > Commit-Queue: Artem Titov > Cr-Commit-Position: refs/heads/master@{#28514} TBR=oprypin@webrtc.org,ossu@webrtc.org,titovartem@webrtc.org Change-Id: Id45abfbb0de47d95fe6cb396b1c29efb97a43797 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10138 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144883 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#28519} --- audio/BUILD.gn | 1 - audio/test/low_bandwidth_audio_test.cc | 11 ++- audio/test/low_bandwidth_audio_test.py | 78 +++++++------------- audio/test/low_bandwidth_audio_test_flags.cc | 27 ------- audio/test/pc_low_bandwidth_audio_test.cc | 16 +--- 5 files changed, 39 insertions(+), 94 deletions(-) delete mode 100644 audio/test/low_bandwidth_audio_test_flags.cc diff --git a/audio/BUILD.gn b/audio/BUILD.gn index c714570e89..4a3d8fd032 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -177,7 +177,6 @@ if (rtc_include_tests) { sources = [ "test/low_bandwidth_audio_test.cc", - "test/low_bandwidth_audio_test_flags.cc", "test/pc_low_bandwidth_audio_test.cc", ] diff --git a/audio/test/low_bandwidth_audio_test.cc b/audio/test/low_bandwidth_audio_test.cc index db1ff2cbba..54191e85a6 100644 --- a/audio/test/low_bandwidth_audio_test.cc +++ b/audio/test/low_bandwidth_audio_test.cc @@ -14,8 +14,15 @@ #include "system_wrappers/include/sleep.h" #include "test/testsupport/file_utils.h" -WEBRTC_DECLARE_int(sample_rate_hz); -WEBRTC_DECLARE_bool(quick); +WEBRTC_DEFINE_int(sample_rate_hz, + 16000, + "Sample rate (Hz) of the produced audio files."); + +WEBRTC_DEFINE_bool( + quick, + false, + "Don't do the full audio recording. " + "Used to quickly check that the test runs without crashing."); namespace webrtc { namespace test { diff --git a/audio/test/low_bandwidth_audio_test.py b/audio/test/low_bandwidth_audio_test.py index 8c3554b1d4..add4f2f72b 100755 --- a/audio/test/low_bandwidth_audio_test.py +++ b/audio/test/low_bandwidth_audio_test.py @@ -23,7 +23,6 @@ import re import shutil import subprocess import sys -import tempfile SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -207,20 +206,7 @@ def _AddChart(charts, metric, test_name, value, units): } -def _AddRunPerfResults(charts, run_perf_results_file): - with open(run_perf_results_file, 'rb') as f: - per_run_perf_results = json.load(f) - if 'charts' not in per_run_perf_results: - return - for metric, cases in per_run_perf_results['charts'].items(): - chart = charts.setdefault(metric, {}) - for case_name, case_value in cases.items(): - if case_name in chart: - logging.error('Overriding results for %s/%s', metric, case_name) - chart[case_name] = case_value - - -Analyzer = collections.namedtuple('Analyzer', ['name', 'func', 'executable', +Analyzer = collections.namedtuple('Analyzer', ['func', 'executable', 'sample_rate_hz']) @@ -242,55 +228,47 @@ def main(): else: test_command = [os.path.join(args.build_dir, 'low_bandwidth_audio_test')] - analyzers = [Analyzer('pesq', _RunPesq, pesq_path, 16000)] + analyzers = [Analyzer(_RunPesq, pesq_path, 16000)] # Check if POLQA can run at all, or skip the 48 kHz tests entirely. example_path = os.path.join(SRC_DIR, 'resources', 'voice_engine', 'audio_tiny48.wav') if polqa_path and _RunPolqa(polqa_path, example_path, example_path): - analyzers.append(Analyzer('polqa', _RunPolqa, polqa_path, 48000)) + analyzers.append(Analyzer(_RunPolqa, polqa_path, 48000)) charts = {} for analyzer in analyzers: - f, cur_perf_results = tempfile.mkstemp(prefix='audio_perf', suffix=".json") + # Start the test executable that produces audio files. + test_process = subprocess.Popen( + _LogCommand(test_command + ['--sample_rate_hz=%d' % + analyzer.sample_rate_hz]), + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) try: - # Start the test executable that produces audio files. - test_process = subprocess.Popen( - _LogCommand(test_command + [ - '--sample_rate_hz=%d' % analyzer.sample_rate_hz, - '--test_case_prefix=%s' % analyzer.name, - '--isolated_script_test_perf_output=%s' % cur_perf_results, - ]), - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - try: - lines = iter(test_process.stdout.readline, '') - for result in ExtractTestRuns(lines, echo=True): - (android_device, test_name, reference_file, degraded_file) = result + lines = iter(test_process.stdout.readline, '') + for result in ExtractTestRuns(lines, echo=True): + (android_device, test_name, reference_file, degraded_file) = result - adb_prefix = (args.adb_path,) - if android_device: - adb_prefix += ('-s', android_device) + adb_prefix = (args.adb_path,) + if android_device: + adb_prefix += ('-s', android_device) - reference_file = _GetFile(reference_file, out_dir, - android=args.android, adb_prefix=adb_prefix) - degraded_file = _GetFile(degraded_file, out_dir, move=True, - android=args.android, adb_prefix=adb_prefix) + reference_file = _GetFile(reference_file, out_dir, + android=args.android, adb_prefix=adb_prefix) + degraded_file = _GetFile(degraded_file, out_dir, move=True, + android=args.android, adb_prefix=adb_prefix) - analyzer_results = analyzer.func(analyzer.executable, - reference_file, degraded_file) - for metric, (value, units) in analyzer_results.items(): - # Output a result for the perf dashboard. - print 'RESULT %s: %s= %s %s' % (metric, test_name, value, units) - _AddChart(charts, metric, test_name, value, units) + analyzer_results = analyzer.func(analyzer.executable, + reference_file, degraded_file) + for metric, (value, units) in analyzer_results.items(): + # Output a result for the perf dashboard. + print 'RESULT %s: %s= %s %s' % (metric, test_name, value, units) + _AddChart(charts, metric, test_name, value, units) - if args.remove: - os.remove(reference_file) - os.remove(degraded_file) - finally: - test_process.terminate() - _AddRunPerfResults(charts, cur_perf_results) + if args.remove: + os.remove(reference_file) + os.remove(degraded_file) finally: - os.remove(cur_perf_results) + test_process.terminate() if args.isolated_script_test_perf_output: with open(args.isolated_script_test_perf_output, 'w') as f: diff --git a/audio/test/low_bandwidth_audio_test_flags.cc b/audio/test/low_bandwidth_audio_test_flags.cc deleted file mode 100644 index a0f12c5bc9..0000000000 --- a/audio/test/low_bandwidth_audio_test_flags.cc +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ -// #ifndef AUDIO_TEST_LOW_BANDWIDTH_AUDIO_TEST_FLAGS_H_ -// #define AUDIO_TEST_LOW_BANDWIDTH_AUDIO_TEST_FLAGS_H_ - -#include "rtc_base/flags.h" - -WEBRTC_DEFINE_int(sample_rate_hz, - 16000, - "Sample rate (Hz) of the produced audio files."); - -WEBRTC_DEFINE_bool( - quick, - false, - "Don't do the full audio recording. " - "Used to quickly check that the test runs without crashing."); - -WEBRTC_DEFINE_string(test_case_prefix, "", "Test case prefix."); - -// #endif // AUDIO_TEST_LOW_BANDWIDTH_AUDIO_TEST_FLAGS_H_ diff --git a/audio/test/pc_low_bandwidth_audio_test.cc b/audio/test/pc_low_bandwidth_audio_test.cc index 8e0b130fb4..4eec672b8b 100644 --- a/audio/test/pc_low_bandwidth_audio_test.cc +++ b/audio/test/pc_low_bandwidth_audio_test.cc @@ -20,8 +20,6 @@ #include "test/pc/e2e/network_quality_metrics_reporter.h" #include "test/testsupport/file_utils.h" -WEBRTC_DECLARE_string(test_case_prefix); - namespace webrtc { namespace test { @@ -35,16 +33,6 @@ namespace { constexpr int kTestDurationSec = 45; -std::string GetMetricTestCaseName() { - const ::testing::TestInfo* const test_info = - ::testing::UnitTest::GetInstance()->current_test_info(); - std::string test_case_prefix(FLAG_test_case_prefix); - if (test_case_prefix.empty()) { - return test_info->name(); - } - return std::string(FLAG_test_case_prefix) + "_" + test_info->name(); -} - EmulatedNetworkNode* CreateEmulatedNodeWithConfig( NetworkEmulationManager* emulation, const BuiltInNetworkBehaviorConfig& config) { @@ -116,7 +104,7 @@ TEST(PCLowBandwidthAudioTest, PCGoodNetworkHighBitrate) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); auto fixture = CreateTestFixture( - GetMetricTestCaseName(), + "pc_good_network", CreateTwoNetworkLinks(network_emulation_manager.get(), BuiltInNetworkBehaviorConfig()), [](PeerConfigurer* alice) { @@ -140,7 +128,7 @@ TEST(PCLowBandwidthAudioTest, PCMobile2GNetwork) { config.queue_length_packets = 1500; config.queue_delay_ms = 400; auto fixture = CreateTestFixture( - GetMetricTestCaseName(), + "pc_mobile_2g_network", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { AudioConfig audio;