initial work to make sequence # and bitfield lead all packets

assignment-client/src/avatars/AvatarMixer.cpp

@@ -259,8 +259,8 @@ void AvatarMixer::broadcastAvatarData() {
                         return;
                     }
 
-                    PacketSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID());
+                    uint16_t lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID());
-                    PacketSequenceNumber lastSeqFromSender = otherNode->getLastSequenceNumberForPacketType(PacketType::AvatarData);
+                    uint16_t lastSeqFromSender = otherNodeData->getLastReceivedSequenceNumber();
 
                     if (lastSeqToReceiver > lastSeqFromSender) {
                         // Did we somehow get out of order packets from the sender?
@@ -289,7 +289,7 @@ void AvatarMixer::broadcastAvatarData() {
 
                     // set the last sent sequence number for this sender on the receiver
                     nodeData->setLastBroadcastSequenceNumber(otherNode->getUUID(),
-                        otherNode->getLastSequenceNumberForPacketType(PacketType::AvatarData));
+                                                             otherNodeData->getLastReceivedSequenceNumber());
 
                     // start a new segment in the PacketList for this avatar
                     avatarPacketList.startSegment();

assignment-client/src/avatars/AvatarMixerClientData.cpp

@@ -14,7 +14,10 @@
 #include "AvatarMixerClientData.h"
 
 int AvatarMixerClientData::parseData(NLPacket& packet) {
-    // compute the offset to the data payload
+    // pull the sequence number from the data first
+    packet.readPrimitive(&_lastReceivedSequenceNumber);
+    
+    // read the remaining data
     return _avatar.parseDataFromBuffer(packet.read(packet.bytesLeftToRead()));
 }
 
@@ -24,13 +27,13 @@ bool AvatarMixerClientData::checkAndSetHasReceivedFirstPackets() {
     return oldValue;
 }
 
-PacketSequenceNumber AvatarMixerClientData::getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const {
+uint16_t AvatarMixerClientData::getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const {
     // return the matching PacketSequenceNumber, or the default if we don't have it
     auto nodeMatch = _lastBroadcastSequenceNumbers.find(nodeUUID);
     if (nodeMatch != _lastBroadcastSequenceNumbers.end()) {
         return nodeMatch->second;
     } else {
-        return DEFAULT_SEQUENCE_NUMBER;
+        return 0;
     }
 }
 

assignment-client/src/avatars/AvatarMixerClientData.h

@@ -36,10 +36,12 @@ public:
     
     bool checkAndSetHasReceivedFirstPackets();
 
-    PacketSequenceNumber getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const;
+    uint16_t getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const;
-    void setLastBroadcastSequenceNumber(const QUuid& nodeUUID, PacketSequenceNumber sequenceNumber)
+    void setLastBroadcastSequenceNumber(const QUuid& nodeUUID, uint16_t sequenceNumber)
         { _lastBroadcastSequenceNumbers[nodeUUID] = sequenceNumber; }
     Q_INVOKABLE void removeLastBroadcastSequenceNumber(const QUuid& nodeUUID) { _lastBroadcastSequenceNumbers.erase(nodeUUID); }
+    
+    uint16_t getLastReceivedSequenceNumber() const { return _lastReceivedSequenceNumber; }
 
     quint64 getBillboardChangeTimestamp() const { return _billboardChangeTimestamp; }
     void setBillboardChangeTimestamp(quint64 billboardChangeTimestamp) { _billboardChangeTimestamp = billboardChangeTimestamp; }
@@ -77,8 +79,9 @@ public:
     void loadJSONStats(QJsonObject& jsonObject) const;
 private:
     AvatarData _avatar;
-
+    
-    std::unordered_map<QUuid, PacketSequenceNumber, UUIDHasher> _lastBroadcastSequenceNumbers;
+    uint16_t _lastReceivedSequenceNumber { 0 };
+    std::unordered_map<QUuid, uint16_t, UUIDHasher> _lastBroadcastSequenceNumbers;
 
     bool _hasReceivedFirstPackets = false;
     quint64 _billboardChangeTimestamp = 0;

libraries/avatars/src/AvatarData.cpp

@@ -1093,8 +1093,11 @@ void AvatarData::sendAvatarDataPacket() {
     auto nodeList = DependencyManager::get<NodeList>();
 
     QByteArray avatarByteArray = toByteArray();
+    
+    static uint16_t sequenceNumber = 0;
 
-    auto avatarPacket = NLPacket::create(PacketType::AvatarData, avatarByteArray.size());
+    auto avatarPacket = NLPacket::create(PacketType::AvatarData, avatarByteArray.size() + sizeof(sequenceNumber));
+    avatarPacket->writePrimitive(sequenceNumber++);
     avatarPacket->write(avatarByteArray);
 
     nodeList->broadcastToNodes(std::move(avatarPacket), NodeSet() << NodeType::AvatarMixer);

libraries/networking/src/HifiSockAddr.cpp

@@ -150,5 +150,5 @@ QHostAddress getLocalAddress() {
 
 uint qHash(const HifiSockAddr& key, uint seed) {
     // use the existing QHostAddress and quint16 hash functions to get our hash
-    return qHash(key.getAddress(), seed) + qHash(key.getPort(), seed);
+    return qHash(key.getAddress(), seed) ^ qHash(key.getPort(), seed);
 }

libraries/networking/src/HifiSockAddr.h

@@ -12,6 +12,8 @@
 #ifndef hifi_HifiSockAddr_h
 #define hifi_HifiSockAddr_h
 
+#include <string>
+
 #ifdef WIN32
 #include <winsock2.h>
 #include <WS2tcpip.h>
@@ -53,6 +55,7 @@ public:
     friend QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr);
     friend QDataStream& operator<<(QDataStream& dataStream, const HifiSockAddr& sockAddr);
     friend QDataStream& operator>>(QDataStream& dataStream, HifiSockAddr& sockAddr);
+    
 private slots:
     void handleLookupResult(const QHostInfo& hostInfo);
 signals:
@@ -65,6 +68,15 @@ private:
 
 uint qHash(const HifiSockAddr& key, uint seed);
 
+template <>
+struct std::hash<HifiSockAddr> {
+    std::size_t operator()(const HifiSockAddr& sockAddr) const {
+        // use XOR of implemented std::hash templates for new hash
+        return std::hash<std::string>()(sockAddr.getAddress().toString().toStdString())
+            ^ std::hash<uint16_t>()((uint16_t) sockAddr.getPort());
+    }
+};
+
 QHostAddress getLocalAddress();
 
 Q_DECLARE_METATYPE(HifiSockAddr)

libraries/networking/src/LimitedNodeList.cpp

@@ -248,12 +248,6 @@ qint64 LimitedNodeList::writePacket(const NLPacket& packet, const Node& destinat
         return 0;
     }
     
-    // TODO Move to transport layer when ready
-    if (SEQUENCE_NUMBERED_PACKETS.contains(packet.getType())) {
-        PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode.getUUID(), packet.getType());
-        const_cast<NLPacket&>(packet).writeSequenceNumber(sequenceNumber);
-    }
-
     emit dataSent(destinationNode.getType(), packet.getDataSize());
     
     return writePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret());
@@ -344,15 +338,6 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const Node&
     return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getConnectionSecret());
 }
 
