transport-cc parsing to be compliant with libwebrtc, make transport-cc reference time to be unsigned, therefore fix 357.

CHANGELOG.md

@@ -9,6 +9,7 @@
 * New C++ `ChannelMessageHandlers` class (PR #894).
 * Fix Rust support after recent changes (PR #898).
 * Update NPM deps.
+* Modify `FeedbackRtpTransport` and tests to be compliant with latest libwebrtc code, make reference time to be unsigned (PR #899 by @penguinol and @sarumjanuch). 
 
 
 ### 3.10.5

worker/deps/libwebrtc/libwebrtc/mediasoup_helpers.h

@@ -39,7 +39,7 @@ namespace mediasoup_helpers
 		int64_t GetBaseDeltaUs(
 		  const RTC::RTCP::FeedbackRtpTransportPacket* packet, int64_t prev_timestamp_us)
 		{
-			return GetBaseTimeUs(packet) - prev_timestamp_us;
+			return packet->GetBaseDelta(prev_timestamp_us / 1000) * 1000;
 		}
 	} // namespace FeedbackRtpTransport
 } // namespace mediasoup_helpers

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, arrival_time_ms)) {
+  } else if (!PacketInOrder(timestamp)) {
     return false;
   } else if (NewTimestampGroup(arrival_time_ms, timestamp)) {
     // First packet of a later frame, the previous frame sample is ready.
@@ -115,17 +115,9 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
   return calculated_deltas;
 }
 
-bool InterArrival::PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms) {
+bool InterArrival::PacketInOrder(uint32_t timestamp) {
   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

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

@@ -71,11 +71,7 @@ class InterArrival {
   };
 
   // Returns true if the packet with timestamp |timestamp| arrived in order.
-  //
-  // 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);
+  bool PacketInOrder(uint32_t timestamp);
 
   // 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/RTC/RTCP/FeedbackRtpTransport.hpp

@@ -41,6 +41,10 @@ namespace RTC
 	{
 		class FeedbackRtpTransportPacket : public FeedbackRtpPacket
 		{
+		public:
+			static constexpr int64_t BaseTimeTick   = 64;
+			static constexpr int64_t TimeWrapPeriod = BaseTimeTick * (1ll << 24);
+
 		public:
 			struct PacketResult
 			{
@@ -237,9 +241,29 @@ namespace RTC
 			{
 				return this->referenceTime;
 			}
+			void SetReferenceTime(uint64_t referenceTime) // We only use this for testing purpose.
+			{
+				this->referenceTime = (referenceTime % TimeWrapPeriod) / BaseTimeTick;
+			}
 			int64_t GetReferenceTimestamp() const // Reference time in ms.
 			{
-				return static_cast<int64_t>(this->referenceTime) * 64;
+				return TimeWrapPeriod + static_cast<int64_t>(this->referenceTime) * BaseTimeTick;
+			}
+			int64_t GetBaseDelta(const int64_t previousTimestampMs) const
+			{
+				int64_t delta = GetReferenceTimestamp() - previousTimestampMs;
+
+				// Compensate for wrap around.
+				if (std::abs(delta - TimeWrapPeriod) < std::abs(delta))
+				{
+					delta -= TimeWrapPeriod;
+				}
+				else if (std::abs(delta + TimeWrapPeriod) < std::abs(delta))
+				{
+					delta += TimeWrapPeriod;
+				}
+
+				return delta;
 			}
 			uint8_t GetFeedbackPacketCount() const
 			{
@@ -290,7 +314,7 @@ namespace RTC
 
 		private:
 			uint16_t baseSequenceNumber{ 0u };
-			int32_t referenceTime{ 0 };
+			uint32_t referenceTime{ 0 };
 			uint16_t latestSequenceNumber{ 0u }; // Just for locally generated packets.
 			uint64_t latestTimestamp{ 0u };      // Just for locally generated packets.
 			uint16_t packetStatusCount{ 0u };

worker/src/RTC/RTCP/FeedbackRtpTransport.cpp

@@ -8,46 +8,6 @@
 #include <limits> // std::numeric_limits()
 #include <sstream>
 
-// Code taken and adapted from libwebrtc (byte_io.h).
-inline static int32_t parseReferenceTime(uint8_t* buffer)
-{
-	int32_t referenceTime;
-	uint32_t unsignedVal = Utils::Byte::Get3Bytes(buffer, 0);
-
-	const auto msb = static_cast<uint8_t>(unsignedVal >> ((3 - 1) * 8));
-
-	if ((msb & 0x80) != 0)
-	{
-		// Create a mask where all bits used by the 3 bytes are set to one, for
-		// instance 0x00FFFFFF. Bit-wise inverts that mask to 0xFF000000 and adds
-		// it to the input value.
-		static uint32_t usedBitMask = (1 << ((3 % sizeof(int32_t)) * 8)) - 1;
-
-		unsignedVal = ~usedBitMask | unsignedVal;
-	}
-
-	// An unsigned value with only the highest order bit set (ex 0x80).
-	static uint32_t unsignedHighestBitMask = static_cast<uint32_t>(1) << ((sizeof(uint32_t) * 8) - 1);
-
-	// A signed value with only the highest bit set. Since this is two's
-	// complement form, we can use the min value from std::numeric_limits.
-	static int32_t signedHighestBitMask = std::numeric_limits<int32_t>::min();
-
-	if ((unsignedVal & unsignedHighestBitMask) != 0)
-	{
-		// Casting is only safe when unsigned value can be represented in the
-		// signed target type, so mask out highest bit and mask it back manually.
-		referenceTime = static_cast<int32_t>(unsignedVal & ~unsignedHighestBitMask);
-		referenceTime |= signedHighestBitMask;
-	}
-	else
-	{
-		referenceTime = static_cast<int32_t>(unsignedVal);
-	}
-
-	return referenceTime;
-}
-
 namespace RTC
 {
 	namespace RTCP
@@ -117,7 +77,7 @@ namespace RTC
 
 			this->baseSequenceNumber  = Utils::Byte::Get2Bytes(data, 0);
 			this->packetStatusCount   = Utils::Byte::Get2Bytes(data, 2);
-			this->referenceTime       = parseReferenceTime(data + 4);
+			this->referenceTime       = Utils::Byte::Get3Bytes(data, 4);
 			this->feedbackPacketCount = Utils::Byte::Get1Byte(data, 7);
 			this->size                = len;