From cfefa0aef329aac0206c5e56efd90b3b0bdb88b6 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Tue, 23 Jul 2019 13:24:26 +0000 Subject: [PATCH] Revert "Record audio/video bytes sent in analyzer stream stats." This reverts commit d978cb43c238ca24b2320acd7b656f446b906101. Reason for revert: It breaks perf tests: https://ci.chromium.org/p/webrtc/builders/perf/Perf%20Android32%20(L%20Nexus4)/1561 Original change's description: > Record audio/video bytes sent in analyzer stream stats. > > For each SSRC report, record the number of bytes sent for that stream > and expose them in analyzer stats. These numbers can be used to > determine useful metrics such as total media throughput (by adding the > bytes sent for all streams) and overhead (by subtracting that amount > from the total bytes sent to the network). > > Bug: webrtc:9719 > Change-Id: I977bbd40acdd0a1ec64763ddd55a642b9a50f309 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146240 > Reviewed-by: Mirko Bonadei > Reviewed-by: Artem Titov > Commit-Queue: Bjorn Mellem > Cr-Commit-Position: refs/heads/master@{#28637} TBR=mbonadei@webrtc.org,mellem@webrtc.org,titovartem@webrtc.org Change-Id: I3e46307dd6ef121b9377b93fc8d9fa788245ea5f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:9719 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146605 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#28646} --- .../audio/default_audio_quality_analyzer.cc | 21 ++------------- .../audio/default_audio_quality_analyzer.h | 1 - .../video/default_video_quality_analyzer.cc | 26 +------------------ .../video/default_video_quality_analyzer.h | 2 -- 4 files changed, 3 insertions(+), 47 deletions(-) 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 b448dc218e..07f3d63b8b 100644 --- a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc +++ b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.cc @@ -45,24 +45,10 @@ void DefaultAudioQualityAnalyzer::OnStatsReports( if (strcmp(media_type->static_string_val(), kStatsAudioMediaType) != 0) { continue; } - if (stats_report->FindValue( webrtc::StatsReport::kStatsValueNameBytesSent)) { - // If kStatsValueNameBytesSent is present, it means it's a send stream. - // All we need from a send stream is bytes sent. - const webrtc::StatsReport::Value* bytes_sent = stats_report->FindValue( - StatsReport::StatsValueName::kStatsValueNameBytesSent); - const webrtc::StatsReport::Value* report_track_id = - stats_report->FindValue( - StatsReport::StatsValueName::kStatsValueNameTrackId); - - rtc::CritScope crit(&lock_); - // Note: outgoing streams have their "stream label" directly in the - // report's track id field. There is no need to look it up using - // GetStreamLabelFromStatsReport(), and in fact doing so will crash. - AudioStreamStats& audio_stream_stats = - streams_stats_[report_track_id->string_val()]; - audio_stream_stats.bytes_sent = bytes_sent->int64_val(); + // If kStatsValueNameBytesSent is present, it means it's a send stream, + // but we need audio metrics for receive stream, so skip it. continue; } @@ -126,9 +112,6 @@ void DefaultAudioQualityAnalyzer::Stop() { item.second.speech_expand_rate, "unitless"); ReportResult("preferred_buffer_size_ms", item.first, item.second.preferred_buffer_size_ms, "ms"); - test::PrintResult("bytes_sent", "", GetTestCaseName(item.first), - item.second.bytes_sent, "sizeInBytes", - /*important=*/false); } } 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 824da607d9..ee34ed345b 100644 --- a/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h +++ b/test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h @@ -29,7 +29,6 @@ struct AudioStreamStats { SamplesStatsCounter preemptive_rate; SamplesStatsCounter speech_expand_rate; SamplesStatsCounter preferred_buffer_size_ms; - int64_t bytes_sent; }; // TODO(bugs.webrtc.org/10430): Migrate to the new GetStats as soon as 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 45516d2ea4..499d04e2d5 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc @@ -27,7 +27,6 @@ constexpr int kMaxActiveComparisons = 10; constexpr int kFreezeThresholdMs = 150; constexpr int kMicrosPerSecond = 1000000; constexpr int kBitsInByte = 8; -constexpr char kStatsVideoMediaType[] = "video"; void LogFrameCounters(const std::string& name, const FrameCounters& counters) { RTC_LOG(INFO) << "[" << name << "] Captured : " << counters.captured; @@ -374,28 +373,7 @@ void DefaultVideoQualityAnalyzer::OnStatsReports( const std::string& pc_label, const StatsReports& stats_reports) { for (const StatsReport* stats_report : stats_reports) { - // Record the number of video bytes sent from outgoing SSRC reports. - if (stats_report->type() == StatsReport::StatsType::kStatsReportTypeSsrc && - strcmp(stats_report - ->FindValue( - StatsReport::StatsValueName::kStatsValueNameMediaType) - ->static_string_val(), - kStatsVideoMediaType) == 0 && - stats_report->FindValue(StatsReport::kStatsValueNameBytesSent)) { - const webrtc::StatsReport::Value* bytes_sent = stats_report->FindValue( - StatsReport::StatsValueName::kStatsValueNameBytesSent); - const webrtc::StatsReport::Value* track_id = stats_report->FindValue( - StatsReport::StatsValueName::kStatsValueNameTrackId); - - rtc::CritScope crit(&comparison_lock_); - // Note: outgoing streams have their "stream label" directly in the - // report's track id field. There is no need to look it up using - // GetStreamLabelFromStatsReport(), and in fact doing so will crash. - StreamStats& stream_stats = stream_stats_[track_id->string_val()]; - stream_stats.bytes_sent = bytes_sent->int64_val(); - } - - // The only other stats collected by this analyzer are present in + // 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; @@ -651,8 +629,6 @@ void DefaultVideoQualityAnalyzer::ReportResults( /*important=*/false); ReportResult("max_skipped", test_case_name, stats.skipped_between_rendered, "unitless"); - test::PrintResult("bytes_sent", "", test_case_name, stats.bytes_sent, - "sizeInBytes", /*important=*/false); } void DefaultVideoQualityAnalyzer::ReportResult( 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 ef10d27c54..e7be2b5af9 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h @@ -96,8 +96,6 @@ struct StreamStats { int64_t dropped_by_encoder = 0; int64_t dropped_before_encoder = 0; - - int64_t bytes_sent = 0; }; struct AnalyzerStats {