-PacketSequenceNumber LimitedNodeList::getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType::Value packetType) {
-    // Thanks to std::map and std::unordered_map this line either default constructs the
-    // PacketType::SequenceMap and the PacketSequenceNumber or returns the existing value.
-    // We use the postfix increment so that the stored value is incremented and the next
-    // return gives the correct value.
-
-    return _packetSequenceNumbers[nodeUUID][packetType]++;
-}
-
 int LimitedNodeList::updateNodeWithDataFromPacket(QSharedPointer<NLPacket> packet, SharedNodePointer sendingNode) {
     QMutexLocker locker(&sendingNode->getMutex());
 

libraries/networking/src/LimitedNodeList.h

@@ -253,8 +253,6 @@ protected:
                        const QUuid& connectionSecret = QUuid());
     qint64 writePacketAndCollectStats(const NLPacket& packet, const HifiSockAddr& destinationSockAddr);
 
-    PacketSequenceNumber getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType::Value packetType);
-    
     bool isPacketVerified(const udt::Packet& packet);
     bool packetVersionMatch(const udt::Packet& packet);
     bool packetSourceAndHashMatch(const udt::Packet& packet);
@@ -289,8 +287,6 @@ protected:
     bool _thisNodeCanAdjustLocks;
     bool _thisNodeCanRez;
 
-    std::unordered_map<QUuid, PacketTypeSequenceMap, UUIDHasher> _packetSequenceNumbers;
-
     QPointer<QTimer> _initialSTUNTimer;
     int _numInitialSTUNRequests = 0;
     bool _hasCompletedInitialSTUN = false;

libraries/networking/src/Node.cpp

@@ -67,14 +67,6 @@ void Node::updateClockSkewUsec(int clockSkewSample) {
     _clockSkewUsec = (int)_clockSkewMovingPercentile.getValueAtPercentile();
 }
 
