From 45d1fdee9d6eeffd51ee39c4d40028234807b81e Mon Sep 17 00:00:00 2001 From: stefan Date: Mon, 13 Jul 2015 08:37:50 -0700 Subject: [PATCH] Revert of Fix simulator issue where chokes didn't apply to non-congested packets. (patchset #2 id:20001 of https://codereview.webrtc.org/1233853002/) Reason for revert: Breaks bots. Original issue's description: > Fix simulator issue where chokes didn't apply to non-congested packets. > > R=magalhaesc@google.com > > Committed: https://chromium.googlesource.com/external/webrtc/+/662ae00efaa71fbb964eba44b60de4567fc36a73 TBR=magalhaesc@webrtc.org,magalhaesc@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.webrtc.org/1230383003 Cr-Commit-Position: refs/heads/master@{#9571} --- webrtc/modules/modules.gyp | 2 -- .../remote_bitrate_estimator.gypi | 2 ++ .../test/bwe_test_framework.cc | 5 +-- .../test/bwe_test_framework_unittest.cc | 36 +++---------------- 4 files changed, 10 insertions(+), 35 deletions(-) diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 3ecf759c45..7f3a1e809e 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -223,8 +223,6 @@ 'remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc', 'remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc', 'remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h', - 'remote_bitrate_estimator/test/bwe_test_framework_unittest.cc', - 'remote_bitrate_estimator/test/estimators/nada_unittest.cc', 'rtp_rtcp/source/mock/mock_rtp_payload_strategy.h', 'rtp_rtcp/source/byte_io_unittest.cc', 'rtp_rtcp/source/fec_receiver_unittest.cc', diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi index b313d435e5..e4b21db375 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi @@ -70,6 +70,7 @@ 'test/bwe_test_fileutils.h', 'test/bwe_test_framework.cc', 'test/bwe_test_framework.h', + 'test/bwe_test_framework_unittest.cc', 'test/bwe_test_logging.cc', 'test/bwe_test_logging.h', 'test/packet_receiver.cc', @@ -81,6 +82,7 @@ 'test/random.h', 'test/estimators/nada.cc', 'test/estimators/nada.h', + 'test/estimators/nada_unittest.cc', 'test/estimators/remb.cc', 'test/estimators/remb.h', 'test/estimators/send_side.cc', diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc index 1f581a271d..5e66697215 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc @@ -431,9 +431,10 @@ void ChokeFilter::RunFor(int64_t /*time_ms*/, Packets* in_out) { assert(in_out); for (PacketsIt it = in_out->begin(); it != in_out->end(); ) { int64_t earliest_send_time_us = - std::max(last_send_time_us_, (*it)->send_time_us()); - int64_t new_send_time_us = earliest_send_time_us + + last_send_time_us_ + ((*it)->payload_size() * 8 * 1000 + kbps_ / 2) / kbps_; + int64_t new_send_time_us = + std::max((*it)->send_time_us(), earliest_send_time_us); if (delay_cap_helper_->ShouldSendPacket(new_send_time_us, (*it)->send_time_us())) { (*it)->set_send_time_us(new_send_time_us); diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc index e01db1fb80..dd7bbfcfeb 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc @@ -657,39 +657,13 @@ class BweTestFramework_ChokeFilterTest : public ::testing::Test { private: int64_t now_ms_; - uint16_t sequence_number_; + uint32_t sequence_number_; Packets output_packets_; std::vector send_times_us_; DISALLOW_COPY_AND_ASSIGN(BweTestFramework_ChokeFilterTest); }; -TEST_F(BweTestFramework_ChokeFilterTest, NoQueue) { - const int kCapacityKbps = 10; - const size_t kPacketSize = 125; - const int64_t kExpectedSendTimeUs = kPacketSize * 8 * 1000 / kCapacityKbps; - uint16_t sequence_number = 0; - int64_t send_time_us = 0; - ChokeFilter filter(NULL, 0); - filter.SetCapacity(10); - Packets packets; - RTPHeader header; - for (uint32_t i = 0; i < 2; ++i) { - header.sequenceNumber = sequence_number++; - // Payload is 1000 bits. - packets.push_back(new MediaPacket(0, send_time_us, 125, header)); - send_time_us += kExpectedSendTimeUs + 1000; - } - ASSERT_TRUE(IsTimeSorted(packets)); - filter.RunFor(2 * kExpectedSendTimeUs + 1000, &packets); - EXPECT_EQ(kExpectedSendTimeUs, packets.front()->send_time_us()); - delete packets.front(); - packets.pop_front(); - EXPECT_EQ(2 * kExpectedSendTimeUs + 1000, packets.front()->send_time_us()); - delete packets.front(); - packets.pop_front(); -} - TEST_F(BweTestFramework_ChokeFilterTest, Short) { // 100ms, 100 packets, 10 kbps choke -> 1 kbit of data should have propagated. // That is actually just a single packet, since each packet has 1000 bits of @@ -749,8 +723,8 @@ TEST_F(BweTestFramework_ChokeFilterTest, MaxDelay) { // 100 ms delay cap filter.SetMaxDelay(100); - // 10100ms, 50 more packets -> 1 packets or 1 kbit through. - TestChoke(&filter, 100, 50, 1); + // 10100ms, 50 more packets -> 2 packets or 2 kbit through. + TestChoke(&filter, 100, 50, 2); CheckMaxDelay(100); // 20000ms, no input, remaining packets in queue should have been dropped. TestChoke(&filter, 9900, 0, 0); @@ -758,8 +732,8 @@ TEST_F(BweTestFramework_ChokeFilterTest, MaxDelay) { // Reset delay cap (0 is no cap) and verify no packets are dropped. filter.SetCapacity(10); filter.SetMaxDelay(0); - TestChoke(&filter, 100, 100, 1); - TestChoke(&filter, 9900, 0, 99); + TestChoke(&filter, 100, 100, 2); + TestChoke(&filter, 9900, 0, 98); } TEST_F(BweTestFramework_ChokeFilterTest, ShortTrace) {