diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc index 43b6fc81dc..9d2e11590c 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc @@ -1122,8 +1122,8 @@ void EventVerifier::VerifyReportBlock( logged_report_block.source_ssrc()); EXPECT_EQ(original_report_block.fraction_lost(), logged_report_block.fraction_lost()); - EXPECT_EQ(original_report_block.cumulative_lost_signed(), - logged_report_block.cumulative_lost_signed()); + EXPECT_EQ(original_report_block.cumulative_lost(), + logged_report_block.cumulative_lost()); EXPECT_EQ(original_report_block.extended_high_seq_num(), logged_report_block.extended_high_seq_num()); EXPECT_EQ(original_report_block.jitter(), logged_report_block.jitter()); diff --git a/modules/rtp_rtcp/include/report_block_data.cc b/modules/rtp_rtcp/include/report_block_data.cc index 81f1b0108a..c2a37131db 100644 --- a/modules/rtp_rtcp/include/report_block_data.cc +++ b/modules/rtp_rtcp/include/report_block_data.cc @@ -22,7 +22,7 @@ void ReportBlockData::SetReportBlock(uint32_t sender_ssrc, report_block_.sender_ssrc = sender_ssrc; report_block_.source_ssrc = report_block.source_ssrc(); report_block_.fraction_lost = report_block.fraction_lost(); - report_block_.packets_lost = report_block.cumulative_lost_signed(); + report_block_.packets_lost = report_block.cumulative_lost(); report_block_.extended_highest_sequence_number = report_block.extended_high_seq_num(); report_block_.jitter = report_block.jitter(); diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc index 92c8f34196..b6593a4c3b 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -285,7 +285,7 @@ TEST_P(ReceiveStatisticsTest, SimpleLossComputation) { // 20% = 51/255. EXPECT_EQ(51u, report_blocks[0].fraction_lost()); - EXPECT_EQ(1, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(1, report_blocks[0].cumulative_lost()); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); EXPECT_EQ(20, statistician->GetFractionLostInPercent()); @@ -308,7 +308,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithReordering) { // 20% = 51/255. EXPECT_EQ(51u, report_blocks[0].fraction_lost()); - EXPECT_EQ(1, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(1, report_blocks[0].cumulative_lost()); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); EXPECT_EQ(20, statistician->GetFractionLostInPercent()); @@ -333,7 +333,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithDuplicates) { // 20% = 51/255. EXPECT_EQ(51u, report_blocks[0].fraction_lost()); - EXPECT_EQ(1, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(1, report_blocks[0].cumulative_lost()); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); EXPECT_EQ(20, statistician->GetFractionLostInPercent()); @@ -359,7 +359,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) { // 20% = 51/255. EXPECT_EQ(51u, report_blocks[0].fraction_lost()); - EXPECT_EQ(1, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(1, report_blocks[0].cumulative_lost()); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); EXPECT_EQ(20, statistician->GetFractionLostInPercent()); @@ -374,7 +374,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) { // 50% = 127/255. EXPECT_EQ(127u, report_blocks[0].fraction_lost()); - EXPECT_EQ(2, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(2, report_blocks[0].cumulative_lost()); // 2 packets lost, 7 expected EXPECT_EQ(28, statistician->GetFractionLostInPercent()); } @@ -396,7 +396,7 @@ TEST_P(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) { EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); EXPECT_EQ(0, report_blocks[0].fraction_lost()); - EXPECT_EQ(0, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(0, report_blocks[0].cumulative_lost()); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); EXPECT_EQ(0, statistician->GetFractionLostInPercent()); @@ -408,7 +408,7 @@ TEST_P(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) { EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); EXPECT_EQ(0, report_blocks[0].fraction_lost()); - EXPECT_EQ(0, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(0, report_blocks[0].cumulative_lost()); EXPECT_EQ(0, statistician->GetFractionLostInPercent()); } @@ -432,7 +432,7 @@ TEST_P(ReceiveStatisticsTest, CountsLossAfterStreamRestart) { ASSERT_THAT(report_blocks, SizeIs(1)); EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); - EXPECT_EQ(1, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(1, report_blocks[0].cumulative_lost()); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); @@ -460,7 +460,7 @@ TEST_P(ReceiveStatisticsTest, StreamCanRestartAtSequenceNumberWrapAround) { ASSERT_THAT(report_blocks, SizeIs(1)); EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); - EXPECT_EQ(1, report_blocks[0].cumulative_lost_signed()); + EXPECT_EQ(1, report_blocks[0].cumulative_lost()); } TEST_P(ReceiveStatisticsTest, StreamRestartNeedsTwoConsecutivePackets) { diff --git a/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc index 23ea49622b..47f8eb13cb 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc @@ -51,7 +51,7 @@ TEST(RtcpPacketReceiverReportTest, ParseWithOneReportBlock) { const ReportBlock& rb = parsed.report_blocks().front(); EXPECT_EQ(kRemoteSsrc, rb.source_ssrc()); EXPECT_EQ(kFractionLost, rb.fraction_lost()); - EXPECT_EQ(kCumulativeLost, rb.cumulative_lost_signed()); + EXPECT_EQ(kCumulativeLost, rb.cumulative_lost()); EXPECT_EQ(kExtHighestSeqNum, rb.extended_high_seq_num()); EXPECT_EQ(kJitter, rb.jitter()); EXPECT_EQ(kLastSr, rb.last_sr()); diff --git a/modules/rtp_rtcp/source/rtcp_packet/report_block.cc b/modules/rtp_rtcp/source/rtcp_packet/report_block.cc index d4579fc8d6..e7e92d2bf1 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/report_block.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/report_block.cc @@ -65,12 +65,11 @@ bool ReportBlock::Parse(const uint8_t* buffer, size_t length) { void ReportBlock::Create(uint8_t* buffer) const { // Runtime check should be done while setting cumulative_lost. - RTC_DCHECK_LT(cumulative_lost_signed(), - (1 << 23)); // Have only 3 bytes for it. + RTC_DCHECK_LT(cumulative_lost(), (1 << 23)); // Have only 3 bytes for it. ByteWriter::WriteBigEndian(&buffer[0], source_ssrc()); ByteWriter::WriteBigEndian(&buffer[4], fraction_lost()); - ByteWriter::WriteBigEndian(&buffer[5], cumulative_lost_signed()); + ByteWriter::WriteBigEndian(&buffer[5], cumulative_lost()); ByteWriter::WriteBigEndian(&buffer[8], extended_high_seq_num()); ByteWriter::WriteBigEndian(&buffer[12], jitter()); ByteWriter::WriteBigEndian(&buffer[16], last_sr()); @@ -88,13 +87,5 @@ bool ReportBlock::SetCumulativeLost(int32_t cumulative_lost) { return true; } -uint32_t ReportBlock::cumulative_lost() const { - if (cumulative_lost_ < 0) { - RTC_LOG(LS_VERBOSE) << "Ignoring negative value of cumulative_lost"; - return 0; - } - return cumulative_lost_; -} - } // namespace rtcp } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtcp_packet/report_block.h b/modules/rtp_rtcp/source/rtcp_packet/report_block.h index eb16640ae2..6ed4f65c0b 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/report_block.h +++ b/modules/rtp_rtcp/source/rtcp_packet/report_block.h @@ -49,9 +49,10 @@ class ReportBlock { uint32_t source_ssrc() const { return source_ssrc_; } uint8_t fraction_lost() const { return fraction_lost_; } - int32_t cumulative_lost_signed() const { return cumulative_lost_; } - // Deprecated - returns max(0, cumulative_lost_), not negative values. - uint32_t cumulative_lost() const; + [[deprecated]] int32_t cumulative_lost_signed() const { + return cumulative_lost_; + } + int32_t cumulative_lost() const { return cumulative_lost_; } uint32_t extended_high_seq_num() const { return extended_high_seq_num_; } uint32_t jitter() const { return jitter_; } uint32_t last_sr() const { return last_sr_; } diff --git a/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc index 5cc102fed0..11031a059a 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc @@ -68,7 +68,7 @@ TEST(RtcpPacketReportBlockTest, ParseMatchCreate) { EXPECT_EQ(kRemoteSsrc, parsed.source_ssrc()); EXPECT_EQ(kFractionLost, parsed.fraction_lost()); - EXPECT_EQ(kCumulativeLost, parsed.cumulative_lost_signed()); + EXPECT_EQ(kCumulativeLost, parsed.cumulative_lost()); EXPECT_EQ(kExtHighestSeqNum, parsed.extended_high_seq_num()); EXPECT_EQ(kJitter, parsed.jitter()); EXPECT_EQ(kLastSr, parsed.last_sr()); @@ -77,9 +77,6 @@ TEST(RtcpPacketReportBlockTest, ParseMatchCreate) { TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) { // CumulativeLost is a signed 24-bit integer. - // However, existing code expects it to be an unsigned integer. - // The case of negative values should be unusual; we return 0 - // when caller wants an unsigned integer. const int32_t kMaxCumulativeLost = 0x7fffff; const int32_t kMinCumulativeLost = -0x800000; ReportBlock rb; @@ -87,8 +84,7 @@ TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) { EXPECT_TRUE(rb.SetCumulativeLost(kMaxCumulativeLost)); EXPECT_FALSE(rb.SetCumulativeLost(kMinCumulativeLost - 1)); EXPECT_TRUE(rb.SetCumulativeLost(kMinCumulativeLost)); - EXPECT_EQ(kMinCumulativeLost, rb.cumulative_lost_signed()); - EXPECT_EQ(0u, rb.cumulative_lost()); + EXPECT_EQ(rb.cumulative_lost(), kMinCumulativeLost); } TEST(RtcpPacketReportBlockTest, ParseNegativeCumulativeLost) { @@ -103,7 +99,7 @@ TEST(RtcpPacketReportBlockTest, ParseNegativeCumulativeLost) { ReportBlock parsed; EXPECT_TRUE(parsed.Parse(buffer, kBufferLength)); - EXPECT_EQ(kNegativeCumulativeLost, parsed.cumulative_lost_signed()); + EXPECT_EQ(kNegativeCumulativeLost, parsed.cumulative_lost()); } } // namespace diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index a1a346718f..79d57bc991 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -1579,8 +1579,7 @@ TEST(RtcpReceiverTest, EXPECT_EQ(rtcp_block.source_ssrc(), report_block.source_ssrc); EXPECT_EQ(kSenderSsrc, report_block.sender_ssrc); EXPECT_EQ(rtcp_block.fraction_lost(), report_block.fraction_lost); - EXPECT_EQ(rtcp_block.cumulative_lost_signed(), - report_block.packets_lost); + EXPECT_EQ(rtcp_block.cumulative_lost(), report_block.packets_lost); EXPECT_EQ(rtcp_block.extended_high_seq_num(), report_block.extended_highest_sequence_number); EXPECT_EQ(rtcp_block.jitter(), report_block.jitter); diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index ae59dc5b0c..0a07ab26c5 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -280,7 +280,7 @@ TEST_F(RtcpSenderTest, SendRrWithOneReportBlock) { const rtcp::ReportBlock& rb = parser()->receiver_report()->report_blocks()[0]; EXPECT_EQ(kRemoteSsrc, rb.source_ssrc()); EXPECT_EQ(0U, rb.fraction_lost()); - EXPECT_EQ(0, rb.cumulative_lost_signed()); + EXPECT_EQ(0, rb.cumulative_lost()); EXPECT_EQ(kSeqNum, rb.extended_high_seq_num()); }