-PacketSequenceNumber Node::getLastSequenceNumberForPacketType(PacketType::Value packetType) const {
-   auto typeMatch = _lastSequenceNumbers.find(packetType);
-   if (typeMatch != _lastSequenceNumbers.end()) {
-        return typeMatch->second;
-   } else {
-       return DEFAULT_SEQUENCE_NUMBER;
-   }
-}
 
 QDataStream& operator<<(QDataStream& out, const Node& node) {
     out << node._type;

libraries/networking/src/Node.h

@@ -24,7 +24,7 @@
 #include "NetworkPeer.h"
 #include "NodeData.h"
 #include "NodeType.h"
-#include "udt/PacketHeaders.h"
+#include "udt/Packet.h"
 #include "SimpleMovingAverage.h"
 #include "MovingPercentile.h"
 
@@ -65,10 +65,6 @@ public:
     void setCanRez(bool canRez) { _canRez = canRez; }
     bool getCanRez() { return _canRez; }
 
-    void setLastSequenceNumberForPacketType(PacketSequenceNumber sequenceNumber, PacketType::Value packetType)
-        { _lastSequenceNumbers[packetType] = sequenceNumber; }
-    PacketSequenceNumber getLastSequenceNumberForPacketType(PacketType::Value packetType) const;
-
     friend QDataStream& operator<<(QDataStream& out, const Node& node);
     friend QDataStream& operator>>(QDataStream& in, Node& node);
 
@@ -89,7 +85,7 @@ private:
     bool _canAdjustLocks;
     bool _canRez;
 
-    PacketTypeSequenceMap _lastSequenceNumbers;
+    std::map<PacketType::Value, udt::Packet::SequenceNumber>  _lastSequenceNumbers;
 };
 
 typedef QSharedPointer<Node> SharedNodePointer;

libraries/networking/src/PacketReceiver.cpp

@@ -246,12 +246,6 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr<udt::Packet> packet) {
             PacketType::Value packetType = nlPacket->getType();
             
             if (matchingNode) {
-                // if this was a sequence numbered packet we should store the last seq number for
-                // a packet of this type for this node
-                if (SEQUENCE_NUMBERED_PACKETS.contains(nlPacket->getType())) {
-                    matchingNode->setLastSequenceNumberForPacketType(nlPacket->readSequenceNumber(), nlPacket->getType());
-                }
-                
                 emit dataReceived(matchingNode->getType(), nlPacket->getDataSize());
                 QMetaMethod metaMethod = listener.second;
                 

libraries/networking/src/UUIDHasher.h

@@ -26,4 +26,4 @@ public:
     }
 };
 
-#endif // hifi_UUIDHasher_h
+#endif // hifi_UUIDHasher_h

libraries/networking/src/udt/Packet.cpp

@@ -16,8 +16,8 @@ using namespace udt;
 const qint64 Packet::PACKET_WRITE_ERROR = -1;
 
 qint64 Packet::localHeaderSize(PacketType::Value type) {
-    qint64 size = numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion) +
+    // TODO: check the bitfield to see if the message is included
-                        ((SEQUENCE_NUMBERED_PACKETS.contains(type)) ? sizeof(SequenceNumber) : 0);
+    qint64 size = sizeof(SequenceNumberAndBitField);
     return size;
 }
 
@@ -75,13 +75,8 @@ Packet::Packet(PacketType::Value type, qint64 size) :
     // Sanity check
     Q_ASSERT(size >= 0 || size < maxPayload);
     
-    // copy packet type and version in header
+    // set the UDT header to default values
-    writePacketTypeAndVersion(type);
+    writeSequenceNumber(0);
-    
-    // Set control bit and sequence number to 0 if necessary
-    if (SEQUENCE_NUMBERED_PACKETS.contains(type)) {
-        writeSequenceNumber(0);
-    }
 }
 
 Packet::Packet(std::unique_ptr<char> data, qint64 size, const HifiSockAddr& senderSockAddr) :
@@ -187,24 +182,20 @@ PacketVersion Packet::readVersion() const {
     return *reinterpret_cast<PacketVersion*>(_packet.get() + numBytesForArithmeticCodedPacketType(_type));
 }
 
