From 2017ea4491a91027cc6d3c498b346bc9d36e6c8e Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 21 Feb 2017 14:18:00 -0800 Subject: [PATCH 01/16] hacking on using sorted avatars for bandwidth budget --- .../src/avatars/AvatarMixerClientData.cpp | 12 ++- .../src/avatars/AvatarMixerClientData.h | 11 +++ .../src/avatars/AvatarMixerSlave.cpp | 76 ++++++++++++++++++- interface/src/avatar/Avatar.cpp | 5 -- interface/src/avatar/Avatar.h | 1 - interface/src/avatar/AvatarManager.cpp | 66 ++++------------ libraries/avatars/src/AvatarData.cpp | 63 +++++++++++++++ libraries/avatars/src/AvatarData.h | 27 ++++++- libraries/avatars/src/AvatarHashMap.cpp | 1 + libraries/avatars/src/AvatarHashMap.h | 1 - 10 files changed, 199 insertions(+), 64 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index df6a64e874..1df6c24a60 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -74,14 +74,22 @@ bool AvatarMixerClientData::checkAndSetHasReceivedFirstPacketsFrom(const QUuid& return true; } +uint64_t AvatarMixerClientData::getLastBroadcastTime(const QUuid& nodeUUID) const { + // return the matching PacketSequenceNumber, or the default if we don't have it + auto nodeMatch = _lastBroadcastTimes.find(nodeUUID); + if (nodeMatch != _lastBroadcastTimes.end()) { + return nodeMatch->second; + } + return 0; +} + uint16_t AvatarMixerClientData::getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const { // return the matching PacketSequenceNumber, or the default if we don't have it auto nodeMatch = _lastBroadcastSequenceNumbers.find(nodeUUID); if (nodeMatch != _lastBroadcastSequenceNumbers.end()) { return nodeMatch->second; - } else { - return 0; } + return 0; } void AvatarMixerClientData::ignoreOther(SharedNodePointer self, SharedNodePointer other) { diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 87f7bb9cc9..7eea31c9ab 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -43,6 +43,7 @@ public: int parseData(ReceivedMessage& message) override; AvatarData& getAvatar() { return *_avatar; } const AvatarData* getConstAvatarData() const { return _avatar.get(); } + AvatarSharedPointer getAvatarSharedPointer() const { return _avatar; } bool checkAndSetHasReceivedFirstPacketsFrom(const QUuid& uuid); @@ -51,6 +52,12 @@ public: { _lastBroadcastSequenceNumbers[nodeUUID] = sequenceNumber; } Q_INVOKABLE void removeLastBroadcastSequenceNumber(const QUuid& nodeUUID) { _lastBroadcastSequenceNumbers.erase(nodeUUID); } + uint64_t getLastBroadcastTime(const QUuid& nodeUUID) const; + void setLastBroadcastTime(const QUuid& nodeUUID, uint64_t broadcastTime) { + _lastBroadcastTimes[nodeUUID] = broadcastTime; + } + Q_INVOKABLE void removeLastBroadcastTime(const QUuid& nodeUUID) { _lastBroadcastTimes.erase(nodeUUID); } + uint16_t getLastReceivedSequenceNumber() const { return _lastReceivedSequenceNumber; } HRCTime getIdentityChangeTimestamp() const { return _identityChangeTimestamp; } @@ -106,6 +113,9 @@ public: bool getRequestsDomainListData() { return _requestsDomainListData; } void setRequestsDomainListData(bool requesting) { _requestsDomainListData = requesting; } + ViewFrustum getViewFrustom() const { return _currentViewFrustum; } + + quint64 getLastOtherAvatarEncodeTime(QUuid otherAvatar) { quint64 result = 0; if (_lastOtherAvatarEncodeTime.find(otherAvatar) != _lastOtherAvatarEncodeTime.end()) { @@ -134,6 +144,7 @@ private: uint16_t _lastReceivedSequenceNumber { 0 }; std::unordered_map _lastBroadcastSequenceNumbers; std::unordered_set _hasReceivedFirstPacketsFrom; + std::unordered_map _lastBroadcastTimes; // this is a map of the last time we encoded an "other" avatar for // sending to "this" node diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 46133a221f..4c95c96483 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -187,9 +187,75 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // setup a PacketList for the avatarPackets auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); + + QList avatarList; + std::unordered_map avatarDataToNodes; + + //qDebug() << "------------------------------"; + int listItem = 0; + std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { + const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); + if (otherNodeData) { + listItem++; + AvatarSharedPointer otherAvatar = otherNodeData->getAvatarSharedPointer(); + avatarList << otherAvatar; + avatarDataToNodes[otherAvatar] = otherNode; + /* + qDebug() << "listItem [" << listItem << "] " + << "otherNode:" << otherNode.data() + << "otherNode->getUUID():" << otherNode->getUUID() + << "otherNodeData:" << otherNodeData + << "otherAvatar:" << otherAvatar.get(); + + qDebug() << "avatarDataToNodes[" << otherAvatar.get() << "]=" << otherNode.data(); + */ + } + + }); + /* + qDebug() << "------------------------------"; + qDebug() << "avatarList.size:" << avatarList.size(); + qDebug() << "avatarDataToNodes.size:" << avatarDataToNodes.size(); + */ + + ViewFrustum cameraView = nodeData->getViewFrustom(); + std::priority_queue sortedAvatars = AvatarData::sortAvatars( + avatarList, cameraView, + + [&](AvatarSharedPointer avatar)->uint64_t{ + auto avatarNode = avatarDataToNodes[avatar]; + if (avatarNode) { + return nodeData->getLastBroadcastTime(avatarNode->getUUID()); + } + return 0; // ??? + }, + + [this](AvatarSharedPointer avatar)->bool{ + // FIXME -- when to ignore this node + return false; + }); + + // this is an AGENT we have received head data from // send back a packet with other active node data to this node - std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { + //std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { + + int avatarRank = 0; + while (!sortedAvatars.empty()) { + AvatarPriority sortData = sortedAvatars.top(); + sortedAvatars.pop(); + const auto& avatarData = sortData.avatar; + avatarRank++; + + auto otherNode = avatarDataToNodes[avatarData]; + + //qDebug() << "otherNode (" << otherNode.data() << ")= avatarDataToNodes[" << avatarData.get() << "]"; + + if (!otherNode) { + //qDebug() << "For viewer:" << node->getUUID() << "... process other avatar [" << avatarRank << ":" << avatarData.get() << "... otherNode unknown!!"; + continue; + } + //qDebug() << "For viewer:" << node->getUUID() << "... process other avatar [" << avatarRank << "] avatarData: " << avatarData.get() << " otherNode:" << otherNode->getUUID() << " ... "; bool shouldConsider = false; quint64 startIgnoreCalculation = usecTimestampNow(); @@ -286,7 +352,8 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // are out of view, this also appears to disable this random distribution. if (distanceToAvatar != 0.0f && !getsOutOfView - && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar)) { + // -- && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) /// FIX ME... no longer doing random + ) { quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); @@ -395,6 +462,9 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { nodeData->setLastBroadcastSequenceNumber(otherNode->getUUID(), otherNodeData->getLastReceivedSequenceNumber()); + // remember the last time we sent details about this other node to the receiver + nodeData->setLastBroadcastTime(otherNode->getUUID(), start); + } } @@ -406,7 +476,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { } } } - }); + }; quint64 startPacketSending = usecTimestampNow(); diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index ed8f083a41..64ecb1dc29 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -334,11 +334,6 @@ void Avatar::updateAvatarEntities() { setAvatarEntityDataChanged(false); } -bool Avatar::shouldDie() const { - const qint64 AVATAR_SILENCE_THRESHOLD_USECS = 5 * USECS_PER_SECOND; - return _owningAvatarMixer.isNull() || getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS; -} - void Avatar::simulate(float deltaTime, bool inView) { PROFILE_RANGE(simulation, "simulate"); diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index 80d387fd33..53f0562bdd 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -178,7 +178,6 @@ public: uint64_t getLastRenderUpdateTime() const { return _lastRenderUpdateTime; } void setLastRenderUpdateTime(uint64_t time) { _lastRenderUpdateTime = time; } - bool shouldDie() const; void animateScaleChanges(float deltaTime); void setTargetScale(float targetScale) override; diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index c3fc974365..1a9866aae4 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -148,16 +148,6 @@ float AvatarManager::getAvatarSimulationRate(const QUuid& sessionID, const QStri } - -class AvatarPriority { -public: - AvatarPriority(AvatarSharedPointer a, float p) : avatar(a), priority(p) {} - AvatarSharedPointer avatar; - float priority; - // NOTE: we invert the less-than operator to sort high priorities to front - bool operator<(const AvatarPriority& other) const { return priority > other.priority; } -}; - void AvatarManager::updateOtherAvatars(float deltaTime) { // lock the hash for read to check the size QReadLocker lock(&_hashLock); @@ -173,57 +163,31 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { QList avatarList = avatarMap.values(); ViewFrustum cameraView; qApp->copyDisplayViewFrustum(cameraView); - glm::vec3 frustumCenter = cameraView.getPosition(); - const float OUT_OF_VIEW_PENALTY = -10.0; + std::priority_queue sortedAvatars = AvatarData::sortAvatars( + avatarList, cameraView, - std::priority_queue sortedAvatars; - { - PROFILE_RANGE(simulation, "sort"); - for (int32_t i = 0; i < avatarList.size(); ++i) { - const auto& avatar = std::static_pointer_cast(avatarList.at(i)); - if (avatar == _myAvatar || !avatar->isInitialized()) { + [](AvatarSharedPointer avatar)->uint64_t{ + return std::static_pointer_cast(avatar)->getLastRenderUpdateTime(); + }, + + [this](AvatarSharedPointer avatar)->bool{ + const auto& castedAvatar = std::static_pointer_cast(avatar); + if (castedAvatar == _myAvatar || !castedAvatar->isInitialized()) { // DO NOT update _myAvatar! Its update has already been done earlier in the main loop. // DO NOT update or fade out uninitialized Avatars - continue; + return true; // ignore it } if (avatar->shouldDie()) { removeAvatar(avatar->getID()); - continue; + return true; // ignore it } if (avatar->isDead()) { - continue; + return true; // ignore it } - // priority = weighted linear combination of: - // (a) apparentSize - // (b) proximity to center of view - // (c) time since last update - // (d) TIME_PENALTY to help recently updated entries sort toward back - glm::vec3 avatarPosition = avatar->getPosition(); - glm::vec3 offset = avatarPosition - frustumCenter; - float distance = glm::length(offset) + 0.001f; // add 1mm to avoid divide by zero - float radius = avatar->getBoundingRadius(); - const glm::vec3& forward = cameraView.getDirection(); - float apparentSize = radius / distance; - float cosineAngle = glm::length(offset - glm::dot(offset, forward) * forward) / distance; - const float TIME_PENALTY = 0.080f; // seconds - float age = (float)(startTime - avatar->getLastRenderUpdateTime()) / (float)(USECS_PER_SECOND) - TIME_PENALTY; - // NOTE: we are adding values of different units to get a single measure of "priority". - // Thus we multiply each component by a conversion "weight" that scales its units - // relative to the others. These weights are pure magic tuning and are hard coded in the - // relation below: (hint: unitary weights are not explicityly shown) - float priority = apparentSize + 0.25f * cosineAngle + age; - - // decrement priority of avatars outside keyhole - if (distance > cameraView.getCenterRadius()) { - if (!cameraView.sphereIntersectsFrustum(avatarPosition, radius)) { - priority += OUT_OF_VIEW_PENALTY; - } - } - sortedAvatars.push(AvatarPriority(avatar, priority)); - } - } + return false; + }); render::PendingChanges pendingChanges; const uint64_t RENDER_UPDATE_BUDGET = 1500; // usec @@ -256,7 +220,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { uint64_t now = usecTimestampNow(); if (now < renderExpiry) { // we're within budget - const float OUT_OF_VIEW_THRESHOLD = 0.5f * OUT_OF_VIEW_PENALTY; + const float OUT_OF_VIEW_THRESHOLD = 0.5f * AvatarData::OUT_OF_VIEW_PENALTY; bool inView = sortData.priority > OUT_OF_VIEW_THRESHOLD; avatar->simulate(deltaTime, inView); avatar->updateRenderItem(pendingChanges); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 2263964e8f..dffdc8c1ac 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include "AvatarLogging.h" @@ -2309,3 +2310,65 @@ void RayToAvatarIntersectionResultFromScriptValue(const QScriptValue& object, Ra vec3FromScriptValue(intersection, value.intersection); } } + +const float AvatarData::OUT_OF_VIEW_PENALTY = -10.0f; + +std::priority_queue AvatarData::sortAvatars( + QList avatarList, + const ViewFrustum& cameraView, + std::function lastUpdated, + std::function shouldIgnore) { + + uint64_t startTime = usecTimestampNow(); + + glm::vec3 frustumCenter = cameraView.getPosition(); + + std::priority_queue sortedAvatars; + { + PROFILE_RANGE(simulation, "sort"); + for (int32_t i = 0; i < avatarList.size(); ++i) { + const auto& avatar = avatarList.at(i); + + // FIXME - probably some lambda that allows the caller to reject some avatars + + if (shouldIgnore(avatar)) { + continue; + } + + // priority = weighted linear combination of: + // (a) apparentSize + // (b) proximity to center of view + // (c) time since last update + // (d) TIME_PENALTY to help recently updated entries sort toward back + glm::vec3 avatarPosition = avatar->getPosition(); + glm::vec3 offset = avatarPosition - frustumCenter; + float distance = glm::length(offset) + 0.001f; // add 1mm to avoid divide by zero + + // FIXME - AvatarData has something equivolent to this + float radius = 1.0f; // avatar->getBoundingRadius(); + + const glm::vec3& forward = cameraView.getDirection(); + float apparentSize = radius / distance; + float cosineAngle = glm::length(glm::dot(offset, forward) * forward) / distance; + + // FIXME - probably some lambda that allows the caller to specify the "updated since" + uint64_t lastUpdatedTime = lastUpdated(avatar); // avatar->getLastRenderUpdateTime() + float age = (float)(startTime - lastUpdatedTime) / (float)(USECS_PER_SECOND); + + // NOTE: we are adding values of different units to get a single measure of "priority". + // Thus we multiply each component by a conversion "weight" that scales its units + // relative to the others. These weights are pure magic tuning and are hard coded in the + // relation below: (hint: unitary weights are not explicityly shown) + float priority = apparentSize + 0.25f * cosineAngle + age; + + // decrement priority of avatars outside keyhole + if (distance > cameraView.getCenterRadius()) { + if (!cameraView.sphereIntersectsFrustum(avatarPosition, radius)) { + priority += OUT_OF_VIEW_PENALTY; + } + } + sortedAvatars.push(AvatarPriority(avatar, priority)); + } + } + return sortedAvatars; +} diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 264da75de2..71ade436ff 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -14,6 +14,8 @@ #include #include +#include + /* VS2010 defines stdint.h, but not inttypes.h */ #if defined(_MSC_VER) typedef signed char int8_t; @@ -57,6 +59,7 @@ typedef unsigned long long quint64; #include #include #include +#include #include "AABox.h" #include "HeadData.h" @@ -304,6 +307,14 @@ public: RateCounter<> jointDataRate; }; +class AvatarPriority { +public: + AvatarPriority(AvatarSharedPointer a, float p) : avatar(a), priority(p) {} + AvatarSharedPointer avatar; + float priority; + // NOTE: we invert the less-than operator to sort high priorities to front + bool operator<(const AvatarPriority& other) const { return priority < other.priority; } +}; class AvatarData : public QObject, public SpatiallyNestable { Q_OBJECT @@ -539,7 +550,7 @@ public: void setOwningAvatarMixer(const QWeakPointer& owningAvatarMixer) { _owningAvatarMixer = owningAvatarMixer; } - const AABox& getLocalAABox() const { return _localAABox; } + //const AABox& getLocalAABox() const { return _localAABox; } int getUsecsSinceLastUpdate() const { return _averageBytesReceived.getUsecsSinceLastEvent(); } int getAverageBytesReceivedPerSecond() const; @@ -578,6 +589,20 @@ public: } + bool shouldDie() const { + const qint64 AVATAR_SILENCE_THRESHOLD_USECS = 5 * USECS_PER_SECOND; + return _owningAvatarMixer.isNull() || getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS; + } + + static const float OUT_OF_VIEW_PENALTY; + + static std::priority_queue sortAvatars( + QList avatarList, + const ViewFrustum& cameraView, + std::function lastUpdated, + std::function shouldIgnore); + + public slots: void sendAvatarDataPacket(); void sendIdentityPacket(); diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 00c515a635..cdbf5f2a85 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -190,3 +190,4 @@ void AvatarHashMap::sessionUUIDChanged(const QUuid& sessionUUID, const QUuid& ol _lastOwnerSessionUUID = oldUUID; emit avatarSessionChangedEvent(sessionUUID, oldUUID); } + diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h index dd097aef22..104ac83261 100644 --- a/libraries/avatars/src/AvatarHashMap.h +++ b/libraries/avatars/src/AvatarHashMap.h @@ -27,7 +27,6 @@ #include "AvatarData.h" - class AvatarHashMap : public QObject, public Dependency { Q_OBJECT SINGLETON_DEPENDENCY From 8cc0b383c4c28d27de675a50465f17d8328ab307 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 21 Feb 2017 16:22:22 -0800 Subject: [PATCH 02/16] wire up bandwidth buget to sorting --- .../src/avatars/AvatarMixerSlave.cpp | 23 ++++++++++++++----- libraries/avatars/src/AvatarData.cpp | 2 ++ .../developer/debugging/debugAvatarMixer.js | 10 +++++--- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 4c95c96483..5c71b88af9 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -213,11 +213,15 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { }); /* - qDebug() << "------------------------------"; qDebug() << "avatarList.size:" << avatarList.size(); qDebug() << "avatarDataToNodes.size:" << avatarDataToNodes.size(); */ + //qDebug() << "------------------------------"; + AvatarSharedPointer thisAvatar = nodeData->getAvatarSharedPointer(); + + //qDebug() << "thisAvatar:" << thisAvatar.get(); + ViewFrustum cameraView = nodeData->getViewFrustom(); std::priority_queue sortedAvatars = AvatarData::sortAvatars( avatarList, cameraView, @@ -230,10 +234,10 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { return 0; // ??? }, - [this](AvatarSharedPointer avatar)->bool{ - // FIXME -- when to ignore this node - return false; + [thisAvatar](AvatarSharedPointer avatar)->bool{ + return (avatar == thisAvatar); // ignore ourselves... }); + //qDebug() << "------------------------------"; // this is an AGENT we have received head data from @@ -269,6 +273,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { || (otherNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { shouldConsider = false; + //qDebug() << "shouldConsider = false ... line:" << __LINE__; } else { const AvatarMixerClientData* otherData = reinterpret_cast(otherNode->getLinkedData()); @@ -304,6 +309,8 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { if (nodeBox.touches(otherNodeBox)) { nodeData->ignoreOther(node, otherNode); shouldConsider = getsAnyIgnored; + //qDebug() << "shouldConsider = getsAnyIgnored " << getsAnyIgnored << " ... line:" << __LINE__; + } } // Not close enough to ignore @@ -352,12 +359,13 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // are out of view, this also appears to disable this random distribution. if (distanceToAvatar != 0.0f && !getsOutOfView - // -- && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) /// FIX ME... no longer doing random + && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) /// FIX ME... we don't want to do this random stuff ) { quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; + //qDebug() << "shouldConsider = false ... line:" << __LINE__; } if (shouldConsider) { @@ -380,7 +388,9 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; - } else if (lastSeqFromSender - lastSeqToReceiver > 1) { + //qDebug() << "shouldConsider = false ... line:" << __LINE__; + } + else if (lastSeqFromSender - lastSeqToReceiver > 1) { // this is a skip - we still send the packet but capture the presence of the skip so we see it happening ++numAvatarsWithSkippedFrames; } @@ -399,6 +409,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; + //qDebug() << "shouldConsider = false ... line:" << __LINE__; } if (shouldConsider) { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index dffdc8c1ac..9416216c5a 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2367,6 +2367,8 @@ std::priority_queue AvatarData::sortAvatars( priority += OUT_OF_VIEW_PENALTY; } } + + //qDebug() << "avatar:" << avatar.get() << "priority:" << priority << "apparentSize:" << apparentSize << "cosineAngle:" << cosineAngle << "age:" << age; sortedAvatars.push(AvatarPriority(avatar, priority)); } } diff --git a/scripts/developer/debugging/debugAvatarMixer.js b/scripts/developer/debugging/debugAvatarMixer.js index ebd43fc2f0..90f2de13a9 100644 --- a/scripts/developer/debugging/debugAvatarMixer.js +++ b/scripts/developer/debugging/debugAvatarMixer.js @@ -58,11 +58,14 @@ function updateOverlays() { // setup a position for the overlay that is just above this avatar's head var overlayPosition = avatar.getJointPosition("Head"); - overlayPosition.y += 1.05; + overlayPosition.y += 1.15; + + var rows = 8; var text = avatarID + "\n" +"--- Data from Mixer ---\n" +"All: " + AvatarManager.getAvatarDataRate(avatarID).toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID).toFixed(2) + "hz)" + "\n" + /* +" GP: " + AvatarManager.getAvatarDataRate(avatarID,"globalPosition").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"globalPosition").toFixed(2) + "hz)" + "\n" +" LP: " + AvatarManager.getAvatarDataRate(avatarID,"localPosition").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"localPosition").toFixed(2) + "hz)" + "\n" +" BB: " + AvatarManager.getAvatarDataRate(avatarID,"avatarBoundingBox").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"avatarBoundingBox").toFixed(2) + "hz)" + "\n" @@ -74,11 +77,12 @@ function updateOverlays() { +" AF: " + AvatarManager.getAvatarDataRate(avatarID,"additionalFlags").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"additionalFlags").toFixed(2) + "hz)" + "\n" +" PI: " + AvatarManager.getAvatarDataRate(avatarID,"parentInfo").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"parentInfo").toFixed(2) + "hz)" + "\n" +" FT: " + AvatarManager.getAvatarDataRate(avatarID,"faceTracker").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"faceTracker").toFixed(2) + "hz)" + "\n" + */ +" JD: " + AvatarManager.getAvatarDataRate(avatarID,"jointData").toFixed(2) + "kbps (" + AvatarManager.getAvatarUpdateRate(avatarID,"jointData").toFixed(2) + "hz)" + "\n" +"--- Simulation ---\n" +"All: " + AvatarManager.getAvatarSimulationRate(avatarID,"avatar").toFixed(2) + "hz \n" +" inView: " + AvatarManager.getAvatarSimulationRate(avatarID,"avatarInView").toFixed(2) + "hz \n" - +" SM: " + AvatarManager.getAvatarSimulationRate(avatarID,"skeletonModel").toFixed(2) + "hz \n" + //+" SM: " + AvatarManager.getAvatarSimulationRate(avatarID,"skeletonModel").toFixed(2) + "hz \n" +" JD: " + AvatarManager.getAvatarSimulationRate(avatarID,"jointData").toFixed(2) + "hz \n" if (avatarID in debugOverlays) { @@ -93,7 +97,7 @@ function updateOverlays() { position: overlayPosition, dimensions: { x: 1.25, - y: 19 * 0.13 + y: rows * 0.13 }, lineHeight: 0.1, font:{size:0.1}, From 4c42e95607852cec20d23ffe716290aba8157c16 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 08:13:37 -0800 Subject: [PATCH 03/16] add some debugging --- .../src/avatars/AvatarMixerSlave.cpp | 23 +++++++++++++------ libraries/avatars/src/AvatarData.cpp | 11 ++++++--- libraries/avatars/src/AvatarData.h | 3 ++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 5c71b88af9..360c6e26fd 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -217,11 +217,19 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { qDebug() << "avatarDataToNodes.size:" << avatarDataToNodes.size(); */ - //qDebug() << "------------------------------"; AvatarSharedPointer thisAvatar = nodeData->getAvatarSharedPointer(); //qDebug() << "thisAvatar:" << thisAvatar.get(); +#ifdef WANT_DEBUG + bool printDebug = nodeData->getAvatarSharedPointer()->getDisplayName() == "ZappoMan"; + + if (printDebug) { + qDebug() << "------------------------------"; + } +#else + bool printDebug = false; +#endif ViewFrustum cameraView = nodeData->getViewFrustom(); std::priority_queue sortedAvatars = AvatarData::sortAvatars( avatarList, cameraView, @@ -236,14 +244,15 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { [thisAvatar](AvatarSharedPointer avatar)->bool{ return (avatar == thisAvatar); // ignore ourselves... - }); - //qDebug() << "------------------------------"; + }, printDebug); +#ifdef WANT_DEBUG + if (printDebug) { + qDebug() << "------------------------------"; + } +#endif - // this is an AGENT we have received head data from - // send back a packet with other active node data to this node - //std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { - + // loop through our sorted avatars and allocate our bandwidth to them accordingly int avatarRank = 0; while (!sortedAvatars.empty()) { AvatarPriority sortData = sortedAvatars.top(); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 9416216c5a..fd21670853 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2317,7 +2317,8 @@ std::priority_queue AvatarData::sortAvatars( QList avatarList, const ViewFrustum& cameraView, std::function lastUpdated, - std::function shouldIgnore) { + std::function shouldIgnore, + bool printDebug) { uint64_t startTime = usecTimestampNow(); @@ -2359,16 +2360,20 @@ std::priority_queue AvatarData::sortAvatars( // Thus we multiply each component by a conversion "weight" that scales its units // relative to the others. These weights are pure magic tuning and are hard coded in the // relation below: (hint: unitary weights are not explicityly shown) - float priority = apparentSize + 0.25f * cosineAngle + age; + float priority = apparentSize + 10.0f * (0.25f * cosineAngle) + age; // decrement priority of avatars outside keyhole + bool outOfView = false; if (distance > cameraView.getCenterRadius()) { if (!cameraView.sphereIntersectsFrustum(avatarPosition, radius)) { priority += OUT_OF_VIEW_PENALTY; + outOfView = true; } } - //qDebug() << "avatar:" << avatar.get() << "priority:" << priority << "apparentSize:" << apparentSize << "cosineAngle:" << cosineAngle << "age:" << age; + if (printDebug) { + qDebug() << "avatar:" << avatar.get() << "priority:" << priority << "apparentSize:" << apparentSize << "cosineAngle:" << cosineAngle << "age:" << age << "outOfView:" << outOfView; + } sortedAvatars.push(AvatarPriority(avatar, priority)); } } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 71ade436ff..a75e81ed3d 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -600,7 +600,8 @@ public: QList avatarList, const ViewFrustum& cameraView, std::function lastUpdated, - std::function shouldIgnore); + std::function shouldIgnore, + bool printDebug = false); public slots: From 05995163cf610405f0519511d8009b220697a613 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 09:49:47 -0800 Subject: [PATCH 04/16] add support for tuning sorting --- assignment-client/src/avatars/AvatarMixer.cpp | 20 ++++++++++++++++++- assignment-client/src/avatars/AvatarMixer.h | 2 ++ interface/src/avatar/AvatarManager.cpp | 15 ++++++++++++++ libraries/avatars/src/AvatarData.h | 6 +++--- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 3a8aa0182c..59896432e7 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -47,7 +47,7 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListener(PacketType::AvatarData, this, "queueIncomingPacket"); - + packetReceiver.registerListener(PacketType::AdjustAvatarSorting, this, "handleAdjustAvatarSorting"); packetReceiver.registerListener(PacketType::ViewFrustum, this, "handleViewFrustumPacket"); packetReceiver.registerListener(PacketType::AvatarIdentity, this, "handleAvatarIdentityPacket"); packetReceiver.registerListener(PacketType::KillAvatar, this, "handleKillAvatarPacket"); @@ -317,6 +317,24 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { } } + +void AvatarMixer::handleAdjustAvatarSorting(QSharedPointer message, SharedNodePointer senderNode) { + auto start = usecTimestampNow(); + + message->readPrimitive(&AvatarData::_avatarSortCoefficientSize); + message->readPrimitive(&AvatarData::_avatarSortCoefficientCenter); + message->readPrimitive(&AvatarData::_avatarSortCoefficientAge); + + qCDebug(avatars) << "New avatar sorting... " + << "size:" << AvatarData::_avatarSortCoefficientSize + << "center:" << AvatarData::_avatarSortCoefficientCenter + << "age:" << AvatarData::_avatarSortCoefficientAge; + + auto end = usecTimestampNow(); + _handleAdjustAvatarSortingElapsedTime += (end - start); +} + + void AvatarMixer::handleViewFrustumPacket(QSharedPointer message, SharedNodePointer senderNode) { auto start = usecTimestampNow(); getOrCreateClientData(senderNode); diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h index 32b0ffed69..1925ec1ebd 100644 --- a/assignment-client/src/avatars/AvatarMixer.h +++ b/assignment-client/src/avatars/AvatarMixer.h @@ -39,6 +39,7 @@ public slots: private slots: void queueIncomingPacket(QSharedPointer message, SharedNodePointer node); + void handleAdjustAvatarSorting(QSharedPointer message, SharedNodePointer senderNode); void handleViewFrustumPacket(QSharedPointer message, SharedNodePointer senderNode); void handleAvatarIdentityPacket(QSharedPointer message, SharedNodePointer senderNode); void handleKillAvatarPacket(QSharedPointer message); @@ -92,6 +93,7 @@ private: quint64 _broadcastAvatarDataNodeTransform { 0 }; quint64 _broadcastAvatarDataNodeFunctor { 0 }; + quint64 _handleAdjustAvatarSortingElapsedTime { 0 }; quint64 _handleViewFrustumPacketElapsedTime { 0 }; quint64 _handleAvatarIdentityPacketElapsedTime { 0 }; quint64 _handleKillAvatarPacketElapsedTime { 0 }; diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 9dc73d9239..4cdf735e96 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -572,14 +572,29 @@ float AvatarManager::getAvatarSortCoefficient(const QString& name) { // HACK void AvatarManager::setAvatarSortCoefficient(const QString& name, const QScriptValue& value) { + bool somethingChanged = false; if (value.isNumber()) { float numericalValue = (float)value.toNumber(); if (name == "size") { AvatarData::_avatarSortCoefficientSize = numericalValue; + somethingChanged = true; } else if (name == "center") { AvatarData::_avatarSortCoefficientCenter = numericalValue; + somethingChanged = true; } else if (name == "age") { AvatarData::_avatarSortCoefficientAge = numericalValue; + somethingChanged = true; } } + if (somethingChanged) { + size_t packetSize = sizeof(AvatarData::_avatarSortCoefficientSize) + + sizeof(AvatarData::_avatarSortCoefficientCenter) + + sizeof(AvatarData::_avatarSortCoefficientAge); + + auto packet = NLPacket::create(PacketType::AdjustAvatarSorting, packetSize); + packet->writePrimitive(AvatarData::_avatarSortCoefficientSize); + packet->writePrimitive(AvatarData::_avatarSortCoefficientCenter); + packet->writePrimitive(AvatarData::_avatarSortCoefficientAge); + DependencyManager::get()->broadcastToNodes(std::move(packet), NodeSet() << NodeType::AvatarMixer); + } } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 9d5bc507e0..daa496f6e1 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -605,9 +605,9 @@ public: // TODO: remove this HACK once we settle on optimal sort coefficients // These coefficients exposed for fine tuning the sort priority for transfering new _jointData to the render pipeline. - static float _avatarSortCoefficientSize { 0.5f }; - static float _avatarSortCoefficientCenter { 0.25 }; - static float _avatarSortCoefficientAge { 1.0f }; + static float _avatarSortCoefficientSize; + static float _avatarSortCoefficientCenter; + static float _avatarSortCoefficientAge; diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 050e3088f8..01973a6786 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -112,7 +112,8 @@ public: ReloadEntityServerScript, EntityPhysics, EntityServerScriptLog, - LAST_PACKET_TYPE = EntityServerScriptLog + AdjustAvatarSorting, + LAST_PACKET_TYPE = AdjustAvatarSorting }; }; From 618f37eb0622abfdbc5e540956535ef20121a9f7 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 11:14:22 -0800 Subject: [PATCH 05/16] fix crash in recording playback when clip loader fails --- libraries/script-engine/src/RecordingScriptingInterface.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libraries/script-engine/src/RecordingScriptingInterface.cpp b/libraries/script-engine/src/RecordingScriptingInterface.cpp index 710f342322..41bb780b47 100644 --- a/libraries/script-engine/src/RecordingScriptingInterface.cpp +++ b/libraries/script-engine/src/RecordingScriptingInterface.cpp @@ -56,6 +56,11 @@ bool RecordingScriptingInterface::loadRecording(const QString& url) { using namespace recording; auto loader = ClipCache::instance().getClipLoader(url); + if (!loader) { + qWarning() << "Clip failed to load from " << url; + return false; + } + if (!loader->isLoaded()) { QEventLoop loop; QObject::connect(loader.data(), &Resource::loaded, &loop, &QEventLoop::quit); From 6ac33dbf61382605d77b5a76b2d086a3e4636632 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 22 Feb 2017 17:23:36 -0800 Subject: [PATCH 06/16] count avatar joint updates and non-updates --- interface/resources/qml/Stats.qml | 4 +-- interface/src/avatar/Avatar.h | 2 ++ interface/src/avatar/AvatarManager.cpp | 42 ++++++++++++++++++++------ interface/src/avatar/AvatarManager.h | 8 ++--- interface/src/ui/Stats.cpp | 4 +-- interface/src/ui/Stats.h | 8 ++--- 6 files changed, 47 insertions(+), 21 deletions(-) diff --git a/interface/resources/qml/Stats.qml b/interface/resources/qml/Stats.qml index faf37d5366..58d589b667 100644 --- a/interface/resources/qml/Stats.qml +++ b/interface/resources/qml/Stats.qml @@ -107,11 +107,11 @@ Item { } StatText { visible: root.expanded - text: "Fully Simulated Avatars: " + root.fullySimulatedAvatarCount + text: "Avatars Updated: " + root.updatedAvatarCount } StatText { visible: root.expanded - text: "Partially Simulated Avatars: " + root.partiallySimulatedAvatarCount + text: "Avatars NOT Updated: " + root.notUpdatedAvatarCount } } } diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index 53f0562bdd..ca4dbd2af8 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -183,6 +183,8 @@ public: Q_INVOKABLE float getSimulationRate(const QString& rateName = QString("")) const; + bool hasNewJointData() const { return _hasNewJointData; } + public slots: // FIXME - these should be migrated to use Pose data instead diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 4cdf735e96..af9f915fec 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -195,8 +195,8 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { uint64_t renderExpiry = startTime + RENDER_UPDATE_BUDGET; uint64_t maxExpiry = startTime + MAX_UPDATE_BUDGET; - int fullySimulatedAvatars = 0; - int partiallySimulatedAvatars = 0; + int numAvatarsUpdated = 0; + int numAVatarsNotUpdated = 0; while (!sortedAvatars.empty()) { const AvatarPriority& sortData = sortedAvatars.top(); const auto& avatar = std::static_pointer_cast(sortData.avatar); @@ -217,33 +217,57 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { } avatar->animateScaleChanges(deltaTime); + const float OUT_OF_VIEW_THRESHOLD = 0.5f * AvatarData::OUT_OF_VIEW_PENALTY; uint64_t now = usecTimestampNow(); if (now < renderExpiry) { // we're within budget - const float OUT_OF_VIEW_THRESHOLD = 0.5f * AvatarData::OUT_OF_VIEW_PENALTY; bool inView = sortData.priority > OUT_OF_VIEW_THRESHOLD; + if (inView && avatar->hasNewJointData()) { + numAvatarsUpdated++; + } avatar->simulate(deltaTime, inView); avatar->updateRenderItem(pendingChanges); avatar->setLastRenderUpdateTime(startTime); - fullySimulatedAvatars++; } else if (now < maxExpiry) { // we've spent most of our time budget, but we still simulate() the avatar as it if were out of view // --> some avatars may freeze until their priority trickles up - const bool inView = false; - avatar->simulate(deltaTime, inView); - partiallySimulatedAvatars++; + bool inView = sortData.priority > OUT_OF_VIEW_THRESHOLD; + if (inView && avatar->hasNewJointData()) { + numAVatarsNotUpdated++; + } + avatar->simulate(deltaTime, false); } else { // we've spent ALL of our time budget --> bail on the rest of the avatar updates + // --> more avatars may freeze until their priority trickles up // --> some scale or fade animations may glitch // --> some avatar velocity measurements may be a little off + + // HACK: no time simulate, but we will take the time to count how many were tragically missed + bool inView = sortData.priority > OUT_OF_VIEW_THRESHOLD; + if (!inView) { + break; + } + if (inView && avatar->hasNewJointData()) { + numAVatarsNotUpdated++; + } + sortedAvatars.pop(); + while (inView && !sortedAvatars.empty()) { + const AvatarPriority& newSortData = sortedAvatars.top(); + const auto& newAvatar = std::static_pointer_cast(newSortData.avatar); + inView = newSortData.priority > OUT_OF_VIEW_THRESHOLD; + if (inView && newAvatar->hasNewJointData()) { + numAVatarsNotUpdated++; + } + sortedAvatars.pop(); + } break; } sortedAvatars.pop(); } _avatarSimulationTime = (float)(usecTimestampNow() - startTime) / (float)USECS_PER_MSEC; - _fullySimulatedAvatars = fullySimulatedAvatars; - _partiallySimulatedAvatars = partiallySimulatedAvatars; + _numAvatarsUpdated = numAvatarsUpdated; + _numAvatarsNotUpdated = numAVatarsNotUpdated; qApp->getMain3DScene()->enqueuePendingChanges(pendingChanges); simulateAvatarFades(deltaTime); diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 8302bf4ed6..e1f5a3b411 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -43,8 +43,8 @@ public: std::shared_ptr getMyAvatar() { return _myAvatar; } AvatarSharedPointer getAvatarBySessionID(const QUuid& sessionID) const override; - int getFullySimulatedAvatars() const { return _fullySimulatedAvatars; } - int getPartiallySimulatedAvatars() const { return _partiallySimulatedAvatars; } + int getNumAvatarsUpdated() const { return _numAvatarsUpdated; } + int getNumAvatarsNotUpdated() const { return _numAvatarsNotUpdated; } float getAvatarSimulationTime() const { return _avatarSimulationTime; } void updateMyAvatar(float deltaTime); @@ -120,8 +120,8 @@ private: VectorOfMotionStates _motionStatesToRemoveFromPhysics; RateCounter<> _myAvatarSendRate; - int _fullySimulatedAvatars { 0 }; - int _partiallySimulatedAvatars { 0 }; + int _numAvatarsUpdated { 0 }; + int _numAvatarsNotUpdated { 0 }; float _avatarSimulationTime { 0.0f }; }; diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index e82f99bed2..1075bbdaa4 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -121,8 +121,8 @@ void Stats::updateStats(bool force) { auto avatarManager = DependencyManager::get(); // we need to take one avatar out so we don't include ourselves STAT_UPDATE(avatarCount, avatarManager->size() - 1); - STAT_UPDATE(fullySimulatedAvatarCount, avatarManager->getFullySimulatedAvatars()); - STAT_UPDATE(partiallySimulatedAvatarCount, avatarManager->getPartiallySimulatedAvatars()); + STAT_UPDATE(updatedAvatarCount, avatarManager->getNumAvatarsUpdated()); + STAT_UPDATE(notUpdatedAvatarCount, avatarManager->getNumAvatarsNotUpdated()); STAT_UPDATE(serverCount, (int)nodeList->size()); STAT_UPDATE(framerate, qApp->getFps()); if (qApp->getActiveDisplayPlugin()) { diff --git a/interface/src/ui/Stats.h b/interface/src/ui/Stats.h index f501f4b09a..6be084100c 100644 --- a/interface/src/ui/Stats.h +++ b/interface/src/ui/Stats.h @@ -49,8 +49,8 @@ class Stats : public QQuickItem { STATS_PROPERTY(int, simrate, 0) STATS_PROPERTY(int, avatarSimrate, 0) STATS_PROPERTY(int, avatarCount, 0) - STATS_PROPERTY(int, fullySimulatedAvatarCount, 0) - STATS_PROPERTY(int, partiallySimulatedAvatarCount, 0) + STATS_PROPERTY(int, updatedAvatarCount, 0) + STATS_PROPERTY(int, notUpdatedAvatarCount, 0) STATS_PROPERTY(int, packetInCount, 0) STATS_PROPERTY(int, packetOutCount, 0) STATS_PROPERTY(float, mbpsIn, 0) @@ -159,8 +159,8 @@ signals: void simrateChanged(); void avatarSimrateChanged(); void avatarCountChanged(); - void fullySimulatedAvatarCountChanged(); - void partiallySimulatedAvatarCountChanged(); + void updatedAvatarCountChanged(); + void notUpdatedAvatarCountChanged(); void packetInCountChanged(); void packetOutCountChanged(); void mbpsInChanged(); From 4c4506b1f7c12f23a3c3401c9b15cf1641806220 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 21:47:40 -0800 Subject: [PATCH 07/16] some cleanup --- assignment-client/src/avatars/AvatarMixer.cpp | 20 ++++---- .../src/avatars/AvatarMixerClientData.h | 10 ++-- .../src/avatars/AvatarMixerSlave.cpp | 46 ++----------------- 3 files changed, 21 insertions(+), 55 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 59896432e7..50363f62e3 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -309,7 +309,8 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { }, [&](const SharedNodePointer& node) { QMetaObject::invokeMethod(node->getLinkedData(), - "removeLastBroadcastSequenceNumber", + //"removeLastBroadcastSequenceNumber", + "cleanupKilledNode", Qt::AutoConnection, Q_ARG(const QUuid&, QUuid(killedNode->getUUID()))); } @@ -321,14 +322,17 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { void AvatarMixer::handleAdjustAvatarSorting(QSharedPointer message, SharedNodePointer senderNode) { auto start = usecTimestampNow(); - message->readPrimitive(&AvatarData::_avatarSortCoefficientSize); - message->readPrimitive(&AvatarData::_avatarSortCoefficientCenter); - message->readPrimitive(&AvatarData::_avatarSortCoefficientAge); + // only allow admins with kick rights to change this value... + if (senderNode->getCanKick()) { + message->readPrimitive(&AvatarData::_avatarSortCoefficientSize); + message->readPrimitive(&AvatarData::_avatarSortCoefficientCenter); + message->readPrimitive(&AvatarData::_avatarSortCoefficientAge); - qCDebug(avatars) << "New avatar sorting... " - << "size:" << AvatarData::_avatarSortCoefficientSize - << "center:" << AvatarData::_avatarSortCoefficientCenter - << "age:" << AvatarData::_avatarSortCoefficientAge; + qCDebug(avatars) << "New avatar sorting... " + << "size:" << AvatarData::_avatarSortCoefficientSize + << "center:" << AvatarData::_avatarSortCoefficientCenter + << "age:" << AvatarData::_avatarSortCoefficientAge; + } auto end = usecTimestampNow(); _handleAdjustAvatarSortingElapsedTime += (end - start); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 7eea31c9ab..9cf73a94f4 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -53,11 +53,14 @@ public: Q_INVOKABLE void removeLastBroadcastSequenceNumber(const QUuid& nodeUUID) { _lastBroadcastSequenceNumbers.erase(nodeUUID); } uint64_t getLastBroadcastTime(const QUuid& nodeUUID) const; - void setLastBroadcastTime(const QUuid& nodeUUID, uint64_t broadcastTime) { - _lastBroadcastTimes[nodeUUID] = broadcastTime; - } + void setLastBroadcastTime(const QUuid& nodeUUID, uint64_t broadcastTime) { _lastBroadcastTimes[nodeUUID] = broadcastTime; } Q_INVOKABLE void removeLastBroadcastTime(const QUuid& nodeUUID) { _lastBroadcastTimes.erase(nodeUUID); } + Q_INVOKABLE void cleanupKilledNode(const QUuid& nodeUUID) { + removeLastBroadcastSequenceNumber(nodeUUID); + removeLastBroadcastTime(nodeUUID); + } + uint16_t getLastReceivedSequenceNumber() const { return _lastReceivedSequenceNumber; } HRCTime getIdentityChangeTimestamp() const { return _identityChangeTimestamp; } @@ -115,7 +118,6 @@ public: ViewFrustum getViewFrustom() const { return _currentViewFrustum; } - quint64 getLastOtherAvatarEncodeTime(QUuid otherAvatar) { quint64 result = 0; if (_lastOtherAvatarEncodeTime.find(otherAvatar) != _lastOtherAvatarEncodeTime.end()) { diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 360c6e26fd..5ad4561aa9 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -191,7 +191,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { QList avatarList; std::unordered_map avatarDataToNodes; - //qDebug() << "------------------------------"; int listItem = 0; std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); @@ -200,36 +199,14 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { AvatarSharedPointer otherAvatar = otherNodeData->getAvatarSharedPointer(); avatarList << otherAvatar; avatarDataToNodes[otherAvatar] = otherNode; - /* - qDebug() << "listItem [" << listItem << "] " - << "otherNode:" << otherNode.data() - << "otherNode->getUUID():" << otherNode->getUUID() - << "otherNodeData:" << otherNodeData - << "otherAvatar:" << otherAvatar.get(); - - qDebug() << "avatarDataToNodes[" << otherAvatar.get() << "]=" << otherNode.data(); - */ } }); - /* - qDebug() << "avatarList.size:" << avatarList.size(); - qDebug() << "avatarDataToNodes.size:" << avatarDataToNodes.size(); - */ AvatarSharedPointer thisAvatar = nodeData->getAvatarSharedPointer(); //qDebug() << "thisAvatar:" << thisAvatar.get(); -#ifdef WANT_DEBUG - bool printDebug = nodeData->getAvatarSharedPointer()->getDisplayName() == "ZappoMan"; - - if (printDebug) { - qDebug() << "------------------------------"; - } -#else - bool printDebug = false; -#endif ViewFrustum cameraView = nodeData->getViewFrustom(); std::priority_queue sortedAvatars = AvatarData::sortAvatars( avatarList, cameraView, @@ -239,18 +216,12 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { if (avatarNode) { return nodeData->getLastBroadcastTime(avatarNode->getUUID()); } - return 0; // ??? + return 0; }, [thisAvatar](AvatarSharedPointer avatar)->bool{ return (avatar == thisAvatar); // ignore ourselves... - }, printDebug); - -#ifdef WANT_DEBUG - if (printDebug) { - qDebug() << "------------------------------"; - } -#endif + }); // loop through our sorted avatars and allocate our bandwidth to them accordingly int avatarRank = 0; @@ -262,13 +233,9 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { auto otherNode = avatarDataToNodes[avatarData]; - //qDebug() << "otherNode (" << otherNode.data() << ")= avatarDataToNodes[" << avatarData.get() << "]"; - if (!otherNode) { - //qDebug() << "For viewer:" << node->getUUID() << "... process other avatar [" << avatarRank << ":" << avatarData.get() << "... otherNode unknown!!"; continue; } - //qDebug() << "For viewer:" << node->getUUID() << "... process other avatar [" << avatarRank << "] avatarData: " << avatarData.get() << " otherNode:" << otherNode->getUUID() << " ... "; bool shouldConsider = false; quint64 startIgnoreCalculation = usecTimestampNow(); @@ -282,8 +249,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { || (otherNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { shouldConsider = false; - //qDebug() << "shouldConsider = false ... line:" << __LINE__; - } else { const AvatarMixerClientData* otherData = reinterpret_cast(otherNode->getLinkedData()); @@ -318,8 +283,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { if (nodeBox.touches(otherNodeBox)) { nodeData->ignoreOther(node, otherNode); shouldConsider = getsAnyIgnored; - //qDebug() << "shouldConsider = getsAnyIgnored " << getsAnyIgnored << " ... line:" << __LINE__; - } } // Not close enough to ignore @@ -397,9 +360,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; - //qDebug() << "shouldConsider = false ... line:" << __LINE__; - } - else if (lastSeqFromSender - lastSeqToReceiver > 1) { + } else if (lastSeqFromSender - lastSeqToReceiver > 1) { // this is a skip - we still send the packet but capture the presence of the skip so we see it happening ++numAvatarsWithSkippedFrames; } @@ -418,7 +379,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; - //qDebug() << "shouldConsider = false ... line:" << __LINE__; } if (shouldConsider) { From edf7c016a12cd51ac5892d495a518142c44233e7 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 22:06:28 -0800 Subject: [PATCH 08/16] more cleanup --- assignment-client/src/avatars/AvatarMixer.cpp | 1 - .../src/avatars/AvatarMixerSlave.cpp | 6 ++---- libraries/avatars/src/AvatarData.cpp | 17 +++-------------- libraries/avatars/src/AvatarData.h | 7 +------ 4 files changed, 6 insertions(+), 25 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 50363f62e3..a347b5ef47 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -309,7 +309,6 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { }, [&](const SharedNodePointer& node) { QMetaObject::invokeMethod(node->getLinkedData(), - //"removeLastBroadcastSequenceNumber", "cleanupKilledNode", Qt::AutoConnection, Q_ARG(const QUuid&, QUuid(killedNode->getUUID()))); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 5ad4561aa9..9b501a5e31 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -187,7 +187,8 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // setup a PacketList for the avatarPackets auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); - + // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes + // for calling the AvatarData::sortAvatars() function and getting our sorted list of client nodes QList avatarList; std::unordered_map avatarDataToNodes; @@ -204,9 +205,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { }); AvatarSharedPointer thisAvatar = nodeData->getAvatarSharedPointer(); - - //qDebug() << "thisAvatar:" << thisAvatar.get(); - ViewFrustum cameraView = nodeData->getViewFrustom(); std::priority_queue sortedAvatars = AvatarData::sortAvatars( avatarList, cameraView, diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 7c1b1baf26..e2574683a2 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -69,8 +69,7 @@ AvatarData::AvatarData() : _displayNameAlpha(1.0f), _errorLogExpiry(0), _owningAvatarMixer(), - _targetVelocity(0.0f), - _localAABox(DEFAULT_LOCAL_AABOX_CORNER, DEFAULT_LOCAL_AABOX_SCALE) + _targetVelocity(0.0f) { setBodyPitch(0.0f); setBodyYaw(-90.0f); @@ -2321,8 +2320,7 @@ std::priority_queue AvatarData::sortAvatars( QList avatarList, const ViewFrustum& cameraView, std::function lastUpdated, - std::function shouldIgnore, - bool printDebug) { + std::function shouldIgnore) { uint64_t startTime = usecTimestampNow(); @@ -2335,8 +2333,6 @@ std::priority_queue AvatarData::sortAvatars( const auto& avatar = avatarList.at(i); bool outOfView = false; - // FIXME - probably some lambda that allows the caller to reject some avatars - if (shouldIgnore(avatar)) { continue; } @@ -2352,13 +2348,10 @@ std::priority_queue AvatarData::sortAvatars( // FIXME - AvatarData has something equivolent to this float radius = 1.0f; // avatar->getBoundingRadius(); - const glm::vec3& forward = cameraView.getDirection(); float apparentSize = 2.0f * radius / distance; float cosineAngle = glm::length(glm::dot(offset, forward) * forward) / distance; - - uint64_t lastUpdatedTime = lastUpdated(avatar); // avatar->getLastRenderUpdateTime() - float age = (float)(startTime - lastUpdatedTime) / (float)(USECS_PER_SECOND); + float age = (float)(startTime - lastUpdated(avatar)) / (float)(USECS_PER_SECOND); // NOTE: we are adding values of different units to get a single measure of "priority". // Thus we multiply each component by a conversion "weight" that scales its units relative to the others. @@ -2375,10 +2368,6 @@ std::priority_queue AvatarData::sortAvatars( outOfView = true; } } - - if (printDebug) { - qDebug() << "avatar:" << avatar.get() << "priority:" << priority << "apparentSize:" << apparentSize << "cosineAngle:" << cosineAngle << "age:" << age << "outOfView:" << outOfView; - } sortedAvatars.push(AvatarPriority(avatar, priority)); } } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index daa496f6e1..8608137b20 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -550,8 +550,6 @@ public: void setOwningAvatarMixer(const QWeakPointer& owningAvatarMixer) { _owningAvatarMixer = owningAvatarMixer; } - //const AABox& getLocalAABox() const { return _localAABox; } - int getUsecsSinceLastUpdate() const { return _averageBytesReceived.getUsecsSinceLastEvent(); } int getAverageBytesReceivedPerSecond() const; int getReceiveRate() const; @@ -600,8 +598,7 @@ public: QList avatarList, const ViewFrustum& cameraView, std::function lastUpdated, - std::function shouldIgnore, - bool printDebug = false); + std::function shouldIgnore); // TODO: remove this HACK once we settle on optimal sort coefficients // These coefficients exposed for fine tuning the sort priority for transfering new _jointData to the render pipeline. @@ -692,8 +689,6 @@ protected: glm::vec3 _targetVelocity; - AABox _localAABox; - SimpleMovingAverage _averageBytesReceived; // During recording, this holds the starting position, orientation & scale of the recorded avatar From 06f0087459a56431f40cd6ec72f7501303b9c36d Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 22:36:37 -0800 Subject: [PATCH 09/16] wire up radius properly --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 5 +++++ interface/src/avatar/AvatarManager.cpp | 4 ++++ libraries/avatars/src/AvatarData.cpp | 7 ++++--- libraries/avatars/src/AvatarData.h | 3 ++- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 9b501a5e31..0004afa97c 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -217,6 +217,11 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { return 0; }, + [&](AvatarSharedPointer avatar)->float{ + glm::vec3 nodeBoxHalfScale = (avatar->getPosition() - avatar->getGlobalBoundingBoxCorner()); + return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); + }, + [thisAvatar](AvatarSharedPointer avatar)->bool{ return (avatar == thisAvatar); // ignore ourselves... }); diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index af9f915fec..b1a09b46ce 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -171,6 +171,10 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { return std::static_pointer_cast(avatar)->getLastRenderUpdateTime(); }, + [](AvatarSharedPointer avatar)->float{ + return std::static_pointer_cast(avatar)->getBoundingRadius(); + }, + [this](AvatarSharedPointer avatar)->bool{ const auto& castedAvatar = std::static_pointer_cast(avatar); if (castedAvatar == _myAvatar || !castedAvatar->isInitialized()) { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index e2574683a2..0c145a6a30 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2319,7 +2319,8 @@ float AvatarData::_avatarSortCoefficientAge { 1.0f }; std::priority_queue AvatarData::sortAvatars( QList avatarList, const ViewFrustum& cameraView, - std::function lastUpdated, + std::function getLastUpdated, + std::function getBoundingRadius, std::function shouldIgnore) { uint64_t startTime = usecTimestampNow(); @@ -2346,12 +2347,12 @@ std::priority_queue AvatarData::sortAvatars( float distance = glm::length(offset) + 0.001f; // add 1mm to avoid divide by zero // FIXME - AvatarData has something equivolent to this - float radius = 1.0f; // avatar->getBoundingRadius(); + float radius = getBoundingRadius(avatar); const glm::vec3& forward = cameraView.getDirection(); float apparentSize = 2.0f * radius / distance; float cosineAngle = glm::length(glm::dot(offset, forward) * forward) / distance; - float age = (float)(startTime - lastUpdated(avatar)) / (float)(USECS_PER_SECOND); + float age = (float)(startTime - getLastUpdated(avatar)) / (float)(USECS_PER_SECOND); // NOTE: we are adding values of different units to get a single measure of "priority". // Thus we multiply each component by a conversion "weight" that scales its units relative to the others. diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 8608137b20..0eddc29cda 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -597,7 +597,8 @@ public: static std::priority_queue sortAvatars( QList avatarList, const ViewFrustum& cameraView, - std::function lastUpdated, + std::function getLastUpdated, + std::function getBoundingRadius, std::function shouldIgnore); // TODO: remove this HACK once we settle on optimal sort coefficients From 40037bee55d063ae1f81c3811e9c7623370f37c5 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 22 Feb 2017 23:28:40 -0800 Subject: [PATCH 10/16] added some stats for the random drops --- assignment-client/src/avatars/AvatarMixer.cpp | 8 +++++++- assignment-client/src/avatars/AvatarMixerSlave.cpp | 10 +++++++--- assignment-client/src/avatars/AvatarMixerSlave.h | 6 ++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index a347b5ef47..a90fdb100c 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -506,6 +506,9 @@ void AvatarMixer::sendStatsPacket() { float averageOthersIncluded = averageNodes ? stats.numOthersIncluded / averageNodes : 0.0f; slaveObject["sent_6_averageOthersIncluded"] = TIGHT_LOOP_STAT(averageOthersIncluded); + float averageRandomDrops = averageNodes ? stats.randomDrops / averageNodes : 0.0f; + slaveObject["sent_7_averageRandomDrops"] = TIGHT_LOOP_STAT(averageRandomDrops); + slaveObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(stats.processIncomingPacketsElapsedTime); slaveObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(stats.ignoreCalculationElapsedTime); slaveObject["timing_3_toByteArray"] = TIGHT_LOOP_STAT_UINT64(stats.toByteArrayElapsedTime); @@ -533,9 +536,12 @@ void AvatarMixer::sendStatsPacket() { float averageOutboundAvatarKbps = averageNodes ? ((aggregateStats.numBytesSent / secondsSinceLastStats) / BYTES_PER_KILOBIT) / averageNodes : 0.0f; slavesAggregatObject["sent_5_averageOutboundAvatarKbps"] = averageOutboundAvatarKbps; + float averageRandomDrops = averageNodes ? aggregateStats.randomDrops / averageNodes : 0.0f; + slavesAggregatObject["sent_7_averageRandomDrops"] = TIGHT_LOOP_STAT(averageRandomDrops); + float averageOthersIncluded = averageNodes ? aggregateStats.numOthersIncluded / averageNodes : 0.0f; slavesAggregatObject["sent_6_averageOthersIncluded"] = TIGHT_LOOP_STAT(averageOthersIncluded); - + slavesAggregatObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.processIncomingPacketsElapsedTime); slavesAggregatObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.ignoreCalculationElapsedTime); slavesAggregatObject["timing_3_toByteArray"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.toByteArrayElapsedTime); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 0004afa97c..5073aab934 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -332,15 +332,19 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // // NOTE: If the recieving node is in "PAL mode" then it's asked to get things even that // are out of view, this also appears to disable this random distribution. + // + // FIXME - This approach for managing the outbound bandwidth is less than ideal, + // it would be better to more directly budget the number of bytes to send + // per frame and simply exit the sorted avatar list once that budget is + // surpassed. We will work on that next. [BHG 2/22/17] if (distanceToAvatar != 0.0f && !getsOutOfView - && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) /// FIX ME... we don't want to do this random stuff + && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) ) { - quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; - //qDebug() << "shouldConsider = false ... line:" << __LINE__; + _stats.randomDrops++; } if (shouldConsider) { diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index b1a5b56c4f..50ae4570b5 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -25,6 +25,8 @@ public: int numBytesSent { 0 }; int numIdentityPackets { 0 }; int numOthersIncluded { 0 }; + int randomDrops { 0 }; + quint64 ignoreCalculationElapsedTime { 0 }; quint64 avatarDataPackingElapsedTime { 0 }; quint64 packetSendingElapsedTime { 0 }; @@ -43,6 +45,8 @@ public: numBytesSent = 0; numIdentityPackets = 0; numOthersIncluded = 0; + randomDrops = 0; + ignoreCalculationElapsedTime = 0; avatarDataPackingElapsedTime = 0; packetSendingElapsedTime = 0; @@ -60,6 +64,8 @@ public: numBytesSent += rhs.numBytesSent; numIdentityPackets += rhs.numIdentityPackets; numOthersIncluded += rhs.numOthersIncluded; + randomDrops += rhs.randomDrops; + ignoreCalculationElapsedTime += rhs.ignoreCalculationElapsedTime; avatarDataPackingElapsedTime += rhs.avatarDataPackingElapsedTime; packetSendingElapsedTime += rhs.packetSendingElapsedTime; From d4adee8b388d6c2cb482f8e0792316892594a68c Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 00:15:07 -0800 Subject: [PATCH 11/16] add over bandwidth exit to sorted avatar loop --- assignment-client/src/avatars/AvatarMixer.cpp | 10 ++++++-- .../src/avatars/AvatarMixerSlave.cpp | 23 +++++++++++++++---- .../src/avatars/AvatarMixerSlave.h | 3 +++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index a90fdb100c..6db9d05234 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -508,6 +508,9 @@ void AvatarMixer::sendStatsPacket() { float averageRandomDrops = averageNodes ? stats.randomDrops / averageNodes : 0.0f; slaveObject["sent_7_averageRandomDrops"] = TIGHT_LOOP_STAT(averageRandomDrops); + + float averageOverBudgetAvatars = averageNodes ? stats.overBudgetAvatars / averageNodes : 0.0f; + slaveObject["sent_8_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); slaveObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(stats.processIncomingPacketsElapsedTime); slaveObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(stats.ignoreCalculationElapsedTime); @@ -536,11 +539,14 @@ void AvatarMixer::sendStatsPacket() { float averageOutboundAvatarKbps = averageNodes ? ((aggregateStats.numBytesSent / secondsSinceLastStats) / BYTES_PER_KILOBIT) / averageNodes : 0.0f; slavesAggregatObject["sent_5_averageOutboundAvatarKbps"] = averageOutboundAvatarKbps; + float averageOthersIncluded = averageNodes ? aggregateStats.numOthersIncluded / averageNodes : 0.0f; + slavesAggregatObject["sent_6_averageOthersIncluded"] = TIGHT_LOOP_STAT(averageOthersIncluded); + float averageRandomDrops = averageNodes ? aggregateStats.randomDrops / averageNodes : 0.0f; slavesAggregatObject["sent_7_averageRandomDrops"] = TIGHT_LOOP_STAT(averageRandomDrops); - float averageOthersIncluded = averageNodes ? aggregateStats.numOthersIncluded / averageNodes : 0.0f; - slavesAggregatObject["sent_6_averageOthersIncluded"] = TIGHT_LOOP_STAT(averageOthersIncluded); + float averageOverBudgetAvatars = averageNodes ? aggregateStats.overBudgetAvatars / averageNodes : 0.0f; + slavesAggregatObject["sent_8_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); slavesAggregatObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.processIncomingPacketsElapsedTime); slavesAggregatObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.ignoreCalculationElapsedTime); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 5073aab934..21eaebc176 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -129,6 +129,11 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // keep track of outbound data rate specifically for avatar data int numAvatarDataBytes = 0; + // max number of avatarBytes per frame + auto maxAvatarBytesPerFrame = (_maxKbpsPerNode * BYTES_PER_KILOBIT) / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND; + + int overBudgetAvatars = 0; + // keep track of the number of other avatars held back in this frame int numAvatarsHeldBack = 0; @@ -333,10 +338,10 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // NOTE: If the recieving node is in "PAL mode" then it's asked to get things even that // are out of view, this also appears to disable this random distribution. // - // FIXME - This approach for managing the outbound bandwidth is less than ideal, - // it would be better to more directly budget the number of bytes to send - // per frame and simply exit the sorted avatar list once that budget is - // surpassed. We will work on that next. [BHG 2/22/17] + // FIXME - This is the old approach for managing the outbound bandwidth. I've left it + // in for now, even though we're also directly managing budget by calculating the + // number of bytes to send per frame and simply exiting the sorted avatar list + // once that budget is surpassed. I need to remove this old logic next. [BHG 2/22/17] if (distanceToAvatar != 0.0f && !getsOutOfView && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) @@ -372,6 +377,16 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { ++numAvatarsWithSkippedFrames; } + // NOTE: Here's where we determine if we are over budget and stop considering avatars after this. + // + // FIXME - revisit this code, remove the random drop logic, and move this code higher up into + // the loop so we don't bother with all these other calculations once over budget. [BHG 2/22/17] + if (numAvatarDataBytes > maxAvatarBytesPerFrame) { + overBudgetAvatars++; + _stats.overBudgetAvatars++; + shouldConsider = false; + } + // we're going to send this avatar if (shouldConsider) { diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 50ae4570b5..1bbd367b2b 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -26,6 +26,7 @@ public: int numIdentityPackets { 0 }; int numOthersIncluded { 0 }; int randomDrops { 0 }; + int overBudgetAvatars { 0 }; quint64 ignoreCalculationElapsedTime { 0 }; quint64 avatarDataPackingElapsedTime { 0 }; @@ -46,6 +47,7 @@ public: numIdentityPackets = 0; numOthersIncluded = 0; randomDrops = 0; + overBudgetAvatars = 0; ignoreCalculationElapsedTime = 0; avatarDataPackingElapsedTime = 0; @@ -65,6 +67,7 @@ public: numIdentityPackets += rhs.numIdentityPackets; numOthersIncluded += rhs.numOthersIncluded; randomDrops += rhs.randomDrops; + overBudgetAvatars += rhs.overBudgetAvatars; ignoreCalculationElapsedTime += rhs.ignoreCalculationElapsedTime; avatarDataPackingElapsedTime += rhs.avatarDataPackingElapsedTime; From 4f17498daf5c22d1e67e302d6b77ae248a5d8e49 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 00:19:06 -0800 Subject: [PATCH 12/16] fix warning --- libraries/avatars/src/AvatarData.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 0c145a6a30..4993c0ac5d 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2332,7 +2332,6 @@ std::priority_queue AvatarData::sortAvatars( PROFILE_RANGE(simulation, "sort"); for (int32_t i = 0; i < avatarList.size(); ++i) { const auto& avatar = avatarList.at(i); - bool outOfView = false; if (shouldIgnore(avatar)) { continue; @@ -2366,7 +2365,6 @@ std::priority_queue AvatarData::sortAvatars( if (distance > cameraView.getCenterRadius()) { if (!cameraView.sphereIntersectsFrustum(avatarPosition, radius)) { priority += OUT_OF_VIEW_PENALTY; - outOfView = true; } } sortedAvatars.push(AvatarPriority(avatar, priority)); From dde9640c66a9be73e8c65c8d87a3b12d22f6a787 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 10:29:17 -0800 Subject: [PATCH 13/16] remove old full rate distance code --- assignment-client/src/avatars/AvatarMixer.cpp | 10 +- .../src/avatars/AvatarMixerClientData.cpp | 2 - .../src/avatars/AvatarMixerClientData.h | 9 -- .../src/avatars/AvatarMixerSlave.cpp | 131 ++++++------------ .../src/avatars/AvatarMixerSlave.h | 5 +- libraries/avatars/src/AvatarData.cpp | 7 + libraries/avatars/src/AvatarData.h | 2 + 7 files changed, 53 insertions(+), 113 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 6db9d05234..0f6863f9ae 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -506,11 +506,8 @@ void AvatarMixer::sendStatsPacket() { float averageOthersIncluded = averageNodes ? stats.numOthersIncluded / averageNodes : 0.0f; slaveObject["sent_6_averageOthersIncluded"] = TIGHT_LOOP_STAT(averageOthersIncluded); - float averageRandomDrops = averageNodes ? stats.randomDrops / averageNodes : 0.0f; - slaveObject["sent_7_averageRandomDrops"] = TIGHT_LOOP_STAT(averageRandomDrops); - float averageOverBudgetAvatars = averageNodes ? stats.overBudgetAvatars / averageNodes : 0.0f; - slaveObject["sent_8_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); + slaveObject["sent_7_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); slaveObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(stats.processIncomingPacketsElapsedTime); slaveObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(stats.ignoreCalculationElapsedTime); @@ -542,11 +539,8 @@ void AvatarMixer::sendStatsPacket() { float averageOthersIncluded = averageNodes ? aggregateStats.numOthersIncluded / averageNodes : 0.0f; slavesAggregatObject["sent_6_averageOthersIncluded"] = TIGHT_LOOP_STAT(averageOthersIncluded); - float averageRandomDrops = averageNodes ? aggregateStats.randomDrops / averageNodes : 0.0f; - slavesAggregatObject["sent_7_averageRandomDrops"] = TIGHT_LOOP_STAT(averageRandomDrops); - float averageOverBudgetAvatars = averageNodes ? aggregateStats.overBudgetAvatars / averageNodes : 0.0f; - slavesAggregatObject["sent_8_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); + slavesAggregatObject["sent_7_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); slavesAggregatObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.processIncomingPacketsElapsedTime); slavesAggregatObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.ignoreCalculationElapsedTime); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 1df6c24a60..717e14535f 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -126,8 +126,6 @@ bool AvatarMixerClientData::otherAvatarInView(const AABox& otherAvatarBox) { void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { jsonObject["display_name"] = _avatar->getDisplayName(); - jsonObject["full_rate_distance"] = _fullRateDistance; - jsonObject["max_av_distance"] = _maxAvatarDistance; jsonObject["num_avs_sent_last_frame"] = _numAvatarsSentLastFrame; jsonObject["avg_other_av_starves_per_second"] = getAvgNumOtherAvatarStarvesPerSecond(); jsonObject["avg_other_av_skips_per_second"] = getAvgNumOtherAvatarSkipsPerSecond(); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 9cf73a94f4..51b8d692e2 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -68,12 +68,6 @@ public: bool getAvatarSessionDisplayNameMustChange() const { return _avatarSessionDisplayNameMustChange; } void setAvatarSessionDisplayNameMustChange(bool set = true) { _avatarSessionDisplayNameMustChange = set; } - void setFullRateDistance(float fullRateDistance) { _fullRateDistance = fullRateDistance; } - float getFullRateDistance() const { return _fullRateDistance; } - - void setMaxAvatarDistance(float maxAvatarDistance) { _maxAvatarDistance = maxAvatarDistance; } - float getMaxAvatarDistance() const { return _maxAvatarDistance; } - void resetNumAvatarsSentLastFrame() { _numAvatarsSentLastFrame = 0; } void incrementNumAvatarsSentLastFrame() { ++_numAvatarsSentLastFrame; } int getNumAvatarsSentLastFrame() const { return _numAvatarsSentLastFrame; } @@ -156,9 +150,6 @@ private: HRCTime _identityChangeTimestamp; bool _avatarSessionDisplayNameMustChange{ false }; - float _fullRateDistance = FLT_MAX; - float _maxAvatarDistance = FLT_MAX; - int _numAvatarsSentLastFrame = 0; int _numFramesSinceAdjustment = 0; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 21eaebc176..1f9fbde89a 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -66,13 +66,16 @@ void AvatarMixerSlave::processIncomingPackets(const SharedNodePointer& node) { } -void AvatarMixerSlave::sendIdentityPacket(const AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode) { +int AvatarMixerSlave::sendIdentityPacket(const AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode) { + int bytesSent = 0; QByteArray individualData = nodeData->getConstAvatarData()->identityByteArray(); auto identityPacket = NLPacket::create(PacketType::AvatarIdentity, individualData.size()); - individualData.replace(0, NUM_BYTES_RFC4122_UUID, nodeData->getNodeID().toRfc4122()); + individualData.replace(0, NUM_BYTES_RFC4122_UUID, nodeData->getNodeID().toRfc4122()); // FIXME, this looks suspicious + bytesSent += individualData.size(); identityPacket->write(individualData); DependencyManager::get()->sendPacket(std::move(identityPacket), *destinationNode); _stats.numIdentityPackets++; + return bytesSent; } static const int AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND = 45; @@ -117,9 +120,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // reset the internal state for correct random number distribution distribution.reset(); - // reset the max distance for this frame - float maxAvatarDistanceThisFrame = 0.0f; - // reset the number of sent avatars nodeData->resetNumAvatarsSentLastFrame(); @@ -128,10 +128,14 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // keep track of outbound data rate specifically for avatar data int numAvatarDataBytes = 0; + int identityBytesSent = 0; // max number of avatarBytes per frame auto maxAvatarBytesPerFrame = (_maxKbpsPerNode * BYTES_PER_KILOBIT) / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND; + // FIXME - find a way to not send the sessionID for every avatar + int minimumBytesPerAvatar = AvatarDataPacket::AVATAR_HAS_FLAGS_SIZE + NUM_BYTES_RFC4122_UUID; + int overBudgetAvatars = 0; // keep track of the number of other avatars held back in this frame @@ -140,9 +144,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // keep track of the number of other avatar frames skipped int numAvatarsWithSkippedFrames = 0; - // use the data rate specifically for avatar data for FRD adjustment checks - float avatarDataRateLastSecond = nodeData->getOutboundAvatarDataKbps(); - // When this is true, the AvatarMixer will send Avatar data to a client about avatars that are not in the view frustrum bool getsOutOfView = nodeData->getRequestsDomainListData(); @@ -152,43 +153,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them bool getsAnyIgnored = getsIgnoredByMe && node->getCanKick(); - // Check if it is time to adjust what we send this client based on the observed - // bandwidth to this node. We do this once a second, which is also the window for - // the bandwidth reported by node->getOutboundBandwidth(); - if (nodeData->getNumFramesSinceFRDAdjustment() > AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND) { - - const float FRD_ADJUSTMENT_ACCEPTABLE_RATIO = 0.8f; - const float HYSTERISIS_GAP = (1 - FRD_ADJUSTMENT_ACCEPTABLE_RATIO); - const float HYSTERISIS_MIDDLE_PERCENTAGE = (1 - (HYSTERISIS_GAP * 0.5f)); - - // get the current full rate distance so we can work with it - float currentFullRateDistance = nodeData->getFullRateDistance(); - - if (avatarDataRateLastSecond > _maxKbpsPerNode) { - - // is the FRD greater than the farthest avatar? - // if so, before we calculate anything, set it to that distance - currentFullRateDistance = std::min(currentFullRateDistance, nodeData->getMaxAvatarDistance()); - - // we're adjusting the full rate distance to target a bandwidth in the middle - // of the hysterisis gap - currentFullRateDistance *= (_maxKbpsPerNode * HYSTERISIS_MIDDLE_PERCENTAGE) / avatarDataRateLastSecond; - - nodeData->setFullRateDistance(currentFullRateDistance); - nodeData->resetNumFramesSinceFRDAdjustment(); - } else if (currentFullRateDistance < nodeData->getMaxAvatarDistance() - && avatarDataRateLastSecond < _maxKbpsPerNode * FRD_ADJUSTMENT_ACCEPTABLE_RATIO) { - // we are constrained AND we've recovered to below the acceptable ratio - // lets adjust the full rate distance to target a bandwidth in the middle of the hyterisis gap - currentFullRateDistance *= (_maxKbpsPerNode * HYSTERISIS_MIDDLE_PERCENTAGE) / avatarDataRateLastSecond; - - nodeData->setFullRateDistance(currentFullRateDistance); - nodeData->resetNumFramesSinceFRDAdjustment(); - } - } else { - nodeData->incrementNumFramesSinceFRDAdjustment(); - } - // setup a PacketList for the avatarPackets auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); @@ -233,18 +197,39 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // loop through our sorted avatars and allocate our bandwidth to them accordingly int avatarRank = 0; + + // this is overly conservative, because it includes some avatars we might not consider + // FIXME - move the ignore logic up into the sorting list so we get a better estimate + int remainingAvatars = (int)sortedAvatars.size(); + while (!sortedAvatars.empty()) { AvatarPriority sortData = sortedAvatars.top(); sortedAvatars.pop(); const auto& avatarData = sortData.avatar; avatarRank++; + remainingAvatars--; auto otherNode = avatarDataToNodes[avatarData]; + // FIXME - should't this be an assert? if (!otherNode) { continue; } + // NOTE: Here's where we determine if we are over budget and drop to bare minimum data + int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars; + bool overBudget = (identityBytesSent + numAvatarDataBytes + minimRemainingAvatarBytes) > maxAvatarBytesPerFrame; + /** + qDebug() << "Budget debugging:" + << "numAvatarDataBytes:" << numAvatarDataBytes << "\n" + << "identityBytesSent:" << identityBytesSent << "\n" + << "remainingAvatars:" << remainingAvatars << "\n" + << "minimumBytesPerAvatar:" << minimumBytesPerAvatar << "\n" + << "minimRemainingAvatarBytes:" << minimRemainingAvatarBytes << "\n" + << "overBudget:" << overBudget << "\n" + << "overBudgetAvatars:" << overBudgetAvatars; + **/ + bool shouldConsider = false; quint64 startIgnoreCalculation = usecTimestampNow(); @@ -313,45 +298,21 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { bool forceSend = !nodeData->checkAndSetHasReceivedFirstPacketsFrom(otherNode->getUUID()); // FIXME - this clause seems suspicious "... || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp ..." - if (otherNodeData->getIdentityChangeTimestamp().time_since_epoch().count() > 0 + if (!overBudget + && otherNodeData->getIdentityChangeTimestamp().time_since_epoch().count() > 0 && (forceSend || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp || distribution(generator) < IDENTITY_SEND_PROBABILITY)) { - sendIdentityPacket(otherNodeData, node); + identityBytesSent += sendIdentityPacket(otherNodeData, node); } const AvatarData* otherAvatar = otherNodeData->getConstAvatarData(); // Decide whether to send this avatar's data based on it's distance from us - // The full rate distance is the distance at which EVERY update will be sent for this avatar - // at twice the full rate distance, there will be a 50% chance of sending this avatar's update glm::vec3 otherPosition = otherAvatar->getClientGlobalPosition(); float distanceToAvatar = glm::length(myPosition - otherPosition); - // potentially update the max full rate distance for this frame - maxAvatarDistanceThisFrame = std::max(maxAvatarDistanceThisFrame, distanceToAvatar); - - // This code handles the random dropping of avatar data based on the ratio of - // "getFullRateDistance" to actual distance. - // - // NOTE: If the recieving node is in "PAL mode" then it's asked to get things even that - // are out of view, this also appears to disable this random distribution. - // - // FIXME - This is the old approach for managing the outbound bandwidth. I've left it - // in for now, even though we're also directly managing budget by calculating the - // number of bytes to send per frame and simply exiting the sorted avatar list - // once that budget is surpassed. I need to remove this old logic next. [BHG 2/22/17] - if (distanceToAvatar != 0.0f - && !getsOutOfView - && distribution(generator) > (nodeData->getFullRateDistance() / distanceToAvatar) - ) { - quint64 endAvatarDataPacking = usecTimestampNow(); - _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); - shouldConsider = false; - _stats.randomDrops++; - } - if (shouldConsider) { AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID()); AvatarDataSequenceNumber lastSeqFromSender = otherNodeData->getLastReceivedSequenceNumber(); @@ -377,16 +338,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { ++numAvatarsWithSkippedFrames; } - // NOTE: Here's where we determine if we are over budget and stop considering avatars after this. - // - // FIXME - revisit this code, remove the random drop logic, and move this code higher up into - // the loop so we don't bother with all these other calculations once over budget. [BHG 2/22/17] - if (numAvatarDataBytes > maxAvatarBytesPerFrame) { - overBudgetAvatars++; - _stats.overBudgetAvatars++; - shouldConsider = false; - } - // we're going to send this avatar if (shouldConsider) { @@ -396,19 +347,26 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { bool isInView = nodeData->otherAvatarInView(otherNodeBox); // this throttles the extra data to only be sent every Nth message + /** if (!isInView && !getsOutOfView && (lastSeqToReceiver % EXTRA_AVATAR_DATA_FRAME_RATIO > 0)) { quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); shouldConsider = false; } + ***/ if (shouldConsider) { // start a new segment in the PacketList for this avatar avatarPacketList->startSegment(); AvatarData::AvatarDataDetail detail; - if (!isInView && !getsOutOfView) { + + if (overBudget) { + overBudgetAvatars++; + _stats.overBudgetAvatars++; + detail = AvatarData::NoData; + } else if (!isInView && !getsOutOfView) { detail = AvatarData::MinimumData; nodeData->incrementAvatarOutOfView(); } else { @@ -498,13 +456,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { nodeData->recordNumOtherAvatarStarves(numAvatarsHeldBack); nodeData->recordNumOtherAvatarSkips(numAvatarsWithSkippedFrames); - if (numOtherAvatars == 0) { - // update the full rate distance to FLOAT_MAX since we didn't have any other avatars to send - nodeData->setMaxAvatarDistance(FLT_MAX); - } else { - nodeData->setMaxAvatarDistance(maxAvatarDistanceThisFrame); - } - quint64 endPacketSending = usecTimestampNow(); _stats.packetSendingElapsedTime += (endPacketSending - startPacketSending); } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 1bbd367b2b..04141d9d72 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -25,7 +25,6 @@ public: int numBytesSent { 0 }; int numIdentityPackets { 0 }; int numOthersIncluded { 0 }; - int randomDrops { 0 }; int overBudgetAvatars { 0 }; quint64 ignoreCalculationElapsedTime { 0 }; @@ -46,7 +45,6 @@ public: numBytesSent = 0; numIdentityPackets = 0; numOthersIncluded = 0; - randomDrops = 0; overBudgetAvatars = 0; ignoreCalculationElapsedTime = 0; @@ -66,7 +64,6 @@ public: numBytesSent += rhs.numBytesSent; numIdentityPackets += rhs.numIdentityPackets; numOthersIncluded += rhs.numOthersIncluded; - randomDrops += rhs.randomDrops; overBudgetAvatars += rhs.overBudgetAvatars; ignoreCalculationElapsedTime += rhs.ignoreCalculationElapsedTime; @@ -94,7 +91,7 @@ public: void harvestStats(AvatarMixerSlaveStats& stats); private: - void sendIdentityPacket(const AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode); + int sendIdentityPacket(const AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode); // frame state ConstIter _begin; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 4993c0ac5d..6a01c2930c 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -192,6 +192,13 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent unsigned char* destinationBuffer = reinterpret_cast(avatarDataByteArray.data()); unsigned char* startPosition = destinationBuffer; + // special case, if we were asked for no data, then just include the flags all set to nothing + if (dataDetail == NoData) { + AvatarDataPacket::HasFlags packetStateFlags = 0; + memcpy(destinationBuffer, &packetStateFlags, sizeof(packetStateFlags)); + return avatarDataByteArray.left(sizeof(packetStateFlags)); + } + // FIXME - // // BUG -- if you enter a space bubble, and then back away, the avatar has wrong orientation until "send all" happens... diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 0eddc29cda..3797132274 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -136,6 +136,7 @@ namespace AvatarDataPacket { const HasFlags PACKET_HAS_AVATAR_LOCAL_POSITION = 1U << 9; const HasFlags PACKET_HAS_FACE_TRACKER_INFO = 1U << 10; const HasFlags PACKET_HAS_JOINT_DATA = 1U << 11; + const size_t AVATAR_HAS_FLAGS_SIZE = 2; // NOTE: AvatarDataPackets start with a uint16_t sequence number that is not reflected in the Header structure. @@ -373,6 +374,7 @@ public: void setHandPosition(const glm::vec3& handPosition); typedef enum { + NoData, MinimumData, CullSmallData, IncludeSmallData, From f61e16fccfe901116ffd75e56608c0be4ac7a6b5 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 11:01:55 -0800 Subject: [PATCH 14/16] more aggressive out of view data culling --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 1f9fbde89a..dfc91b90c1 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -367,7 +367,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { _stats.overBudgetAvatars++; detail = AvatarData::NoData; } else if (!isInView && !getsOutOfView) { - detail = AvatarData::MinimumData; + detail = AvatarData::NoData; nodeData->incrementAvatarOutOfView(); } else { detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO From 8cb8d686ecf33798645243e11b7c7e1c9cc68993 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 13:09:39 -0800 Subject: [PATCH 15/16] more cleanup and some CR feedback fixes --- .../src/avatars/AvatarMixerSlave.cpp | 395 ++++++++---------- 1 file changed, 182 insertions(+), 213 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index dfc91b90c1..a839784cd1 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -80,13 +80,6 @@ int AvatarMixerSlave::sendIdentityPacket(const AvatarMixerClientData* nodeData, static const int AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND = 45; -// only send extra avatar data (avatars out of view, ignored) every Nth AvatarData frame -// Extra avatar data will be sent (AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND/EXTRA_AVATAR_DATA_FRAME_RATIO) times -// per second. -// This value should be a power of two for performance purposes, as the mixer performs a modulo operation every frame -// to determine whether the extra data should be sent. -static const int EXTRA_AVATAR_DATA_FRAME_RATIO = 16; - // FIXME - There is some old logic (unchanged as of 2/17/17) that randomly decides to send an identity // packet. That logic had the following comment about the constants it uses... // @@ -156,6 +149,20 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // setup a PacketList for the avatarPackets auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); + // Define the minimum bubble size + static const glm::vec3 minBubbleSize = glm::vec3(0.3f, 1.3f, 0.3f); + // Define the scale of the box for the current node + glm::vec3 nodeBoxScale = (nodeData->getPosition() - nodeData->getGlobalBoundingBoxCorner()) * 2.0f; + // Set up the bounding box for the current node + AABox nodeBox(nodeData->getGlobalBoundingBoxCorner(), nodeBoxScale); + // Clamp the size of the bounding box to a minimum scale + if (glm::any(glm::lessThan(nodeBoxScale, minBubbleSize))) { + nodeBox.setScaleStayCentered(minBubbleSize); + } + // Quadruple the scale of both bounding boxes + nodeBox.embiggen(4.0f); + + // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes // for calling the AvatarData::sortAvatars() function and getting our sorted list of client nodes QList avatarList; @@ -170,7 +177,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { avatarList << otherAvatar; avatarDataToNodes[otherAvatar] = otherNode; } - }); AvatarSharedPointer thisAvatar = nodeData->getAvatarSharedPointer(); @@ -191,8 +197,87 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); }, - [thisAvatar](AvatarSharedPointer avatar)->bool{ - return (avatar == thisAvatar); // ignore ourselves... + [&](AvatarSharedPointer avatar)->bool{ + if (avatar == thisAvatar) { + return true; // ignore ourselves... + } + + bool shouldIgnore = false; + + // We will also ignore other nodes for a couple of different reasons: + // 1) ignore bubbles and ignore specific node + // 2) the node hasn't really updated it's frame data recently, this can + // happen if for example the avatar is connected on a desktop and sending + // updates at ~30hz. So every 3 frames we skip a frame. + auto avatarNode = avatarDataToNodes[avatar]; + if (avatarNode) { + const AvatarMixerClientData* avatarNodeData = reinterpret_cast(avatarNode->getLinkedData()); + if (avatarNodeData) { + quint64 startIgnoreCalculation = usecTimestampNow(); + + // make sure we have data for this avatar, that it isn't the same node, + // and isn't an avatar that the viewing node has ignored + // or that has ignored the viewing node + if (!avatarNode->getLinkedData() + || avatarNode->getUUID() == node->getUUID() + || (node->isIgnoringNodeWithID(avatarNode->getUUID()) && !getsIgnoredByMe) + || (avatarNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { + shouldIgnore = true; + } else { + + // Check to see if the space bubble is enabled + if (node->isIgnoreRadiusEnabled() || avatarNode->isIgnoreRadiusEnabled()) { + + // Define the scale of the box for the current other node + glm::vec3 otherNodeBoxScale = (avatarNodeData->getPosition() - avatarNodeData->getGlobalBoundingBoxCorner()) * 2.0f; + // Set up the bounding box for the current other node + AABox otherNodeBox(avatarNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); + // Clamp the size of the bounding box to a minimum scale + if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { + otherNodeBox.setScaleStayCentered(minBubbleSize); + } + // Quadruple the scale of both bounding boxes + otherNodeBox.embiggen(4.0f); + + // Perform the collision check between the two bounding boxes + if (nodeBox.touches(otherNodeBox)) { + nodeData->ignoreOther(node, avatarNode); + shouldIgnore = !getsAnyIgnored; + } + } + // Not close enough to ignore + if (!shouldIgnore) { + nodeData->removeFromRadiusIgnoringSet(node, avatarNode->getUUID()); + } + } + quint64 endIgnoreCalculation = usecTimestampNow(); + _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation); + + if (!shouldIgnore) { + AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(avatarNode->getUUID()); + AvatarDataSequenceNumber lastSeqFromSender = avatarNodeData->getLastReceivedSequenceNumber(); + + // FIXME - This code does appear to be working. But it seems brittle. + // It supports determining if the frame of data for this "other" + // avatar has already been sent to the reciever. This has been + // verified to work on a desktop display that renders at 60hz and + // therefore sends to mixer at 30hz. Each second you'd expect to + // have 15 (45hz-30hz) duplicate frames. In this case, the stat + // avg_other_av_skips_per_second does report 15. + // + // make sure we haven't already sent this data from this sender to this receiver + // or that somehow we haven't sent + if (lastSeqToReceiver == lastSeqFromSender && lastSeqToReceiver != 0) { + ++numAvatarsHeldBack; + shouldIgnore = true; + } else if (lastSeqFromSender - lastSeqToReceiver > 1) { + // this is a skip - we still send the packet but capture the presence of the skip so we see it happening + ++numAvatarsWithSkippedFrames; + } + } + } + } + return shouldIgnore; }); // loop through our sorted avatars and allocate our bandwidth to them accordingly @@ -219,223 +304,107 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // NOTE: Here's where we determine if we are over budget and drop to bare minimum data int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars; bool overBudget = (identityBytesSent + numAvatarDataBytes + minimRemainingAvatarBytes) > maxAvatarBytesPerFrame; - /** - qDebug() << "Budget debugging:" - << "numAvatarDataBytes:" << numAvatarDataBytes << "\n" - << "identityBytesSent:" << identityBytesSent << "\n" - << "remainingAvatars:" << remainingAvatars << "\n" - << "minimumBytesPerAvatar:" << minimumBytesPerAvatar << "\n" - << "minimRemainingAvatarBytes:" << minimRemainingAvatarBytes << "\n" - << "overBudget:" << overBudget << "\n" - << "overBudgetAvatars:" << overBudgetAvatars; - **/ - bool shouldConsider = false; - quint64 startIgnoreCalculation = usecTimestampNow(); + quint64 startAvatarDataPacking = usecTimestampNow(); - // make sure we have data for this avatar, that it isn't the same node, - // and isn't an avatar that the viewing node has ignored - // or that has ignored the viewing node - if (!otherNode->getLinkedData() - || otherNode->getUUID() == node->getUUID() - || (node->isIgnoringNodeWithID(otherNode->getUUID()) && !getsIgnoredByMe) - || (otherNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { + ++numOtherAvatars; - shouldConsider = false; + const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); + + // make sure we send out identity packets to and from new arrivals. + bool forceSend = !nodeData->checkAndSetHasReceivedFirstPacketsFrom(otherNode->getUUID()); + + // FIXME - this clause seems suspicious "... || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp ..." + if (!overBudget + && otherNodeData->getIdentityChangeTimestamp().time_since_epoch().count() > 0 + && (forceSend + || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp + || distribution(generator) < IDENTITY_SEND_PROBABILITY)) { + + identityBytesSent += sendIdentityPacket(otherNodeData, node); + } + + const AvatarData* otherAvatar = otherNodeData->getConstAvatarData(); + glm::vec3 otherPosition = otherAvatar->getClientGlobalPosition(); + + // determine if avatar is in view, to determine how much data to include... + glm::vec3 otherNodeBoxScale = (otherPosition - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f; + AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); + bool isInView = nodeData->otherAvatarInView(otherNodeBox); + + // start a new segment in the PacketList for this avatar + avatarPacketList->startSegment(); + + AvatarData::AvatarDataDetail detail; + + if (overBudget) { + overBudgetAvatars++; + _stats.overBudgetAvatars++; + detail = AvatarData::NoData; + } else if (!isInView && !getsOutOfView) { + detail = AvatarData::NoData; + nodeData->incrementAvatarOutOfView(); } else { - const AvatarMixerClientData* otherData = reinterpret_cast(otherNode->getLinkedData()); - - shouldConsider = true; // assume we will consider... - - // Check to see if the space bubble is enabled - if (node->isIgnoreRadiusEnabled() || otherNode->isIgnoreRadiusEnabled()) { - // Define the minimum bubble size - static const glm::vec3 minBubbleSize = glm::vec3(0.3f, 1.3f, 0.3f); - // Define the scale of the box for the current node - glm::vec3 nodeBoxScale = (nodeData->getPosition() - nodeData->getGlobalBoundingBoxCorner()) * 2.0f; - // Define the scale of the box for the current other node - glm::vec3 otherNodeBoxScale = (otherData->getPosition() - otherData->getGlobalBoundingBoxCorner()) * 2.0f; - - // Set up the bounding box for the current node - AABox nodeBox(nodeData->getGlobalBoundingBoxCorner(), nodeBoxScale); - // Clamp the size of the bounding box to a minimum scale - if (glm::any(glm::lessThan(nodeBoxScale, minBubbleSize))) { - nodeBox.setScaleStayCentered(minBubbleSize); - } - // Set up the bounding box for the current other node - AABox otherNodeBox(otherData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); - // Clamp the size of the bounding box to a minimum scale - if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { - otherNodeBox.setScaleStayCentered(minBubbleSize); - } - // Quadruple the scale of both bounding boxes - nodeBox.embiggen(4.0f); - otherNodeBox.embiggen(4.0f); - - // Perform the collision check between the two bounding boxes - if (nodeBox.touches(otherNodeBox)) { - nodeData->ignoreOther(node, otherNode); - shouldConsider = getsAnyIgnored; - } - } - // Not close enough to ignore - if (shouldConsider) { - nodeData->removeFromRadiusIgnoringSet(node, otherNode->getUUID()); - } - - quint64 endIgnoreCalculation = usecTimestampNow(); - _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation); + detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO + ? AvatarData::SendAllData : AvatarData::CullSmallData; + nodeData->incrementAvatarInView(); } - if (shouldConsider) { - quint64 startAvatarDataPacking = usecTimestampNow(); + bool includeThisAvatar = true; + auto lastEncodeForOther = nodeData->getLastOtherAvatarEncodeTime(otherNode->getUUID()); + QVector& lastSentJointsForOther = nodeData->getLastOtherAvatarSentJoints(otherNode->getUUID()); + bool distanceAdjust = true; + glm::vec3 viewerPosition = myPosition; + AvatarDataPacket::HasFlags hasFlagsOut; // the result of the toByteArray + bool dropFaceTracking = false; - ++numOtherAvatars; - - const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); - - // make sure we send out identity packets to and from new arrivals. - bool forceSend = !nodeData->checkAndSetHasReceivedFirstPacketsFrom(otherNode->getUUID()); - - // FIXME - this clause seems suspicious "... || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp ..." - if (!overBudget - && otherNodeData->getIdentityChangeTimestamp().time_since_epoch().count() > 0 - && (forceSend - || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp - || distribution(generator) < IDENTITY_SEND_PROBABILITY)) { - - identityBytesSent += sendIdentityPacket(otherNodeData, node); - } - - const AvatarData* otherAvatar = otherNodeData->getConstAvatarData(); - // Decide whether to send this avatar's data based on it's distance from us - - glm::vec3 otherPosition = otherAvatar->getClientGlobalPosition(); - float distanceToAvatar = glm::length(myPosition - otherPosition); - - if (shouldConsider) { - AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID()); - AvatarDataSequenceNumber lastSeqFromSender = otherNodeData->getLastReceivedSequenceNumber(); - - // FIXME - This code does appear to be working. But it seems brittle. - // It supports determining if the frame of data for this "other" - // avatar has already been sent to the reciever. This has been - // verified to work on a desktop display that renders at 60hz and - // therefore sends to mixer at 30hz. Each second you'd expect to - // have 15 (45hz-30hz) duplicate frames. In this case, the stat - // avg_other_av_skips_per_second does report 15. - // - // make sure we haven't already sent this data from this sender to this receiver - // or that somehow we haven't sent - if (lastSeqToReceiver == lastSeqFromSender && lastSeqToReceiver != 0) { - ++numAvatarsHeldBack; - - quint64 endAvatarDataPacking = usecTimestampNow(); - _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); - shouldConsider = false; - } else if (lastSeqFromSender - lastSeqToReceiver > 1) { - // this is a skip - we still send the packet but capture the presence of the skip so we see it happening - ++numAvatarsWithSkippedFrames; - } - - // we're going to send this avatar - if (shouldConsider) { - - // determine if avatar is in view, to determine how much data to include... - glm::vec3 otherNodeBoxScale = (otherPosition - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f; - AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); - bool isInView = nodeData->otherAvatarInView(otherNodeBox); - - // this throttles the extra data to only be sent every Nth message - /** - if (!isInView && !getsOutOfView && (lastSeqToReceiver % EXTRA_AVATAR_DATA_FRAME_RATIO > 0)) { - quint64 endAvatarDataPacking = usecTimestampNow(); - - _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); - shouldConsider = false; - } - ***/ - - if (shouldConsider) { - // start a new segment in the PacketList for this avatar - avatarPacketList->startSegment(); - - AvatarData::AvatarDataDetail detail; - - if (overBudget) { - overBudgetAvatars++; - _stats.overBudgetAvatars++; - detail = AvatarData::NoData; - } else if (!isInView && !getsOutOfView) { - detail = AvatarData::NoData; - nodeData->incrementAvatarOutOfView(); - } else { - detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO - ? AvatarData::SendAllData : AvatarData::CullSmallData; - nodeData->incrementAvatarInView(); - } - - { - bool includeThisAvatar = true; - auto lastEncodeForOther = nodeData->getLastOtherAvatarEncodeTime(otherNode->getUUID()); - QVector& lastSentJointsForOther = nodeData->getLastOtherAvatarSentJoints(otherNode->getUUID()); - bool distanceAdjust = true; - glm::vec3 viewerPosition = myPosition; - AvatarDataPacket::HasFlags hasFlagsOut; // the result of the toByteArray - bool dropFaceTracking = false; - - quint64 start = usecTimestampNow(); - QByteArray bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, - hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); - quint64 end = usecTimestampNow(); - _stats.toByteArrayElapsedTime += (end - start); - - static const int MAX_ALLOWED_AVATAR_DATA = (1400 - NUM_BYTES_RFC4122_UUID); - if (bytes.size() > MAX_ALLOWED_AVATAR_DATA) { - qCWarning(avatars) << "otherAvatar.toByteArray() resulted in very large buffer:" << bytes.size() << "... attempt to drop facial data"; - - dropFaceTracking = true; // first try dropping the facial data - bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, - hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); - - if (bytes.size() > MAX_ALLOWED_AVATAR_DATA) { - qCWarning(avatars) << "otherAvatar.toByteArray() without facial data resulted in very large buffer:" << bytes.size() << "... reduce to MinimumData"; - bytes = otherAvatar->toByteArray(AvatarData::MinimumData, lastEncodeForOther, lastSentJointsForOther, + quint64 start = usecTimestampNow(); + QByteArray bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); - } + quint64 end = usecTimestampNow(); + _stats.toByteArrayElapsedTime += (end - start); - if (bytes.size() > MAX_ALLOWED_AVATAR_DATA) { - qCWarning(avatars) << "otherAvatar.toByteArray() MinimumData resulted in very large buffer:" << bytes.size() << "... FAIL!!"; - includeThisAvatar = false; - } - } + static const int MAX_ALLOWED_AVATAR_DATA = (1400 - NUM_BYTES_RFC4122_UUID); + if (bytes.size() > MAX_ALLOWED_AVATAR_DATA) { + qCWarning(avatars) << "otherAvatar.toByteArray() resulted in very large buffer:" << bytes.size() << "... attempt to drop facial data"; - if (includeThisAvatar) { - numAvatarDataBytes += avatarPacketList->write(otherNode->getUUID().toRfc4122()); - numAvatarDataBytes += avatarPacketList->write(bytes); - _stats.numOthersIncluded++; + dropFaceTracking = true; // first try dropping the facial data + bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, + hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); - // increment the number of avatars sent to this reciever - nodeData->incrementNumAvatarsSentLastFrame(); + if (bytes.size() > MAX_ALLOWED_AVATAR_DATA) { + qCWarning(avatars) << "otherAvatar.toByteArray() without facial data resulted in very large buffer:" << bytes.size() << "... reduce to MinimumData"; + bytes = otherAvatar->toByteArray(AvatarData::MinimumData, lastEncodeForOther, lastSentJointsForOther, + hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); + } - // set the last sent sequence number for this sender on the receiver - nodeData->setLastBroadcastSequenceNumber(otherNode->getUUID(), - otherNodeData->getLastReceivedSequenceNumber()); - - // remember the last time we sent details about this other node to the receiver - nodeData->setLastBroadcastTime(otherNode->getUUID(), start); - - } - } - - avatarPacketList->endSegment(); - - quint64 endAvatarDataPacking = usecTimestampNow(); - _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); - } - } + if (bytes.size() > MAX_ALLOWED_AVATAR_DATA) { + qCWarning(avatars) << "otherAvatar.toByteArray() MinimumData resulted in very large buffer:" << bytes.size() << "... FAIL!!"; + includeThisAvatar = false; } } + + if (includeThisAvatar) { + numAvatarDataBytes += avatarPacketList->write(otherNode->getUUID().toRfc4122()); + numAvatarDataBytes += avatarPacketList->write(bytes); + _stats.numOthersIncluded++; + + // increment the number of avatars sent to this reciever + nodeData->incrementNumAvatarsSentLastFrame(); + + // set the last sent sequence number for this sender on the receiver + nodeData->setLastBroadcastSequenceNumber(otherNode->getUUID(), + otherNodeData->getLastReceivedSequenceNumber()); + + // remember the last time we sent details about this other node to the receiver + nodeData->setLastBroadcastTime(otherNode->getUUID(), start); + + } + + avatarPacketList->endSegment(); + + quint64 endAvatarDataPacking = usecTimestampNow(); + _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); }; quint64 startPacketSending = usecTimestampNow(); From 1e91f74ce745929d6a502f7f7677640e64b617d6 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 13:32:35 -0800 Subject: [PATCH 16/16] clean up some uneedded nullptr checks, make them asserts --- .../src/avatars/AvatarMixerSlave.cpp | 133 +++++++++--------- 1 file changed, 65 insertions(+), 68 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index a839784cd1..8d9a5e6951 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -171,6 +171,10 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { int listItem = 0; std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); + + // theoretically it's possible for a Node to be in the NodeList (and therefore end up here), + // but not have yet sent data that's linked to the node. Check for that case and don't + // consider those nodes. if (otherNodeData) { listItem++; AvatarSharedPointer otherAvatar = otherNodeData->getAvatarSharedPointer(); @@ -186,10 +190,8 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { [&](AvatarSharedPointer avatar)->uint64_t{ auto avatarNode = avatarDataToNodes[avatar]; - if (avatarNode) { - return nodeData->getLastBroadcastTime(avatarNode->getUUID()); - } - return 0; + assert(avatarNode); // we can't have gotten here without the avatarData being a valid key in the map + return nodeData->getLastBroadcastTime(avatarNode->getUUID()); }, [&](AvatarSharedPointer avatar)->float{ @@ -210,72 +212,72 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // happen if for example the avatar is connected on a desktop and sending // updates at ~30hz. So every 3 frames we skip a frame. auto avatarNode = avatarDataToNodes[avatar]; - if (avatarNode) { - const AvatarMixerClientData* avatarNodeData = reinterpret_cast(avatarNode->getLinkedData()); - if (avatarNodeData) { - quint64 startIgnoreCalculation = usecTimestampNow(); - // make sure we have data for this avatar, that it isn't the same node, - // and isn't an avatar that the viewing node has ignored - // or that has ignored the viewing node - if (!avatarNode->getLinkedData() - || avatarNode->getUUID() == node->getUUID() - || (node->isIgnoringNodeWithID(avatarNode->getUUID()) && !getsIgnoredByMe) - || (avatarNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { - shouldIgnore = true; - } else { + assert(avatarNode); // we can't have gotten here without the avatarData being a valid key in the map - // Check to see if the space bubble is enabled - if (node->isIgnoreRadiusEnabled() || avatarNode->isIgnoreRadiusEnabled()) { + const AvatarMixerClientData* avatarNodeData = reinterpret_cast(avatarNode->getLinkedData()); + assert(avatarNodeData); // we can't have gotten here without avatarNode having valid data + quint64 startIgnoreCalculation = usecTimestampNow(); - // Define the scale of the box for the current other node - glm::vec3 otherNodeBoxScale = (avatarNodeData->getPosition() - avatarNodeData->getGlobalBoundingBoxCorner()) * 2.0f; - // Set up the bounding box for the current other node - AABox otherNodeBox(avatarNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); - // Clamp the size of the bounding box to a minimum scale - if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { - otherNodeBox.setScaleStayCentered(minBubbleSize); - } - // Quadruple the scale of both bounding boxes - otherNodeBox.embiggen(4.0f); + // make sure we have data for this avatar, that it isn't the same node, + // and isn't an avatar that the viewing node has ignored + // or that has ignored the viewing node + if (!avatarNode->getLinkedData() + || avatarNode->getUUID() == node->getUUID() + || (node->isIgnoringNodeWithID(avatarNode->getUUID()) && !getsIgnoredByMe) + || (avatarNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { + shouldIgnore = true; + } else { - // Perform the collision check between the two bounding boxes - if (nodeBox.touches(otherNodeBox)) { - nodeData->ignoreOther(node, avatarNode); - shouldIgnore = !getsAnyIgnored; - } - } - // Not close enough to ignore - if (!shouldIgnore) { - nodeData->removeFromRadiusIgnoringSet(node, avatarNode->getUUID()); - } + // Check to see if the space bubble is enabled + if (node->isIgnoreRadiusEnabled() || avatarNode->isIgnoreRadiusEnabled()) { + + // Define the scale of the box for the current other node + glm::vec3 otherNodeBoxScale = (avatarNodeData->getPosition() - avatarNodeData->getGlobalBoundingBoxCorner()) * 2.0f; + // Set up the bounding box for the current other node + AABox otherNodeBox(avatarNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); + // Clamp the size of the bounding box to a minimum scale + if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { + otherNodeBox.setScaleStayCentered(minBubbleSize); } - quint64 endIgnoreCalculation = usecTimestampNow(); - _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation); + // Quadruple the scale of both bounding boxes + otherNodeBox.embiggen(4.0f); - if (!shouldIgnore) { - AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(avatarNode->getUUID()); - AvatarDataSequenceNumber lastSeqFromSender = avatarNodeData->getLastReceivedSequenceNumber(); - - // FIXME - This code does appear to be working. But it seems brittle. - // It supports determining if the frame of data for this "other" - // avatar has already been sent to the reciever. This has been - // verified to work on a desktop display that renders at 60hz and - // therefore sends to mixer at 30hz. Each second you'd expect to - // have 15 (45hz-30hz) duplicate frames. In this case, the stat - // avg_other_av_skips_per_second does report 15. - // - // make sure we haven't already sent this data from this sender to this receiver - // or that somehow we haven't sent - if (lastSeqToReceiver == lastSeqFromSender && lastSeqToReceiver != 0) { - ++numAvatarsHeldBack; - shouldIgnore = true; - } else if (lastSeqFromSender - lastSeqToReceiver > 1) { - // this is a skip - we still send the packet but capture the presence of the skip so we see it happening - ++numAvatarsWithSkippedFrames; - } + // Perform the collision check between the two bounding boxes + if (nodeBox.touches(otherNodeBox)) { + nodeData->ignoreOther(node, avatarNode); + shouldIgnore = !getsAnyIgnored; } } + // Not close enough to ignore + if (!shouldIgnore) { + nodeData->removeFromRadiusIgnoringSet(node, avatarNode->getUUID()); + } + } + quint64 endIgnoreCalculation = usecTimestampNow(); + _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation); + + if (!shouldIgnore) { + AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(avatarNode->getUUID()); + AvatarDataSequenceNumber lastSeqFromSender = avatarNodeData->getLastReceivedSequenceNumber(); + + // FIXME - This code does appear to be working. But it seems brittle. + // It supports determining if the frame of data for this "other" + // avatar has already been sent to the reciever. This has been + // verified to work on a desktop display that renders at 60hz and + // therefore sends to mixer at 30hz. Each second you'd expect to + // have 15 (45hz-30hz) duplicate frames. In this case, the stat + // avg_other_av_skips_per_second does report 15. + // + // make sure we haven't already sent this data from this sender to this receiver + // or that somehow we haven't sent + if (lastSeqToReceiver == lastSeqFromSender && lastSeqToReceiver != 0) { + ++numAvatarsHeldBack; + shouldIgnore = true; + } else if (lastSeqFromSender - lastSeqToReceiver > 1) { + // this is a skip - we still send the packet but capture the presence of the skip so we see it happening + ++numAvatarsWithSkippedFrames; + } } return shouldIgnore; }); @@ -284,7 +286,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { int avatarRank = 0; // this is overly conservative, because it includes some avatars we might not consider - // FIXME - move the ignore logic up into the sorting list so we get a better estimate int remainingAvatars = (int)sortedAvatars.size(); while (!sortedAvatars.empty()) { @@ -295,11 +296,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { remainingAvatars--; auto otherNode = avatarDataToNodes[avatarData]; - - // FIXME - should't this be an assert? - if (!otherNode) { - continue; - } + assert(otherNode); // we can't have gotten here without the avatarData being a valid key in the map // NOTE: Here's where we determine if we are over budget and drop to bare minimum data int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars;