diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 036c860cf1..883c33fbbe 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -63,6 +63,8 @@ PacedSender::PacedSender(const Clock* clock, : clock_(clock), packet_sender_(packet_sender), alr_detector_(rtc::MakeUnique(event_log)), + send_padding_if_silent_( + field_trial::IsEnabled("WebRTC-Pacer-PadInSilence")), paused_(false), media_budget_(rtc::MakeUnique(0)), padding_budget_(rtc::MakeUnique(0)), @@ -263,11 +265,9 @@ void PacedSender::Process() { << kMaxElapsedTimeMs << " ms"; elapsed_time_ms = kMaxElapsedTimeMs; } - // When congested we send a padding packet every 500 ms to ensure we won't get - // stuck in the congested state due to no feedback being received. - // TODO(srte): Stop sending packet in paused state when pause is no longer - // used for congestion windows. - if (paused_ || Congested()) { + if (send_padding_if_silent_ || paused_ || Congested()) { + // We send a padding packet every 500 ms to ensure we won't get stuck in + // congested state due to no feedback being received. int64_t elapsed_since_last_send_us = now_us - last_send_time_us_; if (elapsed_since_last_send_us >= kCongestedPacketIntervalMs * 1000) { // We can not send padding unless a normal packet has first been sent. If @@ -277,13 +277,13 @@ void PacedSender::Process() { size_t bytes_sent = SendPadding(1, pacing_info); alr_detector_->OnBytesSent(bytes_sent, elapsed_time_ms); } - last_send_time_us_ = clock_->TimeInMicroseconds(); } - return; } + if (paused_) + return; - int target_bitrate_kbps = pacing_bitrate_kbps_; if (elapsed_time_ms > 0) { + int target_bitrate_kbps = pacing_bitrate_kbps_; size_t queue_size_bytes = packets_->SizeInBytes(); if (queue_size_bytes > 0) { // Assuming equal size packets and input/output rate, the average packet @@ -302,8 +302,6 @@ void PacedSender::Process() { UpdateBudgetWithElapsedTime(elapsed_time_ms); } - last_send_time_us_ = clock_->TimeInMicroseconds(); - bool is_probing = prober_->IsProbing(); PacedPacketInfo pacing_info; size_t bytes_sent = 0; @@ -314,7 +312,7 @@ void PacedSender::Process() { } // The paused state is checked in the loop since SendPacket leaves the // critical section allowing the paused state to be changed from other code. - while (!packets_->Empty() && !paused_ && !Congested()) { + while (!packets_->Empty() && !paused_) { // Since we need to release the lock in order to send, we first pop the // element from the priority queue but keep it in storage, so that we can // reinsert it if send fails. @@ -362,8 +360,9 @@ void PacedSender::ProcessThreadAttached(ProcessThread* process_thread) { bool PacedSender::SendPacket(const PacketQueueInterface::Packet& packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK(!paused_); - if (media_budget_->bytes_remaining() == 0 && - pacing_info.probe_cluster_id == PacedPacketInfo::kNotAProbe) { + if (Congested() || + (media_budget_->bytes_remaining() == 0 && + pacing_info.probe_cluster_id == PacedPacketInfo::kNotAProbe)) { return false; } @@ -383,6 +382,7 @@ bool PacedSender::SendPacket(const PacketQueueInterface::Packet& packet, // and we probably don't want to update the budget in such cases. // https://bugs.chromium.org/p/webrtc/issues/detail?id=8052 UpdateBudgetWithBytesSent(packet.bytes); + last_send_time_us_ = clock_->TimeInMicroseconds(); } } @@ -400,6 +400,7 @@ size_t PacedSender::SendPadding(size_t padding_needed, if (bytes_sent > 0) { UpdateBudgetWithBytesSent(bytes_sent); } + last_send_time_us_ = clock_->TimeInMicroseconds(); return bytes_sent; } diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 5ac9ccf98a..959f6d9606 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -162,7 +162,7 @@ class PacedSender : public Pacer { const Clock* const clock_; PacketSender* const packet_sender_; const std::unique_ptr alr_detector_ RTC_PT_GUARDED_BY(critsect_); - + const bool send_padding_if_silent_; rtc::CriticalSection critsect_; bool paused_ RTC_GUARDED_BY(critsect_); // This is the media budget, keeping track of how many bits of media diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc index 62e8c5083d..a8b9baed2e 100644 --- a/modules/pacing/paced_sender_unittest.cc +++ b/modules/pacing/paced_sender_unittest.cc @@ -141,6 +141,64 @@ class PacedSenderTest : public testing::TestWithParam { std::unique_ptr send_bucket_; }; +class PacedSenderFieldTrialTest : public testing::Test { + protected: + struct MediaStream { + const RtpPacketSender::Priority priority; + const uint32_t ssrc; + const size_t packet_size; + uint16_t seq_num; + }; + + const int kProcessIntervalsPerSecond = 1000 / 5; + + PacedSenderFieldTrialTest() : clock_(123456) {} + void InsertPacket(PacedSender* pacer, MediaStream* stream) { + pacer->InsertPacket(stream->priority, stream->ssrc, stream->seq_num++, + clock_.TimeInMilliseconds(), stream->packet_size, + false); + } + void ProcessNext(PacedSender* pacer) { + clock_.AdvanceTimeMilliseconds(5); + pacer->Process(); + } + MediaStream audio{/*priority*/ PacedSender::kHighPriority, + /*ssrc*/ 3333, /*packet_size*/ 100, /*seq_num*/ 1000}; + MediaStream video{/*priority*/ PacedSender::kNormalPriority, + /*ssrc*/ 4444, /*packet_size*/ 1000, /*seq_num*/ 1000}; + SimulatedClock clock_; + MockPacedSenderCallback callback_; +}; + +TEST_F(PacedSenderFieldTrialTest, DefaultNoPaddingInSilence) { + PacedSender pacer(&clock_, &callback_, nullptr); + pacer.SetPacingRates(kTargetBitrateBps, 0); + // Video packet to reset last send time and provide padding data. + InsertPacket(&pacer, &video); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + clock_.AdvanceTimeMilliseconds(5); + pacer.Process(); + EXPECT_CALL(callback_, TimeToSendPadding).Times(0); + // Waiting 500 ms should not trigger sending of padding. + clock_.AdvanceTimeMilliseconds(500); + pacer.Process(); +} + +TEST_F(PacedSenderFieldTrialTest, PaddingInSilenceWithTrial) { + ScopedFieldTrials trial("WebRTC-Pacer-PadInSilence/Enabled/"); + PacedSender pacer(&clock_, &callback_, nullptr); + pacer.SetPacingRates(kTargetBitrateBps, 0); + // Video packet to reset last send time and provide padding data. + InsertPacket(&pacer, &video); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + clock_.AdvanceTimeMilliseconds(5); + pacer.Process(); + EXPECT_CALL(callback_, TimeToSendPadding).WillOnce(Return(1000)); + // Waiting 500 ms should trigger sending of padding. + clock_.AdvanceTimeMilliseconds(500); + pacer.Process(); +} + TEST_F(PacedSenderTest, FirstSentPacketTimeIsSet) { uint16_t sequence_number = 1234; const uint32_t kSsrc = 12345;