From 9701e0ce2fccb0eab5d6426e6ec9f52c1109f0bc Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 9 Aug 2018 11:21:11 +0200 Subject: Makes treatment of received reports of packets lost signed. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9598 Change-Id: I0f6ffe348585b8ec69753089652812da516d33d8 Reviewed-on: https://webrtc-review.googlesource.com/93021 Reviewed-by: Björn Terelius Reviewed-by: Magnus Flodman Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#24291} --- audio/audio_send_stream_unittest.cc | 3 +-- audio/channel.h | 2 +- modules/rtp_rtcp/include/rtp_rtcp_defines.h | 2 +- modules/rtp_rtcp/source/rtcp_receiver_unittest.cc | 2 +- modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc | 2 +- video/report_block_stats.cc | 3 +-- video/report_block_stats_unittest.cc | 10 +++++----- 7 files changed, 11 insertions(+), 13 deletions(-) diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 1e777f1480..f5d6796af5 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -379,8 +379,7 @@ TEST(AudioSendStreamTest, GetStats) { EXPECT_EQ(kSsrc, stats.local_ssrc); EXPECT_EQ(static_cast(kCallStats.bytesSent), stats.bytes_sent); EXPECT_EQ(kCallStats.packetsSent, stats.packets_sent); - EXPECT_EQ(static_cast(kReportBlock.cumulative_num_packets_lost), - stats.packets_lost); + EXPECT_EQ(kReportBlock.cumulative_num_packets_lost, stats.packets_lost); EXPECT_EQ(Q8ToFloat(kReportBlock.fraction_lost), stats.fraction_lost); EXPECT_EQ(std::string(kIsacCodec.plname), stats.codec_name); EXPECT_EQ(static_cast(kReportBlock.extended_highest_sequence_number), diff --git a/audio/channel.h b/audio/channel.h index 8d78665838..670223ca6e 100644 --- a/audio/channel.h +++ b/audio/channel.h @@ -80,7 +80,7 @@ struct ReportBlock { uint32_t sender_SSRC; // SSRC of sender uint32_t source_SSRC; uint8_t fraction_lost; - uint32_t cumulative_num_packets_lost; + int32_t cumulative_num_packets_lost; uint32_t extended_highest_sequence_number; uint32_t interarrival_jitter; uint32_t last_SR_timestamp; diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 260e46170b..34c7428790 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -193,7 +193,7 @@ struct RTCPReportBlock { uint32_t sender_ssrc; // SSRC of sender of this report. uint32_t source_ssrc; // SSRC of the RTP packet sender. uint8_t fraction_lost; - uint32_t packets_lost; // 24 bits valid. + int32_t packets_lost; // 24 bits valid. uint32_t extended_highest_sequence_number; uint32_t jitter; uint32_t last_sender_report_timestamp; diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 91ffd73b10..6cd953c28f 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -441,7 +441,7 @@ TEST_F(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { TEST_F(RtcpReceiverTest, InjectRrPacketsFromTwoRemoteSsrcs) { const uint32_t kSenderSsrc2 = 0x20304; const uint16_t kSequenceNumbers[] = {10, 12423}; - const uint32_t kCumLost[] = {13, 555}; + const int32_t kCumLost[] = {13, 555}; const uint8_t kFracLost[] = {20, 11}; rtcp::ReportBlock rb1; diff --git a/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc b/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc index b6368f7fbb..2edebae79f 100644 --- a/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc +++ b/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc @@ -213,7 +213,7 @@ TEST_F(RtpRtcpRtcpTest, RemoteRTCPStatRemote) { EXPECT_EQ(test_ssrc + 1, report_blocks[0].sender_ssrc); EXPECT_EQ(test_ssrc, report_blocks[0].source_ssrc); - EXPECT_EQ(0u, report_blocks[0].packets_lost); + EXPECT_EQ(0, report_blocks[0].packets_lost); EXPECT_LT(0u, report_blocks[0].delay_since_last_sender_report); EXPECT_EQ(test_sequence_number, report_blocks[0].extended_highest_sequence_number); diff --git a/video/report_block_stats.cc b/video/report_block_stats.cc index 332709e37e..6005043dfa 100644 --- a/video/report_block_stats.cc +++ b/video/report_block_stats.cc @@ -35,8 +35,7 @@ void ReportBlockStats::Store(const RtcpStatistics& rtcp_stats, uint32_t remote_ssrc, uint32_t source_ssrc) { RTCPReportBlock block; - // TODO(srte): Remove this clamp when packets_lost is made signed. - block.packets_lost = std::max(0, rtcp_stats.packets_lost); + block.packets_lost = rtcp_stats.packets_lost; block.fraction_lost = rtcp_stats.fraction_lost; block.extended_highest_sequence_number = rtcp_stats.extended_highest_sequence_number; diff --git a/video/report_block_stats_unittest.cc b/video/report_block_stats_unittest.cc index 3880c4b11a..f80ae16160 100644 --- a/video/report_block_stats_unittest.cc +++ b/video/report_block_stats_unittest.cc @@ -82,7 +82,7 @@ TEST_F(ReportBlockStatsTest, AggregateAndStore_NoSsrc) { std::vector empty; RTCPReportBlock aggregated = stats.AggregateAndStore(empty); EXPECT_EQ(0U, aggregated.fraction_lost); - EXPECT_EQ(0U, aggregated.packets_lost); + EXPECT_EQ(0, aggregated.packets_lost); EXPECT_EQ(0U, aggregated.jitter); EXPECT_EQ(0U, aggregated.extended_highest_sequence_number); } @@ -92,13 +92,13 @@ TEST_F(ReportBlockStatsTest, AggregateAndStore_OneSsrc) { RTCPReportBlock aggregated = stats.AggregateAndStore(ssrc1block1_); // One ssrc, no aggregation done. EXPECT_EQ(123U, aggregated.fraction_lost); - EXPECT_EQ(10U, aggregated.packets_lost); + EXPECT_EQ(10, aggregated.packets_lost); EXPECT_EQ(777U, aggregated.jitter); EXPECT_EQ(24000U, aggregated.extended_highest_sequence_number); aggregated = stats.AggregateAndStore(ssrc1block2_); EXPECT_EQ(0U, aggregated.fraction_lost); - EXPECT_EQ(15U, aggregated.packets_lost); + EXPECT_EQ(15, aggregated.packets_lost); EXPECT_EQ(222U, aggregated.jitter); EXPECT_EQ(24100U, aggregated.extended_highest_sequence_number); @@ -110,14 +110,14 @@ TEST_F(ReportBlockStatsTest, AggregateAndStore_TwoSsrcs) { ReportBlockStats stats; RTCPReportBlock aggregated = stats.AggregateAndStore(ssrc12block1_); EXPECT_EQ(0U, aggregated.fraction_lost); - EXPECT_EQ(10U + 111U, aggregated.packets_lost); + EXPECT_EQ(10 + 111, aggregated.packets_lost); EXPECT_EQ((777U + 555U) / 2, aggregated.jitter); EXPECT_EQ(0U, aggregated.extended_highest_sequence_number); aggregated = stats.AggregateAndStore(ssrc12block2_); // fl: 255 * ((15-10) + (136-111)) / ((24100-24000) + (8800-8500)) = 19 EXPECT_EQ(19U, aggregated.fraction_lost); - EXPECT_EQ(15U + 136U, aggregated.packets_lost); + EXPECT_EQ(15 + 136, aggregated.packets_lost); EXPECT_EQ((222U + 888U) / 2, aggregated.jitter); EXPECT_EQ(0U, aggregated.extended_highest_sequence_number); -- cgit v1.2.3