From 0415a560207e2d56d7dff4ef605ceda76b8312a5 Mon Sep 17 00:00:00 2001 From: stefan Date: Wed, 27 Apr 2016 05:30:25 -0700 Subject: [PATCH] Revert of Deliver reordered packets in arrival time order after parsing feedback message. (patchset #2 id:20001 of https://codereview.webrtc.org/1915993002/ ) Reason for revert: Breaks bots Original issue's description: > Deliver reordered packets in arrival time order after parsing feedback message. > > This is what the remote bitrate estimator expects. > > BUG=5823 > R=sprang@webrtc.org > > Committed: https://crrev.com/ed9535e2cac6be7a45fb03dc8426ef3b0ec43294 > Cr-Commit-Position: refs/heads/master@{#12527} TBR=sprang@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=5823 Review URL: https://codereview.webrtc.org/1928443004 Cr-Commit-Position: refs/heads/master@{#12528} --- .../transport_feedback_adapter.cc | 14 ------ .../transport_feedback_adapter_unittest.cc | 49 ++----------------- 2 files changed, 4 insertions(+), 59 deletions(-) diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc index e6670d6487..f7e07a5dc5 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc +++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc @@ -10,7 +10,6 @@ #include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h" -#include #include #include "webrtc/base/checks.h" @@ -27,17 +26,6 @@ const int64_t kBaseTimestampScaleFactor = rtcp::TransportFeedback::kDeltaScaleFactor * (1 << 8); const int64_t kBaseTimestampRangeSizeUs = kBaseTimestampScaleFactor * (1 << 24); -class PacketInfoComparator { - public: - inline bool operator()(const PacketInfo& lhs, const PacketInfo& rhs) { - if (lhs.arrival_time_ms != rhs.arrival_time_ms) - return lhs.arrival_time_ms < rhs.arrival_time_ms; - if (lhs.send_time_ms !== rhs.send_time_ms) - return lhs.send_time_ms < rhs.send_time_ms; - return lhs.sequence_number < rhs.sequence_number; - } -}; - TransportFeedbackAdapter::TransportFeedbackAdapter( BitrateController* bitrate_controller, Clock* clock) @@ -116,8 +104,6 @@ void TransportFeedbackAdapter::OnTransportFeedback( } ++sequence_number; } - std::sort(packet_feedback_vector.begin(), packet_feedback_vector.end(), - PacketInfoComparator()); RTC_DCHECK(delta_it == delta_vec.end()); if (failed_lookups > 0) { LOG(LS_WARNING) << "Failed to lookup send time for " << failed_lookups diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc index 4f7d83c9eb..0111571262 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc @@ -223,39 +223,6 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { } } -TEST_F(TransportFeedbackAdapterTest, HandlesReordering) { - std::vector packets; - packets.push_back(PacketInfo(120, 200, 0, 1500, true)); - packets.push_back(PacketInfo(110, 210, 1, 1500, true)); - packets.push_back(PacketInfo(100, 220, 2, 1500, true)); - std::vector expected_packets; - expected_packets.push_back(packets[2]); - expected_packets.push_back(packets[1]); - expected_packets.push_back(packets[0]); - - for (const PacketInfo& packet : packets) - OnSentPacket(packet); - - rtcp::TransportFeedback feedback; - feedback.WithBase(packets[0].sequence_number, - packets[0].arrival_time_ms * 1000); - - for (const PacketInfo& packet : packets) { - EXPECT_TRUE(feedback.WithReceivedPacket(packet.sequence_number, - packet.arrival_time_ms * 1000)); - } - - feedback.Build(); - - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke([expected_packets, - this](const std::vector& feedback_vector) { - ComparePacketVectors(expected_packets, feedback_vector); - })); - adapter_->OnTransportFeedback(feedback); -} - TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { std::vector sent_packets; const int64_t kSmallDeltaUs = @@ -290,14 +257,6 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { info.arrival_time_ms += (kLargePositiveDeltaUs + 1000) / 1000; ++info.sequence_number; - // Expected to be ordered on arrival time when the feedback message has been - // parsed. - std::vector expected_packets; - expected_packets.push_back(sent_packets[0]); - expected_packets.push_back(sent_packets[3]); - expected_packets.push_back(sent_packets[1]); - expected_packets.push_back(sent_packets[2]); - // Packets will be added to send history. for (const PacketInfo& packet : sent_packets) OnSentPacket(packet); @@ -325,9 +284,9 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { EXPECT_TRUE(feedback.get() != nullptr); EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) .Times(1) - .WillOnce(Invoke([expected_packets, &received_feedback]( + .WillOnce(Invoke([sent_packets, &received_feedback]( const std::vector& feedback_vector) { - EXPECT_EQ(expected_packets.size(), feedback_vector.size()); + EXPECT_EQ(sent_packets.size(), feedback_vector.size()); received_feedback = feedback_vector; })); adapter_->OnTransportFeedback(*feedback.get()); @@ -351,9 +310,9 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { })); adapter_->OnTransportFeedback(*feedback.get()); - expected_packets.push_back(info); + sent_packets.push_back(info); - ComparePacketVectors(expected_packets, received_feedback); + ComparePacketVectors(sent_packets, received_feedback); } } // namespace test