Adds trial to always send padding packets when not sending video.
This can be used to avoid getting stuck in a state where the encoder
is paused due to low bandwidth estimate which means no additional
feedback is received to update the bandwidth estimate. This could
happen if feedback packets are lost.
Bug: webrtc:8415
Change-Id: I59cd60c0277e8b31a6b911b25e8e488af9008fc2
Reviewed-on: https://webrtc-review.googlesource.com/80880
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Christoffer Rodbro <crodbro@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23632}diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc
index 036c860..883c33f 100644
--- a/modules/pacing/paced_sender.cc
+++ b/modules/pacing/paced_sender.cc
@@ -63,6 +63,8 @@
: clock_(clock),
packet_sender_(packet_sender),
alr_detector_(rtc::MakeUnique<AlrDetector>(event_log)),
+ send_padding_if_silent_(
+ field_trial::IsEnabled("WebRTC-Pacer-PadInSilence")),
paused_(false),
media_budget_(rtc::MakeUnique<IntervalBudget>(0)),
padding_budget_(rtc::MakeUnique<IntervalBudget>(0)),
@@ -263,11 +265,9 @@
<< 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 @@
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 @@
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 @@
}
// 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 @@
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 @@
// 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 @@
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 5ac9ccf..959f6d9 100644
--- a/modules/pacing/paced_sender.h
+++ b/modules/pacing/paced_sender.h
@@ -162,7 +162,7 @@
const Clock* const clock_;
PacketSender* const packet_sender_;
const std::unique_ptr<AlrDetector> 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 62e8c50..a8b9bae 100644
--- a/modules/pacing/paced_sender_unittest.cc
+++ b/modules/pacing/paced_sender_unittest.cc
@@ -141,6 +141,64 @@
std::unique_ptr<PacedSender> 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;
