AvatarData packet overhaul, uses a structure instead of raw memcpy

libraries/avatars/src/AvatarData.cpp

@@ -48,6 +48,38 @@ const glm::vec3 DEFAULT_LOCAL_AABOX_SCALE(1.0f);
 
 const QString AvatarData::FRAME_NAME = "com.highfidelity.recording.AvatarData";
 
+namespace AvatarDataPacket {
+
+    PACKED_BEGIN struct Header {
+        float position[3];            // skeletal model's position
+        float globalPosition[3];      // avatar's position
+        uint16_t localOrientation[3]; // avatar's local euler angles (degrees, compressed) relative to the thing it's attached to
+        uint16_t scale;               // (compressed) 'ratio' encoding uses sign bit as flag.
+        float lookAtPosition[3];      // world space position that eyes are focusing on.
+        float audioLoudness;          // current loundess of microphone
+        uint8_t flags;
+    } PACKED_END;
+    const size_t HEADER_SIZE = 49;
+
+    PACKED_BEGIN struct ParentInfo {
+        uint8_t parentUUID[16];       // rfc 4122 encoded
+        uint16_t parentJointIndex;
+    } PACKED_END;
+    const size_t PARENT_INFO_SIZE = 18;
+
+    PACKED_BEGIN struct FaceTrackerInfo {
+        float leftEyeBlink;
+        float rightEyeBlink;
+        float averageLoudness;
+        float browAudioLift;
+        uint8_t numBlendshapeCoefficients;
+        // float blendshapeCoefficients[numBlendshapeCoefficients];
+    } PACKED_END;
+    const size_t FACE_TRACKER_INFO_SIZE = 17;
+}
+
+#define ASSERT(COND)  do { if (!(COND)) { int* bad = nullptr; *bad = 0xbad; } } while(0)
+
 AvatarData::AvatarData() :
     SpatiallyNestable(NestableType::Avatar, QUuid()),
     _handPosition(0.0f),
@@ -68,6 +100,10 @@ AvatarData::AvatarData() :
     setBodyPitch(0.0f);
     setBodyYaw(-90.0f);
     setBodyRoll(0.0f);
+
+    ASSERT(sizeof AvatarDataPacket::Header == AvatarDataPacket::HEADER_SIZE);
+    ASSERT(sizeof AvatarDataPacket::ParentInfo == AvatarDataPacket::PARENT_INFO_SIZE);
+    ASSERT(sizeof AvatarDataPacket::FaceTrackerInfo == AvatarDataPacket::FACE_TRACKER_INFO_SIZE);
 }
 
 AvatarData::~AvatarData() {
@@ -141,81 +177,67 @@ QByteArray AvatarData::toByteArray(bool cullSmallChanges, bool sendAll) {
     unsigned char* destinationBuffer = reinterpret_cast<unsigned char*>(avatarDataByteArray.data());
     unsigned char* startPosition = destinationBuffer;
 
-    const glm::vec3& position = getLocalPosition();
+    auto header = reinterpret_cast<AvatarDataPacket::Header*>(destinationBuffer);
-    memcpy(destinationBuffer, &position, sizeof(position));
+    header->position[0] = getLocalPosition().x;
-    destinationBuffer += sizeof(position);
+    header->position[1] = getLocalPosition().y;
+    header->position[2] = getLocalPosition().z;
+    header->globalPosition[0] = _globalPosition.x;
+    header->globalPosition[1] = _globalPosition.y;
+    header->globalPosition[2] = _globalPosition.z;
 
-    memcpy(destinationBuffer, &_globalPosition, sizeof(_globalPosition));
-    destinationBuffer += sizeof(_globalPosition);
-
-    // Body rotation
     glm::vec3 bodyEulerAngles = glm::degrees(safeEulerAngles(getLocalOrientation()));
-    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, bodyEulerAngles.y);
+    packFloatAngleToTwoByte((uint8_t*)(header->localOrientation + 0), bodyEulerAngles.y);
-    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, bodyEulerAngles.x);
+    packFloatAngleToTwoByte((uint8_t*)(header->localOrientation + 1), bodyEulerAngles.x);
-    destinationBuffer += packFloatAngleToTwoByte(destinationBuffer, bodyEulerAngles.z);
+    packFloatAngleToTwoByte((uint8_t*)(header->localOrientation + 2), bodyEulerAngles.z);
+    packFloatRatioToTwoByte((uint8_t*)(&header->scale), _targetScale);
+    header->lookAtPosition[0] = _headData->_lookAtPosition.x;
+    header->lookAtPosition[1] = _headData->_lookAtPosition.y;
+    header->lookAtPosition[2] = _headData->_lookAtPosition.z;
+    header->audioLoudness = _headData->_audioLoudness;
 
-    // Body scale
+    setSemiNibbleAt(header->flags, KEY_STATE_START_BIT, _keyState);
-    destinationBuffer += packFloatRatioToTwoByte(destinationBuffer, _targetScale);
-
-    // Lookat Position
-    memcpy(destinationBuffer, &_headData->_lookAtPosition, sizeof(_headData->_lookAtPosition));
-    destinationBuffer += sizeof(_headData->_lookAtPosition);
-
-    // Instantaneous audio loudness (used to drive facial animation)
-    memcpy(destinationBuffer, &_headData->_audioLoudness, sizeof(float));
-    destinationBuffer += sizeof(float);
-
-    // bitMask of less than byte wide items
-    unsigned char bitItems = 0;
-
-    // key state
-    setSemiNibbleAt(bitItems,KEY_STATE_START_BIT,_keyState);
     // hand state
     bool isFingerPointing = _handState & IS_FINGER_POINTING_FLAG;
-    setSemiNibbleAt(bitItems, HAND_STATE_START_BIT, _handState & ~IS_FINGER_POINTING_FLAG);
+    setSemiNibbleAt(header->flags, HAND_STATE_START_BIT, _handState & ~IS_FINGER_POINTING_FLAG);
     if (isFingerPointing) {
-        setAtBit(bitItems, HAND_STATE_FINGER_POINTING_BIT);
+        setAtBit(header->flags, HAND_STATE_FINGER_POINTING_BIT);
     }
     // faceshift state
     if (_headData->_isFaceTrackerConnected) {
-        setAtBit(bitItems, IS_FACESHIFT_CONNECTED);
+        setAtBit(header->flags, IS_FACESHIFT_CONNECTED);
     }
     // eye tracker state
     if (_headData->_isEyeTrackerConnected) {
-        setAtBit(bitItems, IS_EYE_TRACKER_CONNECTED);
+        setAtBit(header->flags, IS_EYE_TRACKER_CONNECTED);
     }
     // referential state
     QUuid parentID = getParentID();
     if (!parentID.isNull()) {
-        setAtBit(bitItems, HAS_REFERENTIAL);
+        setAtBit(header->flags, HAS_REFERENTIAL);
     }
-    *destinationBuffer++ = bitItems;
+    destinationBuffer += sizeof(AvatarDataPacket::Header);
 
     if (!parentID.isNull()) {
+        auto parentInfo = reinterpret_cast<AvatarDataPacket::ParentInfo*>(destinationBuffer);
         QByteArray referentialAsBytes = parentID.toRfc4122();
-        memcpy(destinationBuffer, referentialAsBytes.data(), referentialAsBytes.size());
+        memcpy(parentInfo->parentUUID, referentialAsBytes.data(), referentialAsBytes.size());
-        destinationBuffer += referentialAsBytes.size();
+        parentInfo->parentJointIndex = _parentJointIndex;
-        memcpy(destinationBuffer, &_parentJointIndex, sizeof(_parentJointIndex));
+        destinationBuffer += sizeof(AvatarDataPacket::ParentInfo);
-        destinationBuffer += sizeof(_parentJointIndex);
     }
 
     // If it is connected, pack up the data
     if (_headData->_isFaceTrackerConnected) {
-        memcpy(destinationBuffer, &_headData->_leftEyeBlink, sizeof(float));
+        auto faceTrackerInfo = reinterpret_cast<AvatarDataPacket::FaceTrackerInfo*>(destinationBuffer);
-        destinationBuffer += sizeof(float);
 
-        memcpy(destinationBuffer, &_headData->_rightEyeBlink, sizeof(float));
+        faceTrackerInfo->leftEyeBlink = _headData->_leftEyeBlink;
-        destinationBuffer += sizeof(float);
+        faceTrackerInfo->rightEyeBlink = _headData->_rightEyeBlink;
+        faceTrackerInfo->averageLoudness = _headData->_averageLoudness;
+        faceTrackerInfo->browAudioLift = _headData->_browAudioLift;
+        faceTrackerInfo->numBlendshapeCoefficients = _headData->_blendshapeCoefficients.size();
+        destinationBuffer += sizeof(AvatarDataPacket::FaceTrackerInfo);
 
-        memcpy(destinationBuffer, &_headData->_averageLoudness, sizeof(float));
+        // followed by a variable number of float coefficients
-        destinationBuffer += sizeof(float);
+        memcpy(destinationBuffer, _headData->_blendshapeCoefficients.data(), _headData->_blendshapeCoefficients.size() * sizeof(float));
-
-        memcpy(destinationBuffer, &_headData->_browAudioLift, sizeof(float));
-        destinationBuffer += sizeof(float);
-
-        *destinationBuffer++ = _headData->_blendshapeCoefficients.size();
-        memcpy(destinationBuffer, _headData->_blendshapeCoefficients.data(),
-            _headData->_blendshapeCoefficients.size() * sizeof(float));
         destinationBuffer += _headData->_blendshapeCoefficients.size() * sizeof(float);
     }
 
@@ -377,6 +399,16 @@ bool AvatarData::shouldLogError(const quint64& now) {
     return false;
 }
 
+#define PACKET_READ_CHECK(ITEM_NAME, SIZE_TO_READ)                                        \
+    if ((endPosition - sourceBuffer) < (int)SIZE_TO_READ) {                               \
+        if (shouldLogError(now)) {                                                        \
+            qCWarning(avatars) << "AvatarData packet too small, attempting to read " <<   \
+                #ITEM_NAME << ", only " << (endPosition - sourceBuffer) <<                \
+                " bytes left, " << getSessionUUID();                                      \
+        }                                                                                 \
+        return buffer.size();                                                             \
+    }
+
 // read data in packet starting at byte offset and return number of bytes parsed
 int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
 
@@ -386,124 +418,76 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
     }
 
     const unsigned char* startPosition = reinterpret_cast<const unsigned char*>(buffer.data());
+    const unsigned char* endPosition = startPosition + buffer.size();
     const unsigned char* sourceBuffer = startPosition;
     quint64 now = usecTimestampNow();
 
-    // The absolute minimum size of the update data is as follows:
+    PACKET_READ_CHECK(Header, sizeof(AvatarDataPacket::Header));
-    // 36 bytes of "plain old data" {
+    auto header = reinterpret_cast<const AvatarDataPacket::Header*>(sourceBuffer);
-    //     position      = 12 bytes
+    sourceBuffer += sizeof(AvatarDataPacket::Header);
-    //     bodyYaw       =  2 (compressed float)
-    //     bodyPitch     =  2 (compressed float)
-    //     bodyRoll      =  2 (compressed float)
-    //     targetScale   =  2 (compressed float)
-    //     lookAt        = 12
-    //     audioLoudness =  4
-    // }
-    // + 1 byte for varying data
-    // + 1 byte for numJoints (0)
-    // = 39 bytes
-    int minPossibleSize = 39;
 
