Enable bitrate probing by default in PacedSender.
BUG=crbug:425925
R=mflodman@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/33359004
Cr-Commit-Position: refs/heads/master@{#8379}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8379 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc
index 51c87fb..5475ef3 100644
--- a/webrtc/modules/pacing/bitrate_prober.cc
+++ b/webrtc/modules/pacing/bitrate_prober.cc
@@ -81,7 +81,7 @@
}
if (probe_bitrates_.empty()) {
// No probe started, or waiting for next probe.
- return std::numeric_limits<int>::max();
+ return -1;
}
int64_t elapsed_time_ms = now_ms - time_last_send_ms_;
// We will send the first probe packet immediately if no packet has been
diff --git a/webrtc/modules/pacing/bitrate_prober_unittest.cc b/webrtc/modules/pacing/bitrate_prober_unittest.cc
index fac6a72..c966f5c 100644
--- a/webrtc/modules/pacing/bitrate_prober_unittest.cc
+++ b/webrtc/modules/pacing/bitrate_prober_unittest.cc
@@ -19,7 +19,7 @@
BitrateProber prober;
EXPECT_FALSE(prober.IsProbing());
int64_t now_ms = 0;
- EXPECT_EQ(std::numeric_limits<int>::max(), prober.TimeUntilNextProbe(now_ms));
+ EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
prober.SetEnabled(true);
EXPECT_FALSE(prober.IsProbing());
@@ -45,7 +45,7 @@
prober.PacketSent(now_ms, 1000);
}
- EXPECT_EQ(std::numeric_limits<int>::max(), prober.TimeUntilNextProbe(now_ms));
+ EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
EXPECT_FALSE(prober.IsProbing());
}
} // namespace webrtc
diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h
index ab3ce7f..76d8b60 100644
--- a/webrtc/modules/pacing/include/paced_sender.h
+++ b/webrtc/modules/pacing/include/paced_sender.h
@@ -87,6 +87,11 @@
// Resume sending packets.
void Resume();
+ // Enable bitrate probing. Enabled by default, mostly here to simplify
+ // testing. Must be called before any packets are being sent to have an
+ // effect.
+ void SetProbingEnabled(bool enabled);
+
// Set target bitrates for the pacer.
// We will pace out bursts of packets at a bitrate of |max_bitrate_kbps|.
// |bitrate_kbps| is our estimate of what we are allowed to send on average.
@@ -121,9 +126,6 @@
// Process any pending packets in the queue(s).
virtual 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)
@@ -139,6 +141,7 @@
scoped_ptr<CriticalSectionWrapper> critsect_;
bool enabled_ GUARDED_BY(critsect_);
bool paused_ GUARDED_BY(critsect_);
+ bool probing_enabled_;
// This is the media budget, keeping track of how many bits of media
// we can pace out during the current interval.
scoped_ptr<paced_sender::IntervalBudget> media_budget_ GUARDED_BY(critsect_);
@@ -154,7 +157,7 @@
int64_t time_last_update_us_ GUARDED_BY(critsect_);
scoped_ptr<paced_sender::PacketQueue> packets_ GUARDED_BY(critsect_);
- uint64_t packet_counter_ GUARDED_BY(critsect_);
+ uint64_t packet_counter_;
};
} // namespace webrtc
#endif // WEBRTC_MODULES_PACING_INCLUDE_PACED_SENDER_H_
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index 4b96074..6186f96 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -90,9 +90,9 @@
virtual ~PacketQueue() {}
void Push(const Packet& packet) {
- if (!AddToDupeSet(packet))
+ if (!AddToDupeSet(packet)) {
return;
-
+ }
// Store packet in list, use pointers in priority queue for cheaper moves.
// Packets have a handle to its own iterator in the list, for easy removal
// when popping from queue.
@@ -215,6 +215,7 @@
critsect_(CriticalSectionWrapper::CreateCriticalSection()),
enabled_(true),
paused_(false),
+ probing_enabled_(true),
media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)),
padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)),
prober_(new BitrateProber()),
@@ -237,6 +238,11 @@
paused_ = false;
}
+void PacedSender::SetProbingEnabled(bool enabled) {
+ assert(packet_counter_ == 0);
+ probing_enabled_ = enabled;
+}
+
void PacedSender::SetStatus(bool enable) {
CriticalSectionScoped cs(critsect_.get());
enabled_ = enable;
@@ -264,8 +270,7 @@
if (!enabled_) {
return true; // We can send now.
}
- // Enable probing if the probing experiment is enabled.
- if (!prober_->IsProbing() && ProbingExperimentIsEnabled()) {
+ if (probing_enabled_ && !prober_->IsProbing()) {
prober_->SetEnabled(true);
}
prober_->MaybeInitializeProbe(bitrate_bps_);
@@ -305,7 +310,10 @@
int64_t PacedSender::TimeUntilNextProcess() {
CriticalSectionScoped cs(critsect_.get());
if (prober_->IsProbing()) {
- return prober_->TimeUntilNextProbe(clock_->TimeInMilliseconds());
+ int64_t ret = prober_->TimeUntilNextProbe(clock_->TimeInMilliseconds());
+ if (ret >= 0) {
+ return ret;
+ }
}
int64_t elapsed_time_us = clock_->TimeInMicroseconds() - time_last_update_us_;
int64_t elapsed_time_ms = (elapsed_time_us + 500) / 1000;
@@ -325,10 +333,10 @@
int64_t delta_time_ms = std::min(kMaxIntervalTimeMs, elapsed_time_ms);
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;
+ }
// 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
@@ -337,8 +345,9 @@
if (SendPacket(packet)) {
// Send succeeded, remove it from the queue.
packets_->FinalizePop(packet);
- if (prober_->IsProbing())
+ if (prober_->IsProbing()) {
return 0;
+ }
} else {
// Send failed, put it back into the queue.
packets_->CancelPop(packet);
@@ -386,9 +395,4 @@
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 4303b64..29b856c 100644
--- a/webrtc/modules/pacing/paced_sender_unittest.cc
+++ b/webrtc/modules/pacing/paced_sender_unittest.cc
@@ -108,6 +108,10 @@
kTargetBitrate,
kPaceMultiplier * kTargetBitrate,
0));
+ // Default to bitrate probing disabled for testing purposes. Probing tests
+ // have to enable probing, either by creating a new PacedSender instance or
+ // by calling SetProbingEnabled(true).
+ send_bucket_->SetProbingEnabled(false);
}
void SendAndExpectPacket(PacedSender::Priority priority,
@@ -418,6 +422,7 @@
PacedSenderPadding callback;
send_bucket_.reset(new PacedSender(
&clock_, &callback, kTargetBitrate, kPaceMultiplier * kTargetBitrate, 0));
+ send_bucket_->SetProbingEnabled(false);
send_bucket_->UpdateBitrate(
kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate);
int64_t start_time = clock_.TimeInMilliseconds();
@@ -697,22 +702,6 @@
EXPECT_EQ(0, send_bucket_->QueueInMs());
}
-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) {}
-
- virtual bool ProbingExperimentIsEnabled() const OVERRIDE { return true; }
-};
-
TEST_F(PacedSenderTest, ProbingWithInitialFrame) {
const int kNumPackets = 11;
const int kNumDeltas = kNumPackets - 1;
@@ -725,12 +714,15 @@
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 ProbingPacedSender(&clock_,
- &callback,
- kInitialBitrateKbps,
- kPaceMultiplier * kInitialBitrateKbps,
- 0));
+ new PacedSender(&clock_,
+ &callback,
+ kInitialBitrateKbps,
+ kPaceMultiplier * kInitialBitrateKbps,
+ 0));
+
for (int i = 0; i < kNumPackets; ++i) {
EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
ssrc,
diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h
index d5ea3e8..227be19 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h
+++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h
@@ -85,27 +85,11 @@
int64_t rtt) OVERRIDE;
private:
- 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) {}
-
- virtual bool ProbingExperimentIsEnabled() const OVERRIDE { return true; }
- };
-
int64_t TimeUntilNextProcess(const std::list<Module*>& modules);
void CallProcess(const std::list<Module*>& modules);
void QueuePackets(Packets* batch, int64_t end_of_batch_time_us);
- ProbingPacedSender pacer_;
+ PacedSender pacer_;
Packets queue_;
Packets pacer_queue_;
