From 4a1ec1e639b0e78ca1f189b19205782c4b9ab0af Mon Sep 17 00:00:00 2001 From: philipel Date: Mon, 15 Aug 2016 11:51:06 -0700 Subject: [PATCH] Added ProbeBitrate(bitrate_bps, num_probes) to BitrateProber. Also some minor cleanup. BUG=webrtc:5859 Review-Url: https://codereview.webrtc.org/2243823002 Cr-Commit-Position: refs/heads/master@{#13760} --- webrtc/modules/pacing/bitrate_prober.cc | 59 +++++++++---------- webrtc/modules/pacing/bitrate_prober.h | 11 ++-- .../modules/pacing/bitrate_prober_unittest.cc | 17 +++--- webrtc/modules/pacing/paced_sender.cc | 4 +- 4 files changed, 45 insertions(+), 46 deletions(-) diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc index c5aa983266..4b2a977bbe 100644 --- a/webrtc/modules/pacing/bitrate_prober.cc +++ b/webrtc/modules/pacing/bitrate_prober.cc @@ -10,11 +10,7 @@ #include "webrtc/modules/pacing/bitrate_prober.h" -#include #include -#include -#include -#include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -28,7 +24,7 @@ namespace { constexpr int kInactivityThresholdMs = 5000; int ComputeDeltaFromBitrate(size_t packet_size, uint32_t bitrate_bps) { - assert(bitrate_bps > 0); + RTC_CHECK_GT(bitrate_bps, 0u); // Compute the time delta needed to send packet_size bytes at bitrate_bps // bps. Result is in milliseconds. return static_cast((1000ll * packet_size * 8) / bitrate_bps); @@ -59,41 +55,40 @@ bool BitrateProber::IsProbing() const { return probing_state_ == ProbingState::kActive; } -void BitrateProber::OnIncomingPacket(uint32_t bitrate_bps, - size_t packet_size, - int64_t now_ms) { +void BitrateProber::OnIncomingPacket(size_t packet_size) { // Don't initialize probing unless we have something large enough to start // probing. - if (packet_size < PacedSender::kMinProbePacketSize) - return; - if (probing_state_ != ProbingState::kInactive) - return; - // Max number of packets used for probing. - const int kMaxNumProbes = 2; - const int kPacketsPerProbe = 5; - const float kProbeBitrateMultipliers[kMaxNumProbes] = {3, 6}; - std::stringstream bitrate_log; - bitrate_log << "Start probing for bandwidth, (bitrate:packets): "; - for (int i = 0; i < kMaxNumProbes; ++i) { - ProbeCluster cluster; - // We need one extra to get 5 deltas for the first probe, therefore (i == 0) - cluster.max_probe_packets = kPacketsPerProbe + (i == 0 ? 1 : 0); - cluster.probe_bitrate_bps = kProbeBitrateMultipliers[i] * bitrate_bps; - cluster.id = next_cluster_id_++; - - bitrate_log << "(" << cluster.probe_bitrate_bps << ":" - << cluster.max_probe_packets << ") "; - - clusters_.push(cluster); + if (probing_state_ == ProbingState::kInactive && + packet_size >= PacedSender::kMinProbePacketSize) { + probing_state_ = ProbingState::kActive; } - LOG(LS_INFO) << bitrate_log.str().c_str(); - probing_state_ = ProbingState::kActive; +} + +void BitrateProber::ProbeAtBitrate(uint32_t bitrate_bps, int num_packets) { + ProbeCluster cluster; + cluster.max_probe_packets = num_packets; + cluster.probe_bitrate_bps = bitrate_bps; + cluster.id = next_cluster_id_++; + clusters_.push(cluster); + LOG(LS_INFO) << "Probe cluster (bitrate:packets): (" + << cluster.probe_bitrate_bps << ":" << cluster.max_probe_packets + << ") "; + if (probing_state_ != ProbingState::kActive) + probing_state_ = ProbingState::kInactive; } void BitrateProber::ResetState() { time_last_probe_sent_ms_ = -1; packet_size_last_sent_ = 0; - clusters_ = std::queue(); + + // Recreate all probing clusters. + std::queue clusters; + clusters.swap(clusters_); + while (!clusters.empty()) { + ProbeAtBitrate(clusters.front().probe_bitrate_bps, + clusters.front().max_probe_packets); + clusters.pop(); + } // If its enabled, reset to inactive. if (probing_state_ != ProbingState::kDisabled) probing_state_ = ProbingState::kInactive; diff --git a/webrtc/modules/pacing/bitrate_prober.h b/webrtc/modules/pacing/bitrate_prober.h index c2f9ad8c47..fc0e13fb67 100644 --- a/webrtc/modules/pacing/bitrate_prober.h +++ b/webrtc/modules/pacing/bitrate_prober.h @@ -11,10 +11,9 @@ #ifndef WEBRTC_MODULES_PACING_BITRATE_PROBER_H_ #define WEBRTC_MODULES_PACING_BITRATE_PROBER_H_ -#include -#include #include +#include "webrtc/base/basictypes.h" #include "webrtc/typedefs.h" namespace webrtc { @@ -35,9 +34,11 @@ class BitrateProber { // Initializes a new probing session if the prober is allowed to probe. Does // not initialize the prober unless the packet size is large enough to probe // with. - void OnIncomingPacket(uint32_t bitrate_bps, - size_t packet_size, - int64_t now_ms); + void OnIncomingPacket(size_t packet_size); + + // Create a cluster used to probe for |bitrate_bps| with |num_packets| number + // of packets. + void ProbeAtBitrate(uint32_t bitrate_bps, int num_packets); // Returns the number of milliseconds until the next packet should be sent to // get accurate probing. diff --git a/webrtc/modules/pacing/bitrate_prober_unittest.cc b/webrtc/modules/pacing/bitrate_prober_unittest.cc index ce63781e68..1b2a321573 100644 --- a/webrtc/modules/pacing/bitrate_prober_unittest.cc +++ b/webrtc/modules/pacing/bitrate_prober_unittest.cc @@ -21,10 +21,11 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { int64_t now_ms = 0; EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms)); - prober.SetEnabled(true); + prober.ProbeAtBitrate(900000, 6); + prober.ProbeAtBitrate(1800000, 5); EXPECT_FALSE(prober.IsProbing()); - prober.OnIncomingPacket(300000, 1000, now_ms); + prober.OnIncomingPacket(1000); EXPECT_TRUE(prober.IsProbing()); EXPECT_EQ(0, prober.CurrentClusterId()); @@ -59,10 +60,10 @@ TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) { int64_t now_ms = 0; EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms)); - prober.SetEnabled(true); + prober.ProbeAtBitrate(900000, 6); EXPECT_FALSE(prober.IsProbing()); - prober.OnIncomingPacket(300000, 1000, now_ms); + prober.OnIncomingPacket(1000); EXPECT_TRUE(prober.IsProbing()); EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); prober.PacketSent(now_ms, 1000); @@ -70,16 +71,16 @@ TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) { now_ms += 6000; EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms)); // Insert a small packet, not a candidate for probing. - prober.OnIncomingPacket(300000, 100, now_ms); + prober.OnIncomingPacket(100); prober.PacketSent(now_ms, 100); EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms)); // Insert a large-enough packet after downtime while probing should reset to // perform a new probe since the requested one didn't finish. - prober.OnIncomingPacket(300000, 1000, now_ms); + prober.OnIncomingPacket(1000); EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); prober.PacketSent(now_ms, 1000); // Next packet should be part of new probe and be sent with non-zero delay. - prober.OnIncomingPacket(300000, 1000, now_ms); + prober.OnIncomingPacket(1000); EXPECT_GT(prober.TimeUntilNextProbe(now_ms), 0); } @@ -88,7 +89,7 @@ TEST(BitrateProberTest, DoesntInitializeProbingForSmallPackets) { prober.SetEnabled(true); EXPECT_FALSE(prober.IsProbing()); - prober.OnIncomingPacket(300000, 100, 0); + prober.OnIncomingPacket(100); EXPECT_FALSE(prober.IsProbing()); } diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index a3cb69f2e1..bb071f1656 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -261,6 +261,8 @@ PacedSender::PacedSender(Clock* clock, PacketSender* packet_sender) packets_(new paced_sender::PacketQueue(clock)), packet_counter_(0) { UpdateBytesPerInterval(kMinPacketLimitMs); + prober_->ProbeAtBitrate(900000, 6); + prober_->ProbeAtBitrate(1800000, 5); } PacedSender::~PacedSender() {} @@ -318,7 +320,7 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority, << "SetEstimatedBitrate must be called before InsertPacket."; int64_t now_ms = clock_->TimeInMilliseconds(); - prober_->OnIncomingPacket(estimated_bitrate_bps_, bytes, now_ms); + prober_->OnIncomingPacket(bytes); if (capture_time_ms < 0) capture_time_ms = now_ms;