commit 22ec952829fa5e44f4e5d8842cf8bc9fe9568031
author Niels Möller <nisse@webrtc.org> Thu Oct 05 08:39:15 2017 +0200
committer Commit Bot <commit-bot@chromium.org> Thu Oct 05 07:19:20 2017 +0000
tree e41c1dfb5f2f572eae1813ab28ec72764327c050
parent 69760535055f73255469290ae3e6cb199520d1e0

Delete in_order argument to RtpReceiver::IncomingRtpPacket

Bug: webrtc:7135
Change-Id: I35fbc76a5ca8d50caff918bbfd2cb13dce4cbd21
Reviewed-on: https://webrtc-review.googlesource.com/4141
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20154}

modules/rtp_rtcp/source/rtp_receiver_impl.cc

diff --git a/modules/rtp_rtcp/source/rtp_receiver_impl.cc b/modules/rtp_rtcp/source/rtp_receiver_impl.cc
index a47aa15..792e08b 100644
--- a/modules/rtp_rtcp/source/rtp_receiver_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_receiver_impl.cc
@@ -20,6 +20,7 @@
 
 #include "common_types.h"  // NOLINT(build/include)
 #include "modules/audio_coding/codecs/audio_format_conversion.h"
+#include "modules/include/module_common_types.h"
 #include "modules/rtp_rtcp/include/rtp_payload_registry.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_receiver_strategy.h"
@@ -27,6 +28,26 @@
 
 namespace webrtc {
 
+namespace {
+bool InOrderPacket(rtc::Optional<uint16_t> latest_sequence_number,
+                   uint16_t current_sequence_number) {
+  if (!latest_sequence_number)
+    return true;
+
+  // We need to distinguish between a late or retransmitted packet,
+  // and a sequence number discontinuity.
+  if (IsNewerSequenceNumber(current_sequence_number, *latest_sequence_number)) {
+    return true;
+  } else {
+    // If we have a restart of the remote side this packet is still in order.
+    return !IsNewerSequenceNumber(
+        current_sequence_number,
+        *latest_sequence_number - kDefaultMaxReorderingThreshold);
+  }
+}
+
+}  // namespace
+
 using RtpUtility::Payload;
 
 // Only return the sources in the last 10 seconds.
@@ -142,12 +163,10 @@
   return rtp_media_receiver_->Energy(array_of_energy);
 }
 
-bool RtpReceiverImpl::IncomingRtpPacket(
-  const RTPHeader& rtp_header,
-  const uint8_t* payload,
-  size_t payload_length,
-  PayloadUnion payload_specific,
-  bool in_order) {
+bool RtpReceiverImpl::IncomingRtpPacket(const RTPHeader& rtp_header,
+                                        const uint8_t* payload,
+                                        size_t payload_length,
+                                        PayloadUnion payload_specific) {
   // Trigger our callbacks.
   CheckSSRCChanged(rtp_header);
 
@@ -186,13 +205,18 @@
   {
     rtc::CritScope lock(&critical_section_rtp_receiver_);
 
-    if (in_order) {
-      if (last_received_timestamp_ != rtp_header.timestamp) {
-        last_received_timestamp_ = rtp_header.timestamp;
-        last_received_frame_time_ms_ = clock_->TimeInMilliseconds();
-      }
+    // TODO(nisse): Do not rely on InOrderPacket for recovered packets, when
+    // packet is passed as RtpPacketReceived and that information is available.
+    // We should ideally never record timestamps for retransmitted or recovered
+    // packets.
+    if (InOrderPacket(last_received_sequence_number_,
+                      rtp_header.sequenceNumber)) {
+      last_received_sequence_number_.emplace(rtp_header.sequenceNumber);
+      last_received_timestamp_ = rtp_header.timestamp;
+      last_received_frame_time_ms_ = clock_->TimeInMilliseconds();
     }
   }
+
   return true;
 }
 
@@ -237,7 +261,7 @@
 bool RtpReceiverImpl::GetLatestTimestamps(uint32_t* timestamp,
                                           int64_t* receive_time_ms) const {
   rtc::CritScope lock(&critical_section_rtp_receiver_);
-  if (last_received_frame_time_ms_ < 0)
+  if (!last_received_sequence_number_)
     return false;
 
   *timestamp = last_received_timestamp_;