diff --git a/audio/channel.cc b/audio/channel.cc index 9775243187..3c6376318e 100644 --- a/audio/channel.cc +++ b/audio/channel.cc @@ -866,20 +866,22 @@ void Channel::OnRtpPacket(const RtpPacketReceived& packet) { contributing_sources_.Update(now_ms, csrcs); } - RTPHeader header; - packet.GetHeader(&header); - // Store playout timestamp for the received RTP packet UpdatePlayoutTimestamp(false); - const auto& it = payload_type_frequencies_.find(header.payloadType); + const auto& it = payload_type_frequencies_.find(packet.PayloadType()); if (it == payload_type_frequencies_.end()) return; - header.payload_type_frequency = it->second; + // TODO(nisse): Set payload_type_frequency earlier, when packet is parsed. + RtpPacketReceived packet_copy(packet); + packet_copy.set_payload_type_frequency(it->second); - rtp_receive_statistics_->IncomingPacket(header, packet.size()); + rtp_receive_statistics_->OnRtpPacket(packet_copy); - ReceivePacket(packet.data(), packet.size(), header); + RTPHeader header; + packet_copy.GetHeader(&header); + + ReceivePacket(packet_copy.data(), packet_copy.size(), header); } bool Channel::ReceivePacket(const uint8_t* packet, diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc index 0512ceeca6..679c5c56e0 100644 --- a/call/flexfec_receive_stream_impl.cc +++ b/call/flexfec_receive_stream_impl.cc @@ -177,9 +177,7 @@ void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) { // Do not report media packets in the RTCP RRs generated by |rtp_rtcp_|. if (packet.Ssrc() == config_.remote_ssrc) { - RTPHeader header; - packet.GetHeader(&header); - rtp_receive_statistics_->IncomingPacket(header, packet.size()); + rtp_receive_statistics_->OnRtpPacket(packet); } } diff --git a/call/rtx_receive_stream.cc b/call/rtx_receive_stream.cc index 3e2934fda8..e7065abb00 100644 --- a/call/rtx_receive_stream.cc +++ b/call/rtx_receive_stream.cc @@ -36,9 +36,7 @@ RtxReceiveStream::~RtxReceiveStream() = default; void RtxReceiveStream::OnRtpPacket(const RtpPacketReceived& rtx_packet) { if (rtp_receive_statistics_) { - RTPHeader header; - rtx_packet.GetHeader(&header); - rtp_receive_statistics_->IncomingPacket(header, rtx_packet.size()); + rtp_receive_statistics_->OnRtpPacket(rtx_packet); } rtc::ArrayView payload = rtx_packet.payload(); diff --git a/modules/remote_bitrate_estimator/test/bwe_test_framework.cc b/modules/remote_bitrate_estimator/test/bwe_test_framework.cc index 1e513ee315..0cd8129ad6 100644 --- a/modules/remote_bitrate_estimator/test/bwe_test_framework.cc +++ b/modules/remote_bitrate_estimator/test/bwe_test_framework.cc @@ -14,6 +14,7 @@ #include +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "rtc_base/constructormagic.h" #include "rtc_base/numerics/safe_minmax.h" #include "rtc_base/system/unused.h" @@ -137,6 +138,15 @@ void MediaPacket::SetAbsSendTimeMs(int64_t abs_send_time_ms) { 0x00fffffful; } +// Copies payload size and sequence number. +RtpPacketReceived MediaPacket::GetRtpPacket() const { + RtpPacketReceived packet; + + packet.SetPayloadSize(payload_size()); + packet.SetSequenceNumber(header_.sequenceNumber); + return packet; +} + BbrBweFeedback::BbrBweFeedback( int flow_id, int64_t send_time_us, diff --git a/modules/remote_bitrate_estimator/test/estimators/nada.cc b/modules/remote_bitrate_estimator/test/estimators/nada.cc index 30d36f8d3d..c189d47343 100644 --- a/modules/remote_bitrate_estimator/test/estimators/nada.cc +++ b/modules/remote_bitrate_estimator/test/estimators/nada.cc @@ -57,8 +57,7 @@ void NadaBweReceiver::ReceivePacket(int64_t arrival_time_ms, const int64_t kDelayMaxThresholdMs = 400; // Referred as d_max. clock_.AdvanceTimeMilliseconds(arrival_time_ms - clock_.TimeInMilliseconds()); - recv_stats_->IncomingPacket(media_packet.header(), - media_packet.payload_size()); + recv_stats_->OnRtpPacket(media_packet.GetRtpPacket()); // Refered as x_n. int64_t delay_ms = arrival_time_ms - media_packet.sender_timestamp_ms(); diff --git a/modules/remote_bitrate_estimator/test/estimators/remb.cc b/modules/remote_bitrate_estimator/test/estimators/remb.cc index 324422024d..14e884eb38 100644 --- a/modules/remote_bitrate_estimator/test/estimators/remb.cc +++ b/modules/remote_bitrate_estimator/test/estimators/remb.cc @@ -84,8 +84,7 @@ RembReceiver::~RembReceiver() {} void RembReceiver::ReceivePacket(int64_t arrival_time_ms, const MediaPacket& media_packet) { - recv_stats_->IncomingPacket(media_packet.header(), - media_packet.payload_size()); + recv_stats_->OnRtpPacket(media_packet.GetRtpPacket()); latest_estimate_bps_ = -1; diff --git a/modules/remote_bitrate_estimator/test/packet.h b/modules/remote_bitrate_estimator/test/packet.h index 5e45eb52d6..6f44988b14 100644 --- a/modules/remote_bitrate_estimator/test/packet.h +++ b/modules/remote_bitrate_estimator/test/packet.h @@ -19,6 +19,7 @@ #include "common_types.h" // NOLINT(build/include) #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" namespace webrtc { namespace testing { @@ -77,6 +78,7 @@ class MediaPacket : public Packet { const RTPHeader& header() const { return header_; } Packet::Type GetPacketType() const override; uint16_t sequence_number() const { return header_.sequenceNumber; } + RtpPacketReceived GetRtpPacket() const; private: static const int kAbsSendTimeFraction = 18; diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 378b5526d1..073ea05111 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -206,6 +206,7 @@ rtc_static_library("rtp_rtcp") { "../../api/video:video_bitrate_allocator", "../../api/video:video_frame", "../../api/video_codecs:video_codecs_api", + "../../call:rtp_interfaces", "../../common_video", "../../logging:rtc_event_audio", "../../logging:rtc_event_log_api", diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h index fe72ada419..24d6f81237 100644 --- a/modules/rtp_rtcp/include/receive_statistics.h +++ b/modules/rtp_rtcp/include/receive_statistics.h @@ -14,6 +14,7 @@ #include #include +#include "call/rtp_packet_sink_interface.h" #include "modules/include/module.h" #include "modules/include/module_common_types.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -48,28 +49,22 @@ class StreamStatistician { virtual uint32_t BitrateReceived() const = 0; }; -class ReceiveStatistics : public ReceiveStatisticsProvider { +class ReceiveStatistics : public ReceiveStatisticsProvider, + public RtpPacketSinkInterface { public: ~ReceiveStatistics() override = default; static ReceiveStatistics* Create(Clock* clock); // Updates the receive statistics with this packet. + // TODO(bugs.webrtc.org/8016): Deprecated. Delete as soon as + // downstream code is updated to use OnRtpPacket. + RTC_DEPRECATED virtual void IncomingPacket(const RTPHeader& rtp_header, size_t packet_length) = 0; - // TODO(nisse): Wrapper for backwards compatibility. Delete as soon as - // downstream callers are updated. - RTC_DEPRECATED - void IncomingPacket(const RTPHeader& rtp_header, - size_t packet_length, - bool retransmitted) { - IncomingPacket(rtp_header, packet_length); - } - // Increment counter for number of FEC packets received. - virtual void FecPacketReceived(const RTPHeader& header, - size_t packet_length) = 0; + virtual void FecPacketReceived(const RtpPacketReceived& packet) = 0; // Returns a pointer to the statistician of an ssrc. virtual StreamStatistician* GetStatistician(uint32_t ssrc) const = 0; diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index 80b8c9b0a2..abe028d670 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -16,6 +16,7 @@ #include #include "modules/remote_bitrate_estimator/test/bwe_test_logging.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "modules/rtp_rtcp/source/time_util.h" #include "rtc_base/logging.h" @@ -372,6 +373,12 @@ ReceiveStatisticsImpl::~ReceiveStatisticsImpl() { } } +void ReceiveStatisticsImpl::OnRtpPacket(const RtpPacketReceived& packet) { + RTPHeader header; + packet.GetHeader(&header); + IncomingPacket(header, packet.size()); +} + void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header, size_t packet_length) { StreamStatisticianImpl* impl; @@ -394,18 +401,19 @@ void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header, impl->IncomingPacket(header, packet_length); } -void ReceiveStatisticsImpl::FecPacketReceived(const RTPHeader& header, - size_t packet_length) { +void ReceiveStatisticsImpl::FecPacketReceived(const RtpPacketReceived& packet) { StreamStatisticianImpl* impl; { rtc::CritScope cs(&receive_statistics_lock_); - auto it = statisticians_.find(header.ssrc); + auto it = statisticians_.find(packet.Ssrc()); // Ignore FEC if it is the first packet. if (it == statisticians_.end()) return; impl = it->second; } - impl->FecPacketReceived(header, packet_length); + RTPHeader header; + packet.GetHeader(&header); + impl->FecPacketReceived(header, packet.size()); } StreamStatistician* ReceiveStatisticsImpl::GetStatistician( diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index f94256ade6..56bfd2b4a4 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -105,10 +105,12 @@ class ReceiveStatisticsImpl : public ReceiveStatistics, // Implement ReceiveStatisticsProvider. std::vector RtcpReportBlocks(size_t max_blocks) override; + // Implement RtpPacketSinkInterface + void OnRtpPacket(const RtpPacketReceived& packet) override; + // Implement ReceiveStatistics. void IncomingPacket(const RTPHeader& header, size_t packet_length) override; - void FecPacketReceived(const RTPHeader& header, - size_t packet_length) override; + void FecPacketReceived(const RtpPacketReceived& packet) override; StreamStatistician* GetStatistician(uint32_t ssrc) const override; void SetMaxReorderingThreshold(int max_reordering_threshold) override; void EnableRetransmitDetection(uint32_t ssrc, bool enable) override; diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc index b721b034f5..18448cb77e 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -12,6 +12,8 @@ #include #include "modules/rtp_rtcp/include/receive_statistics.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" +#include "rtc_base/random.h" #include "system_wrappers/include/clock.h" #include "test/gmock.h" #include "test/gtest.h" @@ -29,40 +31,71 @@ const uint32_t kSsrc2 = 202; const uint32_t kSsrc3 = 203; const uint32_t kSsrc4 = 304; -RTPHeader CreateRtpHeader(uint32_t ssrc) { - RTPHeader header; - memset(&header, 0, sizeof(header)); - header.ssrc = ssrc; - header.sequenceNumber = 100; - header.payload_type_frequency = 90000; - return header; +RtpPacketReceived CreateRtpPacket(uint32_t ssrc, + size_t header_size, + size_t payload_size, + size_t padding_size) { + RtpPacketReceived packet; + packet.SetSsrc(ssrc); + packet.SetSequenceNumber(100); + packet.set_payload_type_frequency(90000); + RTC_CHECK_GE(header_size, 12); + RTC_CHECK_EQ(header_size % 4, 0); + if (header_size > 12) { + // Insert csrcs to increase header size. + const int num_csrcs = (header_size - 12) / 4; + std::vector csrcs(num_csrcs); + packet.SetCsrcs(csrcs); + } + packet.SetPayloadSize(payload_size); + if (padding_size > 0) { + Random random(17); + packet.SetPadding(padding_size, &random); + } + return packet; +} + +RtpPacketReceived CreateRtpPacket(uint32_t ssrc, size_t packet_size) { + return CreateRtpPacket(ssrc, 12, packet_size - 12, 0); +} + +void IncrementSequenceNumber(RtpPacketReceived* packet, uint16_t incr) { + packet->SetSequenceNumber(packet->SequenceNumber() + incr); +} + +void IncrementSequenceNumber(RtpPacketReceived* packet) { + IncrementSequenceNumber(packet, 1); +} + +void IncrementTimestamp(RtpPacketReceived* packet, uint32_t incr) { + packet->SetTimestamp(packet->Timestamp() + incr); } class ReceiveStatisticsTest : public ::testing::Test { public: ReceiveStatisticsTest() : clock_(0), receive_statistics_(ReceiveStatistics::Create(&clock_)) { - header1_ = CreateRtpHeader(kSsrc1); - header2_ = CreateRtpHeader(kSsrc2); + packet1_ = CreateRtpPacket(kSsrc1, kPacketSize1); + packet2_ = CreateRtpPacket(kSsrc2, kPacketSize2); } protected: SimulatedClock clock_; std::unique_ptr receive_statistics_; - RTPHeader header1_; - RTPHeader header2_; + RtpPacketReceived packet1_; + RtpPacketReceived packet2_; }; TEST_F(ReceiveStatisticsTest, TwoIncomingSsrcs) { - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; - receive_statistics_->IncomingPacket(header2_, kPacketSize2); - ++header2_.sequenceNumber; + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); + receive_statistics_->OnRtpPacket(packet2_); + IncrementSequenceNumber(&packet2_); clock_.AdvanceTimeMilliseconds(100); - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; - receive_statistics_->IncomingPacket(header2_, kPacketSize2); - ++header2_.sequenceNumber; + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); + receive_statistics_->OnRtpPacket(packet2_); + IncrementSequenceNumber(&packet2_); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); @@ -84,10 +117,10 @@ TEST_F(ReceiveStatisticsTest, TwoIncomingSsrcs) { EXPECT_EQ(2u, receive_statistics_->RtcpReportBlocks(3).size()); // Add more incoming packets and verify that they are registered in both // access methods. - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; - receive_statistics_->IncomingPacket(header2_, kPacketSize2); - ++header2_.sequenceNumber; + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); + receive_statistics_->OnRtpPacket(packet2_); + IncrementSequenceNumber(&packet2_); receive_statistics_->GetStatistician(kSsrc1)->GetDataCounters( &bytes_received, &packets_received); @@ -101,12 +134,12 @@ TEST_F(ReceiveStatisticsTest, TwoIncomingSsrcs) { TEST_F(ReceiveStatisticsTest, RtcpReportBlocksReturnsMaxBlocksWhenThereAreMoreStatisticians) { - RTPHeader header1 = CreateRtpHeader(kSsrc1); - RTPHeader header2 = CreateRtpHeader(kSsrc2); - RTPHeader header3 = CreateRtpHeader(kSsrc3); - receive_statistics_->IncomingPacket(header1, kPacketSize1); - receive_statistics_->IncomingPacket(header2, kPacketSize1); - receive_statistics_->IncomingPacket(header3, kPacketSize1); + RtpPacketReceived packet1 = CreateRtpPacket(kSsrc1, kPacketSize1); + RtpPacketReceived packet2 = CreateRtpPacket(kSsrc2, kPacketSize1); + RtpPacketReceived packet3 = CreateRtpPacket(kSsrc3, kPacketSize1); + receive_statistics_->OnRtpPacket(packet1); + receive_statistics_->OnRtpPacket(packet2); + receive_statistics_->OnRtpPacket(packet3); EXPECT_THAT(receive_statistics_->RtcpReportBlocks(2), SizeIs(2)); EXPECT_THAT(receive_statistics_->RtcpReportBlocks(2), SizeIs(2)); @@ -115,14 +148,14 @@ TEST_F(ReceiveStatisticsTest, TEST_F(ReceiveStatisticsTest, RtcpReportBlocksReturnsAllObservedSsrcsWithMultipleCalls) { - RTPHeader header1 = CreateRtpHeader(kSsrc1); - RTPHeader header2 = CreateRtpHeader(kSsrc2); - RTPHeader header3 = CreateRtpHeader(kSsrc3); - RTPHeader header4 = CreateRtpHeader(kSsrc4); - receive_statistics_->IncomingPacket(header1, kPacketSize1); - receive_statistics_->IncomingPacket(header2, kPacketSize1); - receive_statistics_->IncomingPacket(header3, kPacketSize1); - receive_statistics_->IncomingPacket(header4, kPacketSize1); + RtpPacketReceived packet1 = CreateRtpPacket(kSsrc1, kPacketSize1); + RtpPacketReceived packet2 = CreateRtpPacket(kSsrc2, kPacketSize1); + RtpPacketReceived packet3 = CreateRtpPacket(kSsrc3, kPacketSize1); + RtpPacketReceived packet4 = CreateRtpPacket(kSsrc4, kPacketSize1); + receive_statistics_->OnRtpPacket(packet1); + receive_statistics_->OnRtpPacket(packet2); + receive_statistics_->OnRtpPacket(packet3); + receive_statistics_->OnRtpPacket(packet4); std::vector observed_ssrcs; std::vector report_blocks = @@ -141,11 +174,11 @@ TEST_F(ReceiveStatisticsTest, } TEST_F(ReceiveStatisticsTest, ActiveStatisticians) { - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); clock_.AdvanceTimeMilliseconds(1000); - receive_statistics_->IncomingPacket(header2_, kPacketSize2); - ++header2_.sequenceNumber; + receive_statistics_->OnRtpPacket(packet2_); + IncrementSequenceNumber(&packet2_); // Nothing should time out since only 1000 ms has passed since the first // packet came in. EXPECT_EQ(2u, receive_statistics_->RtcpReportBlocks(3).size()); @@ -158,8 +191,8 @@ TEST_F(ReceiveStatisticsTest, ActiveStatisticians) { // kSsrc2 should have timed out. EXPECT_EQ(0u, receive_statistics_->RtcpReportBlocks(3).size()); - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); // kSsrc1 should be active again and the data counters should have survived. EXPECT_EQ(1u, receive_statistics_->RtcpReportBlocks(3).size()); StreamStatistician* statistician = @@ -179,12 +212,12 @@ TEST_F(ReceiveStatisticsTest, EXPECT_TRUE(receive_statistics_->GetStatistician(kSsrc1) != nullptr); EXPECT_EQ(0u, receive_statistics_->RtcpReportBlocks(3).size()); // Receive first packet - receive_statistics_->IncomingPacket(header1_, kPacketSize1); + receive_statistics_->OnRtpPacket(packet1_); EXPECT_EQ(1u, receive_statistics_->RtcpReportBlocks(3).size()); } TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) { - receive_statistics_->IncomingPacket(header1_, kPacketSize1); + receive_statistics_->OnRtpPacket(packet1_); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); ASSERT_TRUE(statistician != NULL); @@ -194,7 +227,7 @@ TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) { EXPECT_GT(counters.first_packet_time_ms, -1); EXPECT_EQ(1u, counters.transmitted.packets); - receive_statistics_->IncomingPacket(header1_, kPacketSize1); + receive_statistics_->OnRtpPacket(packet1_); statistician->GetReceiveStreamDataCounters(&counters); EXPECT_GT(counters.first_packet_time_ms, -1); EXPECT_EQ(2u, counters.transmitted.packets); @@ -225,23 +258,23 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { receive_statistics_->EnableRetransmitDetection(kSsrc1, true); // Add some arbitrary data, with loss and jitter. - header1_.sequenceNumber = 1; + packet1_.SetSequenceNumber(1); clock_.AdvanceTimeMilliseconds(7); - header1_.timestamp += 3; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - header1_.sequenceNumber += 2; + IncrementTimestamp(&packet1_, 3); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_, 2); clock_.AdvanceTimeMilliseconds(9); - header1_.timestamp += 9; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - --header1_.sequenceNumber; + IncrementTimestamp(&packet1_, 9); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_, -1); clock_.AdvanceTimeMilliseconds(13); - header1_.timestamp += 47; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - header1_.sequenceNumber += 3; + IncrementTimestamp(&packet1_, 47); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_, 3); clock_.AdvanceTimeMilliseconds(11); - header1_.timestamp += 17; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; + IncrementTimestamp(&packet1_, 17); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); EXPECT_EQ(0u, callback.num_calls_); @@ -265,23 +298,23 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { receive_statistics_->RegisterRtcpStatisticsCallback(NULL); // Add some more data. - header1_.sequenceNumber = 1; + packet1_.SetSequenceNumber(1); clock_.AdvanceTimeMilliseconds(7); - header1_.timestamp += 3; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - header1_.sequenceNumber += 2; + IncrementTimestamp(&packet1_, 3); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_, 2); clock_.AdvanceTimeMilliseconds(9); - header1_.timestamp += 9; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - --header1_.sequenceNumber; + IncrementTimestamp(&packet1_, 9); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_, -1); clock_.AdvanceTimeMilliseconds(13); - header1_.timestamp += 47; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - header1_.sequenceNumber += 3; + IncrementTimestamp(&packet1_, 47); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_, 3); clock_.AdvanceTimeMilliseconds(11); - header1_.timestamp += 17; - receive_statistics_->IncomingPacket(header1_, kPacketSize1); - ++header1_.sequenceNumber; + IncrementTimestamp(&packet1_, 17); + receive_statistics_->OnRtpPacket(packet1_); + IncrementSequenceNumber(&packet1_); receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, true); @@ -334,9 +367,10 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { const size_t kHeaderLength = 20; const size_t kPaddingLength = 9; - // One packet of size kPacketSize1. - header1_.headerLength = kHeaderLength; - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); + // One packet with payload size kPacketSize1. + RtpPacketReceived packet1 = + CreateRtpPacket(kSsrc1, kHeaderLength, kPacketSize1, 0); + receive_statistics_->OnRtpPacket(packet1); StreamDataCounters expected; expected.transmitted.payload_bytes = kPacketSize1; expected.transmitted.header_bytes = kHeaderLength; @@ -349,12 +383,12 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { expected.fec.packets = 0; callback.Matches(1, kSsrc1, expected); - ++header1_.sequenceNumber; - clock_.AdvanceTimeMilliseconds(5); - header1_.paddingLength = 9; // Another packet of size kPacketSize1 with 9 bytes padding. - receive_statistics_->IncomingPacket( - header1_, kPacketSize1 + kHeaderLength + kPaddingLength); + RtpPacketReceived packet2 = + CreateRtpPacket(kSsrc1, kHeaderLength, kPacketSize1, 9); + packet2.SetSequenceNumber(packet1.SequenceNumber() + 1); + clock_.AdvanceTimeMilliseconds(5); + receive_statistics_->OnRtpPacket(packet2); expected.transmitted.payload_bytes = kPacketSize1 * 2; expected.transmitted.header_bytes = kHeaderLength * 2; expected.transmitted.padding_bytes = kPaddingLength; @@ -363,8 +397,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { clock_.AdvanceTimeMilliseconds(5); // Retransmit last packet. - receive_statistics_->IncomingPacket( - header1_, kPacketSize1 + kHeaderLength + kPaddingLength); + receive_statistics_->OnRtpPacket(packet2); expected.transmitted.payload_bytes = kPacketSize1 * 3; expected.transmitted.header_bytes = kHeaderLength * 3; expected.transmitted.padding_bytes = kPaddingLength * 2; @@ -375,13 +408,11 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { expected.retransmitted.packets = 1; callback.Matches(3, kSsrc1, expected); - header1_.paddingLength = 0; - ++header1_.sequenceNumber; - clock_.AdvanceTimeMilliseconds(5); // One FEC packet. - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); - receive_statistics_->FecPacketReceived(header1_, - kPacketSize1 + kHeaderLength); + packet1.SetSequenceNumber(packet2.SequenceNumber() + 1); + clock_.AdvanceTimeMilliseconds(5); + receive_statistics_->OnRtpPacket(packet1); + receive_statistics_->FecPacketReceived(packet1); expected.transmitted.payload_bytes = kPacketSize1 * 4; expected.transmitted.header_bytes = kHeaderLength * 4; expected.transmitted.packets = 4; @@ -393,9 +424,9 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { receive_statistics_->RegisterRtpStatisticsCallback(NULL); // New stats, but callback should not be called. - ++header1_.sequenceNumber; + IncrementSequenceNumber(&packet1); clock_.AdvanceTimeMilliseconds(5); - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); + receive_statistics_->OnRtpPacket(packet1); callback.Matches(5, kSsrc1, expected); } @@ -404,14 +435,13 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacksFecFirst) { receive_statistics_->RegisterRtpStatisticsCallback(&callback); const uint32_t kHeaderLength = 20; - header1_.headerLength = kHeaderLength; - + RtpPacketReceived packet = + CreateRtpPacket(kSsrc1, kHeaderLength, kPacketSize1, 0); // If first packet is FEC, ignore it. - receive_statistics_->FecPacketReceived(header1_, - kPacketSize1 + kHeaderLength); + receive_statistics_->FecPacketReceived(packet); EXPECT_EQ(0u, callback.num_calls_); - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); + receive_statistics_->OnRtpPacket(packet); StreamDataCounters expected; expected.transmitted.payload_bytes = kPacketSize1; expected.transmitted.header_bytes = kHeaderLength; @@ -420,8 +450,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacksFecFirst) { expected.fec.packets = 0; callback.Matches(1, kSsrc1, expected); - receive_statistics_->FecPacketReceived(header1_, - kPacketSize1 + kHeaderLength); + receive_statistics_->FecPacketReceived(packet); expected.fec.payload_bytes = kPacketSize1; expected.fec.header_bytes = kHeaderLength; expected.fec.packets = 1; diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index bccf96df5e..a08ce0e83e 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -14,6 +14,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "modules/rtp_rtcp/source/rtcp_sender.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_impl.h" #include "rtc_base/rate_limiter.h" #include "test/gmock.h" @@ -93,13 +94,12 @@ class RtcpSenderTest : public ::testing::Test { } void InsertIncomingPacket(uint32_t remote_ssrc, uint16_t seq_num) { - RTPHeader header; - header.ssrc = remote_ssrc; - header.sequenceNumber = seq_num; - header.timestamp = 12345; - header.headerLength = 12; - size_t kPacketLength = 100; - receive_statistics_->IncomingPacket(header, kPacketLength); + RtpPacketReceived packet; + packet.SetSsrc(remote_ssrc); + packet.SetSequenceNumber(seq_num); + packet.SetTimestamp(12345); + packet.SetPayloadSize(100 - 12); + receive_statistics_->OnRtpPacket(packet); } test::RtcpPacketParser* parser() { return &test_transport_.parser_; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index e6e33b722a..5160a64be7 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -17,6 +17,7 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/nack.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_impl.h" #include "rtc_base/rate_limiter.h" #include "test/gmock.h" @@ -318,12 +319,12 @@ TEST_F(RtpRtcpImplTest, SetSelectiveRetransmissions_HigherLayers) { } TEST_F(RtpRtcpImplTest, Rtt) { - RTPHeader header; - header.timestamp = 1; - header.sequenceNumber = 123; - header.ssrc = kSenderSsrc; - header.headerLength = 12; - receiver_.receive_statistics_->IncomingPacket(header, 100); + RtpPacketReceived packet; + packet.SetTimestamp(1); + packet.SetSequenceNumber(123); + packet.SetSsrc(kSenderSsrc); + packet.AllocatePayload(100 - 12); + receiver_.receive_statistics_->OnRtpPacket(packet); // Send Frame before sending an SR. SendFrame(&sender_, kBaseLayerTid); diff --git a/modules/rtp_rtcp/test/testAPI/test_api.cc b/modules/rtp_rtcp/test/testAPI/test_api.cc index b51daa3053..a147483ba6 100644 --- a/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -14,6 +14,7 @@ #include #include +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/checks.h" #include "rtc_base/rate_limiter.h" #include "test/null_transport.h" @@ -43,22 +44,23 @@ bool LoopBackTransport::SendRtp(const uint8_t* data, return true; } } - RTPHeader header; - std::unique_ptr parser(RtpHeaderParser::Create()); - if (!parser->Parse(data, len, &header)) { + RtpPacketReceived parsed_packet; + if (!parsed_packet.Parse(data, len)) { return false; } const auto pl = - rtp_payload_registry_->PayloadTypeToPayload(header.payloadType); + rtp_payload_registry_->PayloadTypeToPayload(parsed_packet.PayloadType()); if (!pl) { return false; } - const uint8_t* payload = data + header.headerLength; - RTC_CHECK_GE(len, header.headerLength); - const size_t payload_length = len - header.headerLength; - receive_statistics_->IncomingPacket(header, len); - return rtp_receiver_->IncomingRtpPacket(header, payload, payload_length, - pl->typeSpecific); + receive_statistics_->OnRtpPacket(parsed_packet); + RTPHeader header; + parsed_packet.GetHeader(&header); + return rtp_receiver_->IncomingRtpPacket( + header, parsed_packet.payload().data(), + // RtpReceiver expects length including padding. + parsed_packet.payload_size() + parsed_packet.padding_size(), + pl->typeSpecific); } bool LoopBackTransport::SendRtcp(const uint8_t* data, size_t len) { diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index ecd62ae082..c4e7905d43 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -320,11 +320,7 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { // Receive statistics will be reset if the payload type changes (make sure // that the first packet is included in the stats). if (!packet.recovered()) { - RTPHeader header; - packet.GetHeader(&header); - // TODO(nisse): We should pass a recovered flag to stats, to aid - // fixing bug bugs.webrtc.org/6339. - rtp_receive_statistics_->IncomingPacket(header, packet.size()); + rtp_receive_statistics_->OnRtpPacket(packet); } for (RtpPacketSinkInterface* secondary_sink : secondary_sinks_) { @@ -421,9 +417,7 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { return; } if (packet.PayloadType() == config_.rtp.red_payload_type) { - RTPHeader header; - packet.GetHeader(&header); - ParseAndHandleEncapsulatingHeader(packet.data(), packet.size(), header); + ParseAndHandleEncapsulatingHeader(packet); return; } @@ -509,21 +503,21 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { } void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( - const uint8_t* packet, - size_t packet_length, - const RTPHeader& header) { + const RtpPacketReceived& packet) { RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); - if (header.payloadType == config_.rtp.red_payload_type && - packet_length > header.headerLength + header.paddingLength) { - if (packet[header.headerLength] == config_.rtp.ulpfec_payload_type) { - rtp_receive_statistics_->FecPacketReceived(header, packet_length); + if (packet.PayloadType() == config_.rtp.red_payload_type && + packet.payload_size() > 0) { + if (packet.payload()[0] == config_.rtp.ulpfec_payload_type) { + rtp_receive_statistics_->FecPacketReceived(packet); // Notify video_receiver about received FEC packets to avoid NACKing these // packets. - NotifyReceiverOfEmptyPacket(header.sequenceNumber); + NotifyReceiverOfEmptyPacket(packet.SequenceNumber()); } + RTPHeader header; + packet.GetHeader(&header); if (ulpfec_receiver_->AddReceivedRedPacket( - header, packet, packet_length, config_.rtp.ulpfec_payload_type) != - 0) { + header, packet.data(), packet.size(), + config_.rtp.ulpfec_payload_type) != 0) { return; } ulpfec_receiver_->ProcessReceivedFec(); diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 54398871a8..b54d38291e 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -143,9 +143,7 @@ class RtpVideoStreamReceiver : public RtpData, void ReceivePacket(const RtpPacketReceived& packet); // Parses and handles RED headers. // This function assumes that it's being called from only one thread. - void ParseAndHandleEncapsulatingHeader(const uint8_t* packet, - size_t packet_length, - const RTPHeader& header); + void ParseAndHandleEncapsulatingHeader(const RtpPacketReceived& packet); void NotifyReceiverOfEmptyPacket(uint16_t seq_num); void UpdateHistograms(); bool IsRedEnabled() const;