From 5fa5e6e0a74eabcfe1ab99ecd216bde4734044fc Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 1 Mar 2017 14:27:50 -0800 Subject: [PATCH] First pass at implementing Brad's simplification ideas --- .../src/avatars/AvatarMixerClientData.cpp | 10 ------- .../src/avatars/AvatarMixerClientData.h | 9 ++---- .../src/avatars/AvatarMixerSlave.cpp | 28 ++----------------- .../src/avatars/AvatarMixerSlave.h | 12 -------- 4 files changed, 6 insertions(+), 53 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 717e14535f..43b8816111 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -65,15 +65,6 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) { // compute the offset to the data payload return _avatar->parseDataFromBuffer(message.readWithoutCopy(message.getBytesLeftToRead())); } - -bool AvatarMixerClientData::checkAndSetHasReceivedFirstPacketsFrom(const QUuid& uuid) { - if (_hasReceivedFirstPacketsFrom.find(uuid) == _hasReceivedFirstPacketsFrom.end()) { - _hasReceivedFirstPacketsFrom.insert(uuid); - return false; - } - 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); @@ -103,7 +94,6 @@ void AvatarMixerClientData::ignoreOther(SharedNodePointer self, SharedNodePointe killPacket->writePrimitive(KillAvatarReason::YourAvatarEnteredTheirBubble); } DependencyManager::get()->sendUnreliablePacket(*killPacket, *self); - _hasReceivedFirstPacketsFrom.erase(other->getUUID()); } } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 51b8d692e2..1449005246 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -45,8 +45,6 @@ public: const AvatarData* getConstAvatarData() const { return _avatar.get(); } AvatarSharedPointer getAvatarSharedPointer() const { return _avatar; } - bool checkAndSetHasReceivedFirstPacketsFrom(const QUuid& uuid); - uint16_t getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const; void setLastBroadcastSequenceNumber(const QUuid& nodeUUID, uint16_t sequenceNumber) { _lastBroadcastSequenceNumbers[nodeUUID] = sequenceNumber; } @@ -63,8 +61,8 @@ public: uint16_t getLastReceivedSequenceNumber() const { return _lastReceivedSequenceNumber; } - HRCTime getIdentityChangeTimestamp() const { return _identityChangeTimestamp; } - void flagIdentityChange() { _identityChangeTimestamp = p_high_resolution_clock::now(); } + uint64_t getIdentityChangeTimestamp() const { return _identityChangeTimestamp; } + void flagIdentityChange() { _identityChangeTimestamp = usecTimestampNow(); } bool getAvatarSessionDisplayNameMustChange() const { return _avatarSessionDisplayNameMustChange; } void setAvatarSessionDisplayNameMustChange(bool set = true) { _avatarSessionDisplayNameMustChange = set; } @@ -139,7 +137,6 @@ 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 @@ -147,7 +144,7 @@ private: std::unordered_map _lastOtherAvatarEncodeTime; std::unordered_map> _lastOtherAvatarSentJoints; - HRCTime _identityChangeTimestamp; + uint64_t _identityChangeTimestamp; bool _avatarSessionDisplayNameMustChange{ false }; int _numAvatarsSentLastFrame = 0; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 47efb39c3c..60dfad7687 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -138,19 +138,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { // Increase minimumBytesPerAvatar if the PAL is open minimumBytesPerAvatar += sizeof(AvatarDataPacket::AvatarGlobalPosition) + sizeof(AvatarDataPacket::AudioLoudness); - if (_identitySendProbability == DEFAULT_IDENTITY_SEND_PROBABILITY) { - // The client has just opened the PAL. Force all identity packets to be sent to - // this client. - _identitySendProbability = 1.0f; - } else { - // The user recently opened the PAL, but we've already gone through the above conditional. - // We want to receive identity updates more often than default when the PAL is open - // to be more confident that the user will see the most up-to-date information in the PAL. - _identitySendProbability = DEFAULT_IDENTITY_SEND_PROBABILITY * 2; - } - } else { - // If the PAL is closed, reset the identitySendProbability to the default. - _identitySendProbability = DEFAULT_IDENTITY_SEND_PROBABILITY; } // setup a PacketList for the avatarPackets @@ -313,18 +300,9 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { 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) < _identitySendProbability)) || - // Also make sure we send identity packets if the PAL is open. - (PALIsOpen && distribution(generator) < _identitySendProbability)) { - + // If the time that the mixer sent AVATAR DATA about Avatar B to Avatar A is BEFORE OR EQUAL TO + // the time that Avatar B flagged an IDENTITY DATA change, send IDENTITY DATA about Avatar B to Avatar A. + if (nodeData->getLastBroadcastTime(otherNode->getUUID()) <= otherNodeData->getIdentityChangeTimestamp()) { identityBytesSent += sendIdentityPacket(otherNodeData, node); } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 2fa5fa4484..04141d9d72 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -101,18 +101,6 @@ private: float _maxKbpsPerNode { 0.0f }; float _throttlingRatio { 0.0f }; - - // 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... - // - // An 80% chance of sending a identity packet within a 5 second interval. - // assuming 60 htz update rate. - // - // Assuming the calculation of the constant is in fact correct for 80% and 60hz and 5 seconds (an assumption - // that I have not verified) then the constant is definitely wrong now, since we send at 45hz. - const float DEFAULT_IDENTITY_SEND_PROBABILITY = 1.0f / 187.0f; - float _identitySendProbability = DEFAULT_IDENTITY_SEND_PROBABILITY; - AvatarMixerSlaveStats _stats; };