From 12d64deb6c65910b3e22708d6188e2d85057325f Mon Sep 17 00:00:00 2001 From: Chen Xing Date: Thu, 13 Jun 2019 20:36:12 +0200 Subject: [PATCH] Remove sequence_number from RtpPacketInfo. This change removes sequence_number from RtpPacketInfo since it's currently not used. Bug: webrtc:10668 Change-Id: I9b45c7476457df1d18173f37c421374108678931 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/141873 Reviewed-by: Stefan Holmer Reviewed-by: Niels Moller Commit-Queue: Chen Xing Cr-Commit-Position: refs/heads/master@{#28281} --- api/rtp_packet_info.cc | 6 +- api/rtp_packet_info.h | 11 ++-- api/rtp_packet_info_unittest.cc | 37 ++---------- api/rtp_packet_infos_unittest.cc | 12 ++-- .../source/source_tracker_unittest.cc | 57 ++++++++----------- 5 files changed, 40 insertions(+), 83 deletions(-) diff --git a/api/rtp_packet_info.cc b/api/rtp_packet_info.cc index a61b1733fc..efb78381e6 100644 --- a/api/rtp_packet_info.cc +++ b/api/rtp_packet_info.cc @@ -16,17 +16,15 @@ namespace webrtc { RtpPacketInfo::RtpPacketInfo() - : ssrc_(0), sequence_number_(0), rtp_timestamp_(0), receive_time_ms_(-1) {} + : ssrc_(0), rtp_timestamp_(0), receive_time_ms_(-1) {} RtpPacketInfo::RtpPacketInfo(uint32_t ssrc, std::vector csrcs, - uint16_t sequence_number, uint32_t rtp_timestamp, absl::optional audio_level, int64_t receive_time_ms) : ssrc_(ssrc), csrcs_(std::move(csrcs)), - sequence_number_(sequence_number), rtp_timestamp_(rtp_timestamp), audio_level_(audio_level), receive_time_ms_(receive_time_ms) {} @@ -34,7 +32,6 @@ RtpPacketInfo::RtpPacketInfo(uint32_t ssrc, RtpPacketInfo::RtpPacketInfo(const RTPHeader& rtp_header, int64_t receive_time_ms) : ssrc_(rtp_header.ssrc), - sequence_number_(rtp_header.sequenceNumber), rtp_timestamp_(rtp_header.timestamp), receive_time_ms_(receive_time_ms) { const auto& extension = rtp_header.extension; @@ -49,7 +46,6 @@ RtpPacketInfo::RtpPacketInfo(const RTPHeader& rtp_header, bool operator==(const RtpPacketInfo& lhs, const RtpPacketInfo& rhs) { return (lhs.ssrc() == rhs.ssrc()) && (lhs.csrcs() == rhs.csrcs()) && - (lhs.sequence_number() == rhs.sequence_number()) && (lhs.rtp_timestamp() == rhs.rtp_timestamp()) && (lhs.audio_level() == rhs.audio_level()) && (lhs.receive_time_ms() == rhs.receive_time_ms()); diff --git a/api/rtp_packet_info.h b/api/rtp_packet_info.h index 7809067694..a9e86553d4 100644 --- a/api/rtp_packet_info.h +++ b/api/rtp_packet_info.h @@ -20,14 +20,17 @@ namespace webrtc { -// Structure to hold information about a received |RtpPacket|. +// +// Structure to hold information about a received |RtpPacket|. It is primarily +// used to carry per-packet information from when a packet is received until +// the information is passed to |SourceTracker|. +// class RtpPacketInfo { public: RtpPacketInfo(); RtpPacketInfo(uint32_t ssrc, std::vector csrcs, - uint16_t sequence_number, uint32_t rtp_timestamp, absl::optional audio_level, int64_t receive_time_ms); @@ -45,9 +48,6 @@ class RtpPacketInfo { const std::vector& csrcs() const { return csrcs_; } void set_csrcs(std::vector value) { csrcs_ = std::move(value); } - uint16_t sequence_number() const { return sequence_number_; } - void set_sequence_number(uint16_t value) { sequence_number_ = value; } - uint32_t rtp_timestamp() const { return rtp_timestamp_; } void set_rtp_timestamp(uint32_t value) { rtp_timestamp_ = value; } @@ -62,7 +62,6 @@ class RtpPacketInfo { // https://tools.ietf.org/html/rfc3550#section-5.1 uint32_t ssrc_; std::vector csrcs_; - uint16_t sequence_number_; uint32_t rtp_timestamp_; // Fields from the Audio Level header extension: diff --git a/api/rtp_packet_info_unittest.cc b/api/rtp_packet_info_unittest.cc index b9ad1ce359..ee8766a306 100644 --- a/api/rtp_packet_info_unittest.cc +++ b/api/rtp_packet_info_unittest.cc @@ -38,7 +38,7 @@ TEST(RtpPacketInfoTest, Ssrc) { rhs = RtpPacketInfo(); EXPECT_NE(rhs.ssrc(), value); - rhs = RtpPacketInfo(value, {}, {}, {}, {}, {}); + rhs = RtpPacketInfo(value, {}, {}, {}, {}); EXPECT_EQ(rhs.ssrc(), value); } @@ -65,37 +65,10 @@ TEST(RtpPacketInfoTest, Csrcs) { rhs = RtpPacketInfo(); EXPECT_NE(rhs.csrcs(), value); - rhs = RtpPacketInfo({}, value, {}, {}, {}, {}); + rhs = RtpPacketInfo({}, value, {}, {}, {}); EXPECT_EQ(rhs.csrcs(), value); } -TEST(RtpPacketInfoTest, SequenceNumber) { - const uint16_t value = 20238; - - RtpPacketInfo lhs; - RtpPacketInfo rhs; - - EXPECT_TRUE(lhs == rhs); - EXPECT_FALSE(lhs != rhs); - - rhs.set_sequence_number(value); - EXPECT_EQ(rhs.sequence_number(), value); - - EXPECT_FALSE(lhs == rhs); - EXPECT_TRUE(lhs != rhs); - - lhs = rhs; - - EXPECT_TRUE(lhs == rhs); - EXPECT_FALSE(lhs != rhs); - - rhs = RtpPacketInfo(); - EXPECT_NE(rhs.sequence_number(), value); - - rhs = RtpPacketInfo({}, {}, value, {}, {}, {}); - EXPECT_EQ(rhs.sequence_number(), value); -} - TEST(RtpPacketInfoTest, RtpTimestamp) { const uint32_t value = 4038189233; @@ -119,7 +92,7 @@ TEST(RtpPacketInfoTest, RtpTimestamp) { rhs = RtpPacketInfo(); EXPECT_NE(rhs.rtp_timestamp(), value); - rhs = RtpPacketInfo({}, {}, {}, value, {}, {}); + rhs = RtpPacketInfo({}, {}, value, {}, {}); EXPECT_EQ(rhs.rtp_timestamp(), value); } @@ -146,7 +119,7 @@ TEST(RtpPacketInfoTest, AudioLevel) { rhs = RtpPacketInfo(); EXPECT_NE(rhs.audio_level(), value); - rhs = RtpPacketInfo({}, {}, {}, {}, value, {}); + rhs = RtpPacketInfo({}, {}, {}, value, {}); EXPECT_EQ(rhs.audio_level(), value); } @@ -173,7 +146,7 @@ TEST(RtpPacketInfoTest, ReceiveTimeMs) { rhs = RtpPacketInfo(); EXPECT_NE(rhs.receive_time_ms(), value); - rhs = RtpPacketInfo({}, {}, {}, {}, {}, value); + rhs = RtpPacketInfo({}, {}, {}, {}, value); EXPECT_EQ(rhs.receive_time_ms(), value); } diff --git a/api/rtp_packet_infos_unittest.cc b/api/rtp_packet_infos_unittest.cc index 0e1b89a86d..a14d4485f4 100644 --- a/api/rtp_packet_infos_unittest.cc +++ b/api/rtp_packet_infos_unittest.cc @@ -27,9 +27,9 @@ RtpPacketInfos::vector_type ToVector(Iterator begin, Iterator end) { } // namespace TEST(RtpPacketInfosTest, BasicFunctionality) { - RtpPacketInfo p0(123, {1, 2}, 42, 89, 5, 7); - RtpPacketInfo p1(456, {3, 4}, 37, 89, 4, 1); - RtpPacketInfo p2(789, {5, 6}, 91, 88, 1, 7); + RtpPacketInfo p0(123, {1, 2}, 89, 5, 7); + RtpPacketInfo p1(456, {3, 4}, 89, 4, 1); + RtpPacketInfo p2(789, {5, 6}, 88, 1, 7); RtpPacketInfos x({p0, p1, p2}); @@ -52,9 +52,9 @@ TEST(RtpPacketInfosTest, BasicFunctionality) { } TEST(RtpPacketInfosTest, CopyShareData) { - RtpPacketInfo p0(123, {1, 2}, 42, 89, 5, 7); - RtpPacketInfo p1(456, {3, 4}, 37, 89, 4, 1); - RtpPacketInfo p2(789, {5, 6}, 91, 88, 1, 7); + RtpPacketInfo p0(123, {1, 2}, 89, 5, 7); + RtpPacketInfo p1(456, {3, 4}, 89, 4, 1); + RtpPacketInfo p2(789, {5, 6}, 88, 1, 7); RtpPacketInfos lhs({p0, p1, p2}); RtpPacketInfos rhs = lhs; diff --git a/modules/rtp_rtcp/source/source_tracker_unittest.cc b/modules/rtp_rtcp/source/source_tracker_unittest.cc index d48785411b..23426978ad 100644 --- a/modules/rtp_rtcp/source/source_tracker_unittest.cc +++ b/modules/rtp_rtcp/source/source_tracker_unittest.cc @@ -108,7 +108,6 @@ class SourceTrackerRandomTest RtpPacketInfos::vector_type packet_infos; for (size_t i = 0; i < count; ++i) { packet_infos.emplace_back(GenerateSsrc(), GenerateCsrcs(), - GenerateSequenceNumber(), GenerateRtpTimestamp(), GenerateAudioLevel(), GenerateReceiveTimeMs()); } @@ -157,10 +156,6 @@ class SourceTrackerRandomTest return csrcs; } - uint16_t GenerateSequenceNumber() { - return std::uniform_int_distribution()(generator_); - } - uint32_t GenerateRtpTimestamp() { return std::uniform_int_distribution()(generator_); } @@ -224,8 +219,8 @@ TEST(SourceTrackerTest, StartEmpty) { TEST(SourceTrackerTest, OnFrameDeliveredRecordsSources) { constexpr uint32_t kSsrc = 10; - constexpr uint32_t kCsrcs[] = {20, 21}; - constexpr uint16_t kSequenceNumber = 30; + constexpr uint32_t kCsrcs0 = 20; + constexpr uint32_t kCsrcs1 = 21; constexpr uint32_t kRtpTimestamp = 40; constexpr absl::optional kAudioLevel = 50; constexpr int64_t kReceiveTimeMs = 60; @@ -233,20 +228,18 @@ TEST(SourceTrackerTest, OnFrameDeliveredRecordsSources) { SimulatedClock clock(1000000000000ULL); SourceTracker tracker(&clock); - tracker.OnFrameDelivered(RtpPacketInfos( - {RtpPacketInfo(kSsrc, {kCsrcs[0], kCsrcs[1]}, kSequenceNumber, - kRtpTimestamp, kAudioLevel, kReceiveTimeMs)})); + tracker.OnFrameDelivered(RtpPacketInfos({RtpPacketInfo( + kSsrc, {kCsrcs0, kCsrcs1}, kRtpTimestamp, kAudioLevel, kReceiveTimeMs)})); int64_t timestamp_ms = clock.TimeInMilliseconds(); - EXPECT_THAT( - tracker.GetSources(), - ElementsAre(RtpSource(timestamp_ms, kSsrc, RtpSourceType::SSRC, - kAudioLevel, kRtpTimestamp), - RtpSource(timestamp_ms, kCsrcs[1], RtpSourceType::CSRC, - kAudioLevel, kRtpTimestamp), - RtpSource(timestamp_ms, kCsrcs[0], RtpSourceType::CSRC, - kAudioLevel, kRtpTimestamp))); + EXPECT_THAT(tracker.GetSources(), + ElementsAre(RtpSource(timestamp_ms, kSsrc, RtpSourceType::SSRC, + kAudioLevel, kRtpTimestamp), + RtpSource(timestamp_ms, kCsrcs1, RtpSourceType::CSRC, + kAudioLevel, kRtpTimestamp), + RtpSource(timestamp_ms, kCsrcs0, RtpSourceType::CSRC, + kAudioLevel, kRtpTimestamp))); } TEST(SourceTrackerTest, OnFrameDeliveredUpdatesSources) { @@ -254,8 +247,6 @@ TEST(SourceTrackerTest, OnFrameDeliveredUpdatesSources) { constexpr uint32_t kCsrcs0 = 20; constexpr uint32_t kCsrcs1 = 21; constexpr uint32_t kCsrcs2 = 22; - constexpr uint16_t kSequenceNumber0 = 30; - constexpr uint16_t kSequenceNumber1 = 31; constexpr uint32_t kRtpTimestamp0 = 40; constexpr uint32_t kRtpTimestamp1 = 41; constexpr absl::optional kAudioLevel0 = 50; @@ -266,17 +257,17 @@ TEST(SourceTrackerTest, OnFrameDeliveredUpdatesSources) { SimulatedClock clock(1000000000000ULL); SourceTracker tracker(&clock); - tracker.OnFrameDelivered(RtpPacketInfos( - {RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs1}, kSequenceNumber0, - kRtpTimestamp0, kAudioLevel0, kReceiveTimeMs0)})); + tracker.OnFrameDelivered( + RtpPacketInfos({RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs1}, kRtpTimestamp0, + kAudioLevel0, kReceiveTimeMs0)})); int64_t timestamp_ms_0 = clock.TimeInMilliseconds(); clock.AdvanceTimeMilliseconds(17); - tracker.OnFrameDelivered(RtpPacketInfos( - {RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs2}, kSequenceNumber1, - kRtpTimestamp1, kAudioLevel1, kReceiveTimeMs1)})); + tracker.OnFrameDelivered( + RtpPacketInfos({RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs2}, kRtpTimestamp1, + kAudioLevel1, kReceiveTimeMs1)})); int64_t timestamp_ms_1 = clock.TimeInMilliseconds(); @@ -297,8 +288,6 @@ TEST(SourceTrackerTest, TimedOutSourcesAreRemoved) { constexpr uint32_t kCsrcs0 = 20; constexpr uint32_t kCsrcs1 = 21; constexpr uint32_t kCsrcs2 = 22; - constexpr uint16_t kSequenceNumber0 = 30; - constexpr uint16_t kSequenceNumber1 = 31; constexpr uint32_t kRtpTimestamp0 = 40; constexpr uint32_t kRtpTimestamp1 = 41; constexpr absl::optional kAudioLevel0 = 50; @@ -309,15 +298,15 @@ TEST(SourceTrackerTest, TimedOutSourcesAreRemoved) { SimulatedClock clock(1000000000000ULL); SourceTracker tracker(&clock); - tracker.OnFrameDelivered(RtpPacketInfos( - {RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs1}, kSequenceNumber0, - kRtpTimestamp0, kAudioLevel0, kReceiveTimeMs0)})); + tracker.OnFrameDelivered( + RtpPacketInfos({RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs1}, kRtpTimestamp0, + kAudioLevel0, kReceiveTimeMs0)})); clock.AdvanceTimeMilliseconds(17); - tracker.OnFrameDelivered(RtpPacketInfos( - {RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs2}, kSequenceNumber1, - kRtpTimestamp1, kAudioLevel1, kReceiveTimeMs1)})); + tracker.OnFrameDelivered( + RtpPacketInfos({RtpPacketInfo(kSsrc, {kCsrcs0, kCsrcs2}, kRtpTimestamp1, + kAudioLevel1, kReceiveTimeMs1)})); int64_t timestamp_ms_1 = clock.TimeInMilliseconds();