diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index 791f54e939..e73516f90e 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -200,6 +200,31 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, return total_bytes_sent; } +void PacketRouter::GeneratePadding(size_t target_size_bytes) { + rtc::CritScope cs(&modules_crit_); + // First try on the last rtp module to have sent media. This increases the + // the chance that any payload based padding will be useful as it will be + // somewhat distributed over modules according the packet rate, even if it + // 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) { + 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()); + last_send_module_->GeneratePadding(target_size_bytes); + return; + } + + // 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()) { + rtp_module->GeneratePadding(target_size_bytes); + return; + } + } +} + void PacketRouter::SetTransportWideSequenceNumber(uint16_t sequence_number) { rtc::AtomicOps::ReleaseStore(&transport_seq_, sequence_number); } diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h index 0ca4607d61..a14a55e922 100644 --- a/modules/pacing/packet_router.h +++ b/modules/pacing/packet_router.h @@ -65,6 +65,8 @@ class PacketRouter : public TransportSequenceNumberAllocator, virtual size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& packet_info); + virtual void GeneratePadding(size_t target_size_bytes); + void SetTransportWideSequenceNumber(uint16_t sequence_number); uint16_t AllocateSequenceNumber() override; diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc index 1677b0ce3d..92ecdd6a36 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/modules/pacing/packet_router_unittest.cc @@ -272,6 +272,38 @@ TEST(PacketRouterTest, TimeToSendPadding) { packet_router.RemoveSendRtpModule(&rtp_2); } +TEST(PacketRouterTest, GeneratePaddingPicksCorrectModule) { + PacketRouter packet_router; + + // Two RTP modules. The first (prioritized due to rtx) isn't sending media so + // should not be called. + const uint16_t kSsrc1 = 1234; + const uint16_t kSsrc2 = 4567; + + 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)); + + 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)); + + packet_router.AddSendRtpModule(&rtp_1, false); + packet_router.AddSendRtpModule(&rtp_2, false); + + const size_t kPaddingSize = 123; + EXPECT_CALL(rtp_1, GeneratePadding(_)).Times(0); + EXPECT_CALL(rtp_2, GeneratePadding(kPaddingSize)).Times(1); + packet_router.GeneratePadding(kPaddingSize); + + packet_router.RemoveSendRtpModule(&rtp_1); + packet_router.RemoveSendRtpModule(&rtp_2); +} + TEST(PacketRouterTest, PadsOnLastActiveMediaStream) { PacketRouter packet_router; diff --git a/modules/pacing/round_robin_packet_queue.cc b/modules/pacing/round_robin_packet_queue.cc index 9f52a6a9c8..a1deb06239 100644 --- a/modules/pacing/round_robin_packet_queue.cc +++ b/modules/pacing/round_robin_packet_queue.cc @@ -98,7 +98,7 @@ void RoundRobinPacketQueue::Push(int priority, uint32_t ssrc = packet->Ssrc(); uint16_t sequence_number = packet->SequenceNumber(); int64_t capture_time_ms = packet->capture_time_ms(); - size_t size_bytes = packet->payload_size(); + size_t size_bytes = packet->payload_size() + packet->padding_size(); auto type = packet->packet_type(); RTC_DCHECK(type.has_value()); diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 8dcb760d8c..fcad876a4a 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -279,6 +279,8 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { virtual size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info) = 0; + virtual void GeneratePadding(size_t target_size_bytes) = 0; + // Called on generation of new statistics after an RTP send. virtual void RegisterSendChannelRtpStatisticsCallback( StreamDataCountersCallback* callback) = 0; diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 2791d0e070..fc2bb36d81 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -95,6 +95,7 @@ class MockRtpRtcp : public RtpRtcp { const PacedPacketInfo& pacing_info)); MOCK_METHOD2(TimeToSendPadding, size_t(size_t bytes, const PacedPacketInfo& pacing_info)); + MOCK_METHOD1(GeneratePadding, void(size_t target_size_bytes)); MOCK_METHOD2(RegisterRtcpObservers, void(RtcpIntraFrameObserver* intra_frame_callback, RtcpBandwidthObserver* bandwidth_callback)); diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc index 9e0c8c8aca..da6fc61d01 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history.cc @@ -346,6 +346,15 @@ std::unique_ptr RtpPacketHistory::GetBestFittingPacket( } std::unique_ptr RtpPacketHistory::GetPayloadPaddingPacket() { + // Default implementation always just returns a copy of the packet. + return GetPayloadPaddingPacket([](const RtpPacketToSend& packet) { + return absl::make_unique(packet); + }); +} + +std::unique_ptr RtpPacketHistory::GetPayloadPaddingPacket( + rtc::FunctionView(const RtpPacketToSend&)> + encapsulate) { rtc::CritScope cs(&lock_); if (mode_ == StorageMode::kDisabled || padding_priority_.empty()) { return nullptr; @@ -362,11 +371,15 @@ std::unique_ptr RtpPacketHistory::GetPayloadPaddingPacket() { return nullptr; } + auto padding_packet = encapsulate(*best_packet->packet_); + if (!padding_packet) { + return nullptr; + } + best_packet->send_time_ms_ = clock_->TimeInMilliseconds(); best_packet->IncrementTimesRetransmitted(&padding_priority_); - // Return a copy of the packet. - return absl::make_unique(*best_packet->packet_); + return padding_packet; } void RtpPacketHistory::CullAcknowledgedPackets( diff --git a/modules/rtp_rtcp/source/rtp_packet_history.h b/modules/rtp_rtcp/source/rtp_packet_history.h index b988b68068..a0d42827be 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.h +++ b/modules/rtp_rtcp/source/rtp_packet_history.h @@ -119,6 +119,14 @@ class RtpPacketHistory { // a const method. std::unique_ptr GetPayloadPaddingPacket(); + // Same as GetPayloadPaddingPacket(void), but adds an encapsulation + // that can be used for instance to encapsulate the packet in an RTX + // container, or to abort getting the packet if the function returns + // nullptr. + std::unique_ptr GetPayloadPaddingPacket( + rtc::FunctionView( + const RtpPacketToSend&)> encapsulate); + // Cull packets that have been acknowledged as received by the remote end. void CullAcknowledgedPackets(rtc::ArrayView sequence_numbers); diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc index 950cbe6a14..345ad7a6bc 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc @@ -839,4 +839,27 @@ TEST_F(RtpPacketHistoryTest, NoPendingPacketAsPadding) { EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(), kStartSeqNum); } +TEST_F(RtpPacketHistoryTest, PayloadPaddingWithEncapsulation) { + hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); + + hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission, + fake_clock_.TimeInMilliseconds()); + fake_clock_.AdvanceTimeMilliseconds(1); + + // Aborted padding. + EXPECT_EQ(nullptr, + hist_.GetPayloadPaddingPacket( + [](const RtpPacketToSend& packet) { return nullptr; })); + + // Get copy of packet, but with sequence number modified. + auto padding_packet = + hist_.GetPayloadPaddingPacket([&](const RtpPacketToSend& packet) { + auto encapsulated_packet = absl::make_unique(packet); + encapsulated_packet->SetSequenceNumber(kStartSeqNum + 1); + return encapsulated_packet; + }); + ASSERT_TRUE(padding_packet); + EXPECT_EQ(padding_packet->SequenceNumber(), kStartSeqNum + 1); +} + } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 900599b63c..f8a0d709ae 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -427,6 +427,10 @@ size_t ModuleRtpRtcpImpl::TimeToSendPadding( return rtp_sender_->TimeToSendPadding(bytes, pacing_info); } +void ModuleRtpRtcpImpl::GeneratePadding(size_t target_size_bytes) { + rtp_sender_->GeneratePadding(target_size_bytes); +} + size_t ModuleRtpRtcpImpl::MaxRtpPacketSize() const { return rtp_sender_->MaxRtpPacketSize(); } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 38e1ede8d9..60ac5fd604 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -148,6 +148,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info) override; + void GeneratePadding(size_t target_size_bytes) override; + // RTCP part. // Get RTCP status. diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 2863680e2a..ac82262928 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -354,16 +354,15 @@ size_t RTPSender::SendPadData(size_t bytes, if (audio_configured_) { // Allow smaller padding packets for audio. - padding_bytes_in_packet = rtc::SafeClamp( - bytes, kMinAudioPaddingLength, - rtc::SafeMin(max_payload_size, kMaxPaddingLength)); + padding_bytes_in_packet = + rtc::SafeClamp(bytes, kMinAudioPaddingLength, + rtc::SafeMin(max_payload_size, kMaxPaddingLength)); } else { // Always send full padding packets. This is accounted for by the // RtpPacketSender, which will make sure we don't send too much padding even // if a single packet is larger than requested. // We do this to avoid frequently sending small packets on higher bitrates. - padding_bytes_in_packet = - rtc::SafeMin(max_payload_size, kMaxPaddingLength); + padding_bytes_in_packet = rtc::SafeMin(max_payload_size, kMaxPaddingLength); } size_t bytes_sent = 0; while (bytes_sent < bytes) { @@ -837,6 +836,108 @@ size_t RTPSender::TimeToSendPadding(size_t bytes, return bytes_sent; } +void RTPSender::GeneratePadding(size_t target_size_bytes) { + // This method does not actually send packets, it just generates + // them and puts them in the pacer queue. Since this should incur + // low overhead, keep the lock for the scope of the method in order + // to make the code more readable. + rtc::CritScope lock(&send_critsect_); + if (!sending_media_) + return; + + size_t bytes_left = target_size_bytes; + if ((rtx_ & kRtxRedundantPayloads) != 0) { + while (bytes_left >= 0) { + std::unique_ptr packet = + packet_history_.GetPayloadPaddingPacket( + [&](const RtpPacketToSend& packet) + -> std::unique_ptr { + if (packet.payload_size() + kRtxHeaderSize > bytes_left) { + return nullptr; + } + return BuildRtxPacket(packet); + }); + if (!packet) { + break; + } + + bytes_left -= std::min(bytes_left, packet->payload_size()); + packet->set_packet_type(RtpPacketToSend::Type::kPadding); + paced_sender_->EnqueuePacket(std::move(packet)); + } + } + + size_t padding_bytes_in_packet; + const size_t max_payload_size = max_packet_size_ - RtpHeaderLength(); + if (audio_configured_) { + // Allow smaller padding packets for audio. + padding_bytes_in_packet = rtc::SafeClamp( + bytes_left, kMinAudioPaddingLength, + rtc::SafeMin(max_payload_size, kMaxPaddingLength)); + } else { + // Always send full padding packets. This is accounted for by the + // RtpPacketSender, which will make sure we don't send too much padding even + // if a single packet is larger than requested. + // We do this to avoid frequently sending small packets on higher bitrates. + padding_bytes_in_packet = rtc::SafeMin(max_payload_size, kMaxPaddingLength); + } + + while (bytes_left > 0) { + auto padding_packet = + absl::make_unique(&rtp_header_extension_map_); + padding_packet->set_packet_type(RtpPacketToSend::Type::kPadding); + padding_packet->SetMarker(false); + padding_packet->SetTimestamp(last_rtp_timestamp_); + padding_packet->set_capture_time_ms(capture_time_ms_); + if (rtx_ == kRtxOff) { + if (last_payload_type_ == -1) { + break; + } + // Without RTX we can't send padding in the middle of frames. + // For audio marker bits doesn't mark the end of a frame and frames + // are usually a single packet, so for now we don't apply this rule + // for audio. + if (!audio_configured_ && !last_packet_marker_bit_) { + break; + } + + RTC_DCHECK(ssrc_); + padding_packet->SetSsrc(*ssrc_); + padding_packet->SetPayloadType(last_payload_type_); + padding_packet->SetSequenceNumber(sequence_number_++); + } else { + // Without abs-send-time or transport sequence number a media packet + // must be sent before padding so that the timestamps used for + // estimation are correct. + if (!media_has_been_sent_ && + !(rtp_header_extension_map_.IsRegistered(AbsoluteSendTime::kId) || + rtp_header_extension_map_.IsRegistered( + TransportSequenceNumber::kId))) { + break; + } + // Only change the timestamp of padding packets sent over RTX. + // Padding only packets over RTP has to be sent as part of a media + // frame (and therefore the same timestamp). + int64_t now_ms = clock_->TimeInMilliseconds(); + if (last_timestamp_time_ms_ > 0) { + padding_packet->SetTimestamp(padding_packet->Timestamp() + + (now_ms - last_timestamp_time_ms_) * + kTimestampTicksPerMs); + padding_packet->set_capture_time_ms(padding_packet->capture_time_ms() + + (now_ms - last_timestamp_time_ms_)); + } + RTC_DCHECK(ssrc_rtx_); + padding_packet->SetSsrc(*ssrc_rtx_); + padding_packet->SetSequenceNumber(sequence_number_rtx_++); + padding_packet->SetPayloadType(rtx_payload_type_map_.begin()->second); + } + + padding_packet->SetPadding(padding_bytes_in_packet); + bytes_left -= std::min(bytes_left, padding_bytes_in_packet); + paced_sender_->EnqueuePacket(std::move(padding_packet)); + } +} + bool RTPSender::SendToNetwork(std::unique_ptr packet, StorageType storage) { RTC_DCHECK(packet); diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index d8d7865000..c931a15c9a 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -114,6 +114,7 @@ class RTPSender { bool TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info); size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info); + void GeneratePadding(size_t target_size_bytes); // NACK. void OnReceivedNack(const std::vector& nack_sequence_numbers, diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 7cd1367b96..fd60b73aa3 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -78,8 +78,11 @@ using ::testing::DoAll; using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::Field; +using ::testing::Gt; using ::testing::Invoke; using ::testing::NiceMock; +using ::testing::Pointee; +using ::testing::Property; using ::testing::Return; using ::testing::SaveArg; using ::testing::SizeIs; @@ -2085,6 +2088,79 @@ TEST_P(RtpSenderTest, TrySendPacketUpdatesStats) { EXPECT_EQ(rtx_stats.retransmitted.packets, 1u); } +TEST_P(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) { + rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); + rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); + rtp_sender_->SetStorePacketsStatus(true, 1); + + const size_t kPayloadPacketSize = 1234; + std::unique_ptr packet = + BuildRtpPacket(kPayload, true, 0, fake_clock_.TimeInMilliseconds()); + packet->set_allow_retransmission(true); + packet->SetPayloadSize(kPayloadPacketSize); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + + // Send a dummy video packet so it ends up in the packet history. + EXPECT_TRUE(rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo())); + + // Generated padding has large enough budget that the video packet should be + // retransmitted as padding. + EXPECT_CALL(mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::packet_type, + RtpPacketToSend::Type::kPadding)), + Pointee(Property(&RtpPacketToSend::Ssrc, kRtxSsrc)), + Pointee(Property(&RtpPacketToSend::payload_size, + kPayloadPacketSize + kRtxHeaderSize))))) + .Times(1); + rtp_sender_->GeneratePadding(kPayloadPacketSize + kRtxHeaderSize); + + // Not enough budged for payload padding, use plain padding instead. + EXPECT_CALL(mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::packet_type, + RtpPacketToSend::Type::kPadding)), + Pointee(Property(&RtpPacketToSend::Ssrc, kRtxSsrc)), + Pointee(Property(&RtpPacketToSend::payload_size, 0)), + Pointee(Property(&RtpPacketToSend::padding_size, Gt(0u)))))) + .Times((kPayloadPacketSize + kMaxPaddingSize - 1) / kMaxPaddingSize); + rtp_sender_->GeneratePadding(kPayloadPacketSize + kRtxHeaderSize - 1); +} + +TEST_P(RtpSenderTest, GeneratePaddingCreatesPurePaddingWithoutRtx) { + rtp_sender_->SetStorePacketsStatus(true, 1); + + const size_t kPayloadPacketSize = 1234; + // Send a dummy video packet so it ends up in the packet history. Since we + // are not using RTX, it should never be used as padding. + std::unique_ptr packet = + BuildRtpPacket(kPayload, true, 0, fake_clock_.TimeInMilliseconds()); + packet->set_allow_retransmission(true); + packet->SetPayloadSize(kPayloadPacketSize); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + EXPECT_TRUE(rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo())); + + // Payload padding not available without RTX, only generate plain padding on + // the media SSRC. + // Number of padding packets is the requested padding size divided by max + // padding packet size, rounded up. Pure padding packets are always of the + // maximum size. + const size_t kPaddingBytesRequested = kPayloadPacketSize + kRtxHeaderSize; + const size_t kExpectedNumPaddingPackets = + (kPaddingBytesRequested + kMaxPaddingSize - 1) / kMaxPaddingSize; + size_t padding_bytes_sent = 0; + EXPECT_CALL(mock_paced_sender_, EnqueuePacket) + .WillRepeatedly([&](std::unique_ptr packet) { + EXPECT_EQ(packet->packet_type(), RtpPacketToSend::Type::kPadding); + EXPECT_EQ(packet->Ssrc(), kSsrc); + EXPECT_EQ(packet->payload_size(), 0u); + EXPECT_GT(packet->padding_size(), 0u); + padding_bytes_sent += packet->padding_size(); + }); + rtp_sender_->GeneratePadding(kPaddingBytesRequested); + EXPECT_EQ(padding_bytes_sent, kExpectedNumPaddingPackets * kMaxPaddingSize); +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTest, ::testing::Bool());