diff --git a/call/video_send_stream.cc b/call/video_send_stream.cc index 244d78089c..25513e4e4c 100644 --- a/call/video_send_stream.cc +++ b/call/video_send_stream.cc @@ -51,8 +51,13 @@ std::string VideoSendStream::StreamStats::ToString() const { ss << "retransmit_bps: " << retransmit_bitrate_bps << ", "; ss << "avg_delay_ms: " << avg_delay_ms << ", "; ss << "max_delay_ms: " << max_delay_ms << ", "; - ss << "cum_loss: " << rtcp_stats.packets_lost << ", "; - ss << "max_ext_seq: " << rtcp_stats.extended_highest_sequence_number << ", "; + if (report_block_data) { + ss << "cum_loss: " << report_block_data->report_block().packets_lost + << ", "; + ss << "max_ext_seq: " + << report_block_data->report_block().extended_highest_sequence_number + << ", "; + } ss << "nack: " << rtcp_packet_type_counts.nack_packets << ", "; ss << "fir: " << rtcp_packet_type_counts.fir_packets << ", "; ss << "pli: " << rtcp_packet_type_counts.pli_packets; diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 5c8906fbaf..fd7a101b0a 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -82,7 +82,6 @@ class VideoSendStream { uint64_t total_packet_send_delay_ms = 0; StreamDataCounters rtp_stats; RtcpPacketTypeCounter rtcp_packet_type_counts; - RtcpStatistics rtcp_stats; // A snapshot of the most recent Report Block with additional data of // interest to statistics. Used to implement RTCRemoteInboundRtpStreamStats. absl::optional report_block_data; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 9fd5751820..0bf4f20639 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2661,15 +2661,18 @@ WebRtcVideoChannel::WebRtcVideoSendStream::GetPerLayerVideoSenderInfos( stream_stats.rtp_stats.retransmitted.payload_bytes; info.retransmitted_packets_sent = stream_stats.rtp_stats.retransmitted.packets; - info.packets_lost = stream_stats.rtcp_stats.packets_lost; info.firs_rcvd = stream_stats.rtcp_packet_type_counts.fir_packets; info.nacks_rcvd = stream_stats.rtcp_packet_type_counts.nack_packets; info.plis_rcvd = stream_stats.rtcp_packet_type_counts.pli_packets; if (stream_stats.report_block_data.has_value()) { - info.report_block_datas.push_back(stream_stats.report_block_data.value()); + info.packets_lost = + stream_stats.report_block_data->report_block().packets_lost; + info.fraction_lost = + static_cast( + stream_stats.report_block_data->report_block().fraction_lost) / + (1 << 8); + info.report_block_datas.push_back(*stream_stats.report_block_data); } - info.fraction_lost = - static_cast(stream_stats.rtcp_stats.fraction_lost) / (1 << 8); info.qp_sum = stream_stats.qp_sum; info.total_encode_time_ms = stream_stats.total_encode_time_ms; info.total_encoded_bytes_target = stream_stats.total_encoded_bytes_target; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 6d29e3d89a..0c32f8ade0 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -5585,9 +5585,11 @@ TEST_F(WebRtcVideoChannelTest, GetAggregatedStatsReportForSubStreams) { substream.rtcp_packet_type_counts.fir_packets = 14; substream.rtcp_packet_type_counts.nack_packets = 15; substream.rtcp_packet_type_counts.pli_packets = 16; - substream.rtcp_stats.packets_lost = 17; - substream.rtcp_stats.fraction_lost = 18; + webrtc::RTCPReportBlock report_block; + report_block.packets_lost = 17; + report_block.fraction_lost = 18; webrtc::ReportBlockData report_block_data; + report_block_data.SetReportBlock(report_block, 0); report_block_data.AddRoundTripTimeSample(19); substream.report_block_data = report_block_data; substream.encode_frame_rate = 20.0; @@ -5621,9 +5623,12 @@ TEST_F(WebRtcVideoChannelTest, GetAggregatedStatsReportForSubStreams) { static_cast(2 * substream.rtp_stats.transmitted.packets)); EXPECT_EQ(sender.retransmitted_packets_sent, 2u * substream.rtp_stats.retransmitted.packets); - EXPECT_EQ(sender.packets_lost, 2 * substream.rtcp_stats.packets_lost); + EXPECT_EQ(sender.packets_lost, + 2 * substream.report_block_data->report_block().packets_lost); EXPECT_EQ(sender.fraction_lost, - static_cast(substream.rtcp_stats.fraction_lost) / (1 << 8)); + static_cast( + substream.report_block_data->report_block().fraction_lost) / + (1 << 8)); EXPECT_EQ(sender.rtt_ms, 0); EXPECT_EQ(sender.codec_name, DefaultCodec().name); EXPECT_EQ(sender.codec_payload_type, DefaultCodec().id); @@ -5703,9 +5708,11 @@ TEST_F(WebRtcVideoChannelTest, GetPerLayerStatsReportForSubStreams) { substream.rtcp_packet_type_counts.fir_packets = 14; substream.rtcp_packet_type_counts.nack_packets = 15; substream.rtcp_packet_type_counts.pli_packets = 16; - substream.rtcp_stats.packets_lost = 17; - substream.rtcp_stats.fraction_lost = 18; + webrtc::RTCPReportBlock report_block; + report_block.packets_lost = 17; + report_block.fraction_lost = 18; webrtc::ReportBlockData report_block_data; + report_block_data.SetReportBlock(report_block, 0); report_block_data.AddRoundTripTimeSample(19); substream.report_block_data = report_block_data; substream.encode_frame_rate = 20.0; @@ -5739,9 +5746,12 @@ TEST_F(WebRtcVideoChannelTest, GetPerLayerStatsReportForSubStreams) { static_cast(substream.rtp_stats.transmitted.packets)); EXPECT_EQ(sender.retransmitted_packets_sent, substream.rtp_stats.retransmitted.packets); - EXPECT_EQ(sender.packets_lost, substream.rtcp_stats.packets_lost); + EXPECT_EQ(sender.packets_lost, + substream.report_block_data->report_block().packets_lost); EXPECT_EQ(sender.fraction_lost, - static_cast(substream.rtcp_stats.fraction_lost) / (1 << 8)); + static_cast( + substream.report_block_data->report_block().fraction_lost) / + (1 << 8)); EXPECT_EQ(sender.rtt_ms, 0); EXPECT_EQ(sender.codec_name, DefaultCodec().name); EXPECT_EQ(sender.codec_payload_type, DefaultCodec().id); diff --git a/video/end_to_end_tests/stats_tests.cc b/video/end_to_end_tests/stats_tests.cc index 349f5510e9..bb613a544e 100644 --- a/video/end_to_end_tests/stats_tests.cc +++ b/video/end_to_end_tests/stats_tests.cc @@ -182,9 +182,7 @@ TEST_F(StatsEndToEndTest, GetStats) { const VideoSendStream::StreamStats& stream_stats = kv.second; send_stats_filled_[CompoundKey("StatisticsUpdated", kv.first)] |= - stream_stats.rtcp_stats.packets_lost != 0 || - stream_stats.rtcp_stats.extended_highest_sequence_number != 0 || - stream_stats.rtcp_stats.fraction_lost != 0; + stream_stats.report_block_data.has_value(); send_stats_filled_[CompoundKey("DataCountersUpdated", kv.first)] |= stream_stats.rtp_stats.fec.packets != 0 || diff --git a/video/report_block_stats.cc b/video/report_block_stats.cc index e3e95f9aed..bf60364682 100644 --- a/video/report_block_stats.cc +++ b/video/report_block_stats.cc @@ -31,16 +31,13 @@ ReportBlockStats::ReportBlockStats() ReportBlockStats::~ReportBlockStats() {} -void ReportBlockStats::Store(uint32_t ssrc, const RtcpStatistics& rtcp_stats) { +void ReportBlockStats::Store(uint32_t ssrc, + int packets_lost, + uint32_t extended_highest_sequence_number) { Report report; - report.packets_lost = rtcp_stats.packets_lost; - report.extended_highest_sequence_number = - rtcp_stats.extended_highest_sequence_number; - StoreAndAddPacketIncrement(ssrc, report); -} + report.packets_lost = packets_lost; + report.extended_highest_sequence_number = extended_highest_sequence_number; -void ReportBlockStats::StoreAndAddPacketIncrement(uint32_t ssrc, - const Report& report) { // Get diff with previous report block. const auto prev_report = prev_reports_.find(ssrc); if (prev_report != prev_reports_.end()) { diff --git a/video/report_block_stats.h b/video/report_block_stats.h index de4a079032..1d1140295c 100644 --- a/video/report_block_stats.h +++ b/video/report_block_stats.h @@ -15,8 +15,6 @@ #include -#include "modules/rtp_rtcp/include/rtcp_statistics.h" - namespace webrtc { // TODO(nisse): Usefulness of this class is somewhat unclear. The inputs are @@ -32,7 +30,9 @@ class ReportBlockStats { ~ReportBlockStats(); // Updates stats and stores report block. - void Store(uint32_t ssrc, const RtcpStatistics& rtcp_stats); + void Store(uint32_t ssrc, + int packets_lost, + uint32_t extended_highest_sequence_number); // Returns the total fraction of lost packets (or -1 if less than two report // blocks have been stored). @@ -45,10 +45,6 @@ class ReportBlockStats { int32_t packets_lost; }; - // Updates the total number of packets/lost packets. - // Stores the report. - void StoreAndAddPacketIncrement(uint32_t ssrc, const Report& report); - // The total number of packets/lost packets. uint32_t num_sequence_numbers_; uint32_t num_lost_sequence_numbers_; diff --git a/video/report_block_stats_unittest.cc b/video/report_block_stats_unittest.cc index 0b0230941f..bd66e571a0 100644 --- a/video/report_block_stats_unittest.cc +++ b/video/report_block_stats_unittest.cc @@ -13,65 +13,51 @@ #include "test/gtest.h" namespace webrtc { +namespace { -class ReportBlockStatsTest : public ::testing::Test { - protected: - ReportBlockStatsTest() { - // kSsrc1: report 1-3. - stats1_1_.packets_lost = 10; - stats1_1_.extended_highest_sequence_number = 24000; - stats1_2_.packets_lost = 15; - stats1_2_.extended_highest_sequence_number = 24100; - stats1_3_.packets_lost = 50; - stats1_3_.extended_highest_sequence_number = 24200; - // kSsrc2: report 1,2. - stats2_1_.packets_lost = 111; - stats2_1_.extended_highest_sequence_number = 8500; - stats2_2_.packets_lost = 136; - stats2_2_.extended_highest_sequence_number = 8800; - } +constexpr uint32_t kSsrc1 = 123; +constexpr uint32_t kSsrc2 = 234; - const uint32_t kSsrc1 = 123; - const uint32_t kSsrc2 = 234; - RtcpStatistics stats1_1_; - RtcpStatistics stats1_2_; - RtcpStatistics stats1_3_; - RtcpStatistics stats2_1_; - RtcpStatistics stats2_2_; -}; - -TEST_F(ReportBlockStatsTest, StoreAndGetFractionLost) { +TEST(ReportBlockStatsTest, StoreAndGetFractionLost) { ReportBlockStats stats; EXPECT_EQ(-1, stats.FractionLostInPercent()); // First report. - stats.Store(kSsrc1, stats1_1_); + stats.Store(kSsrc1, /*packets_lost=*/10, + /*extended_highest_sequence_number=*/24'000); EXPECT_EQ(-1, stats.FractionLostInPercent()); // fl: 100 * (15-10) / (24100-24000) = 5% - stats.Store(kSsrc1, stats1_2_); + stats.Store(kSsrc1, /*packets_lost=*/15, + /*extended_highest_sequence_number=*/24'100); EXPECT_EQ(5, stats.FractionLostInPercent()); // fl: 100 * (50-10) / (24200-24000) = 20% - stats.Store(kSsrc1, stats1_3_); + stats.Store(kSsrc1, /*packets_lost=*/50, + /*extended_highest_sequence_number=*/24'200); EXPECT_EQ(20, stats.FractionLostInPercent()); } -TEST_F(ReportBlockStatsTest, StoreAndGetFractionLost_TwoSsrcs) { +TEST(ReportBlockStatsTest, StoreAndGetFractionLost_TwoSsrcs) { ReportBlockStats stats; EXPECT_EQ(-1, stats.FractionLostInPercent()); // First report. - stats.Store(kSsrc1, stats1_1_); + stats.Store(kSsrc1, /*packets_lost=*/10, + /*extended_highest_sequence_number=*/24'000); EXPECT_EQ(-1, stats.FractionLostInPercent()); // fl: 100 * (15-10) / (24100-24000) = 5% - stats.Store(kSsrc1, stats1_2_); + stats.Store(kSsrc1, /*packets_lost=*/15, + /*extended_highest_sequence_number=*/24'100); EXPECT_EQ(5, stats.FractionLostInPercent()); // First report, kSsrc2. - stats.Store(kSsrc2, stats2_1_); + stats.Store(kSsrc2, /*packets_lost=*/111, + /*extended_highest_sequence_number=*/8'500); EXPECT_EQ(5, stats.FractionLostInPercent()); // fl: 100 * ((15-10) + (136-111)) / ((24100-24000) + (8800-8500)) = 7% - stats.Store(kSsrc2, stats2_2_); + stats.Store(kSsrc2, /*packets_lost=*/136, + /*extended_highest_sequence_number=*/8'800); EXPECT_EQ(7, stats.FractionLostInPercent()); } +} // namespace } // namespace webrtc diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index 971e42f0e0..1b968ef8f7 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -1293,13 +1293,11 @@ void SendStatisticsProxy::OnReportBlockDataUpdated( if (!stats) return; const RTCPReportBlock& report_block = report_block_data.report_block(); - stats->rtcp_stats.fraction_lost = report_block.fraction_lost; - stats->rtcp_stats.packets_lost = report_block.packets_lost; - stats->rtcp_stats.extended_highest_sequence_number = - report_block.extended_highest_sequence_number; - stats->rtcp_stats.jitter = report_block.jitter; - uma_container_->report_block_stats_.Store(report_block.source_ssrc, - stats->rtcp_stats); + uma_container_->report_block_stats_.Store( + /*ssrc=*/report_block.source_ssrc, + /*packets_lost=*/report_block.packets_lost, + /*extended_highest_sequence_number=*/ + report_block.extended_highest_sequence_number); stats->report_block_data = std::move(report_block_data); } diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index aa133022a4..d4a7a49e39 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -159,11 +159,19 @@ class SendStatisticsProxyTest : public ::testing::Test { b.rtp_stats.retransmitted.packets); EXPECT_EQ(a.rtp_stats.fec.packets, b.rtp_stats.fec.packets); - EXPECT_EQ(a.rtcp_stats.fraction_lost, b.rtcp_stats.fraction_lost); - EXPECT_EQ(a.rtcp_stats.packets_lost, b.rtcp_stats.packets_lost); - EXPECT_EQ(a.rtcp_stats.extended_highest_sequence_number, - b.rtcp_stats.extended_highest_sequence_number); - EXPECT_EQ(a.rtcp_stats.jitter, b.rtcp_stats.jitter); + EXPECT_EQ(a.report_block_data.has_value(), + b.report_block_data.has_value()); + if (a.report_block_data.has_value()) { + const RTCPReportBlock& a_rtcp_stats = + a.report_block_data->report_block(); + const RTCPReportBlock& b_rtcp_stats = + b.report_block_data->report_block(); + EXPECT_EQ(a_rtcp_stats.fraction_lost, b_rtcp_stats.fraction_lost); + EXPECT_EQ(a_rtcp_stats.packets_lost, b_rtcp_stats.packets_lost); + EXPECT_EQ(a_rtcp_stats.extended_highest_sequence_number, + b_rtcp_stats.extended_highest_sequence_number); + EXPECT_EQ(a_rtcp_stats.jitter, b_rtcp_stats.jitter); + } } } @@ -177,46 +185,32 @@ class SendStatisticsProxyTest : public ::testing::Test { TEST_F(SendStatisticsProxyTest, ReportBlockDataObserver) { ReportBlockDataObserver* callback = statistics_proxy_.get(); for (uint32_t ssrc : config_.rtp.ssrcs) { - VideoSendStream::StreamStats& ssrc_stats = expected_.substreams[ssrc]; - // Add statistics with some arbitrary, but unique, numbers. - uint32_t offset = ssrc * sizeof(RtcpStatistics); + uint32_t offset = ssrc * 4; RTCPReportBlock report_block; report_block.source_ssrc = ssrc; report_block.packets_lost = offset; report_block.extended_highest_sequence_number = offset + 1; report_block.fraction_lost = offset + 2; report_block.jitter = offset + 3; - - ssrc_stats.rtcp_stats.packets_lost = report_block.packets_lost; - ssrc_stats.rtcp_stats.extended_highest_sequence_number = - report_block.extended_highest_sequence_number; - ssrc_stats.rtcp_stats.fraction_lost = report_block.fraction_lost; - ssrc_stats.rtcp_stats.jitter = report_block.jitter; ReportBlockData data; data.SetReportBlock(report_block, 0); + expected_.substreams[ssrc].report_block_data = data; callback->OnReportBlockDataUpdated(data); } for (uint32_t ssrc : config_.rtp.rtx.ssrcs) { - VideoSendStream::StreamStats& ssrc_stats = expected_.substreams[ssrc]; - // Add statistics with some arbitrary, but unique, numbers. - uint32_t offset = ssrc * sizeof(RtcpStatistics); + uint32_t offset = ssrc * 4; RTCPReportBlock report_block; report_block.source_ssrc = ssrc; report_block.packets_lost = offset; report_block.extended_highest_sequence_number = offset + 1; report_block.fraction_lost = offset + 2; report_block.jitter = offset + 3; - - ssrc_stats.rtcp_stats.packets_lost = report_block.packets_lost; - ssrc_stats.rtcp_stats.extended_highest_sequence_number = - report_block.extended_highest_sequence_number; - ssrc_stats.rtcp_stats.fraction_lost = report_block.fraction_lost; - ssrc_stats.rtcp_stats.jitter = report_block.jitter; ReportBlockData data; data.SetReportBlock(report_block, 0); + expected_.substreams[ssrc].report_block_data = data; callback->OnReportBlockDataUpdated(data); }