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}
This commit is contained in:
@ -10,7 +10,6 @@
|
||||
|
||||
#include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h"
|
||||
|
||||
#include <algorithm>
|
||||
#include <limits>
|
||||
|
||||
#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
|
||||
|
||||
@ -223,39 +223,6 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST_F(TransportFeedbackAdapterTest, HandlesReordering) {
|
||||
std::vector<PacketInfo> 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<PacketInfo> 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<PacketInfo>& feedback_vector) {
|
||||
ComparePacketVectors(expected_packets, feedback_vector);
|
||||
}));
|
||||
adapter_->OnTransportFeedback(feedback);
|
||||
}
|
||||
|
||||
TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
|
||||
std::vector<PacketInfo> 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<PacketInfo> 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<PacketInfo>& 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
|
||||
|
||||
Reference in New Issue
Block a user