From 1a93cdeabe896cf8d2c34609e0e9781a37894368 Mon Sep 17 00:00:00 2001 From: philipel Date: Fri, 3 Jun 2016 01:41:45 -0700 Subject: [PATCH] BitrateProber now correctly change state to kWait when the last probing cluster has been spent. BUG=webrtc:5859 Review-Url: https://codereview.webrtc.org/2033073002 Cr-Commit-Position: refs/heads/master@{#13027} --- webrtc/modules/pacing/bitrate_prober.cc | 2 ++ webrtc/modules/pacing/paced_sender.cc | 11 ++++++----- webrtc/modules/pacing/paced_sender_unittest.cc | 15 +++++++++++---- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc index 8e8e36ea34..19eaff7a69 100644 --- a/webrtc/modules/pacing/bitrate_prober.cc +++ b/webrtc/modules/pacing/bitrate_prober.cc @@ -154,6 +154,8 @@ void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) { ++cluster->sent_probe_packets; if (cluster->sent_probe_packets == cluster->max_probe_packets) clusters_.pop(); + if (clusters_.empty()) + probing_state_ = kWait; } } } // namespace webrtc diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 58c7d2d368..efbf969ac1 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -390,10 +390,11 @@ void PacedSender::Process() { UpdateBytesPerInterval(delta_time_ms); } - int probe_cluster_id = prober_->IsProbing() ? prober_->CurrentClusterId() - : PacketInfo::kNotAProbe; + bool is_probing = prober_->IsProbing(); + int probe_cluster_id = is_probing ? prober_->CurrentClusterId() + : PacketInfo::kNotAProbe; while (!packets_->Empty()) { - if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing()) + if (media_budget_->bytes_remaining() == 0 && !is_probing) return; // Since we need to release the lock in order to send, we first pop the @@ -404,7 +405,7 @@ void PacedSender::Process() { if (SendPacket(packet, probe_cluster_id)) { // Send succeeded, remove it from the queue. packets_->FinalizePop(packet); - if (prober_->IsProbing()) + if (is_probing) return; } else { // Send failed, put it back into the queue. @@ -418,7 +419,7 @@ void PacedSender::Process() { return; size_t padding_needed; - if (prober_->IsProbing()) { + if (is_probing) { padding_needed = prober_->RecommendedPacketSize(); } else { padding_needed = padding_budget_->bytes_remaining(); diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index f698555ce3..063ec1e194 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -870,11 +870,12 @@ TEST_F(PacedSenderTest, AverageQueueTime) { EXPECT_EQ(0, send_bucket_->AverageQueueTimeMs()); } -TEST_F(PacedSenderTest, DISABLED_ProbeClusterId) { +TEST_F(PacedSenderTest, ProbeClusterId) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; const size_t kPacketSize = 1200; + send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); send_bucket_->SetProbingEnabled(true); for (int i = 0; i < 11; ++i) { send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, @@ -886,18 +887,24 @@ TEST_F(PacedSenderTest, DISABLED_ProbeClusterId) { EXPECT_CALL(callback_, TimeToSendPacket(_, _, _, _, 0)) .Times(6) .WillRepeatedly(Return(true)); - for (int i = 0; i < 6; ++i) + for (int i = 0; i < 6; ++i) { + clock_.AdvanceTimeMilliseconds(20); send_bucket_->Process(); + } // Second probing cluster. EXPECT_CALL(callback_, TimeToSendPacket(_, _, _, _, 1)) .Times(5) .WillRepeatedly(Return(true)); - for (int i = 0; i < 5; ++i) + for (int i = 0; i < 5; ++i) { + clock_.AdvanceTimeMilliseconds(20); send_bucket_->Process(); + } // No more probing packets. - EXPECT_CALL(callback_, TimeToSendPadding(_, _)).Times(1); + EXPECT_CALL(callback_, TimeToSendPadding(_, PacketInfo::kNotAProbe)) + .Times(1) + .WillRepeatedly(Return(500)); send_bucket_->Process(); }