-    int maxAvailableSize = buffer.size();
+    glm::vec3 position = glm::vec3(header->position[0], header->position[1], header->position[2]);
-    if (minPossibleSize > maxAvailableSize) {
+    _globalPosition = glm::vec3(header->globalPosition[0], header->globalPosition[1], header->globalPosition[2]);
+    if (glm::isnan(position.x) || glm::isnan(position.y) || glm::isnan(position.z)) {
         if (shouldLogError(now)) {
-            qCDebug(avatars) << "Malformed AvatarData packet at the start; "
+            qCWarning(avatars) << "Discard AvatarData packet: position NaN, uuid " << getSessionUUID();
-                << " displayName = '" << _displayName << "'"
-                << " minPossibleSize = " << minPossibleSize
-                << " maxAvailableSize = " << maxAvailableSize;
         }
-        // this packet is malformed so we report all bytes as consumed
+        return buffer.size();
-        return maxAvailableSize;
+    }
+    setLocalPosition(position);
+
+    float pitch, yaw, roll;
+    unpackFloatAngleFromTwoByte(header->localOrientation + 0, &yaw);
+    unpackFloatAngleFromTwoByte(header->localOrientation + 1, &pitch);
+    unpackFloatAngleFromTwoByte(header->localOrientation + 2, &roll);
+    if (glm::isnan(yaw) || glm::isnan(pitch) || glm::isnan(roll)) {
+        if (shouldLogError(now)) {
+            qCWarning(avatars) << "Discard AvatarData packet: localOriention is NaN, uuid " << getSessionUUID();
+        }
+        return buffer.size();
     }
 
-    { // Body world position, rotation, and scale
+    glm::quat currentOrientation = getLocalOrientation();
-        // position
+    glm::vec3 newEulerAngles(pitch, yaw, roll);
-        glm::vec3 position;
+    glm::quat newOrientation = glm::quat(glm::radians(newEulerAngles));
-        memcpy(&position, sourceBuffer, sizeof(position));
+    if (currentOrientation != newOrientation) {
-        sourceBuffer += sizeof(position);
+        _hasNewJointRotations = true;
+        setLocalOrientation(newOrientation);
+    }
 
-        memcpy(&_globalPosition, sourceBuffer, sizeof(_globalPosition));
+    float scale;
-        sourceBuffer += sizeof(_globalPosition);
+    unpackFloatRatioFromTwoByte((uint8_t*)&header->scale, scale);
-
+    if (glm::isnan(scale)) {
-        if (glm::isnan(position.x) || glm::isnan(position.y) || glm::isnan(position.z)) {
+        if (shouldLogError(now)) {
-            if (shouldLogError(now)) {
+            qCWarning(avatars) << "Discard AvatarData packet: scale NaN, uuid " << getSessionUUID();
-                qCDebug(avatars) << "Discard nan AvatarData::position; displayName = '" << _displayName << "'";
-            }
-            return maxAvailableSize;
         }
-        setLocalPosition(position);
+        return buffer.size();
+    }
+    _targetScale = std::max(MIN_AVATAR_SCALE, std::min(MAX_AVATAR_SCALE, scale));
 
-        // rotation (NOTE: This needs to become a quaternion to save two bytes)
+    glm::vec3 lookAt = glm::vec3(header->lookAtPosition[0], header->lookAtPosition[1], header->lookAtPosition[2]);
-        float yaw, pitch, roll;
+    if (glm::isnan(lookAt.x) || glm::isnan(lookAt.y) || glm::isnan(lookAt.z)) {
-        sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &yaw);
+        if (shouldLogError(now)) {
-        sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &pitch);
+            qCWarning(avatars) << "Discard AvatarData packet: lookAtPosition is NaN, uuid " << getSessionUUID();
-        sourceBuffer += unpackFloatAngleFromTwoByte((uint16_t*) sourceBuffer, &roll);
-        if (glm::isnan(yaw) || glm::isnan(pitch) || glm::isnan(roll)) {
-            if (shouldLogError(now)) {
-                qCDebug(avatars) << "Discard nan AvatarData::yaw,pitch,roll; displayName = '" << _displayName << "'";
-            }
-            return maxAvailableSize;
         }
+        return buffer.size();
+    }
+    _headData->_lookAtPosition = lookAt;
 
-        // TODO is this safe? will the floats not exactly match?
+    float audioLoudness = header->audioLoudness;
-        // Andrew says:
+    if (glm::isnan(audioLoudness)) {
-        // Yes, there is a possibility that the transmitted will not quite match the extracted despite being originally
+        if (shouldLogError(now)) {
-        // extracted from the exact same quaternion. I followed the code through and it appears the risk is that the
+            qCWarning(avatars) << "Discard AvatarData packet: audioLoudness is NaN, uuid " << getSessionUUID();
-        // avatar's SkeletonModel might fall into the CPU expensive part of Model::updateClusterMatrices() when otherwise it
-        // would not have required it.  However, we know we can update many simultaneously animating avatars, and most
-        // avatars will be moving constantly anyway, so I don't think we need to worry.
-        glm::quat currentOrientation = getLocalOrientation();
-        glm::vec3 newEulerAngles(pitch, yaw, roll);
-        glm::quat newOrientation = glm::quat(glm::radians(newEulerAngles));
-        if (currentOrientation != newOrientation) {
-            _hasNewJointRotations = true;
-            setLocalOrientation(newOrientation);
         }
-
+        return buffer.size();
-        // scale
+    }
-        float scale;
+    _headData->_audioLoudness = audioLoudness;
-        sourceBuffer += unpackFloatRatioFromTwoByte(sourceBuffer, scale);
-        if (glm::isnan(scale)) {
-            if (shouldLogError(now)) {
-                qCDebug(avatars) << "Discard nan AvatarData::scale; displayName = '" << _displayName << "'";
-            }
-            return maxAvailableSize;
-        }
-        _targetScale = std::max(MIN_AVATAR_SCALE, std::min(MAX_AVATAR_SCALE, scale));
-    } // 20 bytes
-
-    { // Lookat Position
-        glm::vec3 lookAt;
-        memcpy(&lookAt, sourceBuffer, sizeof(lookAt));
-        sourceBuffer += sizeof(lookAt);
-        if (glm::isnan(lookAt.x) || glm::isnan(lookAt.y) || glm::isnan(lookAt.z)) {
-            if (shouldLogError(now)) {
-                qCDebug(avatars) << "Discard nan AvatarData::lookAt; displayName = '" << _displayName << "'";
-            }
-            return maxAvailableSize;
-        }
-        _headData->_lookAtPosition = lookAt;
-    } // 12 bytes
-
-    { // AudioLoudness
-        // Instantaneous audio loudness (used to drive facial animation)
-        float audioLoudness;
-        memcpy(&audioLoudness, sourceBuffer, sizeof(float));
-        sourceBuffer += sizeof(float);
-        if (glm::isnan(audioLoudness)) {
-            if (shouldLogError(now)) {
-                qCDebug(avatars) << "Discard nan AvatarData::audioLoudness; displayName = '" << _displayName << "'";
-            }
-            return maxAvailableSize;
-         }
-        _headData->_audioLoudness = audioLoudness;
-    } // 4 bytes
 
     { // bitFlags and face data
-        unsigned char bitItems = *sourceBuffer++;
+        uint8_t bitItems = header->flags;
 
         // key state, stored as a semi-nibble in the bitItems
-        _keyState = (KeyState)getSemiNibbleAt(bitItems,KEY_STATE_START_BIT);
+        _keyState = (KeyState)getSemiNibbleAt(bitItems, KEY_STATE_START_BIT);
 
         // hand state, stored as a semi-nibble plus a bit in the bitItems
         // we store the hand state as well as other items in a shared bitset. The hand state is an octal, but is split
@@ -520,95 +504,48 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
         bool hasReferential = oneAtBit(bitItems, HAS_REFERENTIAL);
 
         if (hasReferential) {
-            const int sizeOfPackedUuid = 16;
+            PACKET_READ_CHECK(ParentInfo, sizeof(AvatarDataPacket::ParentInfo));
-            QByteArray referentialAsBytes((const char*)sourceBuffer, sizeOfPackedUuid);
+            auto parentInfo = reinterpret_cast<const AvatarDataPacket::ParentInfo*>(sourceBuffer);
-            _parentID = QUuid::fromRfc4122(referentialAsBytes);
+            sourceBuffer += sizeof(AvatarDataPacket::ParentInfo);
-            sourceBuffer += sizeOfPackedUuid;
+
-            memcpy(&_parentJointIndex, sourceBuffer, sizeof(_parentJointIndex));
+            const size_t RFC_4122_SIZE = 16;
-            sourceBuffer += sizeof(_parentJointIndex);
+            QByteArray byteArray((const char*)parentInfo->parentUUID, RFC_4122_SIZE);
+            _parentID = QUuid::fromRfc4122(byteArray);
+            _parentJointIndex = parentInfo->parentJointIndex;
         } else {
             _parentID = QUuid();
         }
 
         if (_headData->_isFaceTrackerConnected) {
-            float leftEyeBlink, rightEyeBlink, averageLoudness, browAudioLift;
+            PACKET_READ_CHECK(FaceTrackerInfo, sizeof(AvatarDataPacket::FaceTrackerInfo));
-            minPossibleSize += sizeof(leftEyeBlink) + sizeof(rightEyeBlink) + sizeof(averageLoudness) + sizeof(browAudioLift);
+            auto faceTrackerInfo = reinterpret_cast<const AvatarDataPacket::FaceTrackerInfo*>(sourceBuffer);
-            minPossibleSize++; // one byte for blendDataSize
+            sourceBuffer += sizeof(AvatarDataPacket::FaceTrackerInfo);
-            if (minPossibleSize > maxAvailableSize) {
-                if (shouldLogError(now)) {
-                    qCDebug(avatars) << "Malformed AvatarData packet after BitItems;"
-                        << " displayName = '" << _displayName << "'"
-                        << " minPossibleSize = " << minPossibleSize
-                        << " maxAvailableSize = " << maxAvailableSize;
-                }
-                return maxAvailableSize;
-            }
-            // unpack face data
-            memcpy(&leftEyeBlink, sourceBuffer, sizeof(float));
-            sourceBuffer += sizeof(float);
 
-            memcpy(&rightEyeBlink, sourceBuffer, sizeof(float));
+            _headData->_leftEyeBlink = faceTrackerInfo->leftEyeBlink;
-            sourceBuffer += sizeof(float);
+            _headData->_rightEyeBlink = faceTrackerInfo->rightEyeBlink;
+            _headData->_averageLoudness = faceTrackerInfo->averageLoudness;
+            _headData->_browAudioLift = faceTrackerInfo->browAudioLift;
 
-            memcpy(&averageLoudness, sourceBuffer, sizeof(float));
+            int numCoefficients = faceTrackerInfo->numBlendshapeCoefficients;
-            sourceBuffer += sizeof(float);
+            const int coefficientsSize = sizeof(float) * numCoefficients;
-
+            PACKET_READ_CHECK(FaceTrackerCoefficients, coefficientsSize);
-            memcpy(&browAudioLift, sourceBuffer, sizeof(float));
+            _headData->_blendshapeCoefficients.resize(numCoefficients);  // make sure there's room for the copy!
-            sourceBuffer += sizeof(float);
+            memcpy(_headData->_blendshapeCoefficients.data(), sourceBuffer, coefficientsSize);
-
+            sourceBuffer += coefficientsSize;
-            if (glm::isnan(leftEyeBlink) || glm::isnan(rightEyeBlink)
-                    || glm::isnan(averageLoudness) || glm::isnan(browAudioLift)) {
-                if (shouldLogError(now)) {
-                    qCDebug(avatars) << "Discard nan AvatarData::faceData; displayName = '" << _displayName << "'";
-                }
-                return maxAvailableSize;
-            }
-            _headData->_leftEyeBlink = leftEyeBlink;
-            _headData->_rightEyeBlink = rightEyeBlink;
-            _headData->_averageLoudness = averageLoudness;
-            _headData->_browAudioLift = browAudioLift;
-
-            int numCoefficients = (int)(*sourceBuffer++);
-            int blendDataSize = numCoefficients * sizeof(float);
-            minPossibleSize += blendDataSize;
-            if (minPossibleSize > maxAvailableSize) {
-                if (shouldLogError(now)) {
-                    qCDebug(avatars) << "Malformed AvatarData packet after Blendshapes;"
-                        << " displayName = '" << _displayName << "'"
-                        << " minPossibleSize = " << minPossibleSize
-                        << " maxAvailableSize = " << maxAvailableSize;
-                }
-                return maxAvailableSize;
-            }
-
-            _headData->_blendshapeCoefficients.resize(numCoefficients);
-            memcpy(_headData->_blendshapeCoefficients.data(), sourceBuffer, blendDataSize);
-            sourceBuffer += numCoefficients * sizeof(float);
-
-            //bitItemsDataSize = 4 * sizeof(float) + 1 + blendDataSize;
         }
-    } // 1 + bitItemsDataSize bytes
+    }
 
-    // joint rotations
+    PACKET_READ_CHECK(NumJoints, sizeof(uint8_t));
     int numJoints = *sourceBuffer++;
 
-    int bytesOfValidity = (int)ceil((float)numJoints / (float)BITS_IN_BYTE);
-    minPossibleSize += bytesOfValidity;
-    if (minPossibleSize > maxAvailableSize) {
-        if (shouldLogError(now)) {
-            qCDebug(avatars) << "Malformed AvatarData packet after JointValidityBits;"
-                << " displayName = '" << _displayName << "'"
-                << " minPossibleSize = " << minPossibleSize
-                << " maxAvailableSize = " << maxAvailableSize;
-        }
-        return maxAvailableSize;
-    }
-    int numValidJointRotations = 0;
     _jointData.resize(numJoints);
 
+    const int bytesOfValidity = (int)ceil((float)numJoints / (float)BITS_IN_BYTE);
+    PACKET_READ_CHECK(JointRotationValidityBits, bytesOfValidity);
+
+    int numValidJointRotations = 0;
     QVector<bool> validRotations;
     validRotations.resize(numJoints);
-
     { // rotation validity bits
         unsigned char validity = 0;
         int validityBit = 0;
@@ -623,39 +560,26 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
             validRotations[i] = valid;
             validityBit = (validityBit + 1) % BITS_IN_BYTE;
         }
-    } // 1 + bytesOfValidity bytes
-
-    // each joint rotation is stored in 6 bytes.
-
-    const size_t COMPRESSED_QUATERNION_SIZE = 6;
-    minPossibleSize += numValidJointRotations * COMPRESSED_QUATERNION_SIZE;
-    if (minPossibleSize > maxAvailableSize) {
-        if (shouldLogError(now)) {
-            qCDebug(avatars) << "Malformed AvatarData packet after JointData rotation validity;"
-                << " displayName = '" << _displayName << "'"
-                << " minPossibleSize = " << minPossibleSize
-                << " maxAvailableSize = " << maxAvailableSize;
-        }
-        return maxAvailableSize;
     }
 
