diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc index 41ad5fa11a..fbd9b81741 100644 --- a/webrtc/modules/pacing/bitrate_prober.cc +++ b/webrtc/modules/pacing/bitrate_prober.cc @@ -52,7 +52,13 @@ bool BitrateProber::IsProbing() const { return probing_state_ == kProbing; } -void BitrateProber::MaybeInitializeProbe(int bitrate_bps) { +void BitrateProber::OnIncomingPacket(int bitrate_bps, + size_t packet_size, + int64_t now_ms) { + // Don't initialize probing unless we have something large enough to start + // probing. + if (packet_size < PacedSender::kMinProbePacketSize) + return; if (probing_state_ != kAllowedToProbe) return; probe_bitrates_.clear(); @@ -74,6 +80,9 @@ void BitrateProber::MaybeInitializeProbe(int bitrate_bps) { } bitrate_log << ", num packets: " << probe_bitrates_.size(); LOG(LS_INFO) << bitrate_log.str().c_str(); + // Set last send time to current time so TimeUntilNextProbe doesn't short + // circuit due to inactivity. + time_last_send_ms_ = now_ms; probing_state_ = kProbing; } @@ -81,16 +90,23 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) { if (probing_state_ != kDisabled && probe_bitrates_.empty()) { probing_state_ = kWait; } - if (probe_bitrates_.empty()) { - // No probe started, or waiting for next probe. + if (probe_bitrates_.empty() || time_last_send_ms_ == -1) { + // No probe started, probe finished, or too long since last probe packet. return -1; } int64_t elapsed_time_ms = now_ms - time_last_send_ms_; + // If no packets have been sent for n milliseconds, temporarily deactivate to + // not keep spinning. + static const int kInactiveSendDeltaMs = 5000; + if (elapsed_time_ms > kInactiveSendDeltaMs) { + time_last_send_ms_ = -1; + probing_state_ = kAllowedToProbe; + return -1; + } // We will send the first probe packet immediately if no packet has been // sent before. int time_until_probe_ms = 0; - if (packet_size_last_send_ > PacedSender::kMinProbePacketSize && - probing_state_ == kProbing) { + if (packet_size_last_send_ != 0 && probing_state_ == kProbing) { int next_delta_ms = ComputeDeltaFromBitrate(packet_size_last_send_, probe_bitrates_.front()); time_until_probe_ms = next_delta_ms - elapsed_time_ms; @@ -119,6 +135,8 @@ size_t BitrateProber::RecommendedPacketSize() const { void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) { assert(packet_size > 0); + if (packet_size < PacedSender::kMinProbePacketSize) + return; packet_size_last_send_ = packet_size; time_last_send_ms_ = now_ms; if (probing_state_ != kProbing) diff --git a/webrtc/modules/pacing/bitrate_prober.h b/webrtc/modules/pacing/bitrate_prober.h index b3f52afeb6..84fbc522fc 100644 --- a/webrtc/modules/pacing/bitrate_prober.h +++ b/webrtc/modules/pacing/bitrate_prober.h @@ -31,8 +31,10 @@ class BitrateProber { // TimeUntilNextProbe(). bool IsProbing() const; - // Initializes a new probing session if the prober is allowed to probe. - void MaybeInitializeProbe(int bitrate_bps); + // 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(int bitrate_bps, size_t packet_size, int64_t now_ms); // 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 c966f5cfa8..59ee479973 100644 --- a/webrtc/modules/pacing/bitrate_prober_unittest.cc +++ b/webrtc/modules/pacing/bitrate_prober_unittest.cc @@ -24,9 +24,10 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { prober.SetEnabled(true); EXPECT_FALSE(prober.IsProbing()); - prober.MaybeInitializeProbe(300000); + prober.OnIncomingPacket(300000, 1000, now_ms); EXPECT_TRUE(prober.IsProbing()); + // First packet should probe as soon as possible. EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); prober.PacketSent(now_ms, 1000); @@ -48,4 +49,43 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms)); EXPECT_FALSE(prober.IsProbing()); } + +TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) { + BitrateProber prober; + EXPECT_FALSE(prober.IsProbing()); + int64_t now_ms = 0; + EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms)); + + prober.SetEnabled(true); + EXPECT_FALSE(prober.IsProbing()); + + prober.OnIncomingPacket(300000, 1000, now_ms); + EXPECT_TRUE(prober.IsProbing()); + EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); + // Let time pass, no large enough packets put into prober. + 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.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); + 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); + EXPECT_GT(prober.TimeUntilNextProbe(now_ms), 0); +} + +TEST(BitrateProberTest, DoesntInitializeProbingForSmallPackets) { + BitrateProber prober; + prober.SetEnabled(true); + EXPECT_FALSE(prober.IsProbing()); + + prober.OnIncomingPacket(300000, 100, 0); + EXPECT_FALSE(prober.IsProbing()); +} + } // namespace webrtc diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 23b172c666..c32d365668 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -323,9 +323,9 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority, if (probing_enabled_ && !prober_->IsProbing()) prober_->SetEnabled(true); - prober_->MaybeInitializeProbe(bitrate_bps_); - int64_t now_ms = clock_->TimeInMilliseconds(); + prober_->OnIncomingPacket(bitrate_bps_, bytes, now_ms); + if (capture_time_ms < 0) capture_time_ms = now_ms;