diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 3a3176b78d..1b0d93e2c5 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -259,8 +259,8 @@ void AvatarMixer::broadcastAvatarData() { return; } - PacketSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID()); - PacketSequenceNumber lastSeqFromSender = otherNode->getLastSequenceNumberForPacketType(PacketType::AvatarData); + uint16_t lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID()); + 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(); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index fec51f76cb..eb47f748c8 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/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; } } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index dfd61dafbc..fd0971294c 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -36,10 +36,12 @@ public: bool checkAndSetHasReceivedFirstPackets(); - PacketSequenceNumber getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const; - void setLastBroadcastSequenceNumber(const QUuid& nodeUUID, PacketSequenceNumber sequenceNumber) + uint16_t getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const; + 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 _lastBroadcastSequenceNumbers; + + uint16_t _lastReceivedSequenceNumber { 0 }; + std::unordered_map _lastBroadcastSequenceNumbers; bool _hasReceivedFirstPackets = false; quint64 _billboardChangeTimestamp = 0; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 3a3b895c66..19f1c35a2b 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1093,8 +1093,11 @@ void AvatarData::sendAvatarDataPacket() { auto nodeList = DependencyManager::get(); 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); diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index 8951da58c9..39210db81e 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/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); } diff --git a/libraries/networking/src/HifiSockAddr.h b/libraries/networking/src/HifiSockAddr.h index 9151e51af2..25f7ad2194 100644 --- a/libraries/networking/src/HifiSockAddr.h +++ b/libraries/networking/src/HifiSockAddr.h @@ -12,6 +12,8 @@ #ifndef hifi_HifiSockAddr_h #define hifi_HifiSockAddr_h +#include + #ifdef WIN32 #include #include @@ -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 { + std::size_t operator()(const HifiSockAddr& sockAddr) const { + // use XOR of implemented std::hash templates for new hash + return std::hash()(sockAddr.getAddress().toString().toStdString()) + ^ std::hash()((uint16_t) sockAddr.getPort()); + } +}; + QHostAddress getLocalAddress(); Q_DECLARE_METATYPE(HifiSockAddr) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 1aee0e2da4..2760ed8277 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/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(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 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 packet, SharedNodePointer sendingNode) { QMutexLocker locker(&sendingNode->getMutex()); diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 67dc707815..ead858a239 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/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 _packetSequenceNumbers; - QPointer _initialSTUNTimer; int _numInitialSTUNRequests = 0; bool _hasCompletedInitialSTUN = false; diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index 667e975398..8aff66e3a0 100644 --- a/libraries/networking/src/Node.cpp +++ b/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; diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h index 0b5a985b35..c8106f5fd8 100644 --- a/libraries/networking/src/Node.h +++ b/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 _lastSequenceNumbers; }; typedef QSharedPointer SharedNodePointer; diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 5dfd35f70d..6fb34f05e7 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -246,12 +246,6 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr 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; diff --git a/libraries/networking/src/UUIDHasher.h b/libraries/networking/src/UUIDHasher.h index 5429e151c7..3ece236812 100644 --- a/libraries/networking/src/UUIDHasher.h +++ b/libraries/networking/src/UUIDHasher.h @@ -26,4 +26,4 @@ public: } }; -#endif // hifi_UUIDHasher_h \ No newline at end of file +#endif // hifi_UUIDHasher_h diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index 91f9661b9d..154d526833 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/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) + - ((SEQUENCE_NUMBERED_PACKETS.contains(type)) ? sizeof(SequenceNumber) : 0); + // TODO: check the bitfield to see if the message is included + 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 - writePacketTypeAndVersion(type); - - // Set control bit and sequence number to 0 if necessary - if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { - writeSequenceNumber(0); - } + // set the UDT header to default values + writeSequenceNumber(0); } Packet::Packet(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr) : @@ -187,24 +182,20 @@ PacketVersion Packet::readVersion() const { return *reinterpret_cast(_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)) { - SequenceNumber seqNum = *reinterpret_cast(_packet.get() + - numBytesForArithmeticCodedPacketType(_type) + - sizeof(PacketVersion)); - return seqNum & ~(1 << 15); // remove control bit - } - return -1; + SequenceNumberAndBitField seqNumBitField = *reinterpret_cast(_packet.get()); + return seqNumBitField & ~BIT_FIELD_MASK; // Remove the bit field } bool Packet::readIsControlPacket() const { - if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) { - SequenceNumber seqNum = *reinterpret_cast(_packet.get() + - numBytesForArithmeticCodedPacketType(_type) + - sizeof(PacketVersion)); - return seqNum & (1 << 15); // Only keep control bit - } - return false; + SequenceNumberAndBitField seqNumBitField = *reinterpret_cast(_packet.get()); + return seqNumBitField & CONTROL_BIT_MASK; // Only keep control bit } void Packet::writePacketTypeAndVersion(PacketType::Value type) { diff --git a/libraries/networking/src/udt/Packet.h b/libraries/networking/src/udt/Packet.h index 96e8f8c3d4..b74834c82a 100644 --- a/libraries/networking/src/udt/Packet.h +++ b/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; diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 0591ac30fe..b71f4107df 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -22,8 +22,6 @@ const QSet NON_VERIFIED_PACKETS = QSet() << OctreeDataNack << EntityEditNack << DomainListRequest << StopNode; -const QSet SEQUENCE_NUMBERED_PACKETS = QSet() << AvatarData; - const QSet NON_SOURCED_PACKETS = QSet() << StunResponse << CreateAssignment << RequestAssignment << DomainServerRequireDTLS << DomainConnectRequest diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 3f3f165e87..d134125ca3 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/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 PacketTypeSequenceMap; - extern const QSet NON_VERIFIED_PACKETS; -extern const QSet SEQUENCE_NUMBERED_PACKETS; extern const QSet NON_SOURCED_PACKETS; QString nameForPacketType(PacketType::Value packetType); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index ea4340065a..8b912637a8 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/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(packet).writeSequenceNumber(sequenceNumber); + if (bytesWritten < 0) { qCDebug(networking) << "ERROR in writeDatagram:" << _udpSocket.error() << "-" << _udpSocket.errorString(); } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index e8d6e5c6fa..f4b4ea4ee0 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -15,6 +15,7 @@ #define hifi_Socket_h #include +#include #include #include @@ -59,6 +60,8 @@ private: PacketHandler _packetHandler; QSet _unfilteredSockAddrs; + + std::unordered_map _packetSequenceNumbers; }; } // namespace udt