diff --git a/CHANGELOG.md b/CHANGELOG.md index b7eb77399e..9e13c5b206 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc index 0e362e55da..7a7f25cf5f 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc +++ b/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; diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h index 8ce1c145d1..1602a62bb7 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h +++ b/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. diff --git a/worker/include/DepLibUV.hpp b/worker/include/DepLibUV.hpp index 36c25322f7..965f59cab9 100644 --- a/worker/include/DepLibUV.hpp +++ b/worker/include/DepLibUV.hpp @@ -3,7 +3,6 @@ #include "common.hpp" #include -#include 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::max() }; - - uint64_t time = DepLibUV::GetTimeMs(); - - if (time > MaxInt64) - { - time -= MaxInt64 - 1; - } - - return static_cast(time); + return static_cast(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::max() }; - - uint64_t time = DepLibUV::GetTimeUs(); - - if (time > MaxInt64) - { - time -= MaxInt64 - 1; - } - - return static_cast(time); + return static_cast(DepLibUV::GetTimeUs()); } private: diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 6cfc26ad6e..763f565fb7 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/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: diff --git a/worker/src/RTC/PortManager.cpp b/worker/src/RTC/PortManager.cpp index d0514fe288..37d88b8892 100644 --- a/worker/src/RTC/PortManager.cpp +++ b/worker/src/RTC/PortManager.cpp @@ -162,8 +162,15 @@ namespace RTC { case Transport::UDP: uvHandle = reinterpret_cast(new uv_udp_t()); - err = uv_udp_init_ex( - DepLibUV::GetLoop(), reinterpret_cast(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(uvHandle)); +#else + err = uv_udp_init_ex( + DepLibUV::GetLoop(), reinterpret_cast(uvHandle), UV_UDP_RECVMMSG); +#endif break; case Transport::TCP: diff --git a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp index 6f0b398160..a130c88913 100644 --- a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp +++ b/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(this->referenceTime * 64); for (size_t idx{ 0u }; idx < packetResults.size(); ++idx) { diff --git a/worker/src/RTC/SenderBandwidthEstimator.cpp b/worker/src/RTC/SenderBandwidthEstimator.cpp index 83f4efeac4..68a7ff4fc7 100644 --- a/worker/src/RTC/SenderBandwidthEstimator.cpp +++ b/worker/src/RTC/SenderBandwidthEstimator.cpp @@ -110,16 +110,12 @@ namespace RTC if (!sentInfo.isProbation) { this->cummulativeResult.AddPacket( - sentInfo.size, - static_cast(sentInfo.sentAtMs), - static_cast(result.receivedAtMs)); + sentInfo.size, static_cast(sentInfo.sentAtMs), result.receivedAtMs); } else { this->probationCummulativeResult.AddPacket( - sentInfo.size, - static_cast(sentInfo.sentAtMs), - static_cast(result.receivedAtMs)); + sentInfo.size, static_cast(sentInfo.sentAtMs), result.receivedAtMs); } } diff --git a/worker/src/RTC/Transport.cpp b/worker/src/RTC/Transport.cpp index 4224eb8603..9fda641d6e 100644 --- a/worker/src/RTC/Transport.cpp +++ b/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); diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index d5a81e7473..e8e1b272d9 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/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(nowMs); + + this->rembServer->IncomingPacket(nowMsInt64, packet->GetPayloadLength(), *packet, absSendTime); break; }