+static const uint32_t CONTROL_BIT_MASK = 1 << (sizeof(Packet::SequenceNumberAndBitField) - 1);
+static const uint32_t RELIABILITY_BIT_MASK = 1 << (sizeof(Packet::SequenceNumberAndBitField) - 2);
+static const uint32_t MESSAGE_BIT_MASK = 1 << (sizeof(Packet::SequenceNumberAndBitField) - 3);
+static const uint32_t BIT_FIELD_MASK = CONTROL_BIT_MASK | RELIABILITY_BIT_MASK | MESSAGE_BIT_MASK;
+
+
 Packet::SequenceNumber Packet::readSequenceNumber() const {
-    if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) {
+    SequenceNumberAndBitField seqNumBitField = *reinterpret_cast<SequenceNumberAndBitField*>(_packet.get());
-        SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() +
+    return seqNumBitField & ~BIT_FIELD_MASK; // Remove the bit field
-                                                                   numBytesForArithmeticCodedPacketType(_type) +
-                                                                   sizeof(PacketVersion));
-        return seqNum & ~(1 << 15); // remove control bit
-    }
-    return -1;
 }
 
 bool Packet::readIsControlPacket() const {
-    if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) {
+    SequenceNumberAndBitField seqNumBitField = *reinterpret_cast<SequenceNumberAndBitField*>(_packet.get());
-        SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() +
+    return seqNumBitField & CONTROL_BIT_MASK; // Only keep control bit
-                                                                   numBytesForArithmeticCodedPacketType(_type) +
-                                                                   sizeof(PacketVersion));
-        return seqNum & (1 << 15); // Only keep control bit
-    }
-    return false;
 }
 
 void Packet::writePacketTypeAndVersion(PacketType::Value type) {

libraries/networking/src/udt/Packet.h

@@ -24,7 +24,11 @@ namespace udt {
 class Packet : public QIODevice {
     Q_OBJECT
 public:
-    using SequenceNumber = uint16_t;
+    // NOTE: The SequenceNumber must actually only be 29 bits MAX to leave room for a bit field
+    using SequenceNumber = uint32_t;
+    using SequenceNumberAndBitField = uint32_t;
+    
+    static const uint32_t DEFAULT_SEQUENCE_NUMBER = 0;
 
     static const qint64 PACKET_WRITE_ERROR;
 

libraries/networking/src/udt/PacketHeaders.cpp

@@ -22,8 +22,6 @@ const QSet<PacketType::Value> NON_VERIFIED_PACKETS = QSet<PacketType::Value>()
     << OctreeDataNack << EntityEditNack
     << DomainListRequest << StopNode;
 
-const QSet<PacketType::Value> SEQUENCE_NUMBERED_PACKETS = QSet<PacketType::Value>() << AvatarData;
-
 const QSet<PacketType::Value> NON_SOURCED_PACKETS = QSet<PacketType::Value>()
     << StunResponse << CreateAssignment << RequestAssignment
     << DomainServerRequireDTLS << DomainConnectRequest

libraries/networking/src/udt/PacketHeaders.h

@@ -84,13 +84,7 @@ const int MAX_PACKET_HEADER_BYTES = 4 + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_H
 
 typedef char PacketVersion;
 
-typedef uint16_t PacketSequenceNumber;
-const PacketSequenceNumber DEFAULT_SEQUENCE_NUMBER = 0;
-
-typedef std::map<PacketType::Value, PacketSequenceNumber> PacketTypeSequenceMap;
-
 extern const QSet<PacketType::Value> NON_VERIFIED_PACKETS;
-extern const QSet<PacketType::Value> SEQUENCE_NUMBERED_PACKETS;
 extern const QSet<PacketType::Value> NON_SOURCED_PACKETS;
 
 QString nameForPacketType(PacketType::Value packetType);

libraries/networking/src/udt/Socket.cpp

@@ -65,6 +65,9 @@ qint64 Socket::writeDatagram(const QByteArray& datagram, const HifiSockAddr& soc
     
     qint64 bytesWritten = _udpSocket.writeDatagram(datagram, sockAddr.getAddress(), sockAddr.getPort());
     
+    // TODO::
+    // const_cast<NLPacket&>(packet).writeSequenceNumber(sequenceNumber);
+    
     if (bytesWritten < 0) {
         qCDebug(networking) << "ERROR in writeDatagram:" << _udpSocket.error() << "-" << _udpSocket.errorString();
     }

libraries/networking/src/udt/Socket.h

@@ -15,6 +15,7 @@
 #define hifi_Socket_h
 
 #include <functional>
+#include <unordered_map>
 
 #include <QtCore/QObject>
 #include <QtNetwork/QUdpSocket>
@@ -59,6 +60,8 @@ private:
     PacketHandler _packetHandler;
     
     QSet<HifiSockAddr> _unfilteredSockAddrs;
+
+    std::unordered_map<HifiSockAddr, Packet::SequenceNumber> _packetSequenceNumbers;
 };
     
 } // namespace udt