Couple changes to packets header size computation

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));
-    Q_ASSERT(!NON_SOURCED_PACKETS.contains(type));
+    auto offset = Packet::totalHeadersSize();
-    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));
-    Q_ASSERT(!NON_VERIFIED_PACKETS.contains(type));
+    auto offset = Packet::totalHeadersSize() +
-    auto offset = Packet::headerSize(type) + NLPacket::headerSize(type) +
+                    ((NON_SOURCED_PACKETS.contains(_type)) ? 0 : NUM_BYTES_RFC4122_UUID);
-                    ((NON_SOURCED_PACKETS.contains(type)) ? 0 : NUM_BYTES_RFC4122_UUID);
     memcpy(_packet.get() + offset, connectionUuid.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID);
 }

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

libraries/networking/src/Packet.cpp

@@ -13,17 +13,17 @@
 
 #include "LimitedNodeList.h"
 
-int64_t Packet::headerSize(PacketType::Value type) {
+qint64 Packet::localHeaderSize(PacketType::Value type) {
-    int64_t size = numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion) +
+    qint64 size = numBytesForArithmeticCodedPacketType(type) + sizeof(PacketVersion) +
                         ((SEQUENCE_NUMBERED_PACKETS.contains(type)) ? sizeof(SequenceNumber) : 0);
     return size;
 }
 
-int64_t Packet::maxPayloadSize(PacketType::Value type) {
+qint64 Packet::maxPayloadSize(PacketType::Value type) {
-    return MAX_PACKET_SIZE - headerSize(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) :
+qint64 Packet::totalHeadersSize() const {
-    _packetSize(headerSize(type) + size),
+    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));
 }
 

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 payload ONLY and NOT the entire packet
-    // 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; }
@@ -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