From 999a72a401a7f8379ee7ba369840caece1dac4b3 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Fri, 12 Jul 2019 17:33:46 +0000 Subject: [PATCH] Reland "Optimize PacketRouter/RTPSender interactions." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 66147e892dd6b7b1beaddbcab456a1ce28b2ad22. Reason for revert: The culprit was https://webrtc-review.googlesource.com/c/src/+/133169. Original change's description: > Revert "Optimize PacketRouter/RTPSender interactions." > > This reverts commit 6f129b3b7605dc69c8c188ca02d133250130570e. > > Reason for revert: Speculative revert (some perf test are failing) > > Original change's description: > > Optimize PacketRouter/RTPSender interactions. > > > > The legacy code-path uses a hashmap as cache in order to speed up > > finding the right rtp module to send on. The new path should use that > > as well. > > In addition, there are checks that verify if an RTP module can send > > padding, in some cases payload based. These result in a number of > > calls to methods in RTPSender requiring its lock to be taken. This CL > > introduces a combined SupportsPadding() check method which performs > > all those checks in one go. > > > > Bug: None > > Change-Id: I2d18d0d6e7d8cfe92c81d08cef248a4daa7dcd4b > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144780 > > Reviewed-by: Åsa Persson > > Reviewed-by: Sebastian Jansson > > Commit-Queue: Erik Språng > > Cr-Commit-Position: refs/heads/master@{#28535} > > TBR=asapersson@webrtc.org,sprang@webrtc.org,srte@webrtc.org > > Change-Id: I8499dc0fd6e6d0b9fa7a0886c8754655e5589780 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: None > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145326 > Reviewed-by: Mirko Bonadei > Commit-Queue: Mirko Bonadei > Cr-Commit-Position: refs/heads/master@{#28552} TBR=mbonadei@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org,srte@webrtc.org Change-Id: I3bff3ecb2b776e30f77c1884f6faa72b21788017 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: None Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145401 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#28563} --- modules/pacing/packet_router.cc | 68 +++++++++++++------ modules/pacing/packet_router.h | 4 ++ modules/pacing/packet_router_unittest.cc | 48 ++++--------- modules/rtp_rtcp/include/rtp_rtcp.h | 8 ++- modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 3 +- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 17 +++-- modules/rtp_rtcp/source/rtp_rtcp_impl.h | 3 +- modules/rtp_rtcp/source/rtp_sender.cc | 32 ++++++++- modules/rtp_rtcp/source/rtp_sender.h | 3 + .../rtp_rtcp/source/rtp_sender_unittest.cc | 38 +++++++++++ 10 files changed, 156 insertions(+), 68 deletions(-) diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index 7492f1309e..de2621790b 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -114,14 +114,15 @@ RtpPacketSendResult PacketRouter::TimeToSendPacket( return RtpPacketSendResult::kPacketNotFound; } - if ((rtp_module->RtxSendStatus() & kRtxRedundantPayloads) && - rtp_module->HasBweExtensions()) { + RtpPacketSendResult result = rtp_module->TimeToSendPacket( + ssrc, sequence_number, capture_timestamp, retransmission, pacing_info); + if (result == RtpPacketSendResult::kSuccess && + rtp_module->SupportsRtxPayloadPadding()) { // This is now the last module to send media, and has the desired // properties needed for payload based padding. Cache it for later use. last_send_module_ = rtp_module; } - return rtp_module->TimeToSendPacket(ssrc, sequence_number, capture_timestamp, - retransmission, pacing_info); + return result; } RtpRtcp* PacketRouter::FindRtpModule(uint32_t ssrc) { @@ -152,16 +153,19 @@ void PacketRouter::SendPacket(std::unique_ptr packet, packet->SetExtension(transport_seq_)) { ++transport_seq_; } + + auto it = rtp_module_cache_map_.find(packet->Ssrc()); + if (it != rtp_module_cache_map_.end()) { + if (TrySendPacket(packet.get(), cluster_info, it->second)) { + return; + } + // Entry is stale, remove it. + rtp_module_cache_map_.erase(it); + } + + // Slow path, find the correct send module. for (auto* rtp_module : rtp_send_modules_) { - if (rtp_module->TrySendPacket(packet.get(), cluster_info)) { - const bool can_send_padding = - (rtp_module->RtxSendStatus() & kRtxRedundantPayloads) && - rtp_module->HasBweExtensions(); - if (can_send_padding) { - // This is now the last module to send media, and has the desired - // properties needed for payload based padding. Cache it for later use. - last_send_module_ = rtp_module; - } + if (TrySendPacket(packet.get(), cluster_info, rtp_module)) { return; } } @@ -182,10 +186,10 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, // will be more skewed towards the highest bitrate stream. At the very least // this prevents sending payload padding on a disabled stream where it's // guaranteed not to be useful. - if (last_send_module_ != nullptr) { + if (last_send_module_ != nullptr && + last_send_module_->SupportsRtxPayloadPadding()) { RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), last_send_module_) != rtp_send_modules_.end()); - RTC_DCHECK(last_send_module_->HasBweExtensions()); total_bytes_sent += last_send_module_->TimeToSendPadding( bytes_to_send - total_bytes_sent, pacing_info); if (total_bytes_sent >= bytes_to_send) { @@ -194,8 +198,9 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, } // Rtp modules are ordered by which stream can most benefit from padding. + // Don't require RTX payload padding in the general case. for (RtpRtcp* module : rtp_send_modules_) { - if (module->SendingMedia() && module->HasBweExtensions()) { + if (module->SupportsPadding()) { size_t bytes_sent = module->TimeToSendPadding( bytes_to_send - total_bytes_sent, pacing_info); total_bytes_sent += bytes_sent; @@ -215,17 +220,21 @@ std::vector> PacketRouter::GeneratePadding( // will be more skewed towards the highest bitrate stream. At the very least // this prevents sending payload padding on a disabled stream where it's // guaranteed not to be useful. - if (last_send_module_ != nullptr) { + if (last_send_module_ != nullptr && + last_send_module_->SupportsRtxPayloadPadding()) { RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), last_send_module_) != rtp_send_modules_.end()); - RTC_DCHECK(last_send_module_->HasBweExtensions()); return last_send_module_->GeneratePadding(target_size_bytes); } // Rtp modules are ordered by which stream can most benefit from padding. for (RtpRtcp* rtp_module : rtp_send_modules_) { - if (rtp_module->SendingMedia() && rtp_module->HasBweExtensions()) { - return rtp_module->GeneratePadding(target_size_bytes); + if (rtp_module->SupportsPadding()) { + auto padding_packets = rtp_module->GeneratePadding(target_size_bytes); + if (!padding_packets.empty()) { + last_send_module_ = rtp_module; + } + return padding_packets; } } @@ -402,4 +411,23 @@ void PacketRouter::DetermineActiveRembModule() { active_remb_module_ = new_active_remb_module; } +bool PacketRouter::TrySendPacket(RtpPacketToSend* packet, + const PacedPacketInfo& cluster_info, + RtpRtcp* rtp_module) { + uint32_t ssrc = packet->Ssrc(); + if (rtp_module->TrySendPacket(packet, cluster_info)) { + // Sending succeeded, make sure this SSRC mapping for future use. + rtp_module_cache_map_[ssrc] = rtp_module; + + if (rtp_module->SupportsRtxPayloadPadding()) { + // This is now the last module to send media, and has the desired + // properties needed for payload based padding. Cache it for later use. + last_send_module_ = rtp_module; + } + + return true; + } + return false; +} + } // namespace webrtc diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h index 79d3fa9eb0..309d4382df 100644 --- a/modules/pacing/packet_router.h +++ b/modules/pacing/packet_router.h @@ -102,6 +102,10 @@ class PacketRouter : public TransportSequenceNumberAllocator, bool media_sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); void UnsetActiveRembModule() RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); void DetermineActiveRembModule() RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); + bool TrySendPacket(RtpPacketToSend* packet, + const PacedPacketInfo& cluster_info, + RtpRtcp* rtp_module) + RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); rtc::CriticalSection modules_crit_; // Rtp and Rtcp modules of the rtp senders. diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc index 5add5f7374..ac59a4424d 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/modules/pacing/packet_router_unittest.cc @@ -203,15 +203,13 @@ TEST(PacketRouterTest, TimeToSendPadding) { // ordered by priority (based on rtx mode). const size_t requested_padding_bytes = 1000; const size_t sent_padding_bytes = 890; - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, SupportsPadding).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, Field(&PacedPacketInfo::probe_cluster_id, 111))) .Times(1) .WillOnce(Return(sent_padding_bytes)); - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_1, SupportsPadding).WillOnce(Return(true)); EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes - sent_padding_bytes, Field(&PacedPacketInfo::probe_cluster_id, 111))) @@ -224,10 +222,9 @@ TEST(PacketRouterTest, TimeToSendPadding) { // Let only the lower priority module be sending and verify the padding // request is routed there. - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_2, SupportsPadding).WillOnce(Return(false)); EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(0); - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_1, SupportsPadding).WillOnce(Return(true)); EXPECT_CALL(rtp_1, TimeToSendPadding(_, _)) .Times(1) .WillOnce(Return(sent_padding_bytes)); @@ -238,36 +235,20 @@ TEST(PacketRouterTest, TimeToSendPadding) { kProbeMinBytes))); // No sending module at all. - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_1, SupportsPadding).WillOnce(Return(false)); EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes, _)).Times(0); - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_2, SupportsPadding).WillOnce(Return(false)); EXPECT_CALL(rtp_2, TimeToSendPadding(_, _)).Times(0); EXPECT_EQ(0u, packet_router.TimeToSendPadding( requested_padding_bytes, PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, kProbeMinBytes))); - // Only one module has BWE extensions. - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(false)); - EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes, _)).Times(0); - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)) - .Times(1) - .WillOnce(Return(sent_padding_bytes)); - EXPECT_EQ(sent_padding_bytes, - packet_router.TimeToSendPadding( - requested_padding_bytes, - PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, - kProbeMinBytes))); - packet_router.RemoveSendRtpModule(&rtp_1); // rtp_1 has been removed, try sending padding and make sure rtp_1 isn't asked // to send by not expecting any calls. Instead verify rtp_2 is called. - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, SupportsPadding).WillOnce(Return(true)); EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(1); EXPECT_EQ(0u, packet_router.TimeToSendPadding( requested_padding_bytes, @@ -288,14 +269,12 @@ TEST(PacketRouterTest, GeneratePaddingPicksCorrectModule) { NiceMock rtp_1; ON_CALL(rtp_1, RtxSendStatus()).WillByDefault(Return(kRtxRedundantPayloads)); ON_CALL(rtp_1, SSRC()).WillByDefault(Return(kSsrc1)); - ON_CALL(rtp_1, SendingMedia()).WillByDefault(Return(false)); - ON_CALL(rtp_1, HasBweExtensions()).WillByDefault(Return(false)); + ON_CALL(rtp_1, SupportsPadding).WillByDefault(Return(false)); NiceMock rtp_2; ON_CALL(rtp_2, RtxSendStatus()).WillByDefault(Return(kRtxOff)); ON_CALL(rtp_2, SSRC()).WillByDefault(Return(kSsrc2)); - ON_CALL(rtp_2, SendingMedia()).WillByDefault(Return(true)); - ON_CALL(rtp_2, HasBweExtensions()).WillByDefault(Return(true)); + ON_CALL(rtp_2, SupportsPadding).WillByDefault(Return(true)); packet_router.AddSendRtpModule(&rtp_1, false); packet_router.AddSendRtpModule(&rtp_2, false); @@ -328,21 +307,24 @@ TEST(PacketRouterTest, PadsOnLastActiveMediaStream) { .WillRepeatedly(Return(kRtxRedundantPayloads)); EXPECT_CALL(rtp_1, SSRC()).WillRepeatedly(Return(kSsrc1)); EXPECT_CALL(rtp_1, SendingMedia()).WillRepeatedly(Return(true)); - EXPECT_CALL(rtp_1, HasBweExtensions()).WillRepeatedly(Return(true)); + EXPECT_CALL(rtp_1, SupportsPadding).WillRepeatedly(Return(true)); + EXPECT_CALL(rtp_1, SupportsRtxPayloadPadding).WillRepeatedly(Return(true)); NiceMock rtp_2; EXPECT_CALL(rtp_2, RtxSendStatus()) .WillRepeatedly(Return(kRtxRedundantPayloads)); EXPECT_CALL(rtp_2, SSRC()).WillRepeatedly(Return(kSsrc2)); EXPECT_CALL(rtp_2, SendingMedia()).WillRepeatedly(Return(true)); - EXPECT_CALL(rtp_2, HasBweExtensions()).WillRepeatedly(Return(true)); + EXPECT_CALL(rtp_2, SupportsPadding).WillRepeatedly(Return(true)); + EXPECT_CALL(rtp_2, SupportsRtxPayloadPadding).WillRepeatedly(Return(true)); // Third module is sending media, but does not support rtx. NiceMock rtp_3; EXPECT_CALL(rtp_3, RtxSendStatus()).WillRepeatedly(Return(kRtxOff)); EXPECT_CALL(rtp_3, SSRC()).WillRepeatedly(Return(kSsrc3)); EXPECT_CALL(rtp_3, SendingMedia()).WillRepeatedly(Return(true)); - EXPECT_CALL(rtp_3, HasBweExtensions()).WillRepeatedly(Return(true)); + EXPECT_CALL(rtp_3, SupportsPadding).WillRepeatedly(Return(true)); + EXPECT_CALL(rtp_3, SupportsRtxPayloadPadding).WillRepeatedly(Return(true)); packet_router.AddSendRtpModule(&rtp_1, false); packet_router.AddSendRtpModule(&rtp_2, false); diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 42dd27dbcd..83c4cfc9cb 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -175,7 +175,13 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { virtual int32_t DeregisterSendRtpHeaderExtension(RTPExtensionType type) = 0; - virtual bool HasBweExtensions() const = 0; + // Returns true if RTP module is send media, and any of the extensions + // required for bandwidth estimation is registered. + virtual bool SupportsPadding() const = 0; + // Same as SupportsPadding(), but additionally requires that + // SetRtxSendStatus() has been called with the kRtxRedundantPayloads option + // enabled. + virtual bool SupportsRtxPayloadPadding() const = 0; // Returns start timestamp. virtual uint32_t StartTimestamp() const = 0; diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index f3812ffb9b..68ded28afc 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -49,7 +49,8 @@ class MockRtpRtcp : public RtpRtcp { bool(const std::string& uri, int id)); MOCK_METHOD1(DeregisterSendRtpHeaderExtension, int32_t(RTPExtensionType type)); - MOCK_CONST_METHOD0(HasBweExtensions, bool()); + MOCK_CONST_METHOD0(SupportsPadding, bool()); + MOCK_CONST_METHOD0(SupportsRtxPayloadPadding, bool()); MOCK_CONST_METHOD0(StartTimestamp, uint32_t()); MOCK_METHOD1(SetStartTimestamp, void(uint32_t timestamp)); MOCK_CONST_METHOD0(SequenceNumber, uint16_t()); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 765f76f70f..13f1b354e1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -395,6 +395,14 @@ bool ModuleRtpRtcpImpl::TrySendPacket(RtpPacketToSend* packet, return rtp_sender_->TrySendPacket(packet, pacing_info); } +bool ModuleRtpRtcpImpl::SupportsPadding() const { + return rtp_sender_->SupportsPadding(); +} + +bool ModuleRtpRtcpImpl::SupportsRtxPayloadPadding() const { + return rtp_sender_->SupportsRtxPayloadPadding(); +} + size_t ModuleRtpRtcpImpl::TimeToSendPadding( size_t bytes, const PacedPacketInfo& pacing_info) { @@ -582,15 +590,6 @@ int32_t ModuleRtpRtcpImpl::DeregisterSendRtpHeaderExtension( return rtp_sender_->DeregisterRtpHeaderExtension(type); } -bool ModuleRtpRtcpImpl::HasBweExtensions() const { - return rtp_sender_->IsRtpHeaderExtensionRegistered( - kRtpExtensionTransportSequenceNumber) || - rtp_sender_->IsRtpHeaderExtensionRegistered( - kRtpExtensionAbsoluteSendTime) || - rtp_sender_->IsRtpHeaderExtensionRegistered( - kRtpExtensionTransmissionTimeOffset); -} - // (TMMBR) Temporary Max Media Bit Rate. bool ModuleRtpRtcpImpl::TMMBR() const { return rtcp_sender_.TMMBR(); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index ec62aeb2ed..8cb01be914 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -76,7 +76,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { int32_t DeregisterSendRtpHeaderExtension(RTPExtensionType type) override; - bool HasBweExtensions() const override; + bool SupportsPadding() const override; + bool SupportsRtxPayloadPadding() const override; // Get start timestamp. uint32_t StartTimestamp() const override; diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 0fa719e11c..76cc19c2fc 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -139,6 +139,13 @@ bool IsDisabled(absl::string_view name, return trials.Lookup(name).find("Disabled") == 0; } +bool HasBweExtension(const RtpHeaderExtensionMap& extensions_map) { + return extensions_map.IsRegistered(kRtpExtensionTransportSequenceNumber) || + extensions_map.IsRegistered(kRtpExtensionTransportSequenceNumber02) || + extensions_map.IsRegistered(kRtpExtensionAbsoluteSendTime) || + extensions_map.IsRegistered(kRtpExtensionTransmissionTimeOffset); +} + } // namespace RTPSender::RTPSender(const RtpRtcp::Configuration& config) @@ -185,6 +192,7 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) rtx_(kRtxOff), ssrc_rtx_(config.rtx_send_ssrc), rtp_overhead_bytes_per_packet_(0), + supports_bwe_extension_(false), retransmission_rate_limiter_(config.retransmission_rate_limiter), overhead_observer_(config.overhead_observer), populate_network2_timestamp_(config.populate_network2_timestamp), @@ -275,6 +283,7 @@ RTPSender::RTPSender( csrcs_(), rtx_(kRtxOff), rtp_overhead_bytes_per_packet_(0), + supports_bwe_extension_(false), retransmission_rate_limiter_(retransmission_rate_limiter), overhead_observer_(overhead_observer), populate_network2_timestamp_(populate_network2_timestamp), @@ -351,12 +360,16 @@ void RTPSender::SetExtmapAllowMixed(bool extmap_allow_mixed) { int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id) { rtc::CritScope lock(&send_critsect_); - return rtp_header_extension_map_.RegisterByType(id, type) ? 0 : -1; + bool registered = rtp_header_extension_map_.RegisterByType(id, type); + supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); + return registered ? 0 : -1; } bool RTPSender::RegisterRtpHeaderExtension(const std::string& uri, int id) { rtc::CritScope lock(&send_critsect_); - return rtp_header_extension_map_.RegisterByUri(id, uri); + bool registered = rtp_header_extension_map_.RegisterByUri(id, uri); + supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); + return registered; } bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) const { @@ -366,7 +379,9 @@ bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) const { int32_t RTPSender::DeregisterRtpHeaderExtension(RTPExtensionType type) { rtc::CritScope lock(&send_critsect_); - return rtp_header_extension_map_.Deregister(type); + int32_t deregistered = rtp_header_extension_map_.Deregister(type); + supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); + return deregistered; } void RTPSender::SetMaxRtpPacketSize(size_t max_packet_size) { @@ -853,6 +868,17 @@ bool RTPSender::TrySendPacket(RtpPacketToSend* packet, return true; } +bool RTPSender::SupportsPadding() const { + rtc::CritScope lock(&send_critsect_); + return sending_media_ && supports_bwe_extension_; +} + +bool RTPSender::SupportsRtxPayloadPadding() const { + rtc::CritScope lock(&send_critsect_); + return sending_media_ && supports_bwe_extension_ && + (rtx_ & kRtxRedundantPayloads); +} + bool RTPSender::PrepareAndSendPacket(std::unique_ptr packet, bool send_over_rtx, bool is_retransmit, diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 8e505750a5..032e65cc54 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -118,6 +118,8 @@ class RTPSender { const PacedPacketInfo& pacing_info); bool TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info); + bool SupportsPadding() const; + bool SupportsRtxPayloadPadding() const; size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info); std::vector> GeneratePadding( size_t target_size_bytes); @@ -309,6 +311,7 @@ class RTPSender { // Mapping rtx_payload_type_map_[associated] = rtx. std::map rtx_payload_type_map_ RTC_GUARDED_BY(send_critsect_); size_t rtp_overhead_bytes_per_packet_ RTC_GUARDED_BY(send_critsect_); + bool supports_bwe_extension_ RTC_GUARDED_BY(send_critsect_); RateLimiter* const retransmission_rate_limiter_; OverheadObserver* overhead_observer_; diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 495ffee664..d2761ea347 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -2631,6 +2631,44 @@ TEST_P(RtpSenderTest, GeneratePaddingCreatesPurePaddingWithoutRtx) { kExpectedNumPaddingPackets * kMaxPaddingSize); } +TEST_P(RtpSenderTest, SupportsPadding) { + bool kSendingMediaStats[] = {true, false}; + bool kEnableRedundantPayloads[] = {true, false}; + RTPExtensionType kBweExtensionTypes[] = { + kRtpExtensionTransportSequenceNumber, + kRtpExtensionTransportSequenceNumber02, kRtpExtensionAbsoluteSendTime, + kRtpExtensionTransmissionTimeOffset}; + const int kExtensionsId = 7; + + for (bool sending_media : kSendingMediaStats) { + rtp_sender_->SetSendingMediaStatus(sending_media); + for (bool redundant_payloads : kEnableRedundantPayloads) { + int rtx_mode = kRtxRetransmitted; + if (redundant_payloads) { + rtx_mode |= kRtxRedundantPayloads; + } + rtp_sender_->SetRtxStatus(rtx_mode); + + for (auto extension_type : kBweExtensionTypes) { + EXPECT_FALSE(rtp_sender_->SupportsPadding()); + rtp_sender_->RegisterRtpHeaderExtension(extension_type, kExtensionsId); + if (!sending_media) { + EXPECT_FALSE(rtp_sender_->SupportsPadding()); + } else { + EXPECT_TRUE(rtp_sender_->SupportsPadding()); + if (redundant_payloads) { + EXPECT_TRUE(rtp_sender_->SupportsRtxPayloadPadding()); + } else { + EXPECT_FALSE(rtp_sender_->SupportsRtxPayloadPadding()); + } + } + rtp_sender_->DeregisterRtpHeaderExtension(extension_type); + EXPECT_FALSE(rtp_sender_->SupportsPadding()); + } + } + } +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTest, ::testing::Values(TestConfig{false, false},