From f948eb66aa2cf7ab125c9068d9f2bf8b78df9aca Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Fri, 5 Apr 2019 15:13:23 +0200 Subject: [PATCH] Implement DefaultAudioQualityAnalyzer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DefaultAudioQualityAnalyzer will read stats reports (temporarily using the old PeerConnectionInterface::GetStats) and for each audio stream it will collect some NetEq related stats. When DefaultAudioQualityAnalyzer::Stop is invoked by the framework, it will report the following metrics: - expand_rate - accelerate_rate - preemptive_rate - speech_expand_rate - preferred_buffer_size_ms Bug: webrtc:10138 Change-Id: Ie493456fcb9ed86455b12dabdab98a317387ef46 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/125980 Reviewed-by: Karl Wiberg Reviewed-by: Henrik Boström Reviewed-by: Artem Titov Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#27474} --- api/BUILD.gn | 8 ++ api/test/audio_quality_analyzer_interface.h | 13 +- api/test/track_id_stream_label_map.h | 36 ++++++ test/pc/e2e/BUILD.gn | 17 +++ .../audio/default_audio_quality_analyzer.cc | 111 +++++++++++++++++- .../audio/default_audio_quality_analyzer.h | 30 ++++- test/pc/e2e/analyzer_helper.cc | 37 ++++++ test/pc/e2e/analyzer_helper.h | 50 ++++++++ test/pc/e2e/peer_connection_quality_test.cc | 14 ++- test/pc/e2e/peer_connection_quality_test.h | 6 +- 10 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 api/test/track_id_stream_label_map.h create mode 100644 test/pc/e2e/analyzer_helper.cc create mode 100644 test/pc/e2e/analyzer_helper.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 0d26945fef..ba7570dad3 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -213,6 +213,13 @@ rtc_source_set("video_quality_analyzer_api") { ] } +rtc_source_set("track_id_stream_label_map") { + visibility = [ "*" ] + sources = [ + "test/track_id_stream_label_map.h", + ] +} + rtc_source_set("audio_quality_analyzer_api") { visibility = [ "*" ] testonly = true @@ -222,6 +229,7 @@ rtc_source_set("audio_quality_analyzer_api") { deps = [ ":stats_observer_interface", + ":track_id_stream_label_map", ] } diff --git a/api/test/audio_quality_analyzer_interface.h b/api/test/audio_quality_analyzer_interface.h index 1f67ce69f1..88392d7fd2 100644 --- a/api/test/audio_quality_analyzer_interface.h +++ b/api/test/audio_quality_analyzer_interface.h @@ -14,6 +14,7 @@ #include #include "api/test/stats_observer_interface.h" +#include "api/test/track_id_stream_label_map.h" namespace webrtc { namespace webrtc_pc_e2e { @@ -23,10 +24,18 @@ class AudioQualityAnalyzerInterface : public StatsObserverInterface { public: ~AudioQualityAnalyzerInterface() override = default; - // Will be called by framework before test. + // Will be called by the framework before the test. // |test_case_name| is name of test case, that should be used to report all // audio metrics. - virtual void Start(std::string test_case_name) = 0; + // |analyzer_helper| is a pointer to a class that will allow track_id to + // stream_id matching. The caller is responsible for ensuring the + // AnalyzerHelper outlives the instance of the AudioQualityAnalyzerInterface. + virtual void Start(std::string test_case_name, + TrackIdStreamLabelMap* analyzer_helper) = 0; + + // Will be called by the framework at the end of the test. The analyzer + // has to finalize all its stats and it should report them. + virtual void Stop() = 0; }; } // namespace webrtc_pc_e2e diff --git a/api/test/track_id_stream_label_map.h b/api/test/track_id_stream_label_map.h new file mode 100644 index 0000000000..9f8e1216a3 --- /dev/null +++ b/api/test/track_id_stream_label_map.h @@ -0,0 +1,36 @@ +/* + * 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 API_TEST_TRACK_ID_STREAM_LABEL_MAP_H_ +#define API_TEST_TRACK_ID_STREAM_LABEL_MAP_H_ + +#include + +namespace webrtc { +namespace webrtc_pc_e2e { + +// Instances of |TrackIdStreamLabelMap| provide bookkeeing capabilities that +// are useful to associate stats reports track_ids to the remote stream_id. +class TrackIdStreamLabelMap { + public: + virtual ~TrackIdStreamLabelMap() = default; + + // This method must be called on the same thread where + // StatsObserverInterface::OnStatsReports is invoked. + // Returns a reference to a stream label owned by the TrackIdStreamLabelMap. + // Precondition: |track_id| must be already mapped to a stream_label. + virtual const std::string& GetStreamLabelFromTrackId( + const std::string& track_id) const = 0; +}; + +} // namespace webrtc_pc_e2e +} // namespace webrtc + +#endif // API_TEST_TRACK_ID_STREAM_LABEL_MAP_H_ diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 65ddb7f8a5..f11c89eec9 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -228,6 +228,7 @@ if (rtc_include_tests) { "peer_connection_quality_test.h", ] deps = [ + ":analyzer_helper", ":default_audio_quality_analyzer", ":default_video_quality_analyzer", ":peer_connection_quality_test_params", @@ -337,6 +338,19 @@ if (rtc_include_tests) { } } +rtc_source_set("analyzer_helper") { + visibility = [ "*" ] + sources = [ + "analyzer_helper.cc", + "analyzer_helper.h", + ] + deps = [ + "../../../api:track_id_stream_label_map", + "../../../rtc_base:macromagic", + "../../../rtc_base/synchronization:sequence_checker", + ] +} + rtc_source_set("default_audio_quality_analyzer") { visibility = [ "*" ] testonly = true @@ -346,10 +360,13 @@ rtc_source_set("default_audio_quality_analyzer") { ] deps = [ + "../..:perf_test", "../../../api:audio_quality_analyzer_api", "../../../api:libjingle_peerconnection_api", "../../../api:stats_observer_interface", + "../../../api:track_id_stream_label_map", "../../../rtc_base:logging", + "../../../rtc_base:rtc_numerics", "//third_party/abseil-cpp/absl/strings", ] } diff --git a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc index ed21024d8c..0c025a129b 100644 --- a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc +++ b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc @@ -10,19 +10,126 @@ #include "test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h" +#include + +#include "api/stats_types.h" #include "rtc_base/logging.h" +#include "test/testsupport/perf_test.h" namespace webrtc { namespace webrtc_pc_e2e { +namespace { -void DefaultAudioQualityAnalyzer::Start(std::string test_case_name) { +static const char kStatsAudioMediaType[] = "audio"; + +} // namespace + +void DefaultAudioQualityAnalyzer::Start( + std::string test_case_name, + TrackIdStreamLabelMap* analyzer_helper) { test_case_name_ = std::move(test_case_name); + analyzer_helper_ = analyzer_helper; } void DefaultAudioQualityAnalyzer::OnStatsReports( absl::string_view pc_label, const StatsReports& stats_reports) { - // TODO(bugs.webrtc.org/10138): Implement audio stats collection. + for (const StatsReport* stats_report : stats_reports) { + // NetEq stats are only present in kStatsReportTypeSsrc reports, so all + // other reports are just ignored. + if (stats_report->type() != StatsReport::StatsType::kStatsReportTypeSsrc) { + continue; + } + // Ignoring stats reports of "video" SSRC. + const webrtc::StatsReport::Value* media_type = stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNameMediaType); + RTC_CHECK(media_type); + if (strcmp(media_type->static_string_val(), kStatsAudioMediaType) != 0) { + continue; + } + const webrtc::StatsReport::Value* packets_received = + stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNamePacketsReceived); + if (!packets_received || packets_received->int_val() == 0) { + // Discarding stats in the following situations: + // - When packets_received is not present, because NetEq stats are only + // available in recv-side SSRC. + // - When packets_received is present but its value is 0. This means + // that media is not yet flowing so there is no need to keep this + // stats report into account (since all its fields would be 0). + continue; + } + + const webrtc::StatsReport::Value* expand_rate = stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNameExpandRate); + const webrtc::StatsReport::Value* accelerate_rate = stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNameAccelerateRate); + const webrtc::StatsReport::Value* preemptive_rate = stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNamePreemptiveExpandRate); + const webrtc::StatsReport::Value* speech_expand_rate = + stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNameSpeechExpandRate); + const webrtc::StatsReport::Value* preferred_buffer_size_ms = + stats_report->FindValue(StatsReport::StatsValueName:: + kStatsValueNamePreferredJitterBufferMs); + RTC_CHECK(expand_rate); + RTC_CHECK(accelerate_rate); + RTC_CHECK(preemptive_rate); + RTC_CHECK(speech_expand_rate); + RTC_CHECK(preferred_buffer_size_ms); + + const std::string& stream_label = + GetStreamLabelFromStatsReport(stats_report); + AudioStreamStats& audio_stream_stats = streams_stats_[stream_label]; + audio_stream_stats.expand_rate.AddSample(expand_rate->float_val()); + audio_stream_stats.accelerate_rate.AddSample(accelerate_rate->float_val()); + audio_stream_stats.preemptive_rate.AddSample(preemptive_rate->float_val()); + audio_stream_stats.speech_expand_rate.AddSample( + speech_expand_rate->float_val()); + audio_stream_stats.preferred_buffer_size_ms.AddSample( + preferred_buffer_size_ms->int_val()); + } +} + +const std::string& DefaultAudioQualityAnalyzer::GetStreamLabelFromStatsReport( + const StatsReport* stats_report) const { + const webrtc::StatsReport::Value* report_track_id = stats_report->FindValue( + StatsReport::StatsValueName::kStatsValueNameTrackId); + RTC_CHECK(report_track_id); + return analyzer_helper_->GetStreamLabelFromTrackId( + report_track_id->string_val()); +} + +std::string DefaultAudioQualityAnalyzer::GetTestCaseName( + const std::string& stream_label) const { + return test_case_name_ + "/" + stream_label; +} + +void DefaultAudioQualityAnalyzer::Stop() { + for (auto& item : streams_stats_) { + ReportResult("expand_rate", item.first, item.second.expand_rate, + "unitless"); + ReportResult("accelerate_rate", item.first, item.second.accelerate_rate, + "unitless"); + ReportResult("preemptive_rate", item.first, item.second.preemptive_rate, + "unitless"); + ReportResult("speech_expand_rate", item.first, + item.second.speech_expand_rate, "unitless"); + ReportResult("preferred_buffer_size_ms", item.first, + item.second.preferred_buffer_size_ms, "ms"); + } +} + +void DefaultAudioQualityAnalyzer::ReportResult( + const std::string& metric_name, + const std::string& stream_label, + const SamplesStatsCounter& counter, + const std::string& unit) const { + test::PrintResultMeanAndError( + metric_name, /*modifier=*/"", GetTestCaseName(stream_label), + counter.IsEmpty() ? 0 : counter.GetAverage(), + counter.IsEmpty() ? 0 : counter.GetStandardDeviation(), unit, + /*important=*/false); } } // namespace webrtc_pc_e2e diff --git a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h index 5ff639a38f..75f9f34cd5 100644 --- a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h +++ b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h @@ -11,21 +11,49 @@ #ifndef TEST_PC_E2E_ANALYZER_AUDIO_DEFAULT_AUDIO_QUALITY_ANALYZER_H_ #define TEST_PC_E2E_ANALYZER_AUDIO_DEFAULT_AUDIO_QUALITY_ANALYZER_H_ +#include +#include + #include "absl/strings/string_view.h" #include "api/stats_types.h" #include "api/test/audio_quality_analyzer_interface.h" +#include "api/test/track_id_stream_label_map.h" +#include "rtc_base/numerics/samples_stats_counter.h" namespace webrtc { namespace webrtc_pc_e2e { +struct AudioStreamStats { + public: + SamplesStatsCounter expand_rate; + SamplesStatsCounter accelerate_rate; + SamplesStatsCounter preemptive_rate; + SamplesStatsCounter speech_expand_rate; + SamplesStatsCounter preferred_buffer_size_ms; +}; + +// TODO(bugs.webrtc.org/10430): Migrate to the new GetStats as soon as +// bugs.webrtc.org/10428 is fixed. class DefaultAudioQualityAnalyzer : public AudioQualityAnalyzerInterface { public: - void Start(std::string test_case_name) override; + void Start(std::string test_case_name, + TrackIdStreamLabelMap* analyzer_helper) override; void OnStatsReports(absl::string_view pc_label, const StatsReports& stats_reports) override; + void Stop() override; private: + const std::string& GetStreamLabelFromStatsReport( + const StatsReport* stats_report) const; + std::string GetTestCaseName(const std::string& stream_label) const; + void ReportResult(const std::string& metric_name, + const std::string& stream_label, + const SamplesStatsCounter& counter, + const std::string& unit) const; + std::string test_case_name_; + TrackIdStreamLabelMap* analyzer_helper_; + std::map streams_stats_; }; } // namespace webrtc_pc_e2e diff --git a/test/pc/e2e/analyzer_helper.cc b/test/pc/e2e/analyzer_helper.cc new file mode 100644 index 0000000000..a0cf7923da --- /dev/null +++ b/test/pc/e2e/analyzer_helper.cc @@ -0,0 +1,37 @@ +/* + * 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. + */ + +#include + +#include "test/pc/e2e/analyzer_helper.h" + +namespace webrtc { +namespace webrtc_pc_e2e { + +AnalyzerHelper::AnalyzerHelper() { + signaling_sequence_checker_.Detach(); +} + +void AnalyzerHelper::AddTrackToStreamMapping(std::string track_id, + std::string stream_label) { + RTC_DCHECK_RUN_ON(&signaling_sequence_checker_); + track_to_stream_map_.insert({std::move(track_id), std::move(stream_label)}); +} + +const std::string& AnalyzerHelper::GetStreamLabelFromTrackId( + const std::string& track_id) const { + RTC_DCHECK_RUN_ON(&signaling_sequence_checker_); + auto track_to_stream_pair = track_to_stream_map_.find(track_id); + RTC_CHECK(track_to_stream_pair != track_to_stream_map_.end()); + return track_to_stream_pair->second; +} + +} // namespace webrtc_pc_e2e +} // namespace webrtc diff --git a/test/pc/e2e/analyzer_helper.h b/test/pc/e2e/analyzer_helper.h new file mode 100644 index 0000000000..9a847e6cc4 --- /dev/null +++ b/test/pc/e2e/analyzer_helper.h @@ -0,0 +1,50 @@ +/* + * 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 TEST_PC_E2E_ANALYZER_HELPER_H_ +#define TEST_PC_E2E_ANALYZER_HELPER_H_ + +#include +#include + +#include "api/test/track_id_stream_label_map.h" +#include "rtc_base/synchronization/sequence_checker.h" +#include "rtc_base/thread_annotations.h" + +namespace webrtc { +namespace webrtc_pc_e2e { + +// This class is a utility that provides bookkeeing capabilities that +// are useful to associate stats reports track_ids to the remote stream_id. +// The framework will populate an instance of this class and it will pass +// it to the Start method of Media Quality Analyzers. +// An instance of AnalyzerHelper must only be accessed from a single +// thread and since stats collection happens on the signaling thread, +// both AddTrackToStreamMapping and GetStreamLabelFromTrackId must be +// invoked from the signaling thread. +class AnalyzerHelper : public TrackIdStreamLabelMap { + public: + AnalyzerHelper(); + + void AddTrackToStreamMapping(std::string track_id, std::string stream_label); + + const std::string& GetStreamLabelFromTrackId( + const std::string& track_id) const override; + + private: + SequenceChecker signaling_sequence_checker_; + std::map track_to_stream_map_ + RTC_GUARDED_BY(signaling_sequence_checker_); +}; + +} // namespace webrtc_pc_e2e +} // namespace webrtc + +#endif // TEST_PC_E2E_ANALYZER_HELPER_H_ diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index 738892d315..dfee644aca 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -254,7 +254,7 @@ void PeerConnectionE2EQualityTest::Run( absl::make_unique( [this, bob_video_configs]( rtc::scoped_refptr transceiver) { - SetupVideoSink(transceiver, bob_video_configs); + OnTrackCallback(transceiver, bob_video_configs); }, [this]() { StartVideo(alice_video_sources_); }), video_quality_analyzer_injection_helper_.get(), signaling_thread.get(), @@ -265,7 +265,7 @@ void PeerConnectionE2EQualityTest::Run( absl::make_unique( [this, alice_video_configs]( rtc::scoped_refptr transceiver) { - SetupVideoSink(transceiver, alice_video_configs); + OnTrackCallback(transceiver, alice_video_configs); }, [this]() { StartVideo(bob_video_sources_); }), video_quality_analyzer_injection_helper_.get(), signaling_thread.get(), @@ -286,7 +286,7 @@ void PeerConnectionE2EQualityTest::Run( video_quality_analyzer_injection_helper_->Start(test_case_name_, video_analyzer_threads); - audio_quality_analyzer_->Start(test_case_name_); + audio_quality_analyzer_->Start(test_case_name_, &analyzer_helper_); // Start RTCEventLog recording if requested. if (alice_->params()->rtc_event_log_path) { @@ -355,6 +355,7 @@ void PeerConnectionE2EQualityTest::Run( rtc::Bind(&PeerConnectionE2EQualityTest::TearDownCallOnSignalingThread, this)); + audio_quality_analyzer_->Stop(); video_quality_analyzer_injection_helper_->Stop(); // Ensuring that TestPeers have been destroyed in order to correctly close @@ -456,17 +457,18 @@ void PeerConnectionE2EQualityTest::ValidateParams(const RunParams& run_params, RTC_CHECK_GT(media_streams_count, 0) << "No media in the call."; } -void PeerConnectionE2EQualityTest::SetupVideoSink( +void PeerConnectionE2EQualityTest::OnTrackCallback( rtc::scoped_refptr transceiver, std::vector remote_video_configs) { const rtc::scoped_refptr& track = transceiver->receiver()->track(); + RTC_CHECK_EQ(transceiver->receiver()->stream_ids().size(), 1); + std::string stream_label = transceiver->receiver()->stream_ids().front(); + analyzer_helper_.AddTrackToStreamMapping(track->id(), stream_label); if (track->kind() != MediaStreamTrackInterface::kVideoKind) { return; } - RTC_CHECK_EQ(transceiver->receiver()->stream_ids().size(), 1); - std::string stream_label = transceiver->receiver()->stream_ids().front(); VideoConfig* video_config = nullptr; for (auto& config : remote_video_configs) { if (config.stream_label == stream_label) { diff --git a/test/pc/e2e/peer_connection_quality_test.h b/test/pc/e2e/peer_connection_quality_test.h index 762bdb4613..1be72e7adb 100644 --- a/test/pc/e2e/peer_connection_quality_test.h +++ b/test/pc/e2e/peer_connection_quality_test.h @@ -28,6 +28,7 @@ #include "system_wrappers/include/clock.h" #include "test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h" #include "test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h" +#include "test/pc/e2e/analyzer_helper.h" #include "test/pc/e2e/peer_connection_quality_test_params.h" #include "test/pc/e2e/test_peer.h" #include "test/testsupport/video_frame_writer.h" @@ -191,8 +192,8 @@ class PeerConnectionE2EQualityTest // Validate peer's parameters, also ensure uniqueness of all video stream // labels. void ValidateParams(const RunParams& run_params, std::vector params); - void SetupVideoSink(rtc::scoped_refptr transceiver, - std::vector remote_video_configs); + void OnTrackCallback(rtc::scoped_refptr transceiver, + std::vector remote_video_configs); // Have to be run on the signaling thread. void SetupCallOnSignalingThread(); void TearDownCallOnSignalingThread(); @@ -233,6 +234,7 @@ class PeerConnectionE2EQualityTest std::vector> video_writers_; std::vector>> output_video_sinks_; + AnalyzerHelper analyzer_helper_; rtc::CriticalSection lock_; // Time when test call was started. Minus infinity means that call wasn't