From b2db53998dc4226d41e6c101bf24ff38a8188a32 Mon Sep 17 00:00:00 2001 From: Qingsi Wang Date: Tue, 7 Aug 2018 17:06:45 -0700 Subject: [PATCH] Parse the number of packets lost in RTCP SR as a signed integer. The cumulative number of packets lost in a RTCP sender report can be negative if there are duplicates. This CL fixes a bug that the parser of RTCP reports treats the field as an unsigned integer, and incorrectly reports large packet losses when a negative loss is reported. Bug: webrtc:9601 Change-Id: I1109ac0741614d61bda743e13a390b7d3e147a9c Reviewed-on: https://webrtc-review.googlesource.com/92942 Reviewed-by: Danil Chapovalov Reviewed-by: Sebastian Jansson Commit-Queue: Qingsi Wang Cr-Commit-Position: refs/heads/master@{#24234} --- .../rtp_rtcp/source/rtcp_packet/report_block.cc | 2 +- .../source/rtcp_packet/report_block_unittest.cc | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/modules/rtp_rtcp/source/rtcp_packet/report_block.cc b/modules/rtp_rtcp/source/rtcp_packet/report_block.cc index 4f98963766..d4579fc8d6 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/report_block.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/report_block.cc @@ -54,7 +54,7 @@ bool ReportBlock::Parse(const uint8_t* buffer, size_t length) { source_ssrc_ = ByteReader::ReadBigEndian(&buffer[0]); fraction_lost_ = buffer[4]; - cumulative_lost_ = ByteReader::ReadBigEndian(&buffer[5]); + cumulative_lost_ = ByteReader::ReadBigEndian(&buffer[5]); extended_high_seq_num_ = ByteReader::ReadBigEndian(&buffer[8]); jitter_ = ByteReader::ReadBigEndian(&buffer[12]); last_sr_ = ByteReader::ReadBigEndian(&buffer[16]); 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 a074a2f4f4..5cc102fed0 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc @@ -91,5 +91,20 @@ TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) { EXPECT_EQ(0u, rb.cumulative_lost()); } +TEST(RtcpPacketReportBlockTest, ParseNegativeCumulativeLost) { + // CumulativeLost is a signed 24-bit integer. + const int32_t kNegativeCumulativeLost = -123; + ReportBlock rb; + EXPECT_TRUE(rb.SetCumulativeLost(kNegativeCumulativeLost)); + + uint8_t buffer[kBufferLength]; + rb.Create(buffer); + + ReportBlock parsed; + EXPECT_TRUE(parsed.Parse(buffer, kBufferLength)); + + EXPECT_EQ(kNegativeCumulativeLost, parsed.cumulative_lost_signed()); +} + } // namespace } // namespace webrtc