From e45cfb45b13f18c6f59713745e9af13f2426db9b Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 13 Jun 2022 13:00:33 +0200 Subject: [PATCH] Delete RtcpPacketTypeCounter::first_packet_time_ms as unused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:13757 Change-Id: I358ab99c899b9de5f0135d5293101e7abda4aa31 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265682 Auto-Submit: Danil Chapovalov Commit-Queue: Åsa Persson Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/main@{#37198} --- modules/rtp_rtcp/include/rtcp_statistics.h | 20 +-------------- modules/rtp_rtcp/source/rtcp_receiver.cc | 3 --- modules/rtp_rtcp/source/rtcp_sender.cc | 3 --- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 2 -- .../source/rtp_rtcp_impl2_unittest.cc | 14 +++-------- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 25 +++---------------- video/send_statistics_proxy_unittest.cc | 1 - 7 files changed, 8 insertions(+), 60 deletions(-) diff --git a/modules/rtp_rtcp/include/rtcp_statistics.h b/modules/rtp_rtcp/include/rtcp_statistics.h index de70c14943..6d6246d8a8 100644 --- a/modules/rtp_rtcp/include/rtcp_statistics.h +++ b/modules/rtp_rtcp/include/rtcp_statistics.h @@ -20,8 +20,7 @@ namespace webrtc { // Statistics for RTCP packet types. struct RtcpPacketTypeCounter { RtcpPacketTypeCounter() - : first_packet_time_ms(-1), - nack_packets(0), + : nack_packets(0), fir_packets(0), pli_packets(0), nack_requests(0), @@ -33,12 +32,6 @@ struct RtcpPacketTypeCounter { pli_packets += other.pli_packets; nack_requests += other.nack_requests; unique_nack_requests += other.unique_nack_requests; - if (other.first_packet_time_ms != -1 && - (other.first_packet_time_ms < first_packet_time_ms || - first_packet_time_ms == -1)) { - // Use oldest time. - first_packet_time_ms = other.first_packet_time_ms; - } } void Subtract(const RtcpPacketTypeCounter& other) { @@ -47,16 +40,6 @@ struct RtcpPacketTypeCounter { pli_packets -= other.pli_packets; nack_requests -= other.nack_requests; unique_nack_requests -= other.unique_nack_requests; - if (other.first_packet_time_ms != -1 && - (other.first_packet_time_ms > first_packet_time_ms || - first_packet_time_ms == -1)) { - // Use youngest time. - first_packet_time_ms = other.first_packet_time_ms; - } - } - - int64_t TimeSinceFirstPacketInMs(int64_t now_ms) const { - return (first_packet_time_ms == -1) ? -1 : (now_ms - first_packet_time_ms); } int UniqueNackRequestsInPercent() const { @@ -67,7 +50,6 @@ struct RtcpPacketTypeCounter { 0.5f); } - int64_t first_packet_time_ms; // Time when first packet is sent/received. uint32_t nack_packets; // Number of RTCP NACK packets. uint32_t fir_packets; // Number of RTCP FIR packets. uint32_t pli_packets; // Number of RTCP PLI packets. diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index c117faaf59..3665b60c8e 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -466,9 +466,6 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, break; } - if (packet_type_counter_.first_packet_time_ms == -1) - packet_type_counter_.first_packet_time_ms = clock_->TimeInMilliseconds(); - switch (rtcp_block.type()) { case rtcp::SenderReport::kPacketType: HandleSenderReport(rtcp_block, packet_information); diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index c9543c1d1a..7983371097 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -713,9 +713,6 @@ absl::optional RTCPSender::ComputeCompoundRTCPPacket( } } - if (packet_type_counter_.first_packet_time_ms == -1) - packet_type_counter_.first_packet_time_ms = clock_->TimeInMilliseconds(); - // We need to send our NTP even if we haven't received any reports. RtcpContext context(feedback_state, nack_size, nack_list, clock_->CurrentTime()); diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 73e2f7f797..4007cf14d8 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -603,8 +603,6 @@ TEST_F(RtcpSenderTest, TestRegisterRtcpPacketTypeObserver) { EXPECT_EQ(1, parser()->pli()->num_packets()); EXPECT_EQ(kRemoteSsrc, observer.ssrc_); EXPECT_EQ(1U, observer.counter_.pli_packets); - EXPECT_EQ(clock_.TimeInMilliseconds(), - observer.counter_.first_packet_time_ms); } TEST_F(RtcpSenderTest, SendTmmbr) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 48b0aac037..664fbbadc9 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -511,18 +511,15 @@ TEST_P(RtpRtcpImpl2Test, NoSrBeforeMedia) { // Verify no SR is sent before media has been sent, RR should still be sent // from the receiving module though. AdvanceTime(kDefaultReportInterval / 2); - int64_t current_time = time_controller_.GetClock()->TimeInMilliseconds(); - EXPECT_EQ(-1, sender_.RtcpSent().first_packet_time_ms); - EXPECT_EQ(receiver_.RtcpSent().first_packet_time_ms, current_time); + EXPECT_EQ(sender_.transport_.NumRtcpSent(), 0u); + EXPECT_EQ(receiver_.transport_.NumRtcpSent(), 1u); // RTCP should be triggered by the RTP send. EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); - EXPECT_EQ(sender_.RtcpSent().first_packet_time_ms, current_time); + EXPECT_EQ(sender_.transport_.NumRtcpSent(), 1u); } TEST_P(RtpRtcpImpl2Test, RtcpPacketTypeCounter_Nack) { - EXPECT_EQ(-1, receiver_.RtcpSent().first_packet_time_ms); - EXPECT_EQ(-1, sender_.RtcpReceived().first_packet_time_ms); EXPECT_EQ(0U, sender_.RtcpReceived().nack_packets); EXPECT_EQ(0U, receiver_.RtcpSent().nack_packets); @@ -532,11 +529,9 @@ TEST_P(RtpRtcpImpl2Test, RtcpPacketTypeCounter_Nack) { EXPECT_EQ(0, receiver_.impl_->SendNACK(nack_list, kNackLength)); AdvanceTime(kOneWayNetworkDelay); EXPECT_EQ(1U, receiver_.RtcpSent().nack_packets); - EXPECT_GT(receiver_.RtcpSent().first_packet_time_ms, -1); // Send module receives the NACK. EXPECT_EQ(1U, sender_.RtcpReceived().nack_packets); - EXPECT_GT(sender_.RtcpReceived().first_packet_time_ms, -1); } TEST_P(RtpRtcpImpl2Test, AddStreamDataCounters) { @@ -693,17 +688,14 @@ TEST_P(RtpRtcpImpl2Test, ConfigurableRtcpReportInterval) { EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); // Initial state - EXPECT_EQ(sender_.RtcpSent().first_packet_time_ms, -1); EXPECT_EQ(0u, sender_.transport_.NumRtcpSent()); // Move ahead to the last ms before a rtcp is expected, no action. AdvanceTime(kVideoReportInterval / 2 - TimeDelta::Millis(1)); - EXPECT_EQ(sender_.RtcpSent().first_packet_time_ms, -1); EXPECT_EQ(sender_.transport_.NumRtcpSent(), 0u); // Move ahead to the first rtcp. Send RTCP. AdvanceTime(TimeDelta::Millis(1)); - EXPECT_GT(sender_.RtcpSent().first_packet_time_ms, -1); EXPECT_EQ(sender_.transport_.NumRtcpSent(), 1u); EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 57b564acf4..9b9c1d8970 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -337,24 +337,21 @@ TEST_F(RtpRtcpImplTest, NoSrBeforeMedia) { receiver_.transport_.SimulateNetworkDelay(0, &clock_); sender_.impl_->Process(); - EXPECT_EQ(-1, sender_.RtcpSent().first_packet_time_ms); + EXPECT_EQ(sender_.transport_.NumRtcpSent(), 0u); // Verify no SR is sent before media has been sent, RR should still be sent // from the receiving module though. clock_.AdvanceTimeMilliseconds(2000); - int64_t current_time = clock_.TimeInMilliseconds(); sender_.impl_->Process(); receiver_.impl_->Process(); - EXPECT_EQ(-1, sender_.RtcpSent().first_packet_time_ms); - EXPECT_EQ(receiver_.RtcpSent().first_packet_time_ms, current_time); + EXPECT_EQ(sender_.transport_.NumRtcpSent(), 0u); + EXPECT_EQ(receiver_.transport_.NumRtcpSent(), 1u); SendFrame(&sender_, sender_video_.get(), kBaseLayerTid); - EXPECT_EQ(sender_.RtcpSent().first_packet_time_ms, current_time); + EXPECT_EQ(sender_.transport_.NumRtcpSent(), 1u); } TEST_F(RtpRtcpImplTest, RtcpPacketTypeCounter_Nack) { - EXPECT_EQ(-1, receiver_.RtcpSent().first_packet_time_ms); - EXPECT_EQ(-1, sender_.RtcpReceived().first_packet_time_ms); EXPECT_EQ(0U, sender_.RtcpReceived().nack_packets); EXPECT_EQ(0U, receiver_.RtcpSent().nack_packets); @@ -363,17 +360,13 @@ TEST_F(RtpRtcpImplTest, RtcpPacketTypeCounter_Nack) { uint16_t nack_list[kNackLength] = {123}; EXPECT_EQ(0, receiver_.impl_->SendNACK(nack_list, kNackLength)); EXPECT_EQ(1U, receiver_.RtcpSent().nack_packets); - EXPECT_GT(receiver_.RtcpSent().first_packet_time_ms, -1); // Send module receives the NACK. EXPECT_EQ(1U, sender_.RtcpReceived().nack_packets); - EXPECT_GT(sender_.RtcpReceived().first_packet_time_ms, -1); } TEST_F(RtpRtcpImplTest, AddStreamDataCounters) { StreamDataCounters rtp; - const int64_t kStartTimeMs = 1; - rtp.first_packet_time_ms = kStartTimeMs; rtp.transmitted.packets = 1; rtp.transmitted.payload_bytes = 1; rtp.transmitted.header_bytes = 2; @@ -383,7 +376,6 @@ TEST_F(RtpRtcpImplTest, AddStreamDataCounters) { rtp.transmitted.padding_bytes); StreamDataCounters rtp2; - rtp2.first_packet_time_ms = -1; rtp2.transmitted.packets = 10; rtp2.transmitted.payload_bytes = 10; rtp2.retransmitted.header_bytes = 4; @@ -394,7 +386,6 @@ TEST_F(RtpRtcpImplTest, AddStreamDataCounters) { StreamDataCounters sum = rtp; sum.Add(rtp2); - EXPECT_EQ(kStartTimeMs, sum.first_packet_time_ms); EXPECT_EQ(11U, sum.transmitted.packets); EXPECT_EQ(11U, sum.transmitted.payload_bytes); EXPECT_EQ(2U, sum.transmitted.header_bytes); @@ -406,11 +397,6 @@ TEST_F(RtpRtcpImplTest, AddStreamDataCounters) { EXPECT_EQ(8U, sum.fec.packets); EXPECT_EQ(sum.transmitted.TotalBytes(), rtp.transmitted.TotalBytes() + rtp2.transmitted.TotalBytes()); - - StreamDataCounters rtp3; - rtp3.first_packet_time_ms = kStartTimeMs + 10; - sum.Add(rtp3); - EXPECT_EQ(kStartTimeMs, sum.first_packet_time_ms); // Holds oldest time. } TEST_F(RtpRtcpImplTest, SendsInitialNackList) { @@ -525,19 +511,16 @@ TEST_F(RtpRtcpImplTest, ConfigurableRtcpReportInterval) { // Initial state sender_.impl_->Process(); - EXPECT_EQ(sender_.RtcpSent().first_packet_time_ms, -1); EXPECT_EQ(0u, sender_.transport_.NumRtcpSent()); // Move ahead to the last ms before a rtcp is expected, no action. clock_.AdvanceTimeMilliseconds(kVideoReportInterval / 2 - 1); sender_.impl_->Process(); - EXPECT_EQ(sender_.RtcpSent().first_packet_time_ms, -1); EXPECT_EQ(sender_.transport_.NumRtcpSent(), 0u); // Move ahead to the first rtcp. Send RTCP. clock_.AdvanceTimeMilliseconds(1); sender_.impl_->Process(); - EXPECT_GT(sender_.RtcpSent().first_packet_time_ms, -1); EXPECT_EQ(sender_.transport_.NumRtcpSent(), 1u); SendFrame(&sender_, sender_video_.get(), kBaseLayerTid); diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index b5731b5151..8264065f4b 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -2401,7 +2401,6 @@ TEST_F(SendStatisticsProxyTest, ResetsRtcpCountersOnContentChange) { RtcpPacketTypeCounterObserver* proxy = static_cast(statistics_proxy_.get()); RtcpPacketTypeCounter counters; - counters.first_packet_time_ms = fake_clock_.TimeInMilliseconds(); proxy->RtcpPacketTypesCounterUpdated(kFirstSsrc, counters); proxy->RtcpPacketTypesCounterUpdated(kSecondSsrc, counters);