mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-13 05:40:42 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Delete clamping cumulative loss in ReportBlocks on receiving side

Browse source 
Bug: webrtc:9598
Change-Id: I8a54af88708e5d96e46ba67ab0ef2e0e59fe0b86
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300941
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39887}
This commit is contained in:
Danil Chapovalov 2023-04-13 11:08:57 +02:00 committed by WebRTC LUCI CQ
parent 61e8b59701
commit 95e7a0398c
9 changed files with 24 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
logging/rtc_event_log/rtc_event_log_unittest_helper.cc
View file

@ -1122,8 +1122,8 @@ void EventVerifier::VerifyReportBlock(
logged_report_block.source_ssrc()); logged_report_block.source_ssrc());
EXPECT_EQ(original_report_block.fraction_lost(), EXPECT_EQ(original_report_block.fraction_lost(),
logged_report_block.fraction_lost()); logged_report_block.fraction_lost());
EXPECT_EQ(original_report_block.cumulative_lost_signed(), EXPECT_EQ(original_report_block.cumulative_lost(),
logged_report_block.cumulative_lost_signed()); logged_report_block.cumulative_lost());
EXPECT_EQ(original_report_block.extended_high_seq_num(), EXPECT_EQ(original_report_block.extended_high_seq_num(),
logged_report_block.extended_high_seq_num()); logged_report_block.extended_high_seq_num());
EXPECT_EQ(original_report_block.jitter(), logged_report_block.jitter()); EXPECT_EQ(original_report_block.jitter(), logged_report_block.jitter());

2
modules/rtp_rtcp/include/report_block_data.cc
View file

@ -22,7 +22,7 @@ void ReportBlockData::SetReportBlock(uint32_t sender_ssrc,
report_block_.sender_ssrc = sender_ssrc; report_block_.sender_ssrc = sender_ssrc;
report_block_.source_ssrc = report_block.source_ssrc(); report_block_.source_ssrc = report_block.source_ssrc();
report_block_.fraction_lost = report_block.fraction_lost(); 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_highest_sequence_number =
report_block.extended_high_seq_num(); report_block.extended_high_seq_num();
report_block_.jitter = report_block.jitter(); report_block_.jitter = report_block.jitter();

18
modules/rtp_rtcp/source/receive_statistics_unittest.cc
View file

@ -285,7 +285,7 @@ TEST_P(ReceiveStatisticsTest, SimpleLossComputation) {
// 20% = 51/255. // 20% = 51/255.
EXPECT_EQ(51u, report_blocks[0].fraction_lost()); 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 = StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1); receive_statistics_->GetStatistician(kSsrc1);
EXPECT_EQ(20, statistician->GetFractionLostInPercent()); EXPECT_EQ(20, statistician->GetFractionLostInPercent());
@ -308,7 +308,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithReordering) {
// 20% = 51/255. // 20% = 51/255.
EXPECT_EQ(51u, report_blocks[0].fraction_lost()); 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 = StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1); receive_statistics_->GetStatistician(kSsrc1);
EXPECT_EQ(20, statistician->GetFractionLostInPercent()); EXPECT_EQ(20, statistician->GetFractionLostInPercent());
@ -333,7 +333,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithDuplicates) {
// 20% = 51/255. // 20% = 51/255.
EXPECT_EQ(51u, report_blocks[0].fraction_lost()); 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 = StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1); receive_statistics_->GetStatistician(kSsrc1);
EXPECT_EQ(20, statistician->GetFractionLostInPercent()); EXPECT_EQ(20, statistician->GetFractionLostInPercent());
@ -359,7 +359,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) {
// 20% = 51/255. // 20% = 51/255.
EXPECT_EQ(51u, report_blocks[0].fraction_lost()); 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 = StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1); receive_statistics_->GetStatistician(kSsrc1);
EXPECT_EQ(20, statistician->GetFractionLostInPercent()); EXPECT_EQ(20, statistician->GetFractionLostInPercent());
@ -374,7 +374,7 @@ TEST_P(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) {
// 50% = 127/255. // 50% = 127/255.
EXPECT_EQ(127u, report_blocks[0].fraction_lost()); 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 // 2 packets lost, 7 expected
EXPECT_EQ(28, statistician->GetFractionLostInPercent()); EXPECT_EQ(28, statistician->GetFractionLostInPercent());
} }
@ -396,7 +396,7 @@ TEST_P(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) {
EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc());
EXPECT_EQ(0, report_blocks[0].fraction_lost()); 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 = StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1); receive_statistics_->GetStatistician(kSsrc1);
EXPECT_EQ(0, statistician->GetFractionLostInPercent()); EXPECT_EQ(0, statistician->GetFractionLostInPercent());
@ -408,7 +408,7 @@ TEST_P(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) {
EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc());
EXPECT_EQ(0, report_blocks[0].fraction_lost()); 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()); EXPECT_EQ(0, statistician->GetFractionLostInPercent());
} }
@ -432,7 +432,7 @@ TEST_P(ReceiveStatisticsTest, CountsLossAfterStreamRestart) {
ASSERT_THAT(report_blocks, SizeIs(1)); ASSERT_THAT(report_blocks, SizeIs(1));
EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); 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 = StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1); receive_statistics_->GetStatistician(kSsrc1);
@ -460,7 +460,7 @@ TEST_P(ReceiveStatisticsTest, StreamCanRestartAtSequenceNumberWrapAround) {
ASSERT_THAT(report_blocks, SizeIs(1)); ASSERT_THAT(report_blocks, SizeIs(1));
EXPECT_EQ(kSsrc1, report_blocks[0].source_ssrc()); 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) { TEST_P(ReceiveStatisticsTest, StreamRestartNeedsTwoConsecutivePackets) {

2
modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc
View file

@ -51,7 +51,7 @@ TEST(RtcpPacketReceiverReportTest, ParseWithOneReportBlock) {
const ReportBlock& rb = parsed.report_blocks().front(); const ReportBlock& rb = parsed.report_blocks().front();
EXPECT_EQ(kRemoteSsrc, rb.source_ssrc()); EXPECT_EQ(kRemoteSsrc, rb.source_ssrc());
EXPECT_EQ(kFractionLost, rb.fraction_lost()); 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(kExtHighestSeqNum, rb.extended_high_seq_num());
EXPECT_EQ(kJitter, rb.jitter()); EXPECT_EQ(kJitter, rb.jitter());
EXPECT_EQ(kLastSr, rb.last_sr()); EXPECT_EQ(kLastSr, rb.last_sr());

13
modules/rtp_rtcp/source/rtcp_packet/report_block.cc
View file

@ -65,12 +65,11 @@ bool ReportBlock::Parse(const uint8_t* buffer, size_t length) {
void ReportBlock::Create(uint8_t* buffer) const { void ReportBlock::Create(uint8_t* buffer) const {
// Runtime check should be done while setting cumulative_lost. // Runtime check should be done while setting cumulative_lost.
RTC_DCHECK_LT(cumulative_lost_signed(), RTC_DCHECK_LT(cumulative_lost(), (1 << 23)); // Have only 3 bytes for it.
(1 << 23)); // Have only 3 bytes for it.
ByteWriter<uint32_t>::WriteBigEndian(&buffer[0], source_ssrc()); ByteWriter<uint32_t>::WriteBigEndian(&buffer[0], source_ssrc());
ByteWriter<uint8_t>::WriteBigEndian(&buffer[4], fraction_lost()); ByteWriter<uint8_t>::WriteBigEndian(&buffer[4], fraction_lost());
ByteWriter<int32_t, 3>::WriteBigEndian(&buffer[5], cumulative_lost_signed()); ByteWriter<int32_t, 3>::WriteBigEndian(&buffer[5], cumulative_lost());
ByteWriter<uint32_t>::WriteBigEndian(&buffer[8], extended_high_seq_num()); ByteWriter<uint32_t>::WriteBigEndian(&buffer[8], extended_high_seq_num());
ByteWriter<uint32_t>::WriteBigEndian(&buffer[12], jitter()); ByteWriter<uint32_t>::WriteBigEndian(&buffer[12], jitter());
ByteWriter<uint32_t>::WriteBigEndian(&buffer[16], last_sr()); ByteWriter<uint32_t>::WriteBigEndian(&buffer[16], last_sr());
@ -88,13 +87,5 @@ bool ReportBlock::SetCumulativeLost(int32_t cumulative_lost) {
return true; 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 rtcp
} // namespace webrtc } // namespace webrtc

7
modules/rtp_rtcp/source/rtcp_packet/report_block.h
View file

@ -49,9 +49,10 @@ class ReportBlock {
uint32_t source_ssrc() const { return source_ssrc_; } uint32_t source_ssrc() const { return source_ssrc_; }
uint8_t fraction_lost() const { return fraction_lost_; } uint8_t fraction_lost() const { return fraction_lost_; }
int32_t cumulative_lost_signed() const { return cumulative_lost_; } [[deprecated]] int32_t cumulative_lost_signed() const {
// Deprecated - returns max(0, cumulative_lost_), not negative values. return cumulative_lost_;
uint32_t cumulative_lost() const; }
int32_t cumulative_lost() const { return cumulative_lost_; }
uint32_t extended_high_seq_num() const { return extended_high_seq_num_; } uint32_t extended_high_seq_num() const { return extended_high_seq_num_; }
uint32_t jitter() const { return jitter_; } uint32_t jitter() const { return jitter_; }
uint32_t last_sr() const { return last_sr_; } uint32_t last_sr() const { return last_sr_; }

10
modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
View file

@ -68,7 +68,7 @@ TEST(RtcpPacketReportBlockTest, ParseMatchCreate) {
EXPECT_EQ(kRemoteSsrc, parsed.source_ssrc()); EXPECT_EQ(kRemoteSsrc, parsed.source_ssrc());
EXPECT_EQ(kFractionLost, parsed.fraction_lost()); 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(kExtHighestSeqNum, parsed.extended_high_seq_num());
EXPECT_EQ(kJitter, parsed.jitter()); EXPECT_EQ(kJitter, parsed.jitter());
EXPECT_EQ(kLastSr, parsed.last_sr()); EXPECT_EQ(kLastSr, parsed.last_sr());
@ -77,9 +77,6 @@ TEST(RtcpPacketReportBlockTest, ParseMatchCreate) {
TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) { TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) {
// CumulativeLost is a signed 24-bit integer. // 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 kMaxCumulativeLost = 0x7fffff;
const int32_t kMinCumulativeLost = -0x800000; const int32_t kMinCumulativeLost = -0x800000;
ReportBlock rb; ReportBlock rb;
@ -87,8 +84,7 @@ TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) {
EXPECT_TRUE(rb.SetCumulativeLost(kMaxCumulativeLost)); EXPECT_TRUE(rb.SetCumulativeLost(kMaxCumulativeLost));
EXPECT_FALSE(rb.SetCumulativeLost(kMinCumulativeLost - 1)); EXPECT_FALSE(rb.SetCumulativeLost(kMinCumulativeLost - 1));
EXPECT_TRUE(rb.SetCumulativeLost(kMinCumulativeLost)); EXPECT_TRUE(rb.SetCumulativeLost(kMinCumulativeLost));
EXPECT_EQ(kMinCumulativeLost, rb.cumulative_lost_signed()); EXPECT_EQ(rb.cumulative_lost(), kMinCumulativeLost);
EXPECT_EQ(0u, rb.cumulative_lost());
} }
TEST(RtcpPacketReportBlockTest, ParseNegativeCumulativeLost) { TEST(RtcpPacketReportBlockTest, ParseNegativeCumulativeLost) {
@ -103,7 +99,7 @@ TEST(RtcpPacketReportBlockTest, ParseNegativeCumulativeLost) {
ReportBlock parsed; ReportBlock parsed;
EXPECT_TRUE(parsed.Parse(buffer, kBufferLength)); EXPECT_TRUE(parsed.Parse(buffer, kBufferLength));
EXPECT_EQ(kNegativeCumulativeLost, parsed.cumulative_lost_signed()); EXPECT_EQ(kNegativeCumulativeLost, parsed.cumulative_lost());
} }
} // namespace } // namespace

3
modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
View file

@ -1579,8 +1579,7 @@ TEST(RtcpReceiverTest,
EXPECT_EQ(rtcp_block.source_ssrc(), report_block.source_ssrc); EXPECT_EQ(rtcp_block.source_ssrc(), report_block.source_ssrc);
EXPECT_EQ(kSenderSsrc, report_block.sender_ssrc); EXPECT_EQ(kSenderSsrc, report_block.sender_ssrc);
EXPECT_EQ(rtcp_block.fraction_lost(), report_block.fraction_lost); EXPECT_EQ(rtcp_block.fraction_lost(), report_block.fraction_lost);
EXPECT_EQ(rtcp_block.cumulative_lost_signed(), EXPECT_EQ(rtcp_block.cumulative_lost(), report_block.packets_lost);
report_block.packets_lost);
EXPECT_EQ(rtcp_block.extended_high_seq_num(), EXPECT_EQ(rtcp_block.extended_high_seq_num(),
report_block.extended_highest_sequence_number); report_block.extended_highest_sequence_number);
EXPECT_EQ(rtcp_block.jitter(), report_block.jitter); EXPECT_EQ(rtcp_block.jitter(), report_block.jitter);

2
modules/rtp_rtcp/source/rtcp_sender_unittest.cc
View file

@ -280,7 +280,7 @@ TEST_F(RtcpSenderTest, SendRrWithOneReportBlock) {
const rtcp::ReportBlock& rb = parser()->receiver_report()->report_blocks()[0]; const rtcp::ReportBlock& rb = parser()->receiver_report()->report_blocks()[0];
EXPECT_EQ(kRemoteSsrc, rb.source_ssrc()); EXPECT_EQ(kRemoteSsrc, rb.source_ssrc());
EXPECT_EQ(0U, rb.fraction_lost()); 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()); EXPECT_EQ(kSeqNum, rb.extended_high_seq_num());
} }