From 689a0b5badb2479b382aee3d783e68939fb99b6f Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 17 May 2017 14:22:36 -0700 Subject: [PATCH 1/2] Switch from timestamp to sequence id for avatar identity --- libraries/avatars/src/AvatarData.cpp | 45 ++++++++------------ libraries/avatars/src/AvatarData.h | 36 ++++++++-------- libraries/networking/src/udt/PacketHeaders.h | 3 +- 3 files changed, 38 insertions(+), 46 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index eebb9c2633..d69af74c97 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -170,13 +170,13 @@ QByteArray AvatarData::toByteArrayStateful(AvatarDataDetail dataDetail) { AvatarDataPacket::HasFlags hasFlagsOut; auto lastSentTime = _lastToByteArray; _lastToByteArray = usecTimestampNow(); - return AvatarData::toByteArray(dataDetail, lastSentTime, getLastSentJointData(), + return AvatarData::toByteArray(dataDetail, lastSentTime, getLastSentJointData(), hasFlagsOut, false, false, glm::vec3(0), nullptr, &_outboundDataRate); } QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSentTime, const QVector& lastSentJointData, - AvatarDataPacket::HasFlags& hasFlagsOut, bool dropFaceTracking, bool distanceAdjust, + AvatarDataPacket::HasFlags& hasFlagsOut, bool dropFaceTracking, bool distanceAdjust, glm::vec3 viewerPosition, QVector* sentJointDataOut, AvatarDataRate* outboundDataRateOut) const { bool cullSmallChanges = (dataDetail == CullSmallData); @@ -199,7 +199,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent // FIXME - // - // BUG -- if you enter a space bubble, and then back away, the avatar has wrong orientation until "send all" happens... + // BUG -- if you enter a space bubble, and then back away, the avatar has wrong orientation until "send all" happens... // this is an iFrame issue... what to do about that? // // BUG -- Resizing avatar seems to "take too long"... the avatar doesn't redraw at smaller size right away @@ -1471,13 +1471,13 @@ QStringList AvatarData::getJointNames() const { void AvatarData::parseAvatarIdentityPacket(const QByteArray& data, Identity& identityOut) { QDataStream packetStream(data); - packetStream >> identityOut.uuid - >> identityOut.skeletonModelURL + packetStream >> identityOut.uuid + >> identityOut.skeletonModelURL >> identityOut.attachmentData >> identityOut.displayName >> identityOut.sessionDisplayName >> identityOut.avatarEntityData - >> identityOut.updatedAt; + >> identityOut.sequenceId; #ifdef WANT_DEBUG qCDebug(avatars) << __FUNCTION__ @@ -1500,20 +1500,14 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { } void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew) { - quint64 identityPacketUpdatedAt = identity.updatedAt; - if (identityPacketUpdatedAt <= (uint64_t)(abs(clockSkew))) { // Incoming timestamp is bad - compute our own timestamp - identityPacketUpdatedAt = usecTimestampNow() + clockSkew; - } - - // Consider the case where this packet is being processed on Client A, and Client A is connected to Sandbox B. - // If Client A's system clock is *ahead of* Sandbox B's system clock, "clockSkew" will be *negative*. - // If Client A's system clock is *behind* Sandbox B's system clock, "clockSkew" will be *positive*. - if ((_identityUpdatedAt > identityPacketUpdatedAt - clockSkew) && (_identityUpdatedAt != 0)) { - qCDebug(avatars) << "Ignoring late identity packet for avatar " << getSessionUUID() - << "_identityUpdatedAt (" << _identityUpdatedAt << ") is greater than identityPacketUpdatedAt - clockSkew (" << identityPacketUpdatedAt << "-" << clockSkew << ")"; + if (identity.sequenceId < _identitySequenceId) { + qCDebug(avatars) << "Ignoring older identity packet for avatar" << getSessionUUID() + << "_identitySequenceId (" << _identitySequenceId << ") is greater than" << identity.sequenceId; return; } + // otherwise, set the identitySequenceId to match the incoming identity + _identitySequenceId = identity.sequenceId; if (_firstSkeletonCheck || (identity.skeletonModelURL != cannonicalSkeletonModelURL(emptyURL))) { setSkeletonModelURL(identity.skeletonModelURL); @@ -1545,9 +1539,6 @@ void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityC identityChanged = true; } - // use the timestamp from this identity, since we want to honor the updated times in "server clock" - // this will overwrite any changes we made locally to this AvatarData's _identityUpdatedAt - _identityUpdatedAt = identityPacketUpdatedAt - clockSkew; } QByteArray AvatarData::identityByteArray() const { @@ -1556,13 +1547,13 @@ QByteArray AvatarData::identityByteArray() const { const QUrl& urlToSend = cannonicalSkeletonModelURL(emptyURL); // depends on _skeletonModelURL _avatarEntitiesLock.withReadLock([&] { - identityStream << getSessionUUID() - << urlToSend + identityStream << getSessionUUID() + << urlToSend << _attachmentData << _displayName << getSessionDisplayNameForTransport() // depends on _sessionDisplayName << _avatarEntityData - << _identityUpdatedAt; + << _identitySequenceId; }); return identityData; @@ -2477,11 +2468,11 @@ QScriptValue AvatarEntityMapToScriptValue(QScriptEngine* engine, const AvatarEnt if (!jsonEntityProperties.isObject()) { qCDebug(avatars) << "bad AvatarEntityData in AvatarEntityMap" << QString(entityProperties.toHex()); } - + QVariant variantEntityProperties = jsonEntityProperties.toVariant(); QVariantMap entityPropertiesMap = variantEntityProperties.toMap(); QScriptValue scriptEntityProperties = variantMapToScriptValue(entityPropertiesMap, *engine); - + QString key = entityID.toString(); obj.setProperty(key, scriptEntityProperties); } @@ -2493,12 +2484,12 @@ void AvatarEntityMapFromScriptValue(const QScriptValue& object, AvatarEntityMap& while (itr.hasNext()) { itr.next(); QUuid EntityID = QUuid(itr.name()); - + QScriptValue scriptEntityProperties = itr.value(); QVariant variantEntityProperties = scriptEntityProperties.toVariant(); QJsonDocument jsonEntityProperties = QJsonDocument::fromVariant(variantEntityProperties); QByteArray binaryEntityProperties = jsonEntityProperties.toBinaryData(); - + value[EntityID] = binaryEntityProperties; } } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index f248874943..b4836e3a2e 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -165,15 +165,15 @@ namespace AvatarDataPacket { const size_t AVATAR_ORIENTATION_SIZE = 6; PACKED_BEGIN struct AvatarScale { - SmallFloat scale; // avatar's scale, compressed by packFloatRatioToTwoByte() + SmallFloat scale; // avatar's scale, compressed by packFloatRatioToTwoByte() } PACKED_END; const size_t AVATAR_SCALE_SIZE = 2; PACKED_BEGIN struct LookAtPosition { float lookAtPosition[3]; // world space position that eyes are focusing on. - // FIXME - unless the person has an eye tracker, this is simulated... + // FIXME - unless the person has an eye tracker, this is simulated... // a) maybe we can just have the client calculate this - // b) at distance this will be hard to discern and can likely be + // b) at distance this will be hard to discern and can likely be // descimated or dropped completely // // POTENTIAL SAVINGS - 12 bytes @@ -376,10 +376,10 @@ public: glm::vec3 getHandPosition() const; void setHandPosition(const glm::vec3& handPosition); - typedef enum { + typedef enum { NoData, PALMinimum, - MinimumData, + MinimumData, CullSmallData, IncludeSmallData, SendAllData @@ -388,7 +388,7 @@ public: virtual QByteArray toByteArrayStateful(AvatarDataDetail dataDetail); virtual QByteArray toByteArray(AvatarDataDetail dataDetail, quint64 lastSentTime, const QVector& lastSentJointData, - AvatarDataPacket::HasFlags& hasFlagsOut, bool dropFaceTracking, bool distanceAdjust, glm::vec3 viewerPosition, + AvatarDataPacket::HasFlags& hasFlagsOut, bool dropFaceTracking, bool distanceAdjust, glm::vec3 viewerPosition, QVector* sentJointDataOut, AvatarDataRate* outboundDataRateOut = nullptr) const; virtual void doneEncoding(bool cullSmallChanges); @@ -417,23 +417,23 @@ public: void nextAttitude(glm::vec3 position, glm::quat orientation); // Can be safely called at any time. virtual void updateAttitude() {} // Tell skeleton mesh about changes - glm::quat getHeadOrientation() const { + glm::quat getHeadOrientation() const { lazyInitHeadData(); - return _headData->getOrientation(); + return _headData->getOrientation(); } - void setHeadOrientation(const glm::quat& orientation) { + void setHeadOrientation(const glm::quat& orientation) { if (_headData) { _headData->setOrientation(orientation); } } - void setLookAtPosition(const glm::vec3& lookAtPosition) { + void setLookAtPosition(const glm::vec3& lookAtPosition) { if (_headData) { - _headData->setLookAtPosition(lookAtPosition); + _headData->setLookAtPosition(lookAtPosition); } } - void setBlendshapeCoefficients(const QVector& blendshapeCoefficients) { + void setBlendshapeCoefficients(const QVector& blendshapeCoefficients) { if (_headData) { _headData->setBlendshapeCoefficients(blendshapeCoefficients); } @@ -470,7 +470,7 @@ public: void setDomainMinimumScale(float domainMinimumScale) { _domainMinimumScale = glm::clamp(domainMinimumScale, MIN_AVATAR_SCALE, MAX_AVATAR_SCALE); _scaleChanged = usecTimestampNow(); } - void setDomainMaximumScale(float domainMaximumScale) + void setDomainMaximumScale(float domainMaximumScale) { _domainMaximumScale = glm::clamp(domainMaximumScale, MIN_AVATAR_SCALE, MAX_AVATAR_SCALE); _scaleChanged = usecTimestampNow(); } // Hand State @@ -531,7 +531,7 @@ public: QString displayName; QString sessionDisplayName; AvatarEntityMap avatarEntityData; - quint64 updatedAt; + quint64 sequenceId; }; static void parseAvatarIdentityPacket(const QByteArray& data, Identity& identityOut); @@ -548,8 +548,8 @@ public: virtual void setSkeletonModelURL(const QUrl& skeletonModelURL); virtual void setDisplayName(const QString& displayName); - virtual void setSessionDisplayName(const QString& sessionDisplayName) { - _sessionDisplayName = sessionDisplayName; + virtual void setSessionDisplayName(const QString& sessionDisplayName) { + _sessionDisplayName = sessionDisplayName; markIdentityDataChanged(); } @@ -626,7 +626,7 @@ public: bool getIdentityDataChanged() const { return _identityDataChanged; } // has the identity data changed since the last time sendIdentityPacket() was called void markIdentityDataChanged() { _identityDataChanged = true; - _identityUpdatedAt = usecTimestampNow(); + _identitySequenceId++; } signals: @@ -784,7 +784,7 @@ protected: float _audioAverageLoudness { 0.0f }; bool _identityDataChanged { false }; - quint64 _identityUpdatedAt { 0 }; + quint64 _identitySequenceId { 0 }; private: friend void avatarStateFromFrame(const QByteArray& frameData, AvatarData* _avatar); diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 746ae80361..f88015a4e4 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -234,7 +234,8 @@ enum class AvatarMixerPacketVersion : PacketVersion { VariableAvatarData, AvatarAsChildFixes, StickAndBallDefaultAvatar, - IdentityPacketsIncludeUpdateTime + IdentityPacketsIncludeUpdateTime, + AvatarIdentitySequenceId }; enum class DomainConnectRequestVersion : PacketVersion { From 057718bde3c6d8aff13cfff755d54c6ded3106c5 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 17 May 2017 15:14:56 -0700 Subject: [PATCH 2/2] remove clockSkew, reference new version for Avatar packets --- assignment-client/src/avatars/AvatarMixer.cpp | 10 +++++----- libraries/avatars/src/AvatarData.cpp | 2 +- libraries/avatars/src/AvatarData.h | 2 +- libraries/avatars/src/AvatarHashMap.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 998799f5e6..870149f1bc 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -402,7 +402,7 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes AvatarData::parseAvatarIdentityPacket(message->getMessage(), identity); bool identityChanged = false; bool displayNameChanged = false; - avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged, senderNode->getClockSkewUsec()); + avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged); if (identityChanged) { QMutexLocker nodeDataLocker(&nodeData->getMutex()); nodeData->flagIdentityChange(); @@ -665,12 +665,12 @@ void AvatarMixer::sendStatsPacket() { void AvatarMixer::run() { qCDebug(avatars) << "Waiting for connection to domain to request settings from domain-server."; - + // wait until we have the domain-server settings, otherwise we bail DomainHandler& domainHandler = DependencyManager::get()->getDomainHandler(); connect(&domainHandler, &DomainHandler::settingsReceived, this, &AvatarMixer::domainSettingsRequestComplete); connect(&domainHandler, &DomainHandler::settingsReceiveFail, this, &AvatarMixer::domainSettingsRequestFailed); - + ThreadedAssignment::commonInit(AVATAR_MIXER_LOGGING_NAME, NodeType::AvatarMixer); } @@ -695,7 +695,7 @@ void AvatarMixer::domainSettingsRequestComplete() { // parse the settings to pull out the values we need parseDomainServerSettings(nodeList->getDomainHandler().getSettingsObject()); - + // start our tight loop... start(); } @@ -745,7 +745,7 @@ void AvatarMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { } else { qCDebug(avatars) << "Avatar mixer will automatically determine number of threads to use. Using:" << _slavePool.numThreads() << "threads."; } - + const QString AVATARS_SETTINGS_KEY = "avatars"; static const QString MIN_SCALE_OPTION = "min_avatar_scale"; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index d69af74c97..57ef80000a 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1499,7 +1499,7 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { return _skeletonModelURL.scheme() == "file" ? emptyURL : _skeletonModelURL; } -void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew) { +void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged) { if (identity.sequenceId < _identitySequenceId) { qCDebug(avatars) << "Ignoring older identity packet for avatar" << getSessionUUID() diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index b4836e3a2e..99a437b885 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -538,7 +538,7 @@ public: // identityChanged returns true if identity has changed, false otherwise. // displayNameChanged returns true if displayName has changed, false otherwise. - void processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew); + void processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged); QByteArray identityByteArray() const; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 155ef9a0a2..2ccc64fee2 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -149,7 +149,7 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer bool identityChanged = false; bool displayNameChanged = false; // In this case, the "sendingNode" is the Avatar Mixer. - avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged, sendingNode->getClockSkewUsec()); + avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged); } } diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 82b4bf703d..9d970fa318 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -56,7 +56,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::AvatarData: case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::IdentityPacketsIncludeUpdateTime); + return static_cast(AvatarMixerPacketVersion::AvatarIdentitySequenceId); case PacketType::MessagesData: return static_cast(MessageDataVersion::TextOrBinaryData); case PacketType::ICEServerHeartbeat: