Issue 357 workaround

CHANGELOG.md

@@ -3,6 +3,8 @@
 
 ### 3.5.9 (WIP)
 
+* `libwebrtc`: Apply patch by @sspanak and @Ivaka to avoid crash. Related issue: #357.
+* `PortManager.cpp`: Do not use `UV_UDP_RECVMMSG` in Windows due to a bug in libuv 1.37.0.
 * Update Node deps.
 
 

worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc

@@ -48,7 +48,7 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
     current_timestamp_group_.timestamp = timestamp;
     current_timestamp_group_.first_timestamp = timestamp;
     current_timestamp_group_.first_arrival_ms = arrival_time_ms;
-  } else if (!PacketInOrder(timestamp)) {
+  } else if (!PacketInOrder(timestamp, arrival_time_ms)) {
     return false;
   } else if (NewTimestampGroup(arrival_time_ms, timestamp)) {
     // First packet of a later frame, the previous frame sample is ready.
@@ -65,7 +65,7 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
       if (*arrival_time_delta_ms - system_time_delta_ms >=
           kArrivalTimeOffsetThresholdMs) {
         MS_WARN_TAG(bwe,
-            "The arrival time clock offset has changed (diff = %" PRIi64 "ms, resetting",
+            "the arrival time clock offset has changed (diff = %" PRIi64 "ms, resetting",
             *arrival_time_delta_ms - system_time_delta_ms);
         Reset();
         return false;
@@ -76,7 +76,7 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
         ++num_consecutive_reordered_packets_;
         if (num_consecutive_reordered_packets_ >= kReorderedResetThreshold) {
           MS_WARN_TAG(bwe,
-                 "Packets are being reordered on the path from the "
+                 "packets are being reordered on the path from the "
                  "socket to the bandwidth estimator. Ignoring this "
                  "packet for bandwidth estimation, resetting");
           Reset();
@@ -110,16 +110,31 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
   return calculated_deltas;
 }
 
-bool InterArrival::PacketInOrder(uint32_t timestamp) {
+bool InterArrival::PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms) {
   if (current_timestamp_group_.IsFirstPacket()) {
     return true;
+  } else if (arrival_time_ms < 0) {
+    // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357
+    //
+    // Sometimes we do get negative arrival time, which causes BelongsToBurst()
+    // to fail, which may cause anything that uses InterArrival to crash.
+    //
+    // Credits to @sspanak and @Ivaka.
+    return false;
   } else {
     // Assume that a diff which is bigger than half the timestamp interval
     // (32 bits) must be due to reordering. This code is almost identical to
     // that in IsNewerTimestamp() in module_common_types.h.
     uint32_t timestamp_diff =
         timestamp - current_timestamp_group_.first_timestamp;
-    return timestamp_diff < 0x80000000;
+
+    const static uint32_t int_middle = 0x80000000;
+
+    if (timestamp_diff == int_middle) {
+      return timestamp > current_timestamp_group_.first_timestamp;
+    }
+
+    return timestamp_diff < int_middle;
   }
 }
 
@@ -146,7 +161,8 @@ bool InterArrival::BelongsToBurst(int64_t arrival_time_ms,
 
   MS_ASSERT(
     current_timestamp_group_.complete_time_ms >= 0,
-    "current_timestamp_group_.complete_time_ms < 0");
+    "current_timestamp_group_.complete_time_ms < 0 [current_timestamp_group_.complete_time_ms:%" PRIi64 "]",
+    current_timestamp_group_.complete_time_ms);
 
   int64_t arrival_time_delta_ms =
       arrival_time_ms - current_timestamp_group_.complete_time_ms;

worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h

@@ -71,7 +71,11 @@ class InterArrival {
   };
 
   // Returns true if the packet with timestamp |timestamp| arrived in order.
-  bool PacketInOrder(uint32_t timestamp);
+  //
+  // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357
+  //
+  // bool PacketInOrder(uint32_t timestamp);
+  bool PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms);
 
   // Returns true if the last packet was the end of the current batch and the
   // packet with |timestamp| is the first of a new batch.

worker/include/DepLibUV.hpp

@@ -3,7 +3,6 @@
 
 #include "common.hpp"
 #include <uv.h>
-#include <limits>
 
 class DepLibUV
 {
@@ -28,35 +27,17 @@ class DepLibUV
 	{
 		return uv_hrtime();
 	}
-	// Used within libwebrtc dependency which uses int64_t possitive values for
-	// time representation.
+	// Used within libwebrtc dependency which uses int64_t values for time
+	// representation.
 	static int64_t GetTimeMsInt64()
 	{
-		static constexpr uint64_t MaxInt64{ std::numeric_limits<int64_t>::max() };
-
-		uint64_t time = DepLibUV::GetTimeMs();
-
-		if (time > MaxInt64)
-		{
-			time -= MaxInt64 - 1;
-		}
-
-		return static_cast<int64_t>(time);
+		return static_cast<int64_t>(DepLibUV::GetTimeMs());
 	}
-	// Used within libwebrtc dependency which uses int64_t possitive values for
-	// time representation.
+	// Used within libwebrtc dependency which uses int64_t values for time
+	// representation.
 	static int64_t GetTimeUsInt64()
 	{
-		static constexpr uint64_t MaxInt64{ std::numeric_limits<int64_t>::max() };
-
-		uint64_t time = DepLibUV::GetTimeUs();
-
-		if (time > MaxInt64)
-		{
-			time -= MaxInt64 - 1;
-		}
-
-		return static_cast<int64_t>(time);
+		return static_cast<int64_t>(DepLibUV::GetTimeUs());
 	}
 
 private:

worker/include/RTC/RTCP/FeedbackRtpTransport.hpp

@@ -52,7 +52,7 @@ namespace RTC
 				uint16_t sequenceNumber;   // Wide sequence number.
 				int16_t delta{ 0 };        // Delta.
 				bool received{ false };    // Packet received or not.
-				int32_t receivedAtMs{ 0 }; // Received time (ms) in remote timestamp reference.
+				int64_t receivedAtMs{ 0 }; // Received time (ms) in remote timestamp reference.
 			};
 
 		public:

worker/src/RTC/PortManager.cpp

@@ -162,8 +162,15 @@ namespace RTC
 			{
 				case Transport::UDP:
 					uvHandle = reinterpret_cast<uv_handle_t*>(new uv_udp_t());
-					err      = uv_udp_init_ex(
-            DepLibUV::GetLoop(), reinterpret_cast<uv_udp_t*>(uvHandle), UV_UDP_RECVMMSG);
+#ifdef _WIN32
+					// TODO: Avoid libuv bug in Windows. Let's remove this condition once
+					// the issue is fixed.
+					//   https://github.com/libuv/libuv/issues/2806
+					err = uv_udp_init(DepLibUV::GetLoop(), reinterpret_cast<uv_udp_t*>(uvHandle));
+#else
+					err = uv_udp_init_ex(
+					  DepLibUV::GetLoop(), reinterpret_cast<uv_udp_t*>(uvHandle), UV_UDP_RECVMMSG);
+#endif
 					break;
 
 				case Transport::TCP:

worker/src/RTC/RTCP/FeedbackRtpTransport.cpp

@@ -116,7 +116,7 @@ namespace RTC
 
 			this->baseSequenceNumber  = Utils::Byte::Get2Bytes(data, 0);
 			this->packetStatusCount   = Utils::Byte::Get2Bytes(data, 2);
-			this->referenceTime       = parseReferenceTime(data + 4u);
+			this->referenceTime       = parseReferenceTime(data + 4);
 			this->feedbackPacketCount = Utils::Byte::Get1Byte(data, 7);
 			this->size                = len;
 
@@ -243,7 +243,7 @@ namespace RTC
 				if (packetResult.received)
 				{
 					MS_DUMP(
-					  "    seq:%" PRIu16 ", received:yes, receivedAtMs:%" PRIi32,
+					  "    seq:%" PRIu16 ", received:yes, receivedAtMs:%" PRIi64,
 					  packetResult.sequenceNumber,
 					  packetResult.receivedAtMs);
 				}
@@ -431,7 +431,7 @@ namespace RTC
 			}
 
 			size_t deltaIdx{ 0u };
-			int32_t currentReceivedAtMs = this->referenceTime * 64;
+			int64_t currentReceivedAtMs = static_cast<int64_t>(this->referenceTime * 64);
 
 			for (size_t idx{ 0u }; idx < packetResults.size(); ++idx)
 			{

worker/src/RTC/SenderBandwidthEstimator.cpp

@@ -110,16 +110,12 @@ namespace RTC
 			if (!sentInfo.isProbation)
 			{
 				this->cummulativeResult.AddPacket(
-				  sentInfo.size,
-				  static_cast<int64_t>(sentInfo.sentAtMs),
-				  static_cast<int64_t>(result.receivedAtMs));
+				  sentInfo.size, static_cast<int64_t>(sentInfo.sentAtMs), result.receivedAtMs);
 			}
 			else
 			{
 				this->probationCummulativeResult.AddPacket(
-				  sentInfo.size,
-				  static_cast<int64_t>(sentInfo.sentAtMs),
-				  static_cast<int64_t>(result.receivedAtMs));
+				  sentInfo.size, static_cast<int64_t>(sentInfo.sentAtMs), result.receivedAtMs);
 			}
 		}
 

worker/src/RTC/Transport.cpp

@@ -2323,7 +2323,7 @@ namespace RTC
 			auto* cb = new onSendCallback([tccClient, &packetInfo, senderBwe, &sentInfo](bool sent) {
 				if (sent)
 				{
-					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMs());
+					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMsInt64());
 
 					sentInfo.sentAtMs = DepLibUV::GetTimeMs();
 
@@ -2335,7 +2335,7 @@ namespace RTC
 #else
 			auto* cb = new onSendCallback([tccClient, &packetInfo](bool sent) {
 				if (sent)
-					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMs());
+					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMsInt64());
 			});
 
 			SendRtpPacket(packet, cb);
@@ -2619,7 +2619,7 @@ namespace RTC
 			auto* cb = new onSendCallback([tccClient, &packetInfo, senderBwe, &sentInfo](bool sent) {
 				if (sent)
 				{
-					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMs());
+					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMsInt64());
 
 					sentInfo.sentAtMs = DepLibUV::GetTimeMs();
 
@@ -2631,7 +2631,7 @@ namespace RTC
 #else
 			auto* cb = new onSendCallback([tccClient, &packetInfo](bool sent) {
 				if (sent)
-					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMs());
+					tccClient->PacketSent(packetInfo, DepLibUV::GetTimeMsInt64());
 			});
 
 			SendRtpPacket(packet, cb);

worker/src/RTC/TransportCongestionControlServer.cpp

@@ -180,7 +180,12 @@ namespace RTC
 				if (!packet->ReadAbsSendTime(absSendTime))
 					break;
 
-				this->rembServer->IncomingPacket(nowMs, packet->GetPayloadLength(), *packet, absSendTime);
+				// NOTE: nowMs is uint64_t but we need to "convert" it to int64_t before
+				// we give it to libwebrtc lib (althought this is implicit in the
+				// conversion so it would be converted within the method call).
+				auto nowMsInt64 = static_cast<int64_t>(nowMs);
+
+				this->rembServer->IncomingPacket(nowMsInt64, packet->GetPayloadLength(), *packet, absSendTime);
 
 				break;
 			}