Remove WebRTC.Video.DecodeTimePerFrameInMs. histograms
The decode time per frame and codec profile histograms were added
temporarily to make it possible to get an overview of the decode
time distributions. This fine grained information is not needed
longer and the histograms can be deleted.
Bug: chromium:1007526
Change-Id: Ie59627a88813e0710700cf0e13eedd6627010266
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266496
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37316}
diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc
index 286ef62..66dfe9c 100644
--- a/video/receive_statistics_proxy2.cc
+++ b/video/receive_statistics_proxy2.cc
@@ -97,15 +97,11 @@
} // namespace
-ReceiveStatisticsProxy::ReceiveStatisticsProxy(
- uint32_t remote_ssrc,
- Clock* clock,
- TaskQueueBase* worker_thread,
- const FieldTrialsView& field_trials)
+ReceiveStatisticsProxy::ReceiveStatisticsProxy(uint32_t remote_ssrc,
+ Clock* clock,
+ TaskQueueBase* worker_thread)
: clock_(clock),
start_ms_(clock->TimeInMilliseconds()),
- enable_decode_time_histograms_(
- !field_trials.IsEnabled("WebRTC-DecodeTimeHistogramsKillSwitch")),
last_sample_time_(clock->TimeInMilliseconds()),
fps_threshold_(kLowFpsThreshold,
kHighFpsThreshold,
@@ -583,63 +579,6 @@
stats_.network_frame_rate = static_cast<int>(framerate);
}
-void ReceiveStatisticsProxy::UpdateDecodeTimeHistograms(
- int width,
- int height,
- int decode_time_ms) const {
- RTC_DCHECK_RUN_ON(&main_thread_);
-
- bool is_4k = (width == 3840 || width == 4096) && height == 2160;
- bool is_hd = width == 1920 && height == 1080;
- // Only update histograms for 4k/HD and VP9/H264.
- if ((is_4k || is_hd) && (last_codec_type_ == kVideoCodecVP9 ||
- last_codec_type_ == kVideoCodecH264)) {
- const std::string kDecodeTimeUmaPrefix =
- "WebRTC.Video.DecodeTimePerFrameInMs.";
-
- // Each histogram needs its own line for it to not be reused in the wrong
- // way when the format changes.
- if (last_codec_type_ == kVideoCodecVP9) {
- bool is_sw_decoder =
- stats_.decoder_implementation_name.compare(0, 6, "libvpx") == 0;
- if (is_4k) {
- if (is_sw_decoder)
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "Vp9.4k.Sw",
- decode_time_ms);
- else
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "Vp9.4k.Hw",
- decode_time_ms);
- } else {
- if (is_sw_decoder)
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "Vp9.Hd.Sw",
- decode_time_ms);
- else
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "Vp9.Hd.Hw",
- decode_time_ms);
- }
- } else {
- bool is_sw_decoder =
- stats_.decoder_implementation_name.compare(0, 6, "FFmpeg") == 0;
- if (is_4k) {
- if (is_sw_decoder)
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "H264.4k.Sw",
- decode_time_ms);
- else
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "H264.4k.Hw",
- decode_time_ms);
-
- } else {
- if (is_sw_decoder)
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "H264.Hd.Sw",
- decode_time_ms);
- else
- RTC_HISTOGRAM_COUNTS_1000(kDecodeTimeUmaPrefix + "H264.Hd.Hw",
- decode_time_ms);
- }
- }
- }
-}
-
absl::optional<int64_t>
ReceiveStatisticsProxy::GetCurrentEstimatedPlayoutNtpTimestampMs(
int64_t now_ms) const {
@@ -911,10 +850,6 @@
if (!assembly_time.IsZero()) {
++stats_.frames_assembled_from_multiple_packets;
}
- if (enable_decode_time_histograms_) {
- UpdateDecodeTimeHistograms(frame_meta.width, frame_meta.height,
- decode_time.ms());
- }
last_content_type_ = content_type;
decode_fps_estimator_.Update(1, frame_meta.decode_timestamp.ms());
diff --git a/video/receive_statistics_proxy2.h b/video/receive_statistics_proxy2.h
index 657920f..14f3421 100644
--- a/video/receive_statistics_proxy2.h
+++ b/video/receive_statistics_proxy2.h
@@ -17,7 +17,6 @@
#include <vector>
#include "absl/types/optional.h"
-#include "api/field_trials_view.h"
#include "api/sequence_checker.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "api/task_queue/task_queue_base.h"
@@ -51,8 +50,7 @@
public:
ReceiveStatisticsProxy(uint32_t remote_ssrc,
Clock* clock,
- TaskQueueBase* worker_thread,
- const FieldTrialsView& field_trials);
+ TaskQueueBase* worker_thread);
~ReceiveStatisticsProxy() override;
VideoReceiveStreamInterface::Stats GetStats() const;
@@ -148,16 +146,11 @@
// Removes info about old frames and then updates the framerate.
void UpdateFramerate(int64_t now_ms) const;
- void UpdateDecodeTimeHistograms(int width,
- int height,
- int decode_time_ms) const;
-
absl::optional<int64_t> GetCurrentEstimatedPlayoutNtpTimestampMs(
int64_t now_ms) const;
Clock* const clock_;
const int64_t start_ms_;
- const bool enable_decode_time_histograms_;
int64_t last_sample_time_ RTC_GUARDED_BY(main_thread_);
diff --git a/video/receive_statistics_proxy2_unittest.cc b/video/receive_statistics_proxy2_unittest.cc
index afcba15..fe468f6 100644
--- a/video/receive_statistics_proxy2_unittest.cc
+++ b/video/receive_statistics_proxy2_unittest.cc
@@ -43,11 +43,10 @@
// TODO(sakal): ReceiveStatisticsProxy is lacking unittesting.
class ReceiveStatisticsProxy2Test : public ::testing::Test {
public:
- explicit ReceiveStatisticsProxy2Test(std::string field_trials = "")
- : field_trials_(field_trials), fake_clock_(1234) {
+ ReceiveStatisticsProxy2Test() : fake_clock_(1234) {
metrics::Reset();
statistics_proxy_.reset(new ReceiveStatisticsProxy(
- kRemoteSsrc, &fake_clock_, loop_.task_queue(), field_trials_));
+ kRemoteSsrc, &fake_clock_, loop_.task_queue()));
}
~ReceiveStatisticsProxy2Test() override { statistics_proxy_.reset(); }
@@ -1864,135 +1863,5 @@
}
}
-class ReceiveStatisticsProxy2TestWithDecodeTimeHistograms
- : public ::testing::WithParamInterface<
- std::tuple<bool, int, int, int, VideoCodecType, std::string>>,
- public ReceiveStatisticsProxy2Test {
- public:
- ReceiveStatisticsProxy2TestWithDecodeTimeHistograms()
- : ReceiveStatisticsProxy2Test(
- std::get<0>(GetParam())
- ? "WebRTC-DecodeTimeHistogramsKillSwitch/Enabled/"
- : "") {}
-
- protected:
- const std::string kUmaPrefix = "WebRTC.Video.DecodeTimePerFrameInMs.";
- const int expected_number_of_samples_ = {std::get<1>(GetParam())};
- const int width_ = {std::get<2>(GetParam())};
- const int height_ = {std::get<3>(GetParam())};
- const VideoCodecType codec_type_ = {std::get<4>(GetParam())};
- const std::string implementation_name_ = {std::get<5>(GetParam())};
- const std::string uma_histogram_name_ =
- kUmaPrefix + (codec_type_ == kVideoCodecVP9 ? "Vp9." : "H264.") +
- (height_ == 2160 ? "4k." : "Hd.") +
- (implementation_name_.compare("ExternalDecoder") == 0 ? "Hw" : "Sw");
-};
-
-TEST_P(ReceiveStatisticsProxy2TestWithDecodeTimeHistograms,
- DecodeTimeHistogramsUpdated) {
- constexpr int kNumberOfFrames = 10;
- constexpr int kDecodeTimeMs = 7;
- constexpr int kFrameDurationMs = 1000 / 60;
-
- webrtc::VideoFrame frame = CreateFrame(width_, height_);
-
- statistics_proxy_->OnDecoderImplementationName(implementation_name_.c_str());
- statistics_proxy_->OnPreDecode(codec_type_, /*qp=*/0);
-
- for (int i = 0; i < kNumberOfFrames; ++i) {
- statistics_proxy_->OnDecodedFrame(frame, /*qp=*/absl::nullopt,
- TimeDelta::Millis(kDecodeTimeMs),
- VideoContentType::UNSPECIFIED);
- fake_clock_.AdvanceTimeMilliseconds(kFrameDurationMs);
- }
-
- loop_.Flush();
-
- EXPECT_METRIC_EQ(expected_number_of_samples_,
- metrics::NumSamples(uma_histogram_name_));
- EXPECT_METRIC_EQ(expected_number_of_samples_,
- metrics::NumEvents(uma_histogram_name_, kDecodeTimeMs));
-}
-
-const auto kVp94kHw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/3840,
- /*height=*/2160,
- kVideoCodecVP9,
- /*implementation=*/"ExternalDecoder");
-const auto kVp94kSw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/3840,
- /*height=*/2160,
- kVideoCodecVP9,
- /*implementation=*/"libvpx");
-const auto kVp9HdHw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/1920,
- /*height=*/1080,
- kVideoCodecVP9,
- /*implementation=*/"ExternalDecoder");
-const auto kVp9HdSw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/1920,
- /*height=*/1080,
- kVideoCodecVP9,
- /*implementation=*/"libvpx");
-const auto kH2644kHw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/3840,
- /*height=*/2160,
- kVideoCodecH264,
- /*implementation=*/"ExternalDecoder");
-const auto kH2644kSw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/3840,
- /*height=*/2160,
- kVideoCodecH264,
- /*implementation=*/"FFmpeg");
-const auto kH264HdHw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/1920,
- /*height=*/1080,
- kVideoCodecH264,
- /*implementation=*/"ExternalDecoder");
-const auto kH264HdSw = std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/1920,
- /*height=*/1080,
- kVideoCodecH264,
- /*implementation=*/"FFmpeg");
-
-INSTANTIATE_TEST_SUITE_P(AllHistogramsPopulated,
- ReceiveStatisticsProxy2TestWithDecodeTimeHistograms,
- ::testing::Values(kVp94kHw,
- kVp94kSw,
- kVp9HdHw,
- kVp9HdSw,
- kH2644kHw,
- kH2644kSw,
- kH264HdHw,
- kH264HdSw));
-
-const auto kKillswitchDisabled =
- std::make_tuple(/*killswitch=*/false,
- /*expected_number_of_samples=*/10,
- /*width=*/1920,
- /*height=*/1080,
- kVideoCodecVP9,
- /*implementation=*/"libvpx");
-const auto kKillswitchEnabled =
- std::make_tuple(/*killswitch=*/true,
- /*expected_number_of_samples=*/0,
- /*width=*/1920,
- /*height=*/1080,
- kVideoCodecVP9,
- /*implementation=*/"libvpx");
-
-INSTANTIATE_TEST_SUITE_P(KillswitchEffective,
- ReceiveStatisticsProxy2TestWithDecodeTimeHistograms,
- ::testing::Values(kKillswitchDisabled,
- kKillswitchEnabled));
-
} // namespace internal
} // namespace webrtc
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 80c58c3..f333c44 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -221,10 +221,7 @@
clock_(clock),
call_stats_(call_stats),
source_tracker_(clock_),
- stats_proxy_(remote_ssrc(),
- clock_,
- call->worker_thread(),
- call->trials()),
+ stats_proxy_(remote_ssrc(), clock_, call->worker_thread()),
rtp_receive_statistics_(ReceiveStatistics::Create(clock_)),
timing_(std::move(timing)),
video_receiver_(clock_, timing_.get(), call->trials()),