-    { // joint data
+    // each joint rotation is stored in 6 bytes.
-        for (int i = 0; i < numJoints; i++) {
+    const int COMPRESSED_QUATERNION_SIZE = 6;
-            JointData& data = _jointData[i];
+    PACKET_READ_CHECK(JointRotations, numValidJointRotations * COMPRESSED_QUATERNION_SIZE);
-            if (validRotations[i]) {
+    for (int i = 0; i < numJoints; i++) {
-                sourceBuffer += unpackOrientationQuatFromSixBytes(sourceBuffer, data.rotation);
+        JointData& data = _jointData[i];
-                _hasNewJointRotations = true;
+        if (validRotations[i]) {
-                data.rotationSet = true;
+            sourceBuffer += unpackOrientationQuatFromSixBytes(sourceBuffer, data.rotation);
-            }
+            _hasNewJointRotations = true;
+            data.rotationSet = true;
         }
-    } // numJoints * 6 bytes
+    }
+
+    PACKET_READ_CHECK(JointTranslationValidityBits, bytesOfValidity);
 
-    // joint translations
     // get translation validity bits -- these indicate which translations were packed
     int numValidJointTranslations = 0;
     QVector<bool> validTranslations;
     validTranslations.resize(numJoints);
-
     { // translation validity bits
         unsigned char validity = 0;
         int validityBit = 0;
@@ -673,30 +597,18 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
     } // 1 + bytesOfValidity bytes
 
     // each joint translation component is stored in 6 bytes.
-    const size_t COMPRESSED_TRANSLATION_SIZE = 6;
+    const int COMPRESSED_TRANSLATION_SIZE = 6;
-    minPossibleSize += numValidJointTranslations * COMPRESSED_TRANSLATION_SIZE;
+    PACKET_READ_CHECK(JointTranslation, numValidJointTranslations * COMPRESSED_QUATERNION_SIZE);
-    if (minPossibleSize > maxAvailableSize) {
-        if (shouldLogError(now)) {
-            qCDebug(avatars) << "Malformed AvatarData packet after JointData translation validity;"
-                << " displayName = '" << _displayName << "'"
-                << " minPossibleSize = " << minPossibleSize
-                << " maxAvailableSize = " << maxAvailableSize;
-        }
-        return maxAvailableSize;
-    }
-
     const int TRANSLATION_COMPRESSION_RADIX = 12;
 
-    { // joint data
+    for (int i = 0; i < numJoints; i++) {
-        for (int i = 0; i < numJoints; i++) {
+        JointData& data = _jointData[i];
-            JointData& data = _jointData[i];
+        if (validTranslations[i]) {
-            if (validTranslations[i]) {
+            sourceBuffer += unpackFloatVec3FromSignedTwoByteFixed(sourceBuffer, data.translation, TRANSLATION_COMPRESSION_RADIX);
-                sourceBuffer += unpackFloatVec3FromSignedTwoByteFixed(sourceBuffer, data.translation, TRANSLATION_COMPRESSION_RADIX);
+            _hasNewJointTranslations = true;
-                _hasNewJointTranslations = true;
+            data.translationSet = true;
-                data.translationSet = true;
-            }
         }
-    } // numJoints * 6 bytes
+    }
 
     #ifdef WANT_DEBUG
     if (numValidJointRotations > 15) {
@@ -707,6 +619,10 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
     #endif
 
     int numBytesRead = sourceBuffer - startPosition;
+
+    // AJT: Maybe make this a warning.
+    ASSERT(numBytesRead == buffer.size());
+
     _averageBytesReceived.updateAverage(numBytesRead);
     return numBytesRead;
 }

libraries/avatars/src/AvatarData.h

@@ -53,6 +53,7 @@ typedef unsigned long long quint64;
 #include <SimpleMovingAverage.h>
 #include <SpatiallyNestable.h>
 #include <NumericalConstants.h>
+#include <Packed.h>
 
 #include "AABox.h"
 #include "HeadData.h"
@@ -165,6 +166,7 @@ class AvatarData : public QObject, public SpatiallyNestable {
     Q_PROPERTY(QUuid sessionUUID READ getSessionUUID)
 
 public:
+
     static const QString FRAME_NAME;
 
     static void fromFrame(const QByteArray& frameData, AvatarData& avatar);

libraries/shared/src/Packed.h

@@ -0,0 +1,12 @@
+#ifndef hifi_Packed_h
+#define hifi_Packed_h
+
+#if defined(_MSC_VER)
+#define PACKED_BEGIN __pragma(pack(push, 1))
+#define PACKED_END __pragma(pack(pop));
+#else
+#define PACKED_BEGIN
+#define PACKED_END __attribute__((__packed__));
+#endif
+
+#endif