From 9dfff29bc4c8422a7e18827e325fb98fab80405b Mon Sep 17 00:00:00 2001 From: brandtr Date: Mon, 14 Nov 2016 05:14:50 -0800 Subject: [PATCH] Make FlexFEC packets paceable through RTPSender. Prior to this change, FlexFEC packets that were paced would be lost in the RTPSender, since they were not stored in a packet history. This CL introduces such a packet history, as well as the needed wireup for higher layers to be aware that the particular RTPSender is able to send FlexFEC packets with a particular SSRC. Updated RTPSender unit test to reflect the fact that paced packets are now actually sent. BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2491293002 Cr-Commit-Position: refs/heads/master@{#15066} --- webrtc/modules/pacing/packet_router.cc | 4 +- .../modules/rtp_rtcp/include/flexfec_sender.h | 2 + webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 4 + webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 2 + .../source/flexfec_sender_unittest.cc | 8 ++ .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 12 +-- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 3 + webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 58 +++++++++--- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 9 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 94 +++++++++++-------- .../rtp_rtcp/source/rtp_sender_video.cc | 7 ++ .../rtp_rtcp/source/rtp_sender_video.h | 5 + 12 files changed, 150 insertions(+), 58 deletions(-) diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc index de8c2cee5a..af1d5595be 100644 --- a/webrtc/modules/pacing/packet_router.cc +++ b/webrtc/modules/pacing/packet_router.cc @@ -48,7 +48,9 @@ bool PacketRouter::TimeToSendPacket(uint32_t ssrc, RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread()); rtc::CritScope cs(&modules_crit_); for (auto* rtp_module : rtp_modules_) { - if (rtp_module->SendingMedia() && ssrc == rtp_module->SSRC()) { + if (!rtp_module->SendingMedia()) + continue; + if (ssrc == rtp_module->SSRC() || ssrc == rtp_module->FlexfecSsrc()) { return rtp_module->TimeToSendPacket(ssrc, sequence_number, capture_timestamp, retransmission, probe_cluster_id); diff --git a/webrtc/modules/rtp_rtcp/include/flexfec_sender.h b/webrtc/modules/rtp_rtcp/include/flexfec_sender.h index 058e0160f6..48d3da02ac 100644 --- a/webrtc/modules/rtp_rtcp/include/flexfec_sender.h +++ b/webrtc/modules/rtp_rtcp/include/flexfec_sender.h @@ -38,6 +38,8 @@ class FlexfecSender { Clock* clock); ~FlexfecSender(); + uint32_t ssrc() const { return ssrc_; } + // Sets the FEC rate, max frames sent before FEC packets are sent, // and what type of generator matrices are used. void SetFecParameters(const FecProtectionParams& params); diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 1e4da64220..a2c7098722 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -18,6 +18,7 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/deprecation.h" +#include "webrtc/base/optional.h" #include "webrtc/modules/include/module.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" @@ -202,6 +203,9 @@ class RtpRtcp : public Module { virtual void SetRtxSendPayloadType(int payload_type, int associated_payload_type) = 0; + // Returns the FlexFEC SSRC, if there is one. + virtual rtc::Optional FlexfecSsrc() const = 0; + // Sets sending status. Sends kRtcpByeCode when going from true to false. // Returns -1 on failure else 0. virtual int32_t SetSendingStatus(bool sending) = 0; diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 04603efcea..37ead5ba72 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -15,6 +15,7 @@ #include #include +#include "webrtc/base/optional.h" #include "webrtc/modules/include/module.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -84,6 +85,7 @@ class MockRtpRtcp : public RtpRtcp { MOCK_CONST_METHOD0(RtxSendStatus, int()); MOCK_METHOD1(SetRtxSsrc, void(uint32_t)); MOCK_METHOD2(SetRtxSendPayloadType, void(int, int)); + MOCK_CONST_METHOD0(FlexfecSsrc, rtc::Optional()); MOCK_CONST_METHOD0(RtxSendPayloadType, std::pair()); MOCK_METHOD1(SetSendingStatus, int32_t(bool sending)); MOCK_CONST_METHOD0(Sending, bool()); diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/flexfec_sender_unittest.cc index ca114abeb3..c906750fe3 100644 --- a/webrtc/modules/rtp_rtcp/source/flexfec_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/flexfec_sender_unittest.cc @@ -70,6 +70,14 @@ std::unique_ptr GenerateSingleFlexfecPacket( } // namespace +TEST(FlexfecSenderTest, Ssrc) { + SimulatedClock clock(kInitialSimulatedClockTime); + FlexfecSender sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, + kNoRtpHeaderExtensions, &clock); + + EXPECT_EQ(kFlexfecSsrc, sender.ssrc()); +} + TEST(FlexfecSenderTest, NoFecAvailableBeforeMediaAdded) { SimulatedClock clock(kInitialSimulatedClockTime); FlexfecSender sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 15fb325575..8642b02d68 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -229,6 +229,10 @@ void ModuleRtpRtcpImpl::SetRtxSendPayloadType(int payload_type, rtp_sender_.SetRtxPayloadType(payload_type, associated_payload_type); } +rtc::Optional ModuleRtpRtcpImpl::FlexfecSsrc() const { + return rtp_sender_.FlexfecSsrc(); +} + int32_t ModuleRtpRtcpImpl::IncomingRtcpPacket( const uint8_t* rtcp_packet, const size_t length) { @@ -400,12 +404,8 @@ bool ModuleRtpRtcpImpl::TimeToSendPacket(uint32_t ssrc, int64_t capture_time_ms, bool retransmission, int probe_cluster_id) { - if (SendingMedia() && ssrc == rtp_sender_.SSRC()) { - return rtp_sender_.TimeToSendPacket(sequence_number, capture_time_ms, - retransmission, probe_cluster_id); - } - // No RTP sender is interested in sending this packet. - return true; + return rtp_sender_.TimeToSendPacket(ssrc, sequence_number, capture_time_ms, + retransmission, probe_cluster_id); } size_t ModuleRtpRtcpImpl::TimeToSendPadding(size_t bytes, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 3bda93b929..4808a4e7ea 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -17,6 +17,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/gtest_prod_util.h" +#include "webrtc/base/optional.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/packet_loss_stats.h" @@ -97,6 +98,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { void SetRtxSendPayloadType(int payload_type, int associated_payload_type) override; + rtc::Optional FlexfecSsrc() const override; + // Sends kRtcpByeCode when going from true to false. int32_t SetSendingStatus(bool sending) override; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 0a244d13e9..4bd74a6bb5 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -41,6 +41,8 @@ constexpr uint16_t kMaxInitRtpSeqNumber = 32767; // 2^15 -1. constexpr uint32_t kTimestampTicksPerMs = 90; constexpr int kBitrateStatisticsWindowMs = 1000; +constexpr size_t kMinFlexfecPacketsToStoreForPacing = 50; + const char* FrameTypeToString(FrameType frame_type) { switch (frame_type) { case kEmptyFrame: @@ -94,6 +96,7 @@ RTPSender::RTPSender( payload_type_map_(), rtp_header_extension_map_(), packet_history_(clock), + flexfec_packet_history_(clock), // Statistics rtp_stats_callback_(nullptr), total_bitrate_sent_(kBitrateStatisticsWindowMs, @@ -127,6 +130,13 @@ RTPSender::RTPSender( // Random start, 16 bits. Can't be 0. sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber); sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); + + // Store FlexFEC packets in the packet history data structure, so they can + // be found when paced. + if (flexfec_sender) { + flexfec_packet_history_.SetStorePacketsStatus( + true, kMinFlexfecPacketsToStoreForPacing); + } } RTPSender::~RTPSender() { @@ -685,15 +695,25 @@ void RTPSender::OnReceivedRtcpReportBlocks( } // Called from pacer when we can send the packet. -bool RTPSender::TimeToSendPacket(uint16_t sequence_number, +bool RTPSender::TimeToSendPacket(uint32_t ssrc, + uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, int probe_cluster_id) { - std::unique_ptr packet = - packet_history_.GetPacketAndSetSendTime(sequence_number, 0, - retransmission); + if (!SendingMedia()) + return true; + + std::unique_ptr packet; + if (ssrc == SSRC()) { + packet = packet_history_.GetPacketAndSetSendTime(sequence_number, 0, + retransmission); + } else if (ssrc == FlexfecSsrc()) { + packet = flexfec_packet_history_.GetPacketAndSetSendTime(sequence_number, 0, + retransmission); + } + if (!packet) { - // Packet cannot be found. Allow sending to continue. + // Packet cannot be found. return true; } @@ -836,14 +856,21 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, NackOverheadRate() / 1000, packet->Ssrc()); } + uint32_t ssrc = packet->Ssrc(); + rtc::Optional flexfec_ssrc = FlexfecSsrc(); if (paced_sender_) { uint16_t seq_no = packet->SequenceNumber(); - uint32_t ssrc = packet->Ssrc(); // Correct offset between implementations of millisecond time stamps in // TickTime and Clock. int64_t corrected_time_ms = packet->capture_time_ms() + clock_delta_ms_; size_t payload_length = packet->payload_size(); - packet_history_.PutRtpPacket(std::move(packet), storage, false); + if (ssrc == flexfec_ssrc) { + // Store FlexFEC packets in the history here, so they can be found + // when the pacer calls TimeToSendPacket. + flexfec_packet_history_.PutRtpPacket(std::move(packet), storage, false); + } else { + packet_history_.PutRtpPacket(std::move(packet), storage, false); + } paced_sender_->InsertPacket(priority, ssrc, seq_no, corrected_time_ms, payload_length, false); @@ -879,10 +906,12 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, UpdateRtpStats(*packet, false, false); } - // Mark the packet as sent in the history even if send failed. Dropping a - // packet here should be treated as any other packet drop so we should be - // ready for a retransmission. - packet_history_.PutRtpPacket(std::move(packet), storage, true); + // To support retransmissions, we store the media packet as sent in the + // packet history (even if send failed). + if (storage == kAllowRetransmission) { + RTC_DCHECK_EQ(ssrc, SSRC()); + packet_history_.PutRtpPacket(std::move(packet), storage, true); + } return sent; } @@ -1089,6 +1118,13 @@ uint32_t RTPSender::SSRC() const { return ssrc_; } +rtc::Optional RTPSender::FlexfecSsrc() const { + if (video_) { + return video_->FlexfecSsrc(); + } + return rtc::Optional(); +} + void RTPSender::SetCsrcs(const std::vector& csrcs) { assert(csrcs.size() <= kRtpCsrcSize); rtc::CritScope lock(&send_critsect_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index 09bcebc640..6d83ae54dc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -19,6 +19,7 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/deprecation.h" +#include "webrtc/base/optional.h" #include "webrtc/base/random.h" #include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_annotations.h" @@ -122,7 +123,8 @@ class RTPSender { size_t RtpHeaderExtensionLength() const; - bool TimeToSendPacket(uint16_t sequence_number, + bool TimeToSendPacket(uint32_t ssrc, + uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, int probe_cluster_id); @@ -166,6 +168,8 @@ class RTPSender { uint32_t SSRC() const; + rtc::Optional FlexfecSsrc() const; + bool SendToNetwork(std::unique_ptr packet, StorageType storage, RtpPacketSender::Priority priority); @@ -285,6 +289,9 @@ class RTPSender { PlayoutDelayOracle playout_delay_oracle_; RtpPacketHistory packet_history_; + // TODO(brandtr): Remove |flexfec_packet_history_| when the FlexfecSender + // is hooked up to the PacedSender. + RtpPacketHistory flexfec_packet_history_; // Statistics rtc::CriticalSection statistics_crit_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index d6796e7880..cdac7a7210 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -42,6 +42,7 @@ const int kPayload = 100; const int kRtxPayload = 98; const uint32_t kTimestamp = 10; const uint16_t kSeqNum = 33; +const uint32_t kSsrc = 725242; const int kMaxPacketLength = 1500; const uint8_t kAudioLevel = 0x5a; const uint16_t kTransportSequenceNumber = 0xaabbu; @@ -150,9 +151,10 @@ class RtpSenderTest : public ::testing::Test { nullptr, &seq_num_allocator_, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, &retransmission_rate_limiter_)); - rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetSendPayloadType(kPayload); + rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetTimestampOffset(0); + rtp_sender_->SetSSRC(kSsrc); } SimulatedClock fake_clock_; @@ -490,14 +492,14 @@ TEST_F(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { nullptr, &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, &retransmission_rate_limiter_)); + rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetStorePacketsStatus(true, 10); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); - uint16_t seq_num = 0; - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, _)) - .Times(1).WillRepeatedly(testing::SaveArg<2>(&seq_num)); + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) .WillOnce(testing::Return(kTransportSequenceNumber)); EXPECT_CALL(send_packet_observer_, @@ -511,8 +513,8 @@ TEST_F(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { .Times(1); SendGenericPayload(); - rtp_sender_->TimeToSendPacket(seq_num, fake_clock_.TimeInMilliseconds(), - false, kProbeClusterId); + rtp_sender_->TimeToSendPacket( + kSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), false, kProbeClusterId); const auto& packet = transport_.last_sent_packet(); uint16_t transport_seq_no; @@ -523,7 +525,7 @@ TEST_F(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { TEST_F(RtpSenderTest, TrafficSmoothingWithExtensions) { EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - _, kSeqNum, _, _, _)); + kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _)); @@ -549,7 +551,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingWithExtensions) { const int kStoredTimeInMs = 100; fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - rtp_sender_->TimeToSendPacket(kSeqNum, capture_time_ms, false, + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, PacketInfo::kNotAProbe); // Process send bucket. Packet should now be sent. @@ -568,7 +570,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingWithExtensions) { TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) { EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - _, kSeqNum, _, _, _)); + kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _)); @@ -592,7 +594,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) { EXPECT_EQ(0, transport_.packets_sent()); EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - _, kSeqNum, _, _, _)); + kSsrc, kSeqNum, _, _, _)); const int kStoredTimeInMs = 100; fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); @@ -600,7 +602,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) { EXPECT_EQ(static_cast(packet_size), rtp_sender_->ReSendPacket(kSeqNum)); EXPECT_EQ(0, transport_.packets_sent()); - rtp_sender_->TimeToSendPacket(kSeqNum, capture_time_ms, false, + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, PacketInfo::kNotAProbe); // Process send bucket. Packet should now be sent. @@ -622,7 +624,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) { TEST_F(RtpSenderTest, SendPadding) { // Make all (non-padding) packets go to send queue. EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - _, kSeqNum, _, _, _)); + kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _)) .Times(1 + 4 + 1); @@ -659,7 +661,7 @@ TEST_F(RtpSenderTest, SendPadding) { const int kStoredTimeInMs = 100; fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - rtp_sender_->TimeToSendPacket(seq_num++, capture_time_ms, false, + rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, PacketInfo::kNotAProbe); // Packet should now be sent. This test doesn't verify the regular video // packet, since it is tested in another test. @@ -702,15 +704,15 @@ TEST_F(RtpSenderTest, SendPadding) { packet = BuildRtpPacket(kPayload, kMarkerBit, timestamp, capture_time_ms); packet_size = packet->size(); - EXPECT_CALL(mock_paced_sender_, - InsertPacket(RtpPacketSender::kNormalPriority, _, _, _, _, _)); + EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, + kSsrc, seq_num, _, _, _)); // Packet should be stored in a send bucket. EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission, RtpPacketSender::kNormalPriority)); - rtp_sender_->TimeToSendPacket(seq_num, capture_time_ms, false, + rtp_sender_->TimeToSendPacket(kSsrc, seq_num, capture_time_ms, false, PacketInfo::kNotAProbe); // Process send bucket. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); @@ -738,12 +740,14 @@ TEST_F(RtpSenderTest, OnSendPacketUpdated) { .Times(1); EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) .WillOnce(testing::Return(kTransportSequenceNumber)); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)) + .Times(1); SendGenericPayload(); // Packet passed to pacer. const bool kIsRetransmit = false; - rtp_sender_->TimeToSendPacket(kSeqNum, fake_clock_.TimeInMilliseconds(), - kIsRetransmit, PacketInfo::kNotAProbe); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), kIsRetransmit, + PacketInfo::kNotAProbe); EXPECT_EQ(1, transport_.packets_sent()); } @@ -756,12 +760,14 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { EXPECT_CALL(send_packet_observer_, OnSendPacket(_, _, _)).Times(0); EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) .WillOnce(testing::Return(kTransportSequenceNumber)); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)) + .Times(1); SendGenericPayload(); // Packet passed to pacer. const bool kIsRetransmit = true; - rtp_sender_->TimeToSendPacket(kSeqNum, fake_clock_.TimeInMilliseconds(), - kIsRetransmit, PacketInfo::kNotAProbe); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), kIsRetransmit, + PacketInfo::kNotAProbe); EXPECT_EQ(1, transport_.packets_sent()); } @@ -770,6 +776,8 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr, nullptr /* TransportSequenceNumberAllocator */, nullptr, nullptr, nullptr, nullptr, nullptr, &send_packet_observer_, &retransmission_rate_limiter_)); + rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -777,12 +785,14 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { rtp_sender_->SetStorePacketsStatus(true, 10); EXPECT_CALL(send_packet_observer_, OnSendPacket(_, _, _)).Times(0); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)) + .Times(1); SendGenericPayload(); // Packet passed to pacer. const bool kIsRetransmit = false; - rtp_sender_->TimeToSendPacket(kSeqNum, fake_clock_.TimeInMilliseconds(), - kIsRetransmit, PacketInfo::kNotAProbe); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), kIsRetransmit, + PacketInfo::kNotAProbe); EXPECT_EQ(1, transport_.packets_sent()); } @@ -792,8 +802,8 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { false, &fake_clock_, &transport, &mock_paced_sender_, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, &retransmission_rate_limiter_)); - rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); uint16_t seq_num = kSeqNum; @@ -813,7 +823,7 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { 750, 800, 850, 900, 950}; // Expect all packets go through the pacer. EXPECT_CALL(mock_paced_sender_, - InsertPacket(RtpPacketSender::kNormalPriority, _, _, _, _, _)) + InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, _, _, _, _)) .Times(kNumPayloadSizes); EXPECT_CALL(mock_rtc_event_log_, LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _)) @@ -824,7 +834,7 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); EXPECT_CALL(transport, SendRtp(_, _, _)).WillOnce(testing::Return(true)); SendPacket(capture_time_ms, kPayloadSizes[i]); - rtp_sender_->TimeToSendPacket(seq_num++, capture_time_ms, false, + rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, PacketInfo::kNotAProbe); fake_clock_.AdvanceTimeMilliseconds(33); } @@ -926,25 +936,31 @@ TEST_F(RtpSenderTest, SendFlexfecPackets) { params.fec_mask_type = kFecMaskRandom; rtp_sender_->SetFecParameters(params, params); - uint16_t media_seq_num; + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kLowPriority, kMediaSsrc, kSeqNum, + _, _, false)); + uint16_t flexfec_seq_num; EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, - kMediaSsrc, _, _, _, false)) - .WillOnce(testing::SaveArg<2>(&media_seq_num)); - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, - kFlexfecSsrc, _, _, _, false)); + kFlexfecSsrc, _, _, _, false)) + .WillOnce(testing::SaveArg<2>(&flexfec_seq_num)); SendGenericPayload(); - // TODO(brandtr): Make these tests stricter when the FlexFEC packets are no - // longer lost between PacedSender and RTPSender. EXPECT_CALL(mock_rtc_event_log_, LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _)) - .Times(testing::AtLeast(1)); + .Times(2); EXPECT_TRUE(rtp_sender_->TimeToSendPacket( - media_seq_num, fake_clock_.TimeInMilliseconds(), false, 0)); - EXPECT_LE(1, transport_.packets_sent()); + kMediaSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), false, 0)); + EXPECT_TRUE(rtp_sender_->TimeToSendPacket(kFlexfecSsrc, flexfec_seq_num, + fake_clock_.TimeInMilliseconds(), + false, 0)); + ASSERT_EQ(2, transport_.packets_sent()); const RtpPacketReceived& media_packet = transport_.sent_packets_[0]; EXPECT_EQ(kMediaPayloadType, media_packet.PayloadType()); - EXPECT_EQ(media_seq_num, media_packet.SequenceNumber()); + EXPECT_EQ(kSeqNum, media_packet.SequenceNumber()); EXPECT_EQ(kMediaSsrc, media_packet.Ssrc()); + const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[1]; + EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); + EXPECT_EQ(flexfec_seq_num, flexfec_packet.SequenceNumber()); + EXPECT_EQ(kFlexfecSsrc, flexfec_packet.Ssrc()); } TEST_F(RtpSenderTestWithoutPacer, SendFlexfecPackets) { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index e305ba5c96..2c0267d367 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -272,6 +272,13 @@ void RTPSenderVideo::SetFecParameters(const FecProtectionParams& delta_params, key_fec_params_ = key_params; } +rtc::Optional RTPSenderVideo::FlexfecSsrc() const { + if (flexfec_sender_) { + return rtc::Optional(flexfec_sender_->ssrc()); + } + return rtc::Optional(); +} + bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type, FrameType frame_type, int8_t payload_type, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h index f8c5b7fe67..23a4082af8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h @@ -17,6 +17,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/onetimeevent.h" +#include "webrtc/base/optional.h" #include "webrtc/base/rate_statistics.h" #include "webrtc/base/sequenced_task_checker.h" #include "webrtc/base/thread_annotations.h" @@ -67,9 +68,13 @@ class RTPSenderVideo { void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type); void GetUlpfecConfig(int* red_payload_type, int* ulpfec_payload_type) const; + // FlexFEC/ULPFEC. void SetFecParameters(const FecProtectionParams& delta_params, const FecProtectionParams& key_params); + // FlexFEC. + rtc::Optional FlexfecSsrc() const; + uint32_t VideoBitrateSent() const; uint32_t FecOverheadRate() const;