From 43354c3b2886af2ec05959bb458e064d3345bb7c Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Mon, 6 Jul 2015 15:03:00 -0700 Subject: [PATCH 01/11] More Packet class work + NLPacket shell --- libraries/networking/src/LimitedNodeList.h | 1 + libraries/networking/src/Packet.cpp | 60 +++++++++++++++++++--- libraries/networking/src/Packet.h | 55 +++++++++++++++++--- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index b705b0e2ea..785933120f 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -36,6 +36,7 @@ #include "DomainHandler.h" #include "Node.h" +#include "Packet.h" #include "PacketHeaders.h" #include "UUIDHasher.h" diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index bb5cf98153..bbd001f8d8 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -11,12 +11,60 @@ #include "Packet.h" -#include <QByteArray> - -std::unique_ptr<Packet> Packet::makePacket() { - return std::unique_ptr<Packet>(new Packet()); +int64_t Packet::headerSize() { + return sizeof(Packet::Header); } -QByteArray Packet::payload() { - return QByteArray(); +int64_t Packet::maxPayloadSize() { + return MAX_PACKET_SIZE - Packet::headerSize(); +} + +std::unique_ptr<Packet> Packet::create(PacketType::Value type, int64_t size) { + if (size == -1) { + size = maxPayloadSize(); + } + if (size <= 0 || size > maxPayloadSize()) { + return std::unique_ptr<Packet>(); + } + + return std::unique_ptr<Packet>(new Packet(type, size)); +} + +Packet::Packet(PacketType::Value type, int64_t size) { + _packetSize = headerSize() + size; + _packet = std::unique_ptr<char>(new char(_packetSize)); + _payloadStart = _packet.get() + headerSize(); +} + +const Packet::Header& Packet::getHeader() const { + return *reinterpret_cast<const Header*>(_packet.get()); +} + +Packet::Header& Packet::getHeader() { + return *reinterpret_cast<Header*>(_packet.get()); +} + +char* Packet::getPayload() { + return _payloadStart; +} + + +int64_t NodeListPacket::headerSize() { + return sizeof(NodeListPacket::Header); +} + +int64_t NodeListPacket::maxPayloadSize() { + return Packet::maxPayloadSize() - NodeListPacket::headerSize(); +} + +std::unique_ptr<NodeListPacket> NodeListPacket::create(PacketType::Value type, int64_t size) { + if (size > maxPayloadSize()) { + return std::unique_ptr<NodeListPacket>(); + } + + return std::unique_ptr<NodeListPacket>(new NodeListPacket(type, size)); +} + +NodeListPacket::NodeListPacket(PacketType::Value type, int64_t size) : Packet(type, headerSize() + size) { + } \ No newline at end of file diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 8541bfb881..1fa634621c 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -14,18 +14,61 @@ #include <memory> -class QByteArray; +#include "LimitedNodeList.h" +#include "PacketHeaders.h" class Packet { - std::unique_ptr<Packet> makePacket(); - QByteArray payload(); +public: + struct Header { + // Required + uint8_t _type; + uint8_t _version; + + // Optionnal + uint16_t _messageNumber; + // 1st bit is the data/control bit, rest is the sequence number + uint32_t _controlBitAndSequenceNumber; + }; + // TODO sanity check Header struct size + + static int64_t headerSize(); + static int64_t maxPayloadSize(); + + static std::unique_ptr<Packet> create(PacketType::Value type, int64_t size = -1); + + virtual const Header& getHeader() const; + virtual char* getPayload(); protected: - Packet(); - Packet(const Packet&) = delete; + Packet(PacketType::Value type, int64_t size); Packet(Packet&&) = delete; - Packet& operator=(const Packet&) = delete; + Packet(const Packet&) = delete; Packet& operator=(Packet&&) = delete; + Packet& operator=(const Packet&) = delete; + + Header& getHeader(); + +private: + int64_t _packetSize; + std::unique_ptr<char> _packet; + + char* _payloadStart; +}; + +class NodeListPacket : public Packet { +public: + struct Header { + // Required + char _sourceUuid[NUM_BYTES_RFC4122_UUID]; + char _connectionUuid[NUM_BYTES_RFC4122_UUID]; + }; + static int64_t headerSize(); + static int64_t maxPayloadSize(); + + static std::unique_ptr<NodeListPacket> create(PacketType::Value type, int64_t size = -1); + +protected: + NodeListPacket(PacketType::Value type, int64_t size); }; #endif // hifi_Packet_h \ No newline at end of file From 4a777f600f75cf972a978084583927250203f9fd Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Mon, 6 Jul 2015 15:11:36 -0700 Subject: [PATCH 02/11] Change DDE Packet struct name to avoid collision --- interface/src/devices/DdeFaceTracker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/devices/DdeFaceTracker.cpp b/interface/src/devices/DdeFaceTracker.cpp index 6facf44b91..71039475a4 100644 --- a/interface/src/devices/DdeFaceTracker.cpp +++ b/interface/src/devices/DdeFaceTracker.cpp @@ -118,7 +118,7 @@ static const float DDE_COEFFICIENT_SCALES[] = { 1.0f // CheekSquint_R }; -struct Packet { +struct DDEPacket { //roughly in mm float focal_length[1]; float translation[3]; @@ -347,7 +347,7 @@ void DdeFaceTracker::decodePacket(const QByteArray& buffer) { bool isFiltering = Menu::getInstance()->isOptionChecked(MenuOption::VelocityFilter); - Packet packet; + DDEPacket packet; int bytesToCopy = glm::min((int)sizeof(packet), buffer.size()); memset(&packet.name, '\n', MAX_NAME_SIZE + 1); memcpy(&packet, buffer.data(), bytesToCopy); From 12edec7beadb4f32683f47d74e0efa7b7e06d9f5 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Mon, 6 Jul 2015 17:02:55 -0700 Subject: [PATCH 03/11] Packet class polishing --- libraries/networking/src/Packet.cpp | 78 ++++++++++++++++------------- libraries/networking/src/Packet.h | 48 +++++------------- 2 files changed, 54 insertions(+), 72 deletions(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index bbd001f8d8..bc5a38b0d8 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -1,6 +1,6 @@ // // Packet.cpp -// +// libraries/networking/src // // Created by Clement on 7/2/15. // Copyright 2015 High Fidelity, Inc. @@ -11,60 +11,66 @@ #include "Packet.h" -int64_t Packet::headerSize() { - return sizeof(Packet::Header); +#include "LimitedNodeList.h" + +int64_t Packet::headerSize(PacketType::Value type) { + int64_t size = numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion) + + ((SEQUENCE_NUMBERED_PACKETS.contains(type)) ? sizeof(SequenceNumber) : 0); + return size; } -int64_t Packet::maxPayloadSize() { - return MAX_PACKET_SIZE - Packet::headerSize(); +int64_t Packet::maxPayloadSize(PacketType::Value type) { + return MAX_PACKET_SIZE - headerSize(type); } std::unique_ptr<Packet> Packet::create(PacketType::Value type, int64_t size) { + auto maxPayload = maxPayloadSize(type); if (size == -1) { - size = maxPayloadSize(); + // default size of -1, means biggest packet possible + size = maxPayload; } - if (size <= 0 || size > maxPayloadSize()) { + if (size <= 0 || size > maxPayload) { + // Invalid size, return null pointer return std::unique_ptr<Packet>(); } + // allocate memory return std::unique_ptr<Packet>(new Packet(type, size)); } -Packet::Packet(PacketType::Value type, int64_t size) { - _packetSize = headerSize() + size; - _packet = std::unique_ptr<char>(new char(_packetSize)); - _payloadStart = _packet.get() + headerSize(); +Packet::Packet(PacketType::Value type, int64_t size) : + _packetSize(headerSize(type) + size), + _packet(new char(_packetSize)), + _payload(_packet.get() + headerSize(type), size) { + Q_ASSERT(size <= maxPayloadSize(type)); } -const Packet::Header& Packet::getHeader() const { - return *reinterpret_cast<const Header*>(_packet.get()); +PacketType::Value Packet::getPacketType() const { + return *reinterpret_cast<PacketType::Value*>(_packet.get()); } -Packet::Header& Packet::getHeader() { - return *reinterpret_cast<Header*>(_packet.get()); +PacketVersion Packet::getPacketTypeVersion() const { + return *reinterpret_cast<PacketVersion*>(_packet.get() + numBytesForArithmeticCodedPacketType(getPacketType())); } -char* Packet::getPayload() { - return _payloadStart; -} - - -int64_t NodeListPacket::headerSize() { - return sizeof(NodeListPacket::Header); -} - -int64_t NodeListPacket::maxPayloadSize() { - return Packet::maxPayloadSize() - NodeListPacket::headerSize(); -} - -std::unique_ptr<NodeListPacket> NodeListPacket::create(PacketType::Value type, int64_t size) { - if (size > maxPayloadSize()) { - return std::unique_ptr<NodeListPacket>(); +Packet::SequenceNumber Packet::getSequenceNumber() const { + PacketType::Value type{ getPacketType() }; + if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { + SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + numBytesForArithmeticCodedPacketType(type) + + sizeof(PacketVersion)); + + return seqNum & ~(1 << 15); // remove control bit } - - return std::unique_ptr<NodeListPacket>(new NodeListPacket(type, size)); + return -1; } -NodeListPacket::NodeListPacket(PacketType::Value type, int64_t size) : Packet(type, headerSize() + size) { - -} \ No newline at end of file +bool Packet::isControlPacket() const { + PacketType::Value type{ getPacketType() }; + if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { + SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + numBytesForArithmeticCodedPacketType(type) + + sizeof(PacketVersion)); + + return seqNum & (1 << 15); // Only keep control bit + } + return false; +} diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 1fa634621c..49f1bb0a12 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -1,6 +1,6 @@ // // Packet.h -// +// libraries/networking/src // // Created by Clement on 7/2/15. // Copyright 2015 High Fidelity, Inc. @@ -14,30 +14,24 @@ #include <memory> -#include "LimitedNodeList.h" #include "PacketHeaders.h" +#include "PacketPayload.h" class Packet { public: - struct Header { - // Required - uint8_t _type; - uint8_t _version; - - // Optionnal - uint16_t _messageNumber; - // 1st bit is the data/control bit, rest is the sequence number - uint32_t _controlBitAndSequenceNumber; - }; - // TODO sanity check Header struct size + using SequenceNumber = uint16_t; - static int64_t headerSize(); - static int64_t maxPayloadSize(); + static int64_t headerSize(PacketType::Value type); + static int64_t maxPayloadSize(PacketType::Value type); static std::unique_ptr<Packet> create(PacketType::Value type, int64_t size = -1); - virtual const Header& getHeader() const; - virtual char* getPayload(); + PacketType::Value getPacketType() const; + PacketVersion getPacketTypeVersion() const; + SequenceNumber getSequenceNumber() const; + bool isControlPacket() const; + + PacketPayload& payload() { return _payload; } protected: Packet(PacketType::Value type, int64_t size); @@ -46,29 +40,11 @@ protected: Packet& operator=(Packet&&) = delete; Packet& operator=(const Packet&) = delete; - Header& getHeader(); - private: int64_t _packetSize; std::unique_ptr<char> _packet; - char* _payloadStart; -}; - -class NodeListPacket : public Packet { -public: - struct Header { - // Required - char _sourceUuid[NUM_BYTES_RFC4122_UUID]; - char _connectionUuid[NUM_BYTES_RFC4122_UUID]; - }; - static int64_t headerSize(); - static int64_t maxPayloadSize(); - - static std::unique_ptr<NodeListPacket> create(PacketType::Value type, int64_t size = -1); - -protected: - NodeListPacket(PacketType::Value type, int64_t size); + PacketPayload _payload; }; #endif // hifi_Packet_h \ No newline at end of file From d87679f1151e3c9321d5fc3927c8f2f963cdc252 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Mon, 6 Jul 2015 17:13:03 -0700 Subject: [PATCH 04/11] Partially fill packet header on construction --- libraries/networking/src/Packet.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index bc5a38b0d8..765267faa9 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -42,7 +42,10 @@ Packet::Packet(PacketType::Value type, int64_t size) : _packetSize(headerSize(type) + size), _packet(new char(_packetSize)), _payload(_packet.get() + headerSize(type), size) { - Q_ASSERT(size <= maxPayloadSize(type)); + + Q_ASSERT(size <= maxPayloadSize(type)); + auto offset = packArithmeticallyCodedValue(type, _packet); + _packet[offset] = versionForPacketType(type); } PacketType::Value Packet::getPacketType() const { From 91496d37c78af9f1fffa28bc89a80340404785df Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 10:31:54 -0700 Subject: [PATCH 05/11] Move NLPacket to its own file --- libraries/networking/src/NLPacket.cpp | 48 +++++++++++++++++++++++++++ libraries/networking/src/NLPacket.h | 32 ++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 libraries/networking/src/NLPacket.cpp create mode 100644 libraries/networking/src/NLPacket.h diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp new file mode 100644 index 0000000000..df91b4fdbe --- /dev/null +++ b/libraries/networking/src/NLPacket.cpp @@ -0,0 +1,48 @@ +// +// NLPacket.cpp +// libraries/networking/src +// +// Created by Clement on 7/6/15. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "NLPacket.h" + +int64_t NLPacket::headerSize(PacketType::Value type) { + int64_t size = ((NON_SOURCED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID) + + ((NON_VERIFIED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID); + return size; +} + +int64_t NLPacket::maxPayloadSize(PacketType::Value type) { + return Packet::maxPayloadSize(type) - headerSize(type); +} + +std::unique_ptr<NLPacket> NLPacket::create(PacketType::Value type, int64_t size) { + if (size > maxPayloadSize(type)) { + return std::unique_ptr<NLPacket>(); + } + + return std::unique_ptr<NLPacket>(new NLPacket(type, size)); +} + +NLPacket::NLPacket(PacketType::Value type, int64_t size) : Packet(type, headerSize(type) + size) { +} + +void NLPacket::setSourceUuid(QUuid sourceUuid) { + auto type = getPacketType(); + Q_ASSERT(!NON_SOURCED_PACKETS.contains(type)); + auto offset = Packet::headerSize(type) + NLPacket::headerSize(type); + memcpy(_packet.get() + offset, sourceUuid.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID); +} + +void NLPacket::setConnectionUuid(QUuid connectionUuid) { + auto type = getPacketType(); + Q_ASSERT(!NON_VERIFIED_PACKETS.contains(type)); + auto offset = Packet::headerSize(type) + NLPacket::headerSize(type) + + ((NON_SOURCED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID); + memcpy(_packet.get() + offset, connectionUuid.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID); +} \ No newline at end of file diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h new file mode 100644 index 0000000000..b1ceee9e83 --- /dev/null +++ b/libraries/networking/src/NLPacket.h @@ -0,0 +1,32 @@ +// +// NLPacket.h +// libraries/networking/src +// +// Created by Clement on 7/6/15. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_NLPacket_h +#define hifi_NLPacket_h + +#include "Packet.h" + +class NLPacket : public Packet { +public: + static int64_t headerSize(PacketType::Value type); + static int64_t maxPayloadSize(PacketType::Value type); + + static std::unique_ptr<NLPacket> create(PacketType::Value type, int64_t size = -1); + +protected: + NLPacket(PacketType::Value type, int64_t size); + + void setSourceUuid(QUuid sourceUuid); + void setConnectionUuid(QUuid connectionUuid); + +}; + +#endif // hifi_NLPacket_h \ No newline at end of file From 8e5b040627c46dc1bb671a487caf21ecfbe1e945 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 10:32:31 -0700 Subject: [PATCH 06/11] Packet setters --- libraries/networking/src/Packet.cpp | 39 +++++++++++++++++++++++------ libraries/networking/src/Packet.h | 4 ++- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index 765267faa9..05291f719f 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -43,9 +43,16 @@ Packet::Packet(PacketType::Value type, int64_t size) : _packet(new char(_packetSize)), _payload(_packet.get() + headerSize(type), size) { + // Sanity check Q_ASSERT(size <= maxPayloadSize(type)); - auto offset = packArithmeticallyCodedValue(type, _packet); - _packet[offset] = versionForPacketType(type); + + // copy packet type and version in header + setPacketTypeAndVersion(type); + + // Set control bit and sequence number to 0 if necessary + if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { + setSequenceNumber(0); + } } PacketType::Value Packet::getPacketType() const { @@ -59,21 +66,39 @@ PacketVersion Packet::getPacketTypeVersion() const { Packet::SequenceNumber Packet::getSequenceNumber() const { PacketType::Value type{ getPacketType() }; if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { - SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + numBytesForArithmeticCodedPacketType(type) + + SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + + numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion)); - return seqNum & ~(1 << 15); // remove control bit } return -1; } bool Packet::isControlPacket() const { - PacketType::Value type{ getPacketType() }; + auto type = getPacketType(); if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { - SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + numBytesForArithmeticCodedPacketType(type) + + SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + + numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion)); - return seqNum & (1 << 15); // Only keep control bit } return false; } + +void Packet::setPacketTypeAndVersion(PacketType::Value type) { + // Pack the packet type + auto offset = packArithmeticallyCodedValue(type, _packet.get()); + + // Pack the packet version + auto version { versionForPacketType(type) }; + memcpy(_packet.get() + offset, &version, sizeof(version)); +} + +void Packet::setSequenceNumber(SequenceNumber seqNum) { + auto type = getPacketType(); + // Here we are overriding the control bit to 0. + // But that is not an issue since we should only ever set the seqNum + // for data packets going out + memcpy(_packet.get() + numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion), + &seqNum, sizeof(seqNum)); +} diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 49f1bb0a12..dd275aeb15 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -40,7 +40,9 @@ protected: Packet& operator=(Packet&&) = delete; Packet& operator=(const Packet&) = delete; -private: + void setPacketTypeAndVersion(PacketType::Value type); + void setSequenceNumber(SequenceNumber seqNum); + int64_t _packetSize; std::unique_ptr<char> _packet; From c23e3c0ec2f521e262bb7cfbd4cf10c373afd453 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 10:35:11 -0700 Subject: [PATCH 07/11] Change Packet.h header to NLPacket --- libraries/networking/src/LimitedNodeList.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index b1905fd593..40d43ec02f 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -36,7 +36,7 @@ #include "DomainHandler.h" #include "Node.h" -#include "Packet.h" +#include "NLPacket.h" #include "PacketHeaders.h" #include "UUIDHasher.h" From a07a24788d4118336e273fda13a9ddb63ce299bf Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 10:36:15 -0700 Subject: [PATCH 08/11] Cast type to int before arithmetique packing --- libraries/networking/src/Packet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index 05291f719f..717a79f2ff 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -87,7 +87,7 @@ bool Packet::isControlPacket() const { void Packet::setPacketTypeAndVersion(PacketType::Value type) { // Pack the packet type - auto offset = packArithmeticallyCodedValue(type, _packet.get()); + auto offset = packArithmeticallyCodedValue((int)type, _packet.get()); // Pack the packet version auto version { versionForPacketType(type) }; From 7eddcf383c229cd4ff627f2258722f7fb49aec05 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 11:33:33 -0700 Subject: [PATCH 09/11] Merge Packet and PacketPayload --- libraries/networking/src/Packet.cpp | 53 ++++++++++++++++- libraries/networking/src/Packet.h | 36 ++++++++++-- libraries/networking/src/PacketHeaders.h | 1 + libraries/networking/src/PacketPayload.cpp | 66 ---------------------- libraries/networking/src/PacketPayload.h | 40 ------------- 5 files changed, 83 insertions(+), 113 deletions(-) delete mode 100644 libraries/networking/src/PacketPayload.cpp delete mode 100644 libraries/networking/src/PacketPayload.h diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index 717a79f2ff..fb1a3dabe3 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -41,7 +41,8 @@ std::unique_ptr<Packet> Packet::create(PacketType::Value type, int64_t size) { Packet::Packet(PacketType::Value type, int64_t size) : _packetSize(headerSize(type) + size), _packet(new char(_packetSize)), - _payload(_packet.get() + headerSize(type), size) { + _payloadStart(_packet.get() + headerSize(type)), + _capacity(size) { // Sanity check Q_ASSERT(size <= maxPayloadSize(type)); @@ -87,7 +88,7 @@ bool Packet::isControlPacket() const { void Packet::setPacketTypeAndVersion(PacketType::Value type) { // Pack the packet type - auto offset = packArithmeticallyCodedValue((int)type, _packet.get()); + auto offset = packArithmeticallyCodedValue(type, _packet.get()); // Pack the packet version auto version { versionForPacketType(type) }; @@ -102,3 +103,51 @@ void Packet::setSequenceNumber(SequenceNumber seqNum) { memcpy(_packet.get() + numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion), &seqNum, sizeof(seqNum)); } + +bool Packet::seek(qint64 pos) { + bool valid = (pos >= 0) && (pos < size()); + if (valid) { + _position = pos; + } + return valid; +} + +static const qint64 PACKET_WRITE_ERROR = -1; +qint64 Packet::writeData(const char* data, qint64 maxSize) { + // make sure we have the space required to write this block + if (maxSize <= bytesAvailable()) { + qint64 currentPos = pos(); + + // good to go - write the data + memcpy(_payloadStart + currentPos, data, maxSize); + + // seek to the new position based on where our write just finished + seek(currentPos + maxSize); + + // keep track of _sizeUsed so we can just write the actual data when packet is about to be sent + _sizeUsed = std::max(pos() + 1, _sizeUsed); + + // return the number of bytes written + return maxSize; + } else { + // not enough space left for this write - return an error + return PACKET_WRITE_ERROR; + } +} + +qint64 Packet::readData(char* dest, qint64 maxSize) { + // we're either reading what is left from the current position or what was asked to be read + qint64 numBytesToRead = std::min(bytesAvailable(), maxSize); + + if (numBytesToRead > 0) { + int currentPosition = pos(); + + // read out the data + memcpy(dest, _payloadStart + currentPosition, numBytesToRead); + + // seek to the end of the read + seek(currentPosition + numBytesToRead); + } + + return numBytesToRead; +} diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index dd275aeb15..a2c3c83e50 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -17,7 +17,7 @@ #include "PacketHeaders.h" #include "PacketPayload.h" -class Packet { +class Packet : public QIODevice { public: using SequenceNumber = uint16_t; @@ -26,12 +26,29 @@ public: static std::unique_ptr<Packet> create(PacketType::Value type, int64_t size = -1); + + qint64 getSizeWithHeader() const { return getSizeUsed(); } + qint64 getSizeUsed() const { return _sizeUsed; } + void setSizeUsed(qint64 sizeUsed) { _sizeUsed = sizeUsed; } + + const char* constData() const { return _payloadStart; } + PacketType::Value getPacketType() const; PacketVersion getPacketTypeVersion() const; SequenceNumber getSequenceNumber() const; bool isControlPacket() const; - PacketPayload& payload() { return _payload; } + // QIODevice virtual functions + // + // WARNING: Those methods all refer to the paylaod only and not the entire packet + virtual bool atEnd() const { return _position == _capacity; } + virtual qint64 bytesAvailable() const { return size() - pos(); } + virtual bool canReadLine() const { return false; } + virtual bool isSequential() const { return false; } + virtual qint64 pos() const { return _position; } + virtual bool reset() { return seek(0); } + virtual bool seek(qint64 pos); + virtual qint64 size() const { return _capacity; } protected: Packet(PacketType::Value type, int64_t size); @@ -39,14 +56,23 @@ protected: Packet(const Packet&) = delete; Packet& operator=(Packet&&) = delete; Packet& operator=(const Packet&) = delete; + + // QIODevice virtual functions + virtual qint64 writeData(const char* data, qint64 maxSize); + virtual qint64 readData(char* data, qint64 maxSize); void setPacketTypeAndVersion(PacketType::Value type); void setSequenceNumber(SequenceNumber seqNum); - int64_t _packetSize; - std::unique_ptr<char> _packet; + std::unique_ptr<char> _packet; // Allocated memory + int64_t _packetSize = 0; // Total size of the allocated memory + + char* _payloadStart = nullptr; // Start of the payload + qint64 _position = 0; // Current position in the payload + qint64 _capacity = 0; // Total capacity of the payload + + qint64 _sizeUsed = 0; // How much of the payload is actually used - PacketPayload _payload; }; #endif // hifi_Packet_h \ No newline at end of file diff --git a/libraries/networking/src/PacketHeaders.h b/libraries/networking/src/PacketHeaders.h index 298a0f7246..b164081d0d 100644 --- a/libraries/networking/src/PacketHeaders.h +++ b/libraries/networking/src/PacketHeaders.h @@ -118,6 +118,7 @@ int numBytesForPacketHeader(const QByteArray& packet); int numBytesForPacketHeader(const char* packet); int numBytesForArithmeticCodedPacketType(PacketType::Value packetType); int numBytesForPacketHeaderGivenPacketType(PacketType::Value packetType); +int packArithmeticallyCodedValue(int value, char* destination); QUuid uuidFromPacketHeader(const QByteArray& packet); diff --git a/libraries/networking/src/PacketPayload.cpp b/libraries/networking/src/PacketPayload.cpp deleted file mode 100644 index 5f0bc9b557..0000000000 --- a/libraries/networking/src/PacketPayload.cpp +++ /dev/null @@ -1,66 +0,0 @@ -// -// PacketPayload.cpp -// libraries/networking/src -// -// Created by Stephen Birarda on 07/06/15. -// Copyright 2015 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#include "PacketPayload.h" - -PacketPayload::PacketPayload(char* data, qint64 capacity) : - _data(data) - _capacity(capacity) -{ - -} - -const int PACKET_READ_ERROR = -1; - -qint64 PacketPayload::writeData(const char* data, qint64 maxSize) { - - qint64 currentPos = pos(); - - // make sure we have the space required to write this block - qint64 bytesAvailable = _capacity - currentPos; - - if (bytesAvailable < srcBytes) { - // good to go - write the data - memcpy(_data + currentPos, src, srcBytes); - - // seek to the new position based on where our write just finished - seek(currentPos + srcBytes); - - // keep track of _maxWritten so we can just write the actual data when packet is about to be sent - _size = std::max(pos() + 1, _maxWritten); - - // return the number of bytes written - return srcBytes; - } else { - // not enough space left for this write - return an error - return PACKET_WRITE_ERROR; - } -} - -const qint64 PACKET_READ_ERROR = -1; - -qint64 PacketPayload::readData(char* dest, qint64 maxSize) { - - int currentPosition = pos(); - - // we're either reading what is left from the current position or what was asked to be read - qint64 numBytesToRead = std::min(_maxSize - currentPosition, maxSize); - - if (numBytesToRead > 0) { - // read out the data - memcpy(dest, _data + currentPosition, numBytesToRead); - - // seek to the end of the read - seek(_data + currentPosition + numBytesToRead); - } - - return numBytesToRead; -} diff --git a/libraries/networking/src/PacketPayload.h b/libraries/networking/src/PacketPayload.h deleted file mode 100644 index a9e3811767..0000000000 --- a/libraries/networking/src/PacketPayload.h +++ /dev/null @@ -1,40 +0,0 @@ -// -// PacketPayload.h -// libraries/networking/src -// -// Created by Stephen Birarda on 07/06/15. -// Copyright 2015 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_PacketPayload_h -#define hifi_PacketPayload_h - -#pragma once - -#include <QtCore/QIODevice> - -class PacketPayload : public QIODevice { -public: - PacketPayload(char* data, qint64 capacity); - - qint64 getSizeUsed() const { return _sizeUsed; } - void setSizeUsed(qint64 sizeUsed) { _sizeUsed = sizeUsed; } - - virtual qint64 size() const { return _capacity; } - virtual bool isSequential() const { return false; } - - const char* constData() const { return _data; } -protected: - qint64 writeData(const char* data, qint64 maxSize); - qint64 readData(char* data, qint64 maxSize); - -private: - char* _data; - qint64 _sizeUsed = 0; - qint64 _capacity = 0; -}; - -#endif // hifi_PacketPayload_h From 712b35c4dc43e16613e2f3dd95d2820ac373a5c3 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 12:26:35 -0700 Subject: [PATCH 10/11] Couple changes to packets header size computation --- libraries/networking/src/NLPacket.cpp | 26 +++++++++------- libraries/networking/src/NLPacket.h | 8 +++-- libraries/networking/src/Packet.cpp | 43 +++++++++++++++------------ libraries/networking/src/Packet.h | 35 +++++++++++++--------- 4 files changed, 66 insertions(+), 46 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index df91b4fdbe..524983a8ff 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -11,14 +11,22 @@ #include "NLPacket.h" -int64_t NLPacket::headerSize(PacketType::Value type) { +int64_t NLPacket::localHeaderSize(PacketType::Value type) { int64_t size = ((NON_SOURCED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID) + ((NON_VERIFIED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID); return size; } int64_t NLPacket::maxPayloadSize(PacketType::Value type) { - return Packet::maxPayloadSize(type) - headerSize(type); + return Packet::maxPayloadSize(type) - localHeaderSize(type); +} + +qint64 Packet::totalHeadersSize() const { + return localHeaderSize() + Packet::localHeaderSize(); +} + +qint64 Packet::localHeaderSize() const { + return localHeaderSize(_type); } std::unique_ptr<NLPacket> NLPacket::create(PacketType::Value type, int64_t size) { @@ -29,20 +37,18 @@ std::unique_ptr<NLPacket> NLPacket::create(PacketType::Value type, int64_t size) return std::unique_ptr<NLPacket>(new NLPacket(type, size)); } -NLPacket::NLPacket(PacketType::Value type, int64_t size) : Packet(type, headerSize(type) + size) { +NLPacket::NLPacket(PacketType::Value type, int64_t size) : Packet(type, localHeaderSize(type) + size) { } void NLPacket::setSourceUuid(QUuid sourceUuid) { - auto type = getPacketType(); - Q_ASSERT(!NON_SOURCED_PACKETS.contains(type)); - auto offset = Packet::headerSize(type) + NLPacket::headerSize(type); + Q_ASSERT(!NON_SOURCED_PACKETS.contains(_type)); + auto offset = Packet::totalHeadersSize(); memcpy(_packet.get() + offset, sourceUuid.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID); } void NLPacket::setConnectionUuid(QUuid connectionUuid) { - auto type = getPacketType(); - Q_ASSERT(!NON_VERIFIED_PACKETS.contains(type)); - auto offset = Packet::headerSize(type) + NLPacket::headerSize(type) + - ((NON_SOURCED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID); + Q_ASSERT(!NON_VERIFIED_PACKETS.contains(_type)); + auto offset = Packet::totalHeadersSize() + + ((NON_SOURCED_PACKETS.contains(_type)) ? 0 : NUM_BYTES_RFC4122_UUID); memcpy(_packet.get() + offset, connectionUuid.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID); } \ No newline at end of file diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index b1ceee9e83..7f625fb60c 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -16,17 +16,19 @@ class NLPacket : public Packet { public: - static int64_t headerSize(PacketType::Value type); + static std::unique_ptr<NLPacket> create(PacketType::Value type, int64_t size = -1); + + static int64_t localHeaderSize(PacketType::Value type); static int64_t maxPayloadSize(PacketType::Value type); - static std::unique_ptr<NLPacket> create(PacketType::Value type, int64_t size = -1); + virtual qint64 totalHeadersSize() const; // Cumulated size of all the headers + virtual qint64 localHeaderSize() const; // Current level's header size protected: NLPacket(PacketType::Value type, int64_t size); void setSourceUuid(QUuid sourceUuid); void setConnectionUuid(QUuid connectionUuid); - }; #endif // hifi_NLPacket_h \ No newline at end of file diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index fb1a3dabe3..293763f77b 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -13,17 +13,17 @@ #include "LimitedNodeList.h" -int64_t Packet::headerSize(PacketType::Value type) { - int64_t size = numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion) + +qint64 Packet::localHeaderSize(PacketType::Value type) { + qint64 size = numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion) + ((SEQUENCE_NUMBERED_PACKETS.contains(type)) ? sizeof(SequenceNumber) : 0); return size; } -int64_t Packet::maxPayloadSize(PacketType::Value type) { - return MAX_PACKET_SIZE - headerSize(type); +qint64 Packet::maxPayloadSize(PacketType::Value type) { + return MAX_PACKET_SIZE - localHeaderSize(type); } -std::unique_ptr<Packet> Packet::create(PacketType::Value type, int64_t size) { +std::unique_ptr<Packet> Packet::create(PacketType::Value type, qint64 size) { auto maxPayload = maxPayloadSize(type); if (size == -1) { // default size of -1, means biggest packet possible @@ -38,12 +38,20 @@ std::unique_ptr<Packet> Packet::create(PacketType::Value type, int64_t size) { return std::unique_ptr<Packet>(new Packet(type, size)); } -Packet::Packet(PacketType::Value type, int64_t size) : - _packetSize(headerSize(type) + size), +qint64 Packet::totalHeadersSize() const { + return localHeaderSize(); +} + +qint64 Packet::localHeaderSize() const { + return localHeaderSize(_type); +} + +Packet::Packet(PacketType::Value type, qint64 size) : + _type(type), + _packetSize(localHeaderSize(_type) + size), _packet(new char(_packetSize)), - _payloadStart(_packet.get() + headerSize(type)), + _payloadStart(_packet.get() + localHeaderSize(_type)), _capacity(size) { - // Sanity check Q_ASSERT(size <= maxPayloadSize(type)); @@ -57,18 +65,17 @@ Packet::Packet(PacketType::Value type, int64_t size) : } PacketType::Value Packet::getPacketType() const { - return *reinterpret_cast<PacketType::Value*>(_packet.get()); + return (PacketType::Value)arithmeticCodingValueFromBuffer(_packet.get()); } PacketVersion Packet::getPacketTypeVersion() const { - return *reinterpret_cast<PacketVersion*>(_packet.get() + numBytesForArithmeticCodedPacketType(getPacketType())); + return *reinterpret_cast<PacketVersion*>(_packet.get() + numBytesForArithmeticCodedPacketType(_type)); } Packet::SequenceNumber Packet::getSequenceNumber() const { - PacketType::Value type{ getPacketType() }; - if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { + if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) { SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + - numBytesForArithmeticCodedPacketType(type) + + numBytesForArithmeticCodedPacketType(_type) + sizeof(PacketVersion)); return seqNum & ~(1 << 15); // remove control bit } @@ -76,10 +83,9 @@ Packet::SequenceNumber Packet::getSequenceNumber() const { } bool Packet::isControlPacket() const { - auto type = getPacketType(); - if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { + if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) { SequenceNumber seqNum = *reinterpret_cast<SequenceNumber*>(_packet.get() + - numBytesForArithmeticCodedPacketType(type) + + numBytesForArithmeticCodedPacketType(_type) + sizeof(PacketVersion)); return seqNum & (1 << 15); // Only keep control bit } @@ -96,11 +102,10 @@ void Packet::setPacketTypeAndVersion(PacketType::Value type) { } void Packet::setSequenceNumber(SequenceNumber seqNum) { - auto type = getPacketType(); // Here we are overriding the control bit to 0. // But that is not an issue since we should only ever set the seqNum // for data packets going out - memcpy(_packet.get() + numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion), + memcpy(_packet.get() + numBytesForArithmeticCodedPacketType(_type) + sizeof(PacketVersion), &seqNum, sizeof(seqNum)); } diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index a2c3c83e50..958f56a827 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -14,33 +14,38 @@ #include <memory> +#include <QIODevice> + #include "PacketHeaders.h" -#include "PacketPayload.h" class Packet : public QIODevice { public: - using SequenceNumber = uint16_t; - - static int64_t headerSize(PacketType::Value type); - static int64_t maxPayloadSize(PacketType::Value type); - static std::unique_ptr<Packet> create(PacketType::Value type, int64_t size = -1); + using SequenceNumber = uint16_t; - qint64 getSizeWithHeader() const { return getSizeUsed(); } + static qint64 localHeaderSize(PacketType::Value type); + static qint64 maxPayloadSize(PacketType::Value type); + + virtual qint64 totalHeadersSize() const; // Cumulated size of all the headers + virtual qint64 localHeaderSize() const; // Current level's header size + + // Payload direct access, use responsibly! + char* getPayload() { return _payloadStart; } + const char* getPayload() const { return _payloadStart; } + + qint64 getSizeWithHeader() const { return localHeaderSize() + getSizeUsed(); } qint64 getSizeUsed() const { return _sizeUsed; } void setSizeUsed(qint64 sizeUsed) { _sizeUsed = sizeUsed; } - - const char* constData() const { return _payloadStart; } - + + // Header readers PacketType::Value getPacketType() const; PacketVersion getPacketTypeVersion() const; SequenceNumber getSequenceNumber() const; bool isControlPacket() const; // QIODevice virtual functions - // - // WARNING: Those methods all refer to the paylaod only and not the entire packet + // WARNING: Those methods all refer to the payload ONLY and NOT the entire packet virtual bool atEnd() const { return _position == _capacity; } virtual qint64 bytesAvailable() const { return size() - pos(); } virtual bool canReadLine() const { return false; } @@ -61,18 +66,20 @@ protected: virtual qint64 writeData(const char* data, qint64 maxSize); virtual qint64 readData(char* data, qint64 maxSize); + // Header writers void setPacketTypeAndVersion(PacketType::Value type); void setSequenceNumber(SequenceNumber seqNum); + PacketType::Value _type; + + qint64 _packetSize = 0; // Total size of the allocated memory std::unique_ptr<char> _packet; // Allocated memory - int64_t _packetSize = 0; // Total size of the allocated memory char* _payloadStart = nullptr; // Start of the payload qint64 _position = 0; // Current position in the payload qint64 _capacity = 0; // Total capacity of the payload qint64 _sizeUsed = 0; // How much of the payload is actually used - }; #endif // hifi_Packet_h \ No newline at end of file From e2bd9532b7df80248b51535180e8ad159dfeb3f7 Mon Sep 17 00:00:00 2001 From: Atlante45 <clement.brisset@gmail.com> Date: Tue, 7 Jul 2015 12:47:23 -0700 Subject: [PATCH 11/11] Implement move ctor/assignment --- libraries/networking/src/Packet.cpp | 35 +++++++++++++++++++++++++++++ libraries/networking/src/Packet.h | 12 +++++----- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index 293763f77b..ca49100efe 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -64,6 +64,41 @@ Packet::Packet(PacketType::Value type, qint64 size) : } } +Packet::Packet(const Packet& other) { + *this = other; +} + +Packet& Packet::operator=(const Packet& other) { + _packetSize = other._packetSize; + _packet = std::unique_ptr<char>(new char(_packetSize)); + memcpy(_packet.get(), other._packet.get(), _packetSize); + + _payloadStart = _packet.get() + (other._payloadStart - other._packet.get()); + _position = other._position; + _capacity = other._capacity; + + _sizeUsed = other._sizeUsed; + + return *this; +} + +Packet::Packet(Packet&& other) { + *this = std::move(other); +} + +Packet& Packet::operator=(Packet&& other) { + _packetSize = other._packetSize; + _packet = std::move(other._packet); + + _payloadStart = other._payloadStart; + _position = other._position; + _capacity = other._capacity; + + _sizeUsed = other._sizeUsed; + + return *this; +} + PacketType::Value Packet::getPacketType() const { return (PacketType::Value)arithmeticCodingValueFromBuffer(_packet.get()); } diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 958f56a827..6177cbd60e 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -20,10 +20,10 @@ class Packet : public QIODevice { public: - static std::unique_ptr<Packet> create(PacketType::Value type, int64_t size = -1); - using SequenceNumber = uint16_t; + static std::unique_ptr<Packet> create(PacketType::Value type, int64_t size = -1); + static qint64 localHeaderSize(PacketType::Value type); static qint64 maxPayloadSize(PacketType::Value type); @@ -57,10 +57,10 @@ public: protected: Packet(PacketType::Value type, int64_t size); - Packet(Packet&&) = delete; - Packet(const Packet&) = delete; - Packet& operator=(Packet&&) = delete; - Packet& operator=(const Packet&) = delete; + Packet(const Packet& other); + Packet& operator=(const Packet& other); + Packet(Packet&& other); + Packet& operator=(Packet&& other); // QIODevice virtual functions virtual qint64 writeData(const char* data, qint64 maxSize);