Revert "Add Video Bwe stats collection to DefaultVideoQualityAnalyzer."
This reverts commit 8848229234aae01ec19582ece7b748d557119d66.
Reason for revert: break chromium compilation on iOS
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915214519549611184/+/steps/compile/0/stdout
Original change's description:
> Add Video Bwe stats collection to DefaultVideoQualityAnalyzer.
>
> This CL adds the possibility to collect the following Video BWE stats:
> - available_send_bandwidth
> - transmission_bitrate
> - retransmission_bitrate
> - actual_encode_bitrate
> - target_encode_bitrate
>
> Example of the output:
>
> RESULT available_send_bandwidth: smoke_test/alice= {487754.33,87583.093} bytesPerSecond
> RESULT transmission_bitrate: smoke_test/alice= {465779.17,212075.5} bytesPerSecond
> RESULT retransmission_bitrate: smoke_test/alice= {20036,26326.751} bytesPerSecond
> RESULT actual_encode_bitrate: smoke_test/alice= {418779.33,200486.03} bytesPerSecond
> RESULT target_encode_bitrate: smoke_test/alice= {469491.17,77866.909} bytesPerSecond
> RESULT available_send_bandwidth: smoke_test/bob= {642924.83,168842.34} bytesPerSecond
> RESULT transmission_bitrate: smoke_test/bob= {626115.5,294783.56} bytesPerSecond
> RESULT retransmission_bitrate: smoke_test/bob= {0,0} bytesPerSecond
> RESULT actual_encode_bitrate: smoke_test/bob= {594235.33,297289.54} bytesPerSecond
> RESULT target_encode_bitrate: smoke_test/bob= {640463.5,167676.66} bytesPerSecond
>
> Bug: webrtc:10138
> Change-Id: I0414055af0010b8fb4d909297e6da86d398157c2
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132703
> Commit-Queue: Artem Titov <titovartem@webrtc.org>
> Reviewed-by: Tommi <tommi@webrtc.org>
> Reviewed-by: Artem Titov <titovartem@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@google.com>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27760}
TBR=mbonadei@webrtc.org,mbonadei@google.com,ilnik@webrtc.org,tommi@webrtc.org,titovartem@webrtc.org
Change-Id: Ib0ef94331410d9b22b6425e4da412b75360fa2d9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10138
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134210
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27771}
diff --git a/test/DEPS b/test/DEPS
index 80714fd..80c4719 100644
--- a/test/DEPS
+++ b/test/DEPS
@@ -20,10 +20,6 @@
"+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 f7c90b9..1f398f2 100644
--- a/test/pc/e2e/BUILD.gn
+++ b/test/pc/e2e/BUILD.gn
@@ -161,17 +161,6 @@
]
}
-# TODO(mbonadei): Remove as soon as -Wundef and -Wextra-semi problems are
-# fixed upstream (CLs are under review).
-config("peerconnection_quality_test_cflags") {
- if (is_clang) {
- cflags = [
- "-Wno-undef",
- "-Wno-extra-semi",
- ]
- }
-}
-
if (rtc_include_tests) {
rtc_source_set("video_quality_analyzer_injection_helper") {
visibility = [ "*" ]
@@ -236,10 +225,6 @@
rtc_source_set("peerconnection_quality_test") {
visibility = [ "*" ]
testonly = true
-
- # TODO(mbonadei): Remove as soon as -Wundef and -Wextra-semi problems are
- # fixed upstream (CLs are under review).
- configs += [ ":peerconnection_quality_test_cflags" ]
sources = [
"peer_connection_quality_test.cc",
"peer_connection_quality_test.h",
@@ -321,10 +306,6 @@
rtc_source_set("peer_connection_e2e_smoke_test") {
testonly = true
-
- # TODO(mbonadei): Remove as soon as -Wundef and -Wextra-semi problems are
- # fixed upstream (CLs are under review).
- configs += [ ":peerconnection_quality_test_cflags" ]
sources = [
"peer_connection_e2e_smoke_test.cc",
]
@@ -433,10 +414,6 @@
rtc_source_set("default_video_quality_analyzer") {
visibility = [ "*" ]
-
- # TODO(mbonadei): Remove as soon as -Wundef and -Wextra-semi problems are
- # fixed upstream (CLs are under review).
- configs += [ ":peerconnection_quality_test_cflags" ]
testonly = true
sources = [
"analyzer/video/default_video_quality_analyzer.cc",
@@ -457,7 +434,6 @@
"../../../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/video/default_video_quality_analyzer.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
index d37ef5f..dff5155 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
@@ -352,59 +352,6 @@
return analyzer_stats_;
}
-// 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 StatsReports& stats_reports) {
- for (const StatsReport* stats_report : stats_reports) {
- // The only stats collected by this analyzer are present in
- // kStatsReportTypeBwe reports, so all other reports are just ignored.
- if (stats_report->type() != StatsReport::StatsType::kStatsReportTypeBwe) {
- continue;
- }
- const webrtc::StatsReport::Value* available_send_bandwidth =
- stats_report->FindValue(
- StatsReport::StatsValueName::kStatsValueNameAvailableSendBandwidth);
- const webrtc::StatsReport::Value* retransmission_bitrate =
- stats_report->FindValue(
- StatsReport::StatsValueName::kStatsValueNameRetransmitBitrate);
- const webrtc::StatsReport::Value* transmission_bitrate =
- stats_report->FindValue(
- StatsReport::StatsValueName::kStatsValueNameTransmitBitrate);
- const webrtc::StatsReport::Value* actual_encode_bitrate =
- stats_report->FindValue(
- StatsReport::StatsValueName::kStatsValueNameActualEncBitrate);
- const webrtc::StatsReport::Value* target_encode_bitrate =
- stats_report->FindValue(
- StatsReport::StatsValueName::kStatsValueNameTargetEncBitrate);
- RTC_CHECK(available_send_bandwidth);
- RTC_CHECK(retransmission_bitrate);
- RTC_CHECK(transmission_bitrate);
- RTC_CHECK(actual_encode_bitrate);
- RTC_CHECK(target_encode_bitrate);
-
- rtc::CritScope crit(&video_bwe_stats_lock_);
- VideoBweStats& video_bwe_stats = video_bwe_stats_[pc_label];
- video_bwe_stats.available_send_bandwidth.AddSample(
- available_send_bandwidth->int_val());
- video_bwe_stats.transmission_bitrate.AddSample(
- transmission_bitrate->int_val());
- video_bwe_stats.retransmission_bitrate.AddSample(
- retransmission_bitrate->int_val());
- video_bwe_stats.actual_encode_bitrate.AddSample(
- actual_encode_bitrate->int_val());
- video_bwe_stats.target_encode_bitrate.AddSample(
- target_encode_bitrate->int_val());
- }
-}
-
-absl::flat_hash_map<std::string, VideoBweStats>
-DefaultVideoQualityAnalyzer::GetVideoBweStats() const {
- rtc::CritScope crit(&video_bwe_stats_lock_);
- return video_bwe_stats_;
-}
-
void DefaultVideoQualityAnalyzer::AddComparison(
absl::optional<VideoFrame> captured,
absl::optional<VideoFrame> rendered,
@@ -540,12 +487,6 @@
ReportResults(GetTestCaseName(item.first), item.second,
stream_frame_counters_.at(item.first));
}
- {
- rtc::CritScope video_bwe_crit(&video_bwe_stats_lock_);
- for (const auto& item : video_bwe_stats_) {
- ReportVideoBweResults(GetTestCaseName(item.first), item.second);
- }
- }
LogFrameCounters("Global", frame_counters_);
for (auto& item : stream_stats_) {
LogFrameCounters(item.first, stream_frame_counters_.at(item.first));
@@ -563,25 +504,9 @@
<< analyzer_stats_.overloaded_comparisons_done;
}
-void DefaultVideoQualityAnalyzer::ReportVideoBweResults(
- const std::string& test_case_name,
- const VideoBweStats& video_bwe_stats) {
- ReportResult("available_send_bandwidth", test_case_name,
- video_bwe_stats.available_send_bandwidth, "bytesPerSecond");
- ReportResult("transmission_bitrate", test_case_name,
- video_bwe_stats.transmission_bitrate, "bytesPerSecond");
- ReportResult("retransmission_bitrate", test_case_name,
- video_bwe_stats.retransmission_bitrate, "bytesPerSecond");
- ReportResult("actual_encode_bitrate", test_case_name,
- video_bwe_stats.actual_encode_bitrate, "bytesPerSecond");
- ReportResult("target_encode_bitrate", test_case_name,
- video_bwe_stats.target_encode_bitrate, "bytesPerSecond");
-}
-
-void DefaultVideoQualityAnalyzer::ReportResults(
- const std::string& test_case_name,
- const StreamStats& stats,
- const FrameCounters& frame_counters) {
+void DefaultVideoQualityAnalyzer::ReportResults(std::string test_case_name,
+ StreamStats stats,
+ FrameCounters frame_counters) {
ReportResult("psnr", test_case_name, stats.psnr, "dB");
ReportResult("ssim", test_case_name, stats.ssim, "unitless");
ReportResult("transport_time", test_case_name, stats.transport_time_ms, "ms");
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 22bf4fd..b8d186d 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 <string>
#include <vector>
-#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"
@@ -69,6 +68,7 @@
};
struct StreamStats {
+ public:
SamplesStatsCounter psnr;
SamplesStatsCounter ssim;
// Time from frame encoded (time point on exit from encoder) to the
@@ -100,6 +100,7 @@
};
struct AnalyzerStats {
+ public:
// Size of analyzer internal comparisons queue, measured when new element
// id added to the queue.
SamplesStatsCounter comparisons_queue_size;
@@ -113,14 +114,6 @@
int64_t overloaded_comparisons_done = 0;
};
-struct VideoBweStats {
- SamplesStatsCounter available_send_bandwidth;
- SamplesStatsCounter transmission_bitrate;
- SamplesStatsCounter retransmission_bitrate;
- SamplesStatsCounter actual_encode_bitrate;
- SamplesStatsCounter target_encode_bitrate;
-};
-
class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
public:
DefaultVideoQualityAnalyzer();
@@ -155,12 +148,8 @@
std::map<std::string, StreamStats> GetStats() const;
AnalyzerStats GetAnalyzerStats() const;
- // Will be called everytime new stats reports are available for the
- // Peer Connection identified by |pc_label|.
- void OnStatsReports(absl::string_view pc_label,
- const StatsReports& stats_reports) override;
-
- absl::flat_hash_map<std::string, VideoBweStats> GetVideoBweStats() const;
+ // TODO(bugs.webrtc.org/10138): Provide a real implementation for
+ // OnStatsReport.
private:
struct FrameStats {
@@ -243,11 +232,9 @@
void ProcessComparison(const FrameComparison& comparison);
// Report results for all metrics for all streams.
void ReportResults();
- static void ReportVideoBweResults(const std::string& test_case_name,
- const VideoBweStats& video_bwe_stats);
- static void ReportResults(const std::string& test_case_name,
- const StreamStats& stats,
- const FrameCounters& frame_counters);
+ static void ReportResults(std::string test_case_name,
+ StreamStats stats,
+ FrameCounters frame_counters);
// Report result for single metric for specified stream.
static void ReportResult(const std::string& metric_name,
const std::string& test_case_name,
@@ -284,12 +271,6 @@
std::deque<FrameComparison> comparisons_ RTC_GUARDED_BY(comparison_lock_);
AnalyzerStats analyzer_stats_ RTC_GUARDED_BY(comparison_lock_);
- 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<std::string, VideoBweStats> video_bwe_stats_
- RTC_GUARDED_BY(video_bwe_stats_lock_);
-
std::vector<std::unique_ptr<rtc::PlatformThread>> thread_pool_;
rtc::Event comparison_available_event_;
};
