Only update codec type histogram if lifetime is long enough (10 sec). Add metrics for Call/VideoSendStream/VideoReceiveStream lifetime. BUG= Review-Url: https://codereview.webrtc.org/2136533002 Cr-Commit-Position: refs/heads/master@{#13537}
diff --git a/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java b/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java index 14ca30b..510c895 100644 --- a/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java +++ b/webrtc/api/androidtests/src/org/webrtc/PeerConnectionTest.java
@@ -744,13 +744,13 @@ answeringExpectations.expectStateChange(DataChannel.State.CLOSED); answeringExpectations.dataChannel.close(); offeringExpectations.dataChannel.close(); - getMetrics(); // Free the Java-land objects and collect them. shutdownPC(offeringPC, offeringExpectations); offeringPC = null; shutdownPC(answeringPC, answeringExpectations); answeringPC = null; + getMetrics(); videoSource.dispose(); factory.dispose(); System.gc(); @@ -1026,13 +1026,11 @@ private static void getMetrics() { Metrics metrics = Metrics.getAndReset(); assertTrue(metrics.map.size() > 0); - // Test for example that the configured video codec is recorded when a - // VideoSendStream is created. - String name = "WebRTC.Video.Encoder.CodecType"; + // Test for example that the lifetime of a Call is recorded. + String name = "WebRTC.Call.LifetimeInSeconds"; assertTrue(metrics.map.containsKey(name)); HistogramInfo info = metrics.map.get(name); - assertEquals(1, info.samples.size()); // samples: <sample value, # of events> - assertTrue(info.samples.containsValue(2)); // <codec type, 2>, same codec configured + assertTrue(info.samples.size() > 0); } private static void shutdownPC(
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 6df86e3..f50a8d6 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc
@@ -136,6 +136,7 @@ void UpdateSendHistograms() EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_); void UpdateReceiveHistograms(); + void UpdateHistograms(); void UpdateAggregateNetworkState(); Clock* const clock_; @@ -196,6 +197,7 @@ VieRemb remb_; const std::unique_ptr<CongestionController> congestion_controller_; const std::unique_ptr<SendDelayStats> video_send_delay_stats_; + const int64_t start_ms_; RTC_DISALLOW_COPY_AND_ASSIGN(Call); }; @@ -231,11 +233,11 @@ min_allocated_send_bitrate_bps_(0), num_bitrate_updates_(0), configured_max_padding_bitrate_bps_(0), - remb_(clock_), congestion_controller_( new CongestionController(clock_, this, &remb_, event_log_.get())), - video_send_delay_stats_(new SendDelayStats(clock_)) { + video_send_delay_stats_(new SendDelayStats(clock_)), + start_ms_(clock_->TimeInMilliseconds()) { RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); RTC_DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0); RTC_DCHECK_GE(config.bitrate_config.start_bitrate_bps, @@ -285,10 +287,17 @@ // they won't try to concurrently update stats. UpdateSendHistograms(); UpdateReceiveHistograms(); + UpdateHistograms(); Trace::ReturnTrace(); } +void Call::UpdateHistograms() { + RTC_LOGGED_HISTOGRAM_COUNTS_100000( + "WebRTC.Call.LifetimeInSeconds", + (clock_->TimeInMilliseconds() - start_ms_) / 1000); +} + void Call::UpdateSendHistograms() { if (num_bitrate_updates_ == 0 || first_packet_sent_ms_ == -1) return;
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index d0e99ab..7e25f95 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc
@@ -2134,12 +2134,17 @@ screenshare ? "WebRTC.Video.Screenshare." : "WebRTC.Video."; // Verify that stats have been updated once. + EXPECT_EQ(2, metrics::NumSamples("WebRTC.Call.LifetimeInSeconds")); EXPECT_EQ(1, metrics::NumSamples("WebRTC.Call.VideoBitrateReceivedInKbps")); EXPECT_EQ(1, metrics::NumSamples("WebRTC.Call.RtcpBitrateReceivedInBps")); EXPECT_EQ(1, metrics::NumSamples("WebRTC.Call.BitrateReceivedInKbps")); EXPECT_EQ(1, metrics::NumSamples("WebRTC.Call.EstimatedSendBitrateInKbps")); EXPECT_EQ(1, metrics::NumSamples("WebRTC.Call.PacerBitrateInKbps")); + EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.SendStreamLifetimeInSeconds")); + EXPECT_EQ(1, + metrics::NumSamples("WebRTC.Video.ReceiveStreamLifetimeInSeconds")); + EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.NackPacketsSentPerMinute")); EXPECT_EQ(1, metrics::NumSamples(video_prefix + "NackPacketsReceivedPerMinute"));
diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc index 57d1f56..9057543 100644 --- a/webrtc/video/receive_statistics_proxy.cc +++ b/webrtc/video/receive_statistics_proxy.cc
@@ -24,6 +24,7 @@ Clock* clock) : clock_(clock), config_(*config), + start_ms_(clock->TimeInMilliseconds()), // 1000ms window, scale 1000 for ms to s. decode_fps_estimator_(1000, 1000), renders_fps_estimator_(1000, 1000), @@ -39,6 +40,10 @@ } void ReceiveStatisticsProxy::UpdateHistograms() { + RTC_LOGGED_HISTOGRAM_COUNTS_100000( + "WebRTC.Video.ReceiveStreamLifetimeInSeconds", + (clock_->TimeInMilliseconds() - start_ms_) / 1000); + int fraction_lost = report_block_stats_.FractionLostInPercent(); if (fraction_lost != -1) { RTC_LOGGED_HISTOGRAM_PERCENTAGE("WebRTC.Video.ReceivedPacketsLostInPercent",
diff --git a/webrtc/video/receive_statistics_proxy.h b/webrtc/video/receive_statistics_proxy.h index 86c35e7..09c5f12 100644 --- a/webrtc/video/receive_statistics_proxy.h +++ b/webrtc/video/receive_statistics_proxy.h
@@ -104,6 +104,7 @@ // ReceiveStatisticsProxy() ctor to accept a const& of Config (since we'll // then no longer store a pointer to the object). const VideoReceiveStream::Config& config_; + const int64_t start_ms_; rtc::CriticalSection crit_; VideoReceiveStream::Stats stats_ GUARDED_BY(crit_);
diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index 8852de1..dfa7185f 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc
@@ -77,16 +77,23 @@ : clock_(clock), config_(config), content_type_(content_type), + start_ms_(clock->TimeInMilliseconds()), last_sent_frame_timestamp_(0), encode_time_(kEncodeTimeWeigthFactor), uma_container_( new UmaSamplesContainer(GetUmaPrefix(content_type_), stats_, clock)) { - UpdateCodecTypeHistogram(config_.encoder_settings.payload_name); } SendStatisticsProxy::~SendStatisticsProxy() { rtc::CritScope lock(&crit_); uma_container_->UpdateHistograms(config_, stats_); + + int64_t elapsed_sec = (clock_->TimeInMilliseconds() - start_ms_) / 1000; + RTC_LOGGED_HISTOGRAM_COUNTS_100000("WebRTC.Video.SendStreamLifetimeInSeconds", + elapsed_sec); + + if (elapsed_sec >= metrics::kMinRunTimeInSeconds) + UpdateCodecTypeHistogram(config_.encoder_settings.payload_name); } SendStatisticsProxy::UmaSamplesContainer::UmaSamplesContainer(
diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h index 2c9225f..b47691a 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h
@@ -139,6 +139,7 @@ const VideoSendStream::Config config_; rtc::CriticalSection crit_; VideoEncoderConfig::ContentType content_type_ GUARDED_BY(crit_); + const int64_t start_ms_; VideoSendStream::Stats stats_ GUARDED_BY(crit_); uint32_t last_sent_frame_timestamp_ GUARDED_BY(crit_); std::map<uint32_t, StatsUpdateTimes> update_times_ GUARDED_BY(crit_);
diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc index ad51f2a..b1bf161 100644 --- a/webrtc/video/send_statistics_proxy_unittest.cc +++ b/webrtc/video/send_statistics_proxy_unittest.cc
@@ -302,6 +302,21 @@ EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.InputWidthInPixels")); } +TEST_F(SendStatisticsProxyTest, LifetimeHistogramIsUpdated) { + const int64_t kTimeSec = 3; + fake_clock_.AdvanceTimeMilliseconds(kTimeSec * 1000); + statistics_proxy_.reset(); + EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.SendStreamLifetimeInSeconds")); + EXPECT_EQ(1, metrics::NumEvents("WebRTC.Video.SendStreamLifetimeInSeconds", + kTimeSec)); +} + +TEST_F(SendStatisticsProxyTest, CodecTypeHistogramIsUpdated) { + fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000); + statistics_proxy_.reset(); + EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.Encoder.CodecType")); +} + TEST_F(SendStatisticsProxyTest, VerifyQpHistogramStats_Vp8) { EncodedImage encoded_image; CodecSpecificInfo codec_info;