From 3dd33a6787d15951fb51dda8443595d542ca560e Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Thu, 22 Jan 2015 09:12:23 +0000 Subject: [PATCH] Fix bug in thresholds for bitrate probing and adjust thresholds to allow a larger dispersion and concentration for successful probes. BUG=crbug:425925 R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/38629004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@8121 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../remote_bitrate_estimator_abs_send_time.cc | 143 +++++++++++------- ...itrate_estimator_abs_send_time_unittest.cc | 96 ++++++++++-- 2 files changed, 175 insertions(+), 64 deletions(-) diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index 98f5893d3..c35757785 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc @@ -67,6 +67,42 @@ std::vector Keys(const std::map& map) { return keys; } +struct Probe { + Probe(int64_t send_time_ms, int64_t recv_time_ms, size_t payload_size) + : send_time_ms(send_time_ms), + recv_time_ms(recv_time_ms), + payload_size(payload_size) {} + int64_t send_time_ms; + int64_t recv_time_ms; + size_t payload_size; +}; + +struct Cluster { + Cluster() + : send_mean_ms(0.0f), + recv_mean_ms(0.0f), + mean_size(0), + count(0), + num_above_min_delta(0) {} + + int GetSendBitrateBps() const { + assert(send_mean_ms > 0); + return mean_size * 8 * 1000 / send_mean_ms; + } + + int GetRecvBitrateBps() const { + assert(recv_mean_ms > 0); + return mean_size * 8 * 1000 / recv_mean_ms; + } + + float send_mean_ms; + float recv_mean_ms; + // TODO(holmer): Add some variance metric as well? + size_t mean_size; + int count; + int num_above_min_delta; +}; + class RemoteBitrateEstimatorAbsSendTimeImpl : public RemoteBitrateEstimator { public: RemoteBitrateEstimatorAbsSendTimeImpl(RemoteBitrateObserver* observer, @@ -93,31 +129,6 @@ class RemoteBitrateEstimatorAbsSendTimeImpl : public RemoteBitrateEstimator { private: typedef std::map Ssrcs; - struct Probe { - Probe(int64_t send_time_ms, int64_t recv_time_ms, size_t payload_size) - : send_time_ms(send_time_ms), - recv_time_ms(recv_time_ms), - payload_size(payload_size) {} - int64_t send_time_ms; - int64_t recv_time_ms; - size_t payload_size; - }; - - struct Cluster { - Cluster() - : send_mean_ms(0.0f), - recv_mean_ms(0.0f), - mean_size(0), - count(0), - num_above_min_delta(0) {} - - float send_mean_ms; - float recv_mean_ms; - // TODO(holmer): Add some variance metric as well? - size_t mean_size; - int count; - int num_above_min_delta; - }; static bool IsWithinClusterBounds(int send_delta_ms, const Cluster& cluster_aggregate) { @@ -151,11 +162,16 @@ class RemoteBitrateEstimatorAbsSendTimeImpl : public RemoteBitrateEstimator { void ComputeClusters(std::list* clusters) const; - int FindBestProbeBitrate(const std::list& clusters) const; + std::list::const_iterator FindBestProbe( + const std::list& clusters) const + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_.get()); void ProcessClusters(int64_t now_ms) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_.get()); + bool IsBitrateImproving(int probe_bitrate_bps) const + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_.get()); + scoped_ptr crit_sect_; RemoteBitrateObserver* observer_ GUARDED_BY(crit_sect_.get()); Clock* clock_; @@ -172,6 +188,7 @@ class RemoteBitrateEstimatorAbsSendTimeImpl : public RemoteBitrateEstimator { int total_propagation_delta_ms_ GUARDED_BY(crit_sect_.get()); std::list probes_; + size_t total_probes_received_; int64_t first_packet_time_ms_; DISALLOW_IMPLICIT_CONSTRUCTORS(RemoteBitrateEstimatorAbsSendTimeImpl); @@ -194,6 +211,7 @@ RemoteBitrateEstimatorAbsSendTimeImpl::RemoteBitrateEstimatorAbsSendTimeImpl( last_process_time_(-1), process_interval_ms_(kProcessIntervalMs), total_propagation_delta_ms_(0), + total_probes_received_(0), first_packet_time_ms_(-1) { assert(observer_); assert(clock_); @@ -210,7 +228,7 @@ void RemoteBitrateEstimatorAbsSendTimeImpl::ComputeClusters( if (prev_send_time >= 0) { int send_delta_ms = it->send_time_ms - prev_send_time; int recv_delta_ms = it->recv_time_ms - prev_recv_time; - if (send_delta_ms > 1 && recv_delta_ms > 1) { + if (send_delta_ms >= 1 && recv_delta_ms >= 1) { ++current.num_above_min_delta; } if (!IsWithinClusterBounds(send_delta_ms, current)) { @@ -230,9 +248,11 @@ void RemoteBitrateEstimatorAbsSendTimeImpl::ComputeClusters( AddCluster(clusters, ¤t); } -int RemoteBitrateEstimatorAbsSendTimeImpl::FindBestProbeBitrate( +std::list::const_iterator +RemoteBitrateEstimatorAbsSendTimeImpl::FindBestProbe( const std::list& clusters) const { int highest_probe_bitrate_bps = 0; + std::list::const_iterator best_it = clusters.end(); for (std::list::const_iterator it = clusters.begin(); it != clusters.end(); ++it) { @@ -241,16 +261,14 @@ int RemoteBitrateEstimatorAbsSendTimeImpl::FindBestProbeBitrate( int send_bitrate_bps = it->mean_size * 8 * 1000 / it->send_mean_ms; int recv_bitrate_bps = it->mean_size * 8 * 1000 / it->recv_mean_ms; if (it->num_above_min_delta > it->count / 2 && - (it->recv_mean_ms - it->send_mean_ms <= 1.5f || - it->send_mean_ms - it->recv_mean_ms >= 3.0f)) { - int probe_bitrate_bps = std::min(send_bitrate_bps, recv_bitrate_bps); - if (probe_bitrate_bps > highest_probe_bitrate_bps) + (it->recv_mean_ms - it->send_mean_ms <= 2.0f && + it->send_mean_ms - it->recv_mean_ms <= 5.0f)) { + int probe_bitrate_bps = + std::min(it->GetSendBitrateBps(), it->GetRecvBitrateBps()); + if (probe_bitrate_bps > highest_probe_bitrate_bps) { highest_probe_bitrate_bps = probe_bitrate_bps; - LOG(LS_INFO) << "Probe successful, sent at " << send_bitrate_bps - << " bps, received at " << recv_bitrate_bps - << " bps. Mean send delta: " << it->send_mean_ms - << " ms, mean recv delta: " << it->recv_mean_ms - << " ms, num probes: " << it->count; + best_it = it; + } } else { LOG(LS_INFO) << "Probe failed, sent at " << send_bitrate_bps << " bps, received at " << recv_bitrate_bps @@ -260,7 +278,7 @@ int RemoteBitrateEstimatorAbsSendTimeImpl::FindBestProbeBitrate( break; } } - return highest_probe_bitrate_bps; + return best_it; } void RemoteBitrateEstimatorAbsSendTimeImpl::ProcessClusters(int64_t now_ms) { @@ -273,24 +291,37 @@ void RemoteBitrateEstimatorAbsSendTimeImpl::ProcessClusters(int64_t now_ms) { probes_.pop_front(); return; } - int highest_probe_bitrate_bps = FindBestProbeBitrate(clusters); - bool initial_probe = - !remote_rate_->ValidEstimate() && highest_probe_bitrate_bps > 0; - bool probe_above_estimate = - remote_rate_->ValidEstimate() && - highest_probe_bitrate_bps > - static_cast(remote_rate_->LatestEstimate()); - if (initial_probe || probe_above_estimate) { - LOG(LS_INFO) << "Set new bitrate based on probe: " - << highest_probe_bitrate_bps << " bps."; - remote_rate_->SetEstimate(highest_probe_bitrate_bps, now_ms); + + std::list::const_iterator best_it = FindBestProbe(clusters); + if (best_it != clusters.end()) { + int probe_bitrate_bps = + std::min(best_it->GetSendBitrateBps(), best_it->GetRecvBitrateBps()); + if (IsBitrateImproving(probe_bitrate_bps)) { + LOG(LS_INFO) << "Probe successful, sent at " + << best_it->GetSendBitrateBps() << " bps, received at " + << best_it->GetRecvBitrateBps() + << " bps. Mean send delta: " << best_it->send_mean_ms + << " ms, mean recv delta: " << best_it->recv_mean_ms + << " ms, num probes: " << best_it->count; + remote_rate_->SetEstimate(probe_bitrate_bps, now_ms); + } } + // Not probing and received non-probe packet, or finished with current set // of probes. if (clusters.size() >= kExpectedNumberOfProbes) probes_.clear(); } +bool RemoteBitrateEstimatorAbsSendTimeImpl::IsBitrateImproving( + int new_bitrate_bps) const { + bool initial_probe = !remote_rate_->ValidEstimate() && new_bitrate_bps > 0; + bool bitrate_above_estimate = + remote_rate_->ValidEstimate() && + new_bitrate_bps > static_cast(remote_rate_->LatestEstimate()); + return initial_probe || bitrate_above_estimate; +} + void RemoteBitrateEstimatorAbsSendTimeImpl::IncomingPacket( int64_t arrival_time_ms, size_t payload_size, @@ -321,18 +352,20 @@ void RemoteBitrateEstimatorAbsSendTimeImpl::IncomingPacket( now_ms - first_packet_time_ms_ < kInitialProbingIntervalMs) { int64_t send_time_ms = static_cast(timestamp) * kTimestampToMs; // TODO(holmer): Use a map instead to get correct order? - if (probes_.empty()) { - LOG(LS_INFO) << "Probe packet received: send time=" << send_time_ms - << " ms, recv time=" << arrival_time_ms << " ms"; - } else { - int send_delta_ms = send_time_ms - probes_.back().send_time_ms; - int recv_delta_ms = arrival_time_ms - probes_.back().recv_time_ms; + if (total_probes_received_ < kMaxProbePackets) { + int send_delta_ms = -1; + int recv_delta_ms = -1; + if (!probes_.empty()) { + send_delta_ms = send_time_ms - probes_.back().send_time_ms; + recv_delta_ms = arrival_time_ms - probes_.back().recv_time_ms; + } LOG(LS_INFO) << "Probe packet received: send time=" << send_time_ms << " ms, recv time=" << arrival_time_ms << " ms, send delta=" << send_delta_ms << " ms, recv delta=" << recv_delta_ms << " ms."; } probes_.push_back(Probe(send_time_ms, arrival_time_ms, payload_size)); + ++total_probes_received_; ProcessClusters(now_ms); } if (!inter_arrival_.get()) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc index 863129a02..e1268dad2 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc @@ -136,27 +136,105 @@ TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, TestProbeDetectionTooHighBitrate) { const int kProbeLength = 5; int64_t now_ms = clock_.TimeInMilliseconds(); + int64_t send_time_ms = 0; // First burst sent at 8 * 1000 / 10 = 800 kbps. for (int i = 0; i < kProbeLength; ++i) { clock_.AdvanceTimeMilliseconds(10); now_ms = clock_.TimeInMilliseconds(); - IncomingPacket(0, 1000, now_ms, 90 * now_ms, AbsSendTime(now_ms, 1000)); + send_time_ms += 10; + IncomingPacket(0, 1000, now_ms, 90 * send_time_ms, + AbsSendTime(send_time_ms, 1000)); } - // Second burst sent at 8 * 1000 / 5 = 1600 kbps. + // Second burst sent at 8 * 1000 / 5 = 1600 kbps, arriving at 8 * 1000 / 8 = + // 1000 kbps. for (int i = 0; i < kProbeLength; ++i) { clock_.AdvanceTimeMilliseconds(8); now_ms = clock_.TimeInMilliseconds(); - IncomingPacket(0, - 1000, - now_ms, - 90 * (now_ms - 3 * i), - AbsSendTime(now_ms - 3 * i, 1000)); + send_time_ms += 5; + IncomingPacket(0, 1000, now_ms, send_time_ms, + AbsSendTime(send_time_ms, 1000)); } EXPECT_EQ(0, bitrate_estimator_->Process()); EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_GT(bitrate_observer_->latest_bitrate(), 700000u); - EXPECT_LT(bitrate_observer_->latest_bitrate(), 900000u); + EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 800000u, 10000); +} + +TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, + TestProbeDetectionSlightlyFasterArrival) { + const int kProbeLength = 5; + int64_t now_ms = clock_.TimeInMilliseconds(); + // First burst sent at 8 * 1000 / 10 = 800 kbps. + // Arriving at 8 * 1000 / 5 = 1600 kbps. + int64_t send_time_ms = 0; + for (int i = 0; i < kProbeLength; ++i) { + clock_.AdvanceTimeMilliseconds(5); + send_time_ms += 10; + now_ms = clock_.TimeInMilliseconds(); + IncomingPacket(0, 1000, now_ms, 90 * send_time_ms, + AbsSendTime(send_time_ms, 1000)); + } + + EXPECT_EQ(0, bitrate_estimator_->Process()); + EXPECT_TRUE(bitrate_observer_->updated()); + EXPECT_GT(bitrate_observer_->latest_bitrate(), 800000u); +} + +TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, TestProbeDetectionFasterArrival) { + const int kProbeLength = 5; + int64_t now_ms = clock_.TimeInMilliseconds(); + // First burst sent at 8 * 1000 / 10 = 800 kbps. + // Arriving at 8 * 1000 / 5 = 1600 kbps. + int64_t send_time_ms = 0; + for (int i = 0; i < kProbeLength; ++i) { + clock_.AdvanceTimeMilliseconds(1); + send_time_ms += 10; + now_ms = clock_.TimeInMilliseconds(); + IncomingPacket(0, 1000, now_ms, 90 * send_time_ms, + AbsSendTime(send_time_ms, 1000)); + } + + EXPECT_EQ(0, bitrate_estimator_->Process()); + EXPECT_FALSE(bitrate_observer_->updated()); +} + +TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, TestProbeDetectionSlowerArrival) { + const int kProbeLength = 5; + int64_t now_ms = clock_.TimeInMilliseconds(); + // First burst sent at 8 * 1000 / 5 = 1600 kbps. + // Arriving at 8 * 1000 / 7 = 1142 kbps. + int64_t send_time_ms = 0; + for (int i = 0; i < kProbeLength; ++i) { + clock_.AdvanceTimeMilliseconds(7); + send_time_ms += 5; + now_ms = clock_.TimeInMilliseconds(); + IncomingPacket(0, 1000, now_ms, 90 * send_time_ms, + AbsSendTime(send_time_ms, 1000)); + } + + EXPECT_EQ(0, bitrate_estimator_->Process()); + EXPECT_TRUE(bitrate_observer_->updated()); + EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 1140000, 10000); +} + +TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, + TestProbeDetectionSlowerArrivalHighBitrate) { + const int kProbeLength = 5; + int64_t now_ms = clock_.TimeInMilliseconds(); + // Burst sent at 8 * 1000 / 1 = 8000 kbps. + // Arriving at 8 * 1000 / 2 = 4000 kbps. + int64_t send_time_ms = 0; + for (int i = 0; i < kProbeLength; ++i) { + clock_.AdvanceTimeMilliseconds(2); + send_time_ms += 1; + now_ms = clock_.TimeInMilliseconds(); + IncomingPacket(0, 1000, now_ms, 90 * send_time_ms, + AbsSendTime(send_time_ms, 1000)); + } + + EXPECT_EQ(0, bitrate_estimator_->Process()); + EXPECT_TRUE(bitrate_observer_->updated()); + EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 4000000u, 10000); } } // namespace webrtc