Use padding to achieve bitrate probing if the initial key frame has too few packets.
BUG=4350
R=mflodman@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/44879004
Cr-Commit-Position: refs/heads/master@{#9134}
diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc
index 5475ef3..1ed6298 100644
--- a/webrtc/modules/pacing/bitrate_prober.cc
+++ b/webrtc/modules/pacing/bitrate_prober.cc
@@ -11,6 +11,7 @@
#include "webrtc/modules/pacing/bitrate_prober.h"
#include <assert.h>
+#include <algorithm>
#include <limits>
#include <sstream>
@@ -107,7 +108,11 @@
time_until_probe_ms = 0;
}
}
- return time_until_probe_ms;
+ return std::max(time_until_probe_ms, 0);
+}
+
+size_t BitrateProber::RecommendedPacketSize() const {
+ return packet_size_last_send_;
}
void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) {
diff --git a/webrtc/modules/pacing/bitrate_prober.h b/webrtc/modules/pacing/bitrate_prober.h
index 04a8580..b3f52af 100644
--- a/webrtc/modules/pacing/bitrate_prober.h
+++ b/webrtc/modules/pacing/bitrate_prober.h
@@ -38,6 +38,10 @@
// get accurate probing.
int TimeUntilNextProbe(int64_t now_ms);
+ // Returns the number of bytes that the prober recommends for the next probe
+ // packet.
+ size_t RecommendedPacketSize() const;
+
// Called to report to the prober that a packet has been sent, which helps the
// prober know when to move to the next packet in a probe.
void PacketSent(int64_t now_ms, size_t packet_size);
diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h
index 645999d..730d3b7 100644
--- a/webrtc/modules/pacing/include/paced_sender.h
+++ b/webrtc/modules/pacing/include/paced_sender.h
@@ -126,6 +126,9 @@
// Process any pending packets in the queue(s).
int32_t Process() override;
+ protected:
+ virtual bool ProbingExperimentIsEnabled() const;
+
private:
// Updates the number of bytes that can be sent for the next time interval.
void UpdateBytesPerInterval(int64_t delta_time_in_ms)
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index 6186f96..f251341 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -193,7 +193,9 @@
-500 * target_rate_kbps_ / 8);
}
- int bytes_remaining() const { return bytes_remaining_; }
+ size_t bytes_remaining() const {
+ return static_cast<size_t>(std::max(0, bytes_remaining_));
+ }
int target_rate_kbps() const { return target_rate_kbps_; }
@@ -334,7 +336,7 @@
UpdateBytesPerInterval(delta_time_ms);
}
while (!packets_->Empty()) {
- if (media_budget_->bytes_remaining() <= 0 && !prober_->IsProbing()) {
+ if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing()) {
return 0;
}
@@ -355,10 +357,14 @@
}
}
- int padding_needed = padding_budget_->bytes_remaining();
- if (padding_needed > 0) {
+ size_t padding_needed;
+ if (prober_->IsProbing() && ProbingExperimentIsEnabled())
+ padding_needed = prober_->RecommendedPacketSize();
+ else
+ padding_needed = padding_budget_->bytes_remaining();
+
+ if (padding_needed > 0)
SendPadding(static_cast<size_t>(padding_needed));
- }
}
return 0;
}
@@ -386,13 +392,20 @@
size_t bytes_sent = callback_->TimeToSendPadding(padding_needed);
critsect_->Enter();
- // Update padding bytes sent.
- media_budget_->UseBudget(bytes_sent);
- padding_budget_->UseBudget(bytes_sent);
+ if (bytes_sent > 0) {
+ prober_->PacketSent(clock_->TimeInMilliseconds(), bytes_sent);
+ media_budget_->UseBudget(bytes_sent);
+ padding_budget_->UseBudget(bytes_sent);
+ }
}
void PacedSender::UpdateBytesPerInterval(int64_t delta_time_ms) {
media_budget_->IncreaseBudget(delta_time_ms);
padding_budget_->IncreaseBudget(delta_time_ms);
}
+
+bool PacedSender::ProbingExperimentIsEnabled() const {
+ return webrtc::field_trial::FindFullName("WebRTC-BitrateProbing") ==
+ "Enabled";
+}
} // namespace webrtc
diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc
index e82a49e..0730062 100644
--- a/webrtc/modules/pacing/paced_sender_unittest.cc
+++ b/webrtc/modules/pacing/paced_sender_unittest.cc
@@ -71,22 +71,26 @@
uint16_t sequence_number,
int64_t capture_time_ms,
bool retransmission) {
+ ExpectAndCountPacket();
+ return true;
+ }
+
+ size_t TimeToSendPadding(size_t bytes) {
+ ExpectAndCountPacket();
+ return bytes;
+ }
+
+ void ExpectAndCountPacket() {
++packets_sent_;
EXPECT_FALSE(expected_deltas_.empty());
if (expected_deltas_.empty())
- return false;
+ return;
int64_t now_ms = clock_->TimeInMilliseconds();
if (prev_packet_time_ms_ >= 0) {
EXPECT_EQ(expected_deltas_.front(), now_ms - prev_packet_time_ms_);
expected_deltas_.pop_front();
}
prev_packet_time_ms_ = now_ms;
- return true;
- }
-
- size_t TimeToSendPadding(size_t bytes) {
- EXPECT_TRUE(false);
- return bytes;
}
int packets_sent() const { return packets_sent_; }
@@ -714,8 +718,6 @@
std::list<int> expected_deltas_list(expected_deltas,
expected_deltas + kNumPackets - 1);
PacedSenderProbing callback(expected_deltas_list, &clock_);
- // Probing implicitly enabled by creating a new PacedSender which defaults to
- // probing on.
send_bucket_.reset(
new PacedSender(&clock_,
&callback,
@@ -741,6 +743,58 @@
}
}
+class ProbingPacedSender : public PacedSender {
+ public:
+ ProbingPacedSender(Clock* clock,
+ Callback* callback,
+ int bitrate_kbps,
+ int max_bitrate_kbps,
+ int min_bitrate_kbps)
+ : PacedSender(clock,
+ callback,
+ bitrate_kbps,
+ max_bitrate_kbps,
+ min_bitrate_kbps) {}
+
+ bool ProbingExperimentIsEnabled() const override { return true; }
+};
+
+TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) {
+ const int kNumPackets = 11;
+ const int kNumDeltas = kNumPackets - 1;
+ const size_t kPacketSize = 1200;
+ const int kInitialBitrateKbps = 300;
+ uint32_t ssrc = 12346;
+ uint16_t sequence_number = 1234;
+ const int expected_deltas[kNumDeltas] = {10, 10, 10, 10, 10, 5, 5, 5, 5, 5};
+ std::list<int> expected_deltas_list(expected_deltas,
+ expected_deltas + kNumPackets - 1);
+ PacedSenderProbing callback(expected_deltas_list, &clock_);
+ send_bucket_.reset(
+ new ProbingPacedSender(&clock_, &callback, kInitialBitrateKbps,
+ kPaceMultiplier * kInitialBitrateKbps, 0));
+
+ for (int i = 0; i < kNumPackets - 5; ++i) {
+ EXPECT_FALSE(send_bucket_->SendPacket(
+ PacedSender::kNormalPriority, ssrc, sequence_number++,
+ clock_.TimeInMilliseconds(), kPacketSize, false));
+ }
+ while (callback.packets_sent() < kNumPackets) {
+ int time_until_process = send_bucket_->TimeUntilNextProcess();
+ if (time_until_process <= 0) {
+ send_bucket_->Process();
+ } else {
+ clock_.AdvanceTimeMilliseconds(time_until_process);
+ }
+ }
+
+ // Process one more time and make sure we don't send any more probes.
+ int time_until_process = send_bucket_->TimeUntilNextProcess();
+ clock_.AdvanceTimeMilliseconds(time_until_process);
+ send_bucket_->Process();
+ EXPECT_EQ(kNumPackets, callback.packets_sent());
+}
+
TEST_F(PacedSenderTest, PriorityInversion) {
uint32_t ssrc = 12346;
uint16_t sequence_number = 1234;
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index c260567..1b4e990 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -403,7 +403,8 @@
if (sent_rtp_packets_ % kPacketsBetweenLossBursts == 0)
packets_left_to_drop_ = kLossBurstSize;
- if (packets_left_to_drop_ > 0) {
+ // Never drop padding packets as those won't be retransmitted.
+ if (packets_left_to_drop_ > 0 && header.paddingLength == 0) {
--packets_left_to_drop_;
dropped_packets_.insert(header.sequenceNumber);
return DROP_PACKET;
@@ -463,11 +464,15 @@
RTPHeader header;
EXPECT_TRUE(parser_->Parse(packet, length, &header));
- EXPECT_EQ(kRedPayloadType, header.payloadType);
- int encapsulated_payload_type =
- static_cast<int>(packet[header.headerLength]);
- if (encapsulated_payload_type != kFakeSendPayloadType)
- EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+ int encapsulated_payload_type = -1;
+ if (header.payloadType == kRedPayloadType) {
+ encapsulated_payload_type =
+ static_cast<int>(packet[header.headerLength]);
+ if (encapsulated_payload_type != kFakeSendPayloadType)
+ EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+ } else {
+ EXPECT_EQ(kFakeSendPayloadType, header.payloadType);
+ }
if (protected_sequence_numbers_.count(header.sequenceNumber) != 0) {
// Retransmitted packet, should not count.
@@ -571,12 +576,16 @@
Action OnSendRtp(const uint8_t* packet, size_t length) override {
RTPHeader header;
EXPECT_TRUE(parser_->Parse(packet, length, &header));
- EXPECT_EQ(kRedPayloadType, header.payloadType);
- int encapsulated_payload_type =
- static_cast<int>(packet[header.headerLength]);
- if (encapsulated_payload_type != kFakeSendPayloadType)
- EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+ int encapsulated_payload_type = -1;
+ if (header.payloadType == kRedPayloadType) {
+ encapsulated_payload_type =
+ static_cast<int>(packet[header.headerLength]);
+ if (encapsulated_payload_type != kFakeSendPayloadType)
+ EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+ } else {
+ EXPECT_EQ(kFakeSendPayloadType, header.payloadType);
+ }
if (has_last_sequence_number_ &&
!IsNewerSequenceNumber(header.sequenceNumber,
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc
index eb79e1b..547c811 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -306,14 +306,22 @@
EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr));
}
- EXPECT_EQ(kRedPayloadType, header.payloadType);
-
- uint8_t encapsulated_payload_type = packet[header.headerLength];
-
- if (encapsulated_payload_type == kUlpfecPayloadType) {
- received_fec_ = true;
+ int encapsulated_payload_type = -1;
+ if (header.payloadType == kRedPayloadType) {
+ encapsulated_payload_type =
+ static_cast<int>(packet[header.headerLength]);
+ if (encapsulated_payload_type != kFakeSendPayloadType)
+ EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
} else {
- received_media_ = true;
+ EXPECT_EQ(kFakeSendPayloadType, header.payloadType);
+ }
+
+ if (encapsulated_payload_type != -1) {
+ if (encapsulated_payload_type == kUlpfecPayloadType) {
+ received_fec_ = true;
+ } else {
+ received_media_ = true;
+ }
}
if (received_media_ && received_fec_)
