From 60f14ce21747cd3b485019e97b12571ec8c2ce9a Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Wed, 8 May 2019 10:52:52 +0200 Subject: [PATCH] Do not use absl::flat_hash_map in DefaultVideoQualityAnalyzer. This CL removes the usage of absl::flat_hash_map because it transitively depends on CCTZ which fails to link with lld-link after the switch to libc++. Since std::map doesn't support heterogeneous lookup until C++14, this CL also stops using absl::string_view and switches to `const std::string&`. Bug: webrtc:10605 Change-Id: I4fc93969c6fc0cc7e7e62b4d2f801bdd27cff0f6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/135566 Reviewed-by: Artem Titov Reviewed-by: Karl Wiberg Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#27877} --- api/BUILD.gn | 2 -- api/test/stats_observer_interface.h | 5 +++-- api/test/video_quality_analyzer_interface.h | 3 +-- test/DEPS | 4 ---- test/pc/e2e/BUILD.gn | 3 --- .../e2e/analyzer/audio/default_audio_quality_analyzer.cc | 2 +- .../pc/e2e/analyzer/audio/default_audio_quality_analyzer.h | 3 +-- .../e2e/analyzer/video/default_video_quality_analyzer.cc | 4 ++-- .../pc/e2e/analyzer/video/default_video_quality_analyzer.h | 7 +++---- .../video/video_quality_analyzer_injection_helper.cc | 2 +- .../video/video_quality_analyzer_injection_helper.h | 3 +-- 11 files changed, 13 insertions(+), 25 deletions(-) diff --git a/api/BUILD.gn b/api/BUILD.gn index 510fe4f557..ec274094dd 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -213,7 +213,6 @@ rtc_source_set("video_quality_analyzer_api") { "video:encoded_image", "video:video_frame", "video_codecs:video_codecs_api", - "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", ] } @@ -247,7 +246,6 @@ rtc_source_set("stats_observer_interface") { deps = [ ":libjingle_peerconnection_api", - "//third_party/abseil-cpp/absl/strings", ] } diff --git a/api/test/stats_observer_interface.h b/api/test/stats_observer_interface.h index c6bea63fee..98c8dd937f 100644 --- a/api/test/stats_observer_interface.h +++ b/api/test/stats_observer_interface.h @@ -11,7 +11,8 @@ #ifndef API_TEST_STATS_OBSERVER_INTERFACE_H_ #define API_TEST_STATS_OBSERVER_INTERFACE_H_ -#include "absl/strings/string_view.h" +#include + #include "api/stats_types.h" namespace webrtc { @@ -24,7 +25,7 @@ class StatsObserverInterface { // Method called when stats reports are available for the PeerConnection // identified by |pc_label|. - virtual void OnStatsReports(absl::string_view pc_label, + virtual void OnStatsReports(const std::string& pc_label, const StatsReports& reports) = 0; }; diff --git a/api/test/video_quality_analyzer_interface.h b/api/test/video_quality_analyzer_interface.h index d83d16040a..92224a4b9d 100644 --- a/api/test/video_quality_analyzer_interface.h +++ b/api/test/video_quality_analyzer_interface.h @@ -14,7 +14,6 @@ #include #include -#include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/test/stats_observer_interface.h" #include "api/video/encoded_image.h" @@ -99,7 +98,7 @@ class VideoQualityAnalyzerInterface : public StatsObserverInterface { virtual void OnDecoderError(uint16_t frame_id, int32_t error_code) {} // Will be called everytime new stats reports are available for the // Peer Connection identified by |pc_label|. - void OnStatsReports(absl::string_view pc_label, + void OnStatsReports(const std::string& pc_label, const StatsReports& stats_reports) override {} // Tells analyzer that analysis complete and it should calculate final diff --git a/test/DEPS b/test/DEPS index 80714fd563..80c47191d6 100644 --- a/test/DEPS +++ b/test/DEPS @@ -20,10 +20,6 @@ include_rules = [ "+sdk", "+system_wrappers", "+third_party/libyuv", - - # This should probably go into //DEPS but at the moment we don't know - # yet if this is OK to use in production code. - "+absl/container/flat_hash_map.h", ] specific_include_rules = { diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 0d98206997..b1297c9203 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -192,7 +192,6 @@ if (rtc_include_tests) { "../../../test:video_test_common", "../../../test:video_test_support", "//third_party/abseil-cpp/absl/memory", - "//third_party/abseil-cpp/absl/strings", ] } @@ -412,7 +411,6 @@ rtc_source_set("default_audio_quality_analyzer") { "../../../rtc_base:criticalsection", "../../../rtc_base:logging", "../../../rtc_base:rtc_numerics", - "//third_party/abseil-cpp/absl/strings", ] } @@ -459,7 +457,6 @@ rtc_source_set("default_video_quality_analyzer") { "../../../rtc_base:rtc_event", "../../../rtc_base:rtc_numerics", "../../../system_wrappers", - "//third_party/abseil-cpp/absl/container:flat_hash_map", "//third_party/abseil-cpp/absl/memory", ] } 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 8859ede3ac..0792069e64 100644 --- a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc +++ b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc @@ -30,7 +30,7 @@ void DefaultAudioQualityAnalyzer::Start( } void DefaultAudioQualityAnalyzer::OnStatsReports( - absl::string_view pc_label, + const std::string& pc_label, const StatsReports& stats_reports) { for (const StatsReport* stats_report : stats_reports) { // NetEq stats are only present in kStatsReportTypeSsrc reports, so all 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 019ab2e4da..ee34ed345b 100644 --- a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h +++ b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h @@ -14,7 +14,6 @@ #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" @@ -38,7 +37,7 @@ class DefaultAudioQualityAnalyzer : public AudioQualityAnalyzerInterface { public: void Start(std::string test_case_name, TrackIdStreamLabelMap* analyzer_helper) override; - void OnStatsReports(absl::string_view pc_label, + void OnStatsReports(const std::string& pc_label, const StatsReports& stats_reports) override; void Stop() override; diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc index d37ef5f7d5..49cdb1899b 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc @@ -355,7 +355,7 @@ AnalyzerStats DefaultVideoQualityAnalyzer::GetAnalyzerStats() const { // TODO(bugs.webrtc.org/10430): Migrate to the new GetStats as soon as // bugs.webrtc.org/10428 is fixed. void DefaultVideoQualityAnalyzer::OnStatsReports( - absl::string_view pc_label, + const std::string& pc_label, const StatsReports& stats_reports) { for (const StatsReport* stats_report : stats_reports) { // The only stats collected by this analyzer are present in @@ -399,7 +399,7 @@ void DefaultVideoQualityAnalyzer::OnStatsReports( } } -absl::flat_hash_map +std::map DefaultVideoQualityAnalyzer::GetVideoBweStats() const { rtc::CritScope crit(&video_bwe_stats_lock_); return video_bwe_stats_; diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h index 22bf4fdea9..caac7648d0 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h @@ -19,7 +19,6 @@ #include #include -#include "absl/container/flat_hash_map.h" #include "api/test/video_quality_analyzer_interface.h" #include "api/units/timestamp.h" #include "api/video/encoded_image.h" @@ -157,10 +156,10 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface { // Will be called everytime new stats reports are available for the // Peer Connection identified by |pc_label|. - void OnStatsReports(absl::string_view pc_label, + void OnStatsReports(const std::string& pc_label, const StatsReports& stats_reports) override; - absl::flat_hash_map GetVideoBweStats() const; + std::map GetVideoBweStats() const; private: struct FrameStats { @@ -287,7 +286,7 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface { rtc::CriticalSection video_bwe_stats_lock_; // Map between a peer connection label (provided by the framework) and // its video BWE stats. - absl::flat_hash_map video_bwe_stats_ + std::map video_bwe_stats_ RTC_GUARDED_BY(video_bwe_stats_lock_); std::vector> thread_pool_; diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc index 7b71de0769..6f8e64c505 100644 --- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc +++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc @@ -141,7 +141,7 @@ void VideoQualityAnalyzerInjectionHelper::Start(std::string test_case_name, } void VideoQualityAnalyzerInjectionHelper::OnStatsReports( - absl::string_view pc_label, + const std::string& pc_label, const StatsReports& stats_reports) { analyzer_->OnStatsReports(pc_label, stats_reports); } diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h index c831b845ab..8438edc735 100644 --- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h +++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h @@ -15,7 +15,6 @@ #include #include -#include "absl/strings/string_view.h" #include "api/test/stats_observer_interface.h" #include "api/test/video_quality_analyzer_interface.h" #include "api/video/video_frame.h" @@ -69,7 +68,7 @@ class VideoQualityAnalyzerInjectionHelper : public StatsObserverInterface { // Forwards |stats_reports| for Peer Connection |pc_label| to // |analyzer_|. - void OnStatsReports(absl::string_view pc_label, + void OnStatsReports(const std::string& pc_label, const StatsReports& stats_reports) override; // Stops VideoQualityAnalyzerInterface to populate final data and metrics.