From 0561bdf8338aa58f164b7d82d75368d9777c2684 Mon Sep 17 00:00:00 2001 From: philipel Date: Wed, 24 Aug 2016 03:43:53 -0700 Subject: [PATCH] Only use payload size within the know send/receive interval for probing calculations. BUG=webrtc:5859 Review-Url: https://codereview.webrtc.org/2254733005 Cr-Commit-Position: refs/heads/master@{#13889} --- .../probe_bitrate_estimator.cc | 65 +++++++++++-------- .../probe_bitrate_estimator.h | 4 +- .../probe_bitrate_estimator_unittest.cc | 19 ++++++ 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc index 1378296628..e8bcfbd984 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc @@ -42,17 +42,25 @@ int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( EraseOldClusters(packet_info.arrival_time_ms - kMaxClusterHistoryMs); + int payload_size_bits = packet_info.payload_size * 8; AggregatedCluster* cluster = &clusters_[packet_info.probe_cluster_id]; - cluster->first_send_ms = - std::min(cluster->first_send_ms, packet_info.send_time_ms); - cluster->last_send_ms = - std::max(cluster->last_send_ms, packet_info.send_time_ms); - cluster->first_receive_ms = - std::min(cluster->first_receive_ms, packet_info.arrival_time_ms); - cluster->last_receive_ms = - std::max(cluster->last_receive_ms, packet_info.arrival_time_ms); - cluster->size += packet_info.payload_size * 8; - ++cluster->num_probes; + + if (packet_info.send_time_ms < cluster->first_send_ms) { + cluster->first_send_ms = packet_info.send_time_ms; + } + if (packet_info.send_time_ms > cluster->last_send_ms) { + cluster->last_send_ms = packet_info.send_time_ms; + cluster->size_last_send = payload_size_bits; + } + if (packet_info.arrival_time_ms < cluster->first_receive_ms) { + cluster->first_receive_ms = packet_info.arrival_time_ms; + cluster->size_first_receive = payload_size_bits; + } + if (packet_info.arrival_time_ms > cluster->last_receive_ms) { + cluster->last_receive_ms = packet_info.arrival_time_ms; + } + cluster->size_total += payload_size_bits; + cluster->num_probes += 1; if (cluster->num_probes < kMinNumProbesValidCluster) return -1; @@ -61,14 +69,6 @@ int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( float receive_interval_ms = cluster->last_receive_ms - cluster->first_receive_ms; - // Since the send/receive interval does not include the send/receive time of - // the last/first packet we expand the interval by the average inverval - // between the probing packets. - float interval_correction = - static_cast(cluster->num_probes) / (cluster->num_probes - 1); - send_interval_ms *= interval_correction; - receive_interval_ms *= interval_correction; - if (send_interval_ms <= 0 || send_interval_ms > kMaxProbeIntervalMs || receive_interval_ms <= 0 || receive_interval_ms > kMaxProbeIntervalMs) { LOG(LS_INFO) << "Probing unsuccessful, invalid send/receive interval" @@ -77,16 +77,27 @@ int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( << " [receive interval: " << receive_interval_ms << " ms]"; return -1; } - float send_bps = static_cast(cluster->size) / send_interval_ms * 1000; - float receive_bps = - static_cast(cluster->size) / receive_interval_ms * 1000; + // Since the |send_interval_ms| does not include the time it takes to actually + // send the last packet the size of the last sent packet should not be + // included when calculating the send bitrate. + RTC_DCHECK_GT(cluster->size_total, cluster->size_last_send); + float send_size = cluster->size_total - cluster->size_last_send; + float send_bps = send_size / send_interval_ms * 1000; + + // Since the |receive_interval_ms| does not include the time it takes to + // actually receive the first packet the size of the first received packet + // should not be included when calculating the receive bitrate. + RTC_DCHECK_GT(cluster->size_total, cluster->size_first_receive); + float receive_size = cluster->size_total - cluster->size_first_receive; + float receive_bps = receive_size / receive_interval_ms * 1000; + float ratio = receive_bps / send_bps; if (ratio > kValidRatio) { LOG(LS_INFO) << "Probing unsuccessful, receive/send ratio too high" << " [cluster id: " << packet_info.probe_cluster_id - << "] [send: " << cluster->size << " bytes / " - << send_interval_ms << " ms = " << send_bps / 1000 << " kb/s]" - << " [receive: " << cluster->size << " bytes / " + << "] [send: " << send_size << " bytes / " << send_interval_ms + << " ms = " << send_bps / 1000 << " kb/s]" + << " [receive: " << receive_size << " bytes / " << receive_interval_ms << " ms = " << receive_bps / 1000 << " kb/s]" << " [ratio: " << receive_bps / 1000 << " / " @@ -96,9 +107,9 @@ int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( } LOG(LS_INFO) << "Probing successful" << " [cluster id: " << packet_info.probe_cluster_id - << "] [send: " << cluster->size << " bytes / " - << send_interval_ms << " ms = " << send_bps / 1000 << " kb/s]" - << " [receive: " << cluster->size << " bytes / " + << "] [send: " << send_size << " bytes / " << send_interval_ms + << " ms = " << send_bps / 1000 << " kb/s]" + << " [receive: " << receive_size << " bytes / " << receive_interval_ms << " ms = " << receive_bps / 1000 << " kb/s]"; return std::min(send_bps, receive_bps); diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h index e6f77c0345..c25c727dd9 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h @@ -33,7 +33,9 @@ class ProbeBitrateEstimator { int64_t last_send_ms = 0; int64_t first_receive_ms = std::numeric_limits::max(); int64_t last_receive_ms = 0; - size_t size = 0; + int size_last_send = 0; + int size_first_receive = 0; + int size_total = 0; }; // Erases old cluster data that was seen before |timestamp_ms|. diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc index d895e32f58..0c143b5efd 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc @@ -123,4 +123,23 @@ TEST_F(TestProbeBitrateEstimator, IgnoreOldClusters) { EXPECT_EQ(measured_bps_, INVALID_BPS); } +TEST_F(TestProbeBitrateEstimator, IgnoreSizeLastSendPacket) { + AddPacketFeedback(0, 1000, 0, 10); + AddPacketFeedback(0, 1000, 10, 20); + AddPacketFeedback(0, 1000, 20, 30); + AddPacketFeedback(0, 1000, 30, 40); + AddPacketFeedback(0, 1500, 40, 50); + + EXPECT_NEAR(measured_bps_, 800000, 10); +} + +TEST_F(TestProbeBitrateEstimator, IgnoreSizeFirstReceivePacket) { + AddPacketFeedback(0, 1500, 0, 10); + AddPacketFeedback(0, 1000, 10, 20); + AddPacketFeedback(0, 1000, 20, 30); + AddPacketFeedback(0, 1000, 30, 40); + + EXPECT_NEAR(measured_bps_, 800000, 10); +} + } // namespace webrtc