diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 524985c490..194ef56b25 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -89,7 +89,18 @@ void RemoteEstimatorProxy::Process() { void RemoteEstimatorProxy::OnPacketArrival(uint16_t sequence_number, int64_t arrival_time) { + // TODO(holmer): We should handle a backwards wrap here if the first + // sequence number was small and the new sequence number is large. The + // SequenceNumberUnwrapper doesn't do this, so we should replace this with + // calls to IsNewerSequenceNumber instead. int64_t seq = unwrapper_.Unwrap(sequence_number); + if (seq > window_start_seq_ + 0xFFFF / 2) { + LOG(LS_WARNING) << "Skipping this sequence number (" << sequence_number + << ") since it likely is reordered, but the unwrapper" + "failed to handle it. Feedback window starts at " + << window_start_seq_ << "."; + return; + } if (packet_arrival_times_.lower_bound(window_start_seq_) == packet_arrival_times_.end()) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h index 66373e2977..127886300d 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -54,7 +54,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { private: void OnPacketArrival(uint16_t sequence_number, int64_t arrival_time) EXCLUSIVE_LOCKS_REQUIRED(&lock_); - bool BuildFeedbackPacket(rtcp::TransportFeedback* feedback_packetket); + bool BuildFeedbackPacket(rtcp::TransportFeedback* feedback_packet); Clock* const clock_; PacketRouter* const packet_router_; diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index 62dd2c3956..d999525be0 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -48,7 +48,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { } SimulatedClock clock_; - MockPacketRouter router_; + testing::StrictMock router_; RemoteEstimatorProxy proxy_; const size_t kDefaultPacketSize = 100; @@ -225,6 +225,28 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { Process(); } +TEST_F(RemoteEstimatorProxyTest, GracefullyHandlesReorderingAndWrap) { + const int64_t kDeltaMs = 1000; + const uint16_t kLargeSeq = 62762; + IncomingPacket(kBaseSeq, kBaseTimeMs); + IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs); + + EXPECT_CALL(router_, SendFeedback(_)) + .Times(1) + .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { + packet->Build(); + EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, packet->GetMediaSourceSsrc()); + + std::vector delta_vec = packet->GetReceiveDeltasUs(); + EXPECT_EQ(1u, delta_vec.size()); + EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + return true; + })); + + Process(); +} + TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2);