Fix crash which happens when there's reordering in the beginning of a call.

webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 71 matching lines @@
       RTC_DCHECK(packet_router_ != nullptr);
       packet_router_->SendFeedback(&feedback_packet);
     } else {
       more_to_build = false;
     }
   }
 }
 
 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) {

[pbos-webrtc, 2016/07/18 08:53:57]

Should this check be using IsNewerSequenceNumber?

[stefan-webrtc, 2016/07/18 09:30:20]

That wouldn't be that simple to write I think. We want to capture if there was
reordering which also happened around a wrap, and if that happened when the
window_start_seq_ was <= 0xFFFF, as that would require seq to be negative. An
alternative would be to simply allow the unwrapper to return a negative seq (or
-1 because it failed), but then I'll have to go over more code to see that
nothing else breaks. If you prefer that I can certainly do that.

[pbos-webrtc, 2016/07/18 09:32:39]

Nah, no strong preferences if it doesn't make things simpler.

+    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 "

[pbos-webrtc, 2016/07/18 08:53:57]

". F"

[stefan-webrtc, 2016/07/18 09:30:20]

Acknowledged.

+                    << window_start_seq_ << ".";
+    return;
+  }
 
   if (packet_arrival_times_.lower_bound(window_start_seq_) ==
       packet_arrival_times_.end()) {
     // Start new feedback packet, cull old packets.
     for (auto it = packet_arrival_times_.begin();
          it != packet_arrival_times_.end() && it->first < seq &&
          arrival_time - it->second >= kBackWindowMs;) {
       auto delete_it = it;
       ++it;
       packet_arrival_times_.erase(delete_it);
@@ skipping 50 matching lines @@
     // Note: Don't erase items from packet_arrival_times_ after sending, in case
     // they need to be re-sent after a reordering. Removal will be handled
     // by OnPacketArrival once packets are too old.
     window_start_seq_ = it->first + 1;
   }
 
   return true;
 }
 
 }  // namespace webrtc