Update video histograms that do not have a minimum lifetime limit before being recorded.
Updated histograms:
"WebRTC.Video.ReceivedPacketsLostInPercent" (two RTCP RR previously needed)
"WebRTC.Video.ReceivedFecPacketsInPercent" (one received packet previously needed)
"WebRTC.Video.RecoveredMediaPacketsInPercentOfFec" (one received FEC packet previously needed)
Prevents logging stats if call was shortly in use.
BUG=b/32659204
Review-Url: https://codereview.webrtc.org/2536653002
Cr-Commit-Position: refs/heads/master@{#15315}
diff --git a/webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h b/webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h
index fbb4b4c..af416f9 100644
--- a/webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h
+++ b/webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h
@@ -18,11 +18,15 @@
struct FecPacketCounter {
FecPacketCounter()
- : num_packets(0), num_fec_packets(0), num_recovered_packets(0) {}
+ : num_packets(0),
+ num_fec_packets(0),
+ num_recovered_packets(0),
+ first_packet_time_ms(-1) {}
size_t num_packets; // Number of received packets.
size_t num_fec_packets; // Number of received FEC packets.
size_t num_recovered_packets; // Number of recovered media packets using FEC.
+ int64_t first_packet_time_ms; // Time when first packet is received.
};
class UlpfecReceiver {
diff --git a/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
index 8ba53b5..2752a37 100644
--- a/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
@@ -17,6 +17,7 @@
#include "webrtc/base/logging.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h"
+#include "webrtc/system_wrappers/include/clock.h"
namespace webrtc {
@@ -125,6 +126,10 @@
}
}
++packet_counter_.num_packets;
+ if (packet_counter_.first_packet_time_ms == -1) {
+ packet_counter_.first_packet_time_ms =
+ Clock::GetRealTimeClock()->TimeInMilliseconds();
+ }
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>
second_received_packet;
diff --git a/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
index 89e91c4..ddca655 100644
--- a/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
@@ -187,11 +187,22 @@
std::list<ForwardErrorCorrection::Packet*> fec_packets;
EncodeFec(media_packets, kNumFecPackets, &fec_packets);
+ FecPacketCounter counter = receiver_fec_->GetPacketCounter();
+ EXPECT_EQ(0u, counter.num_packets);
+ EXPECT_EQ(-1, counter.first_packet_time_ms);
+
// Recovery
auto it = augmented_media_packets.begin();
BuildAndAddRedMediaPacket(*it);
VerifyReconstructedMediaPacket(**it, 1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
+ counter = receiver_fec_->GetPacketCounter();
+ EXPECT_EQ(1u, counter.num_packets);
+ EXPECT_EQ(0u, counter.num_fec_packets);
+ EXPECT_EQ(0u, counter.num_recovered_packets);
+ const int64_t first_packet_time_ms = counter.first_packet_time_ms;
+ EXPECT_NE(-1, first_packet_time_ms);
+
// Drop one media packet.
auto fec_it = fec_packets.begin();
BuildAndAddRedFecPacket(*fec_it);
@@ -199,10 +210,11 @@
VerifyReconstructedMediaPacket(**it, 1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
- FecPacketCounter counter = receiver_fec_->GetPacketCounter();
+ counter = receiver_fec_->GetPacketCounter();
EXPECT_EQ(2u, counter.num_packets);
EXPECT_EQ(1u, counter.num_fec_packets);
EXPECT_EQ(1u, counter.num_recovered_packets);
+ EXPECT_EQ(first_packet_time_ms, counter.first_packet_time_ms);
}
TEST_F(UlpfecReceiverTest, InjectGarbageFecHeaderLengthRecovery) {
diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc
index 7d5fbc0..15a2206 100644
--- a/webrtc/video/receive_statistics_proxy.cc
+++ b/webrtc/video/receive_statistics_proxy.cc
@@ -35,7 +35,8 @@
renders_fps_estimator_(1000, 1000),
render_fps_tracker_(100, 10u),
render_pixel_tracker_(100, 10u),
- freq_offset_counter_(clock, nullptr, kFreqOffsetProcessIntervalMs) {
+ freq_offset_counter_(clock, nullptr, kFreqOffsetProcessIntervalMs),
+ first_report_block_time_ms_(-1) {
stats_.ssrc = config_.rtp.remote_ssrc;
for (auto it : config_.rtp.rtx)
rtx_stats_[it.second.ssrc] = StreamDataCounters();
@@ -50,11 +51,16 @@
"WebRTC.Video.ReceiveStreamLifetimeInSeconds",
(clock_->TimeInMilliseconds() - start_ms_) / 1000);
- int fraction_lost = report_block_stats_.FractionLostInPercent();
- if (fraction_lost != -1) {
- RTC_HISTOGRAM_PERCENTAGE("WebRTC.Video.ReceivedPacketsLostInPercent",
- fraction_lost);
+ if (first_report_block_time_ms_ != -1 &&
+ ((clock_->TimeInMilliseconds() - first_report_block_time_ms_) / 1000) >=
+ metrics::kMinRunTimeInSeconds) {
+ int fraction_lost = report_block_stats_.FractionLostInPercent();
+ if (fraction_lost != -1) {
+ RTC_HISTOGRAM_PERCENTAGE("WebRTC.Video.ReceivedPacketsLostInPercent",
+ fraction_lost);
+ }
}
+
const int kMinRequiredSamples = 200;
int samples = static_cast<int>(render_fps_tracker_.TotalSampleCount());
if (samples > kMinRequiredSamples) {
@@ -233,6 +239,9 @@
return;
stats_.rtcp_stats = statistics;
report_block_stats_.Store(statistics, ssrc, 0);
+
+ if (first_report_block_time_ms_ == -1)
+ first_report_block_time_ms_ = clock_->TimeInMilliseconds();
}
void ReceiveStatisticsProxy::CNameChanged(const char* cname, uint32_t ssrc) {
diff --git a/webrtc/video/receive_statistics_proxy.h b/webrtc/video/receive_statistics_proxy.h
index 445a731..b75ad8c 100644
--- a/webrtc/video/receive_statistics_proxy.h
+++ b/webrtc/video/receive_statistics_proxy.h
@@ -123,6 +123,7 @@
SampleCounter delay_counter_ GUARDED_BY(crit_);
SampleCounter e2e_delay_counter_ GUARDED_BY(crit_);
MaxCounter freq_offset_counter_ GUARDED_BY(crit_);
+ int64_t first_report_block_time_ms_ GUARDED_BY(crit_);
ReportBlockStats report_block_stats_ GUARDED_BY(crit_);
QpCounters qp_counters_; // Only accessed on the decoding thread.
std::map<uint32_t, StreamDataCounters> rtx_stats_ GUARDED_BY(crit_);
diff --git a/webrtc/video/receive_statistics_proxy_unittest.cc b/webrtc/video/receive_statistics_proxy_unittest.cc
index 66856a7..74186f0 100644
--- a/webrtc/video/receive_statistics_proxy_unittest.cc
+++ b/webrtc/video/receive_statistics_proxy_unittest.cc
@@ -12,6 +12,7 @@
#include <memory>
+#include "webrtc/system_wrappers/include/metrics.h"
#include "webrtc/system_wrappers/include/metrics_default.h"
#include "webrtc/test/gtest.h"
@@ -112,6 +113,64 @@
kTimeSec));
}
+TEST_F(ReceiveStatisticsProxyTest, PacketLossHistogramIsUpdated) {
+ const uint32_t kCumLost1 = 1;
+ const uint32_t kExtSeqNum1 = 10;
+ const uint32_t kCumLost2 = 2;
+ const uint32_t kExtSeqNum2 = 20;
+
+ // One report block received.
+ RtcpStatistics rtcp_stats1;
+ rtcp_stats1.cumulative_lost = kCumLost1;
+ rtcp_stats1.extended_max_sequence_number = kExtSeqNum1;
+ statistics_proxy_->StatisticsUpdated(rtcp_stats1, kRemoteSsrc);
+
+ // Two report blocks received.
+ RtcpStatistics rtcp_stats2;
+ rtcp_stats2.cumulative_lost = kCumLost2;
+ rtcp_stats2.extended_max_sequence_number = kExtSeqNum2;
+ statistics_proxy_->StatisticsUpdated(rtcp_stats2, kRemoteSsrc);
+
+ // Two received report blocks but min run time has not passed.
+ fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000 - 1);
+ SetUp(); // Reset stat proxy causes histograms to be updated.
+ EXPECT_EQ(0,
+ metrics::NumSamples("WebRTC.Video.ReceivedPacketsLostInPercent"));
+
+ // Two report blocks received.
+ statistics_proxy_->StatisticsUpdated(rtcp_stats1, kRemoteSsrc);
+ statistics_proxy_->StatisticsUpdated(rtcp_stats2, kRemoteSsrc);
+
+ // Two received report blocks and min run time has passed.
+ fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000);
+ SetUp();
+ EXPECT_EQ(1,
+ metrics::NumSamples("WebRTC.Video.ReceivedPacketsLostInPercent"));
+ EXPECT_EQ(1, metrics::NumEvents("WebRTC.Video.ReceivedPacketsLostInPercent",
+ (kCumLost2 - kCumLost1) * 100 /
+ (kExtSeqNum2 - kExtSeqNum1)));
+}
+
+TEST_F(ReceiveStatisticsProxyTest,
+ PacketLossHistogramIsNotUpdatedIfLessThanTwoReportBlocksAreReceived) {
+ RtcpStatistics rtcp_stats1;
+ rtcp_stats1.cumulative_lost = 1;
+ rtcp_stats1.extended_max_sequence_number = 10;
+
+ // Min run time has passed but no received report block.
+ fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000);
+ SetUp(); // Reset stat proxy causes histograms to be updated.
+ EXPECT_EQ(0,
+ metrics::NumSamples("WebRTC.Video.ReceivedPacketsLostInPercent"));
+
+ // Min run time has passed but only one received report block.
+ statistics_proxy_->StatisticsUpdated(rtcp_stats1, kRemoteSsrc);
+ fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000);
+ SetUp();
+ EXPECT_EQ(0,
+ metrics::NumSamples("WebRTC.Video.ReceivedPacketsLostInPercent"));
+}
+
TEST_F(ReceiveStatisticsProxyTest, AvSyncOffsetHistogramIsUpdated) {
const int64_t kSyncOffsetMs = 22;
const double kFreqKhz = 90.0;
diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc
index 2f27d8d..3bbf69e 100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -620,6 +620,14 @@
void RtpStreamReceiver::UpdateHistograms() {
FecPacketCounter counter = ulpfec_receiver_->GetPacketCounter();
+ if (counter.first_packet_time_ms == -1)
+ return;
+
+ int64_t elapsed_sec =
+ (clock_->TimeInMilliseconds() - counter.first_packet_time_ms) / 1000;
+ if (elapsed_sec < metrics::kMinRunTimeInSeconds)
+ return;
+
if (counter.num_packets > 0) {
RTC_HISTOGRAM_PERCENTAGE(
"WebRTC.Video.ReceivedFecPacketsInPercent",
