From 53d0dc3f067d6440c37157db6060d2f2c9fa8446 Mon Sep 17 00:00:00 2001 From: Stefan Holmer Date: Thu, 7 May 2015 15:04:19 +0200 Subject: [PATCH] Wire up RTT to send-side GCC and TCP. BUG=4548 R=magalhaesc@webrtc.org Review URL: https://webrtc-codereview.appspot.com/52429004 Cr-Commit-Position: refs/heads/master@{#9151} --- .../bwe_simulations.cc | 18 ++++-------------- .../test/estimators/send_side.cc | 16 +++++++++++----- .../test/estimators/tcp.cc | 8 ++++++-- .../test/estimators/tcp.h | 1 + .../test/packet_sender.cc | 10 +++++++++- .../test/packet_sender.h | 6 +++++- 6 files changed, 36 insertions(+), 23 deletions(-) diff --git a/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc b/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc index f32c4ff365..d519317273 100644 --- a/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc +++ b/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc @@ -31,7 +31,10 @@ class BweSimulation : public BweTest, virtual ~BweSimulation() {} protected: - void SetUp() override { BweTest::SetUp(); } + void SetUp() override { + BweTest::SetUp(); + VerboseLogging(true); + } private: DISALLOW_COPY_AND_ASSIGN(BweSimulation); @@ -44,7 +47,6 @@ INSTANTIATE_TEST_CASE_P(VideoSendersTest, kNadaEstimator)); TEST_P(BweSimulation, SprintUplinkTest) { - VerboseLogging(true); AdaptiveVideoSource source(0, 30, 300, 0, 0); VideoSender sender(&uplink_, &source, GetParam()); RateCounterFilter counter1(&uplink_, 0, "sender_output"); @@ -56,7 +58,6 @@ TEST_P(BweSimulation, SprintUplinkTest) { } TEST_P(BweSimulation, Verizon4gDownlinkTest) { - VerboseLogging(true); AdaptiveVideoSource source(0, 30, 300, 0, 0); VideoSender sender(&downlink_, &source, GetParam()); RateCounterFilter counter1(&downlink_, 0, "sender_output"); @@ -68,7 +69,6 @@ TEST_P(BweSimulation, Verizon4gDownlinkTest) { } TEST_P(BweSimulation, Choke1000kbps500kbps1000kbpsBiDirectional) { - VerboseLogging(true); const int kFlowIds[] = {0, 1}; const size_t kNumFlows = sizeof(kFlowIds) / sizeof(kFlowIds[0]); @@ -99,7 +99,6 @@ TEST_P(BweSimulation, Choke1000kbps500kbps1000kbpsBiDirectional) { } TEST_P(BweSimulation, Choke1000kbps500kbps1000kbps) { - VerboseLogging(true); AdaptiveVideoSource source(0, 30, 300, 0, 0); VideoSender sender(&uplink_, &source, GetParam()); @@ -117,7 +116,6 @@ TEST_P(BweSimulation, Choke1000kbps500kbps1000kbps) { } TEST_P(BweSimulation, PacerChoke1000kbps500kbps1000kbps) { - VerboseLogging(true); PeriodicKeyFrameSource source(0, 30, 300, 0, 0, 1000); PacedVideoSender sender(&uplink_, &source, GetParam()); ChokeFilter filter(&uplink_, 0); @@ -133,7 +131,6 @@ TEST_P(BweSimulation, PacerChoke1000kbps500kbps1000kbps) { } TEST_P(BweSimulation, PacerChoke10000kbps) { - VerboseLogging(true); PeriodicKeyFrameSource source(0, 30, 300, 0, 0, 1000); PacedVideoSender sender(&uplink_, &source, GetParam()); ChokeFilter filter(&uplink_, 0); @@ -145,7 +142,6 @@ TEST_P(BweSimulation, PacerChoke10000kbps) { } TEST_P(BweSimulation, PacerChoke200kbps30kbps200kbps) { - VerboseLogging(true); PeriodicKeyFrameSource source(0, 30, 300, 0, 0, 1000); PacedVideoSender sender(&uplink_, &source, GetParam()); ChokeFilter filter(&uplink_, 0); @@ -161,7 +157,6 @@ TEST_P(BweSimulation, PacerChoke200kbps30kbps200kbps) { } TEST_P(BweSimulation, Choke200kbps30kbps200kbps) { - VerboseLogging(true); AdaptiveVideoSource source(0, 30, 300, 0, 0); VideoSender sender(&uplink_, &source, GetParam()); ChokeFilter filter(&uplink_, 0); @@ -177,7 +172,6 @@ TEST_P(BweSimulation, Choke200kbps30kbps200kbps) { } TEST_P(BweSimulation, GoogleWifiTrace3Mbps) { - VerboseLogging(true); AdaptiveVideoSource source(0, 30, 300, 0, 0); VideoSender sender(&uplink_, &source, GetParam()); RateCounterFilter counter1(&uplink_, 0, "sender_output"); @@ -190,7 +184,6 @@ TEST_P(BweSimulation, GoogleWifiTrace3Mbps) { } TEST_P(BweSimulation, PacerGoogleWifiTrace3Mbps) { - VerboseLogging(true); PeriodicKeyFrameSource source(0, 30, 300, 0, 0, 1000); PacedVideoSender sender(&uplink_, &source, GetParam()); RateCounterFilter counter1(&uplink_, 0, "sender_output"); @@ -203,7 +196,6 @@ TEST_P(BweSimulation, PacerGoogleWifiTrace3Mbps) { } TEST_P(BweSimulation, SelfFairnessTest) { - VerboseLogging(true); const int kAllFlowIds[] = {0, 1, 2}; const size_t kNumFlows = sizeof(kAllFlowIds) / sizeof(kAllFlowIds[0]); rtc::scoped_ptr sources[kNumFlows]; @@ -238,13 +230,11 @@ TEST_P(BweSimulation, SelfFairnessTest) { } TEST_P(BweSimulation, PacedSelfFairnessTest) { - VerboseLogging(true); srand(Clock::GetRealTimeClock()->TimeInMicroseconds()); RunFairnessTest(GetParam(), 4, 0, 1000, 3000, 50); } TEST_P(BweSimulation, PacedTcpFairnessTest) { - VerboseLogging(true); srand(Clock::GetRealTimeClock()->TimeInMicroseconds()); RunFairnessTest(GetParam(), 4, 0, 1000, 3000, 500); } diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 1e2352e009..5111e921e5 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -55,6 +55,11 @@ void FullBweSender::GiveFeedback(const FeedbackPacket& feedback) { } } + int64_t rtt_ms = + clock_->TimeInMilliseconds() - feedback.latest_send_time_ms(); + rbe_->OnRttUpdate(rtt_ms); + BWE_TEST_LOGGING_PLOT(1, "RTT", clock_->TimeInMilliseconds(), rtt_ms); + rbe_->IncomingPacketFeedbackVector(packet_feedback_vector); if (has_received_ack_) { int expected_packets = fb.packet_feedback_vector().back().sequence_number - @@ -70,7 +75,7 @@ void FullBweSender::GiveFeedback(const FeedbackPacket& feedback) { ReportBlockList report_blocks; report_blocks.push_back(report_block_); feedback_observer_->OnReceivedRtcpReceiverReport( - report_blocks, 0, clock_->TimeInMilliseconds()); + report_blocks, rtt_ms, clock_->TimeInMilliseconds()); } bitrate_controller_->Process(); @@ -118,18 +123,19 @@ SendSideBweReceiver::~SendSideBweReceiver() { void SendSideBweReceiver::ReceivePacket(int64_t arrival_time_ms, const MediaPacket& media_packet) { packet_feedback_vector_.push_back(PacketInfo( - arrival_time_ms, - GetAbsSendTimeInMs(media_packet.header().extension.absoluteSendTime), + arrival_time_ms, media_packet.sender_timestamp_us() / 1000, media_packet.header().sequenceNumber, media_packet.payload_size())); } FeedbackPacket* SendSideBweReceiver::GetFeedback(int64_t now_ms) { if (now_ms - last_feedback_ms_ < 100) return NULL; - int64_t latest_send_time_ms = last_feedback_ms_; last_feedback_ms_ = now_ms; + int64_t corrected_send_time_ms = + packet_feedback_vector_.back().send_time_ms + now_ms - + packet_feedback_vector_.back().arrival_time_ms; FeedbackPacket* fb = new SendSideBweFeedback( - flow_id_, now_ms * 1000, latest_send_time_ms, packet_feedback_vector_); + flow_id_, now_ms * 1000, corrected_send_time_ms, packet_feedback_vector_); packet_feedback_vector_.clear(); return fb; } diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.cc index 6b125564ed..4c16895564 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.cc @@ -23,7 +23,9 @@ namespace testing { namespace bwe { TcpBweReceiver::TcpBweReceiver(int flow_id) - : BweReceiver(flow_id), last_feedback_ms_(0) { + : BweReceiver(flow_id), + last_feedback_ms_(0), + latest_owd_ms_(0) { } TcpBweReceiver::~TcpBweReceiver() { @@ -31,12 +33,14 @@ TcpBweReceiver::~TcpBweReceiver() { void TcpBweReceiver::ReceivePacket(int64_t arrival_time_ms, const MediaPacket& media_packet) { + latest_owd_ms_ = arrival_time_ms - media_packet.sender_timestamp_us() / 1000; acks_.push_back(media_packet.header().sequenceNumber); } FeedbackPacket* TcpBweReceiver::GetFeedback(int64_t now_ms) { + int64_t corrected_send_time_ms = now_ms - latest_owd_ms_; FeedbackPacket* fb = - new TcpFeedback(flow_id_, now_ms * 1000, last_feedback_ms_, acks_); + new TcpFeedback(flow_id_, now_ms * 1000, corrected_send_time_ms, acks_); last_feedback_ms_ = now_ms; acks_.clear(); return fb; diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.h index a5f73a2b25..b33c93eef7 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/tcp.h @@ -29,6 +29,7 @@ class TcpBweReceiver : public BweReceiver { private: int64_t last_feedback_ms_; + int64_t latest_owd_ms_; std::vector acks_; }; } // namespace bwe diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc index a94b4e75cc..c3334460e7 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -274,6 +274,7 @@ void TcpSender::RunFor(int64_t time_ms, Packets* in_out) { clock_.AdvanceTimeMilliseconds(time_ms); return; } + int64_t start_time_ms = clock_.TimeInMilliseconds(); BWE_TEST_LOGGING_CONTEXT("Sender"); BWE_TEST_LOGGING_CONTEXT(*flow_ids().begin()); @@ -283,6 +284,9 @@ void TcpSender::RunFor(int64_t time_ms, Packets* in_out) { // number of packets in_flight_ and the max number of packets in flight // (cwnd_). Therefore SendPackets() isn't directly dependent on time_ms. for (FeedbackPacket* fb : feedbacks) { + clock_.AdvanceTimeMilliseconds(fb->send_time_us() / 1000 - + clock_.TimeInMilliseconds()); + last_rtt_ms_ = fb->send_time_us() / 1000 - fb->latest_send_time_ms(); UpdateCongestionControl(fb); SendPackets(in_out); } @@ -294,8 +298,9 @@ void TcpSender::RunFor(int64_t time_ms, Packets* in_out) { ++it; } + clock_.AdvanceTimeMilliseconds(time_ms - + (clock_.TimeInMilliseconds() - start_time_ms)); SendPackets(in_out); - clock_.AdvanceTimeMilliseconds(time_ms); SetSenderTimestamps(in_out); } @@ -353,6 +358,9 @@ int TcpSender::TriggerTimeouts() { } void TcpSender::HandleLoss() { + if (clock_.TimeInMilliseconds() - last_reduction_time_ms_ < last_rtt_ms_) + return; + last_reduction_time_ms_ = clock_.TimeInMilliseconds(); ssthresh_ = std::max(static_cast(in_flight_.size() / 2), 2); cwnd_ = ssthresh_; } diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h index 367d099320..2e690c8377 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h @@ -119,7 +119,9 @@ class TcpSender : public PacketSender { ack_received_(false), last_acked_seq_num_(0), next_sequence_number_(0), - offset_ms_(offset_ms) {} + offset_ms_(offset_ms), + last_reduction_time_ms_(-1), + last_rtt_ms_(0) {} virtual ~TcpSender() {} @@ -159,6 +161,8 @@ class TcpSender : public PacketSender { uint16_t last_acked_seq_num_; uint16_t next_sequence_number_; int64_t offset_ms_; + int64_t last_reduction_time_ms_; + int64_t last_rtt_ms_; }; } // namespace bwe } // namespace testing