From dde9640c66a9be73e8c65c8d87a3b12d22f6a787 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 23 Feb 2017 10:29:17 -0800 Subject: [PATCH] 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,