diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc index 286ef62fe9..66dfe9cf80 100644 --- a/video/receive_statistics_proxy2.cc +++ b/video/receive_statistics_proxy2.cc @@ -97,15 +97,11 @@ bool IsCurrentTaskQueueOrThread(TaskQueueBase* task_queue) { } // 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 @@ void ReceiveStatisticsProxy::UpdateFramerate(int64_t now_ms) const { stats_.network_frame_rate = static_cast(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 ReceiveStatisticsProxy::GetCurrentEstimatedPlayoutNtpTimestampMs( int64_t now_ms) const { @@ -911,10 +850,6 @@ void ReceiveStatisticsProxy::OnDecodedFrame( 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 657920f539..14f3421600 100644 --- a/video/receive_statistics_proxy2.h +++ b/video/receive_statistics_proxy2.h @@ -17,7 +17,6 @@ #include #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 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback, 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 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback, // 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 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 afcba15dbe..fe468f612f 100644 --- a/video/receive_statistics_proxy2_unittest.cc +++ b/video/receive_statistics_proxy2_unittest.cc @@ -43,11 +43,10 @@ const int kHeight = 720; // 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 @@ TEST_P(ReceiveStatisticsProxy2TestWithContent, } } -class ReceiveStatisticsProxy2TestWithDecodeTimeHistograms - : public ::testing::WithParamInterface< - std::tuple>, - 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 80c58c36fd..f333c446a7 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -221,10 +221,7 @@ VideoReceiveStream2::VideoReceiveStream2( 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()),