diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 999979def8..d523128e38 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -141,10 +141,6 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { GetSendStreamDataCounters, (StreamDataCounters*, StreamDataCounters*), (const, override)); - MOCK_METHOD(int32_t, - RemoteRTCPStat, - (std::vector * receive_blocks), - (const, override)); MOCK_METHOD(std::vector, GetLatestReportBlockData, (), diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 7d2e543a2c..ae0e9e5934 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -385,17 +385,6 @@ RTCPReceiver::ConsumeReceivedXrReferenceTimeInfo() { return last_xr_rtis; } -// We can get multiple receive reports when we receive the report from a CE. -int32_t RTCPReceiver::StatisticsReceived( - std::vector* receive_blocks) const { - RTC_DCHECK(receive_blocks); - MutexLock lock(&rtcp_receiver_lock_); - for (const auto& reports_per_receiver : received_report_blocks_) - for (const auto& report : reports_per_receiver.second) - receive_blocks->push_back(report.second.report_block()); - return 0; -} - std::vector RTCPReceiver::GetLatestReportBlockData() const { std::vector result; MutexLock lock(&rtcp_receiver_lock_); diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index 350ec28310..7b0f38bea0 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -100,8 +100,6 @@ class RTCPReceiver final { absl::optional OnPeriodicRttUpdate(Timestamp newer_than, bool sending); - // Get statistics. - int32_t StatisticsReceived(std::vector* receiveBlocks) const; // A snapshot of Report Blocks with additional data of interest to statistics. // Within this list, the sender-source SSRC pair is unique and per-pair the // ReportBlockData represents the latest Report Block that was received for diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 13b3fe8b35..5739c51f65 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -355,9 +355,7 @@ TEST(RtcpReceiverTest, InjectRrPacket) { OnReceivedRtcpReceiverReport(IsEmpty(), _, now)); receiver.IncomingPacket(rr.Build()); - std::vector report_blocks; - receiver.StatisticsReceived(&report_blocks); - EXPECT_TRUE(report_blocks.empty()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), IsEmpty()); } TEST(RtcpReceiverTest, InjectRrPacketWithReportBlockNotToUsIgnored) { @@ -378,9 +376,7 @@ TEST(RtcpReceiverTest, InjectRrPacketWithReportBlockNotToUsIgnored) { receiver.IncomingPacket(rr.Build()); EXPECT_EQ(0, receiver.LastReceivedReportBlockMs()); - std::vector received_blocks; - receiver.StatisticsReceived(&received_blocks); - EXPECT_TRUE(received_blocks.empty()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), IsEmpty()); } TEST(RtcpReceiverTest, InjectRrPacketWithOneReportBlock) { @@ -402,9 +398,7 @@ TEST(RtcpReceiverTest, InjectRrPacketWithOneReportBlock) { receiver.IncomingPacket(rr.Build()); EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); - std::vector received_blocks; - receiver.StatisticsReceived(&received_blocks); - EXPECT_EQ(1u, received_blocks.size()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(1)); } TEST(RtcpReceiverTest, InjectSrPacketWithOneReportBlock) { @@ -426,9 +420,7 @@ TEST(RtcpReceiverTest, InjectSrPacketWithOneReportBlock) { receiver.IncomingPacket(sr.Build()); EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); - std::vector received_blocks; - receiver.StatisticsReceived(&received_blocks); - EXPECT_EQ(1u, received_blocks.size()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(1)); } TEST(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { @@ -462,11 +454,12 @@ TEST(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { receiver.IncomingPacket(rr1.Build()); EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); - std::vector received_blocks; - receiver.StatisticsReceived(&received_blocks); - EXPECT_THAT(received_blocks, - UnorderedElementsAre(Field(&RTCPReportBlock::fraction_lost, 0), - Field(&RTCPReportBlock::fraction_lost, 10))); + EXPECT_THAT(receiver.GetLatestReportBlockData(), + UnorderedElementsAre( + Property(&ReportBlockData::report_block, + Field(&RTCPReportBlock::fraction_lost, 0)), + Property(&ReportBlockData::report_block, + Field(&RTCPReportBlock::fraction_lost, 10)))); // Insert next receiver report with same ssrc but new values. rtcp::ReportBlock rb3; @@ -495,22 +488,23 @@ TEST(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { OnReceivedRtcpReceiverReport(SizeIs(2), _, now)); receiver.IncomingPacket(rr2.Build()); - received_blocks.clear(); - receiver.StatisticsReceived(&received_blocks); - EXPECT_EQ(2u, received_blocks.size()); EXPECT_THAT( - received_blocks, + receiver.GetLatestReportBlockData(), UnorderedElementsAre( - AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), - Field(&RTCPReportBlock::fraction_lost, kFracLost[0]), - Field(&RTCPReportBlock::packets_lost, kCumLost[0]), - Field(&RTCPReportBlock::extended_highest_sequence_number, - kSequenceNumbers[0])), - AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverExtraSsrc), - Field(&RTCPReportBlock::fraction_lost, kFracLost[1]), - Field(&RTCPReportBlock::packets_lost, kCumLost[1]), - Field(&RTCPReportBlock::extended_highest_sequence_number, - kSequenceNumbers[1])))); + Property( + &ReportBlockData::report_block, + AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), + Field(&RTCPReportBlock::fraction_lost, kFracLost[0]), + Field(&RTCPReportBlock::packets_lost, kCumLost[0]), + Field(&RTCPReportBlock::extended_highest_sequence_number, + kSequenceNumbers[0]))), + Property( + &ReportBlockData::report_block, + AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverExtraSsrc), + Field(&RTCPReportBlock::fraction_lost, kFracLost[1]), + Field(&RTCPReportBlock::packets_lost, kCumLost[1]), + Field(&RTCPReportBlock::extended_highest_sequence_number, + kSequenceNumbers[1]))))); } TEST(RtcpReceiverTest, InjectRrPacketsFromTwoRemoteSsrcs) { @@ -540,15 +534,16 @@ TEST(RtcpReceiverTest, InjectRrPacketsFromTwoRemoteSsrcs) { EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); - std::vector received_blocks; - receiver.StatisticsReceived(&received_blocks); - EXPECT_EQ(1u, received_blocks.size()); - EXPECT_EQ(kSenderSsrc, received_blocks[0].sender_ssrc); - EXPECT_EQ(kReceiverMainSsrc, received_blocks[0].source_ssrc); - EXPECT_EQ(kFracLost[0], received_blocks[0].fraction_lost); - EXPECT_EQ(kCumLost[0], received_blocks[0].packets_lost); - EXPECT_EQ(kSequenceNumbers[0], - received_blocks[0].extended_highest_sequence_number); + EXPECT_THAT( + receiver.GetLatestReportBlockData(), + ElementsAre(Property( + &ReportBlockData::report_block, + AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), + Field(&RTCPReportBlock::sender_ssrc, kSenderSsrc), + Field(&RTCPReportBlock::fraction_lost, kFracLost[0]), + Field(&RTCPReportBlock::packets_lost, kCumLost[0]), + Field(&RTCPReportBlock::extended_highest_sequence_number, + kSequenceNumbers[0]))))); rtcp::ReportBlock rb2; rb2.SetMediaSsrc(kReceiverMainSsrc); @@ -564,24 +559,25 @@ TEST(RtcpReceiverTest, InjectRrPacketsFromTwoRemoteSsrcs) { OnReceivedRtcpReceiverReport(SizeIs(1), _, now)); receiver.IncomingPacket(rr2.Build()); - received_blocks.clear(); - receiver.StatisticsReceived(&received_blocks); - ASSERT_EQ(2u, received_blocks.size()); EXPECT_THAT( - received_blocks, + receiver.GetLatestReportBlockData(), UnorderedElementsAre( - AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), - Field(&RTCPReportBlock::sender_ssrc, kSenderSsrc), - Field(&RTCPReportBlock::fraction_lost, kFracLost[0]), - Field(&RTCPReportBlock::packets_lost, kCumLost[0]), - Field(&RTCPReportBlock::extended_highest_sequence_number, - kSequenceNumbers[0])), - AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), - Field(&RTCPReportBlock::sender_ssrc, kSenderSsrc2), - Field(&RTCPReportBlock::fraction_lost, kFracLost[1]), - Field(&RTCPReportBlock::packets_lost, kCumLost[1]), - Field(&RTCPReportBlock::extended_highest_sequence_number, - kSequenceNumbers[1])))); + Property( + &ReportBlockData::report_block, + AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), + Field(&RTCPReportBlock::sender_ssrc, kSenderSsrc), + Field(&RTCPReportBlock::fraction_lost, kFracLost[0]), + Field(&RTCPReportBlock::packets_lost, kCumLost[0]), + Field(&RTCPReportBlock::extended_highest_sequence_number, + kSequenceNumbers[0]))), + Property( + &ReportBlockData::report_block, + AllOf(Field(&RTCPReportBlock::source_ssrc, kReceiverMainSsrc), + Field(&RTCPReportBlock::sender_ssrc, kSenderSsrc2), + Field(&RTCPReportBlock::fraction_lost, kFracLost[1]), + Field(&RTCPReportBlock::packets_lost, kCumLost[1]), + Field(&RTCPReportBlock::extended_highest_sequence_number, + kSequenceNumbers[1]))))); } TEST(RtcpReceiverTest, GetRtt) { @@ -671,9 +667,7 @@ TEST(RtcpReceiverTest, InjectByePacket_RemovesReportBlocks) { EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr.Build()); - std::vector received_blocks; - receiver.StatisticsReceived(&received_blocks); - EXPECT_EQ(2u, received_blocks.size()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(2)); // Verify that BYE removes the report blocks. rtcp::Bye bye; @@ -681,18 +675,14 @@ TEST(RtcpReceiverTest, InjectByePacket_RemovesReportBlocks) { receiver.IncomingPacket(bye.Build()); - received_blocks.clear(); - receiver.StatisticsReceived(&received_blocks); - EXPECT_TRUE(received_blocks.empty()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), IsEmpty()); // Inject packet again. EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr.Build()); - received_blocks.clear(); - receiver.StatisticsReceived(&received_blocks); - EXPECT_EQ(2u, received_blocks.size()); + EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(2)); } TEST(RtcpReceiverTest, InjectByePacketRemovesReferenceTimeInfo) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 04ef821fb9..fa4af1dedc 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -123,20 +123,18 @@ void ModuleRtpRtcpImpl::Process() { // processed RTT for at least |kRtpRtcpRttProcessTimeMs| milliseconds. // Note that LastReceivedReportBlockMs() grabs a lock, so check // |process_rtt| first. - if (process_rtt && + if (process_rtt && rtt_stats_ != nullptr && rtcp_receiver_.LastReceivedReportBlockMs() > last_rtt_process_time_) { - std::vector receive_blocks; - rtcp_receiver_.StatisticsReceived(&receive_blocks); - int64_t max_rtt = 0; - for (std::vector::iterator it = receive_blocks.begin(); - it != receive_blocks.end(); ++it) { - int64_t rtt = 0; - rtcp_receiver_.RTT(it->sender_ssrc, &rtt, NULL, NULL, NULL); - max_rtt = (rtt > max_rtt) ? rtt : max_rtt; + int64_t max_rtt_ms = 0; + for (const auto& block : rtcp_receiver_.GetLatestReportBlockData()) { + if (block.last_rtt_ms() > max_rtt_ms) { + max_rtt_ms = block.last_rtt_ms(); + } } // Report the rtt. - if (rtt_stats_ && max_rtt != 0) - rtt_stats_->OnRttUpdate(max_rtt); + if (max_rtt_ms > 0) { + rtt_stats_->OnRttUpdate(max_rtt_ms); + } } // Verify receiver reports are delivered and the reported sequence number @@ -536,11 +534,6 @@ void ModuleRtpRtcpImpl::GetSendStreamDataCounters( } // Received RTCP report. -int32_t ModuleRtpRtcpImpl::RemoteRTCPStat( - std::vector* receive_blocks) const { - return rtcp_receiver_.StatisticsReceived(receive_blocks); -} - std::vector ModuleRtpRtcpImpl::GetLatestReportBlockData() const { return rtcp_receiver_.GetLatestReportBlockData(); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 9bcdd0c9e6..5bcabc57b1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -188,9 +188,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { StreamDataCounters* rtp_counters, StreamDataCounters* rtx_counters) const override; - // Get received RTCP report, report block. - int32_t RemoteRTCPStat( - std::vector* receive_blocks) const override; // A snapshot of the most recent Report Block with additional data of // interest to statistics. Used to implement RTCRemoteInboundRtpStreamStats. // Within this list, the ReportBlockData::RTCPReportBlock::source_ssrc(), diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 73baab1da7..78ccf9907f 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -501,11 +501,6 @@ void ModuleRtpRtcpImpl2::GetSendStreamDataCounters( } // Received RTCP report. -int32_t ModuleRtpRtcpImpl2::RemoteRTCPStat( - std::vector* receive_blocks) const { - return rtcp_receiver_.StatisticsReceived(receive_blocks); -} - std::vector ModuleRtpRtcpImpl2::GetLatestReportBlockData() const { return rtcp_receiver_.GetLatestReportBlockData(); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index 7b8de1b050..00f6ff161d 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -200,9 +200,6 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, StreamDataCounters* rtp_counters, StreamDataCounters* rtx_counters) const override; - // Get received RTCP report, report block. - int32_t RemoteRTCPStat( - std::vector* receive_blocks) const override; // A snapshot of the most recent Report Block with additional data of // interest to statistics. Used to implement RTCRemoteInboundRtpStreamStats. // Within this list, the ReportBlockData::RTCPReportBlock::source_ssrc(), diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 4505ba1d48..5ab48c9ad4 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -383,12 +383,6 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { StreamDataCounters* rtp_counters, StreamDataCounters* rtx_counters) const = 0; - // Returns received RTCP report block. - // Returns -1 on failure else 0. - // TODO(https://crbug.com/webrtc/10678): Remove this in favor of - // GetLatestReportBlockData(). - virtual int32_t RemoteRTCPStat( - std::vector* receive_blocks) const = 0; // A snapshot of Report Blocks with additional data of interest to statistics. // Within this list, the sender-source SSRC pair is unique and per-pair the // ReportBlockData represents the latest Report Block that was received for