From e1f905cb3667fafaa9e9935a7a6db333fb38639b Mon Sep 17 00:00:00 2001 From: wangyix Date: Fri, 1 Aug 2014 10:23:03 -0700 Subject: [PATCH] fixed >100% loss rate bug in SequenceNumberStats --- interface/src/Audio.cpp | 4 +- interface/src/ui/OctreeStatsDialog.cpp | 14 ++--- libraries/audio/src/InboundAudioStream.cpp | 2 +- libraries/audio/src/InboundAudioStream.h | 2 +- .../networking/src/SequenceNumberStats.cpp | 55 +++++++++-------- .../networking/src/SequenceNumberStats.h | 61 ++++++++++--------- 6 files changed, 73 insertions(+), 65 deletions(-) diff --git a/interface/src/Audio.cpp b/interface/src/Audio.cpp index 4ed1f7aeb3..4fcab5b949 100644 --- a/interface/src/Audio.cpp +++ b/interface/src/Audio.cpp @@ -1424,9 +1424,9 @@ void Audio::renderAudioStreamStats(const AudioStreamStats& streamStats, int hori sprintf(stringBuffer, " Packet loss | overall: %5.2f%% (%d lost), last_30s: %5.2f%% (%d lost)", streamStats._packetStreamStats.getLostRate() * 100.0f, - streamStats._packetStreamStats._numLost, + streamStats._packetStreamStats._lost, streamStats._packetStreamWindowStats.getLostRate() * 100.0f, - streamStats._packetStreamWindowStats._numLost); + streamStats._packetStreamWindowStats._lost); verticalOffset += STATS_HEIGHT_PER_LINE; drawText(horizontalOffset, verticalOffset, scale, rotation, font, stringBuffer, color); diff --git a/interface/src/ui/OctreeStatsDialog.cpp b/interface/src/ui/OctreeStatsDialog.cpp index afa799815f..7abc42d0e3 100644 --- a/interface/src/ui/OctreeStatsDialog.cpp +++ b/interface/src/ui/OctreeStatsDialog.cpp @@ -366,13 +366,13 @@ void OctreeStatsDialog::showOctreeServersOfType(int& serverCount, NodeType_t ser QString incomingBytesString = locale.toString((uint)stats.getIncomingBytes()); QString incomingWastedBytesString = locale.toString((uint)stats.getIncomingWastedBytes()); const SequenceNumberStats& seqStats = stats.getIncomingOctreeSequenceNumberStats(); - QString incomingOutOfOrderString = locale.toString((uint)seqStats.getNumOutOfOrder()); - QString incomingLateString = locale.toString((uint)seqStats.getNumLate()); - QString incomingUnreasonableString = locale.toString((uint)seqStats.getNumUnreasonable()); - QString incomingEarlyString = locale.toString((uint)seqStats.getNumEarly()); - QString incomingLikelyLostString = locale.toString((uint)seqStats.getNumLost()); - QString incomingRecovered = locale.toString((uint)seqStats.getNumRecovered()); - QString incomingDuplicateString = locale.toString((uint)seqStats.getNumDuplicate()); + QString incomingOutOfOrderString = locale.toString((uint)seqStats.getOutOfOrder()); + QString incomingLateString = locale.toString((uint)seqStats.getLate()); + QString incomingUnreasonableString = locale.toString((uint)seqStats.getUnreasonable()); + QString incomingEarlyString = locale.toString((uint)seqStats.getEarly()); + QString incomingLikelyLostString = locale.toString((uint)seqStats.getLost()); + QString incomingRecovered = locale.toString((uint)seqStats.getRecovered()); + QString incomingDuplicateString = locale.toString((uint)seqStats.getDuplicate()); int clockSkewInMS = node->getClockSkewUsec() / (int)USECS_PER_MSEC; QString incomingFlightTimeString = locale.toString((int)stats.getIncomingFlightTimeAverage()); diff --git a/libraries/audio/src/InboundAudioStream.cpp b/libraries/audio/src/InboundAudioStream.cpp index 6ade4b17e9..36145a35f6 100644 --- a/libraries/audio/src/InboundAudioStream.cpp +++ b/libraries/audio/src/InboundAudioStream.cpp @@ -212,7 +212,7 @@ SequenceNumberStats::ArrivalInfo InboundAudioStream::frameReceivedUpdateNetworkS // discard the first few packets we receive since they usually have gaps that aren't represensative of normal jitter const int NUM_INITIAL_PACKETS_DISCARD = 3; quint64 now = usecTimestampNow(); - if (_incomingSequenceNumberStats.getNumReceived() > NUM_INITIAL_PACKETS_DISCARD) { + if (_incomingSequenceNumberStats.getReceived() > NUM_INITIAL_PACKETS_DISCARD) { quint64 gap = now - _lastFrameReceivedTime; _interframeTimeGapStatsForStatsPacket.update(gap); diff --git a/libraries/audio/src/InboundAudioStream.h b/libraries/audio/src/InboundAudioStream.h index 06bd329fee..1a196a1c61 100644 --- a/libraries/audio/src/InboundAudioStream.h +++ b/libraries/audio/src/InboundAudioStream.h @@ -108,7 +108,7 @@ public: int getSilentFramesDropped() const { return _silentFramesDropped; } int getOverflowCount() const { return _ringBuffer.getOverflowCount(); } - int getPacketReceived() const { return _incomingSequenceNumberStats.getNumReceived(); } + int getPacketsReceived() const { return _incomingSequenceNumberStats.getReceived(); } private: void starved(); diff --git a/libraries/networking/src/SequenceNumberStats.cpp b/libraries/networking/src/SequenceNumberStats.cpp index 66d57500a5..00a3e66877 100644 --- a/libraries/networking/src/SequenceNumberStats.cpp +++ b/libraries/networking/src/SequenceNumberStats.cpp @@ -14,7 +14,8 @@ #include SequenceNumberStats::SequenceNumberStats(int statsHistoryLength) - : _lastReceived(std::numeric_limits::max()), + : _received(0), + _lastReceivedSequence(0), _missingSet(), _stats(), _lastSenderUUID(), @@ -23,8 +24,10 @@ SequenceNumberStats::SequenceNumberStats(int statsHistoryLength) } void SequenceNumberStats::reset() { + _received = 0; _missingSet.clear(); _stats = PacketStreamStats(); + _lastSenderUUID = QUuid(); _statsHistory.clear(); } @@ -36,7 +39,7 @@ SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(qui // if the sender node has changed, reset all stats if (senderUUID != _lastSenderUUID) { - if (_stats._numReceived > 0) { + if (_received > 0) { qDebug() << "sequence number stats was reset due to new sender node"; qDebug() << "previous:" << _lastSenderUUID << "current:" << senderUUID; reset(); @@ -45,13 +48,14 @@ SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(qui } // determine our expected sequence number... handle rollover appropriately - quint16 expected = _stats._numReceived > 0 ? _lastReceived + (quint16)1 : incoming; + quint16 expected = _received > 0 ? _lastReceivedSequence + (quint16)1 : incoming; - _stats._numReceived++; + _received++; if (incoming == expected) { // on time arrivalInfo._status = OnTime; - _lastReceived = incoming; + _lastReceivedSequence = incoming; + _stats._expectedReceived++; } else { // out of order if (wantExtraDebugging) { @@ -76,8 +80,8 @@ SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(qui // ignore packet if gap is unreasonable qDebug() << "ignoring unreasonable sequence number:" << incoming - << "previous:" << _lastReceived; - _stats._numUnreasonable++; + << "previous:" << _lastReceivedSequence; + _stats._unreasonable++; return arrivalInfo; } @@ -94,10 +98,11 @@ SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(qui qDebug() << "this packet is earlier than expected..."; qDebug() << ">>>>>>>> missing gap=" << (incomingInt - expectedInt); } - - _stats._numEarly++; - _stats._numLost += (incomingInt - expectedInt); - _lastReceived = incoming; + int skipped = incomingInt - expectedInt; + _stats._early++; + _stats._lost += skipped; + _stats._expectedReceived += (skipped + 1); + _lastReceivedSequence = incoming; // add all sequence numbers that were skipped to the missing sequence numbers list for (int missingInt = expectedInt; missingInt < incomingInt; missingInt++) { @@ -114,7 +119,7 @@ SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(qui qDebug() << "this packet is later than expected..."; } - _stats._numLate++; + _stats._late++; // do not update _lastReceived; it shouldn't become smaller @@ -125,15 +130,15 @@ SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(qui if (wantExtraDebugging) { qDebug() << "found it in _missingSet"; } - _stats._numLost--; - _stats._numRecovered++; + _stats._lost--; + _stats._recovered++; } else { arrivalInfo._status = Duplicate; if (wantExtraDebugging) { qDebug() << "sequence:" << incoming << "was NOT found in _missingSet and is probably a duplicate"; } - _stats._numDuplicate++; + _stats._duplicate++; } } } @@ -148,7 +153,7 @@ void SequenceNumberStats::pruneMissingSet(const bool wantExtraDebugging) { // some older sequence numbers may be from before a rollover point; this must be handled. // some sequence numbers in this list may be larger than _incomingLastSequence, indicating that they were received // before the most recent rollover. - int cutoff = (int)_lastReceived - MAX_REASONABLE_SEQUENCE_GAP; + int cutoff = (int)_lastReceivedSequence - MAX_REASONABLE_SEQUENCE_GAP; if (cutoff >= 0) { quint16 nonRolloverCutoff = (quint16)cutoff; QSet::iterator i = _missingSet.begin(); @@ -159,7 +164,7 @@ void SequenceNumberStats::pruneMissingSet(const bool wantExtraDebugging) { qDebug() << "old age cutoff:" << nonRolloverCutoff; } - if (missing > _lastReceived || missing < nonRolloverCutoff) { + if (missing > _lastReceivedSequence || missing < nonRolloverCutoff) { i = _missingSet.erase(i); if (wantExtraDebugging) { qDebug() << "pruning really old missing sequence:" << missing; @@ -178,7 +183,7 @@ void SequenceNumberStats::pruneMissingSet(const bool wantExtraDebugging) { qDebug() << "old age cutoff:" << rolloverCutoff; } - if (missing > _lastReceived && missing < rolloverCutoff) { + if (missing > _lastReceivedSequence && missing < rolloverCutoff) { i = _missingSet.erase(i); if (wantExtraDebugging) { qDebug() << "pruning really old missing sequence:" << missing; @@ -202,13 +207,13 @@ PacketStreamStats SequenceNumberStats::getStatsForHistoryWindow() const { // calculate difference between newest stats and oldest stats to get window stats PacketStreamStats windowStats; - windowStats._numReceived = newestStats->_numReceived - oldestStats->_numReceived; - windowStats._numUnreasonable = newestStats->_numUnreasonable - oldestStats->_numUnreasonable; - windowStats._numEarly = newestStats->_numEarly - oldestStats->_numEarly; - windowStats._numLate = newestStats->_numLate - oldestStats->_numLate; - windowStats._numLost = newestStats->_numLost - oldestStats->_numLost; - windowStats._numRecovered = newestStats->_numRecovered - oldestStats->_numRecovered; - windowStats._numDuplicate = newestStats->_numDuplicate - oldestStats->_numDuplicate; + windowStats._expectedReceived = newestStats->_expectedReceived - oldestStats->_expectedReceived; + windowStats._unreasonable = newestStats->_unreasonable - oldestStats->_unreasonable; + windowStats._early = newestStats->_early - oldestStats->_early; + windowStats._late = newestStats->_late - oldestStats->_late; + windowStats._lost = newestStats->_lost - oldestStats->_lost; + windowStats._recovered = newestStats->_recovered - oldestStats->_recovered; + windowStats._duplicate = newestStats->_duplicate - oldestStats->_duplicate; return windowStats; } diff --git a/libraries/networking/src/SequenceNumberStats.h b/libraries/networking/src/SequenceNumberStats.h index f4e85b6fb3..d611a494ad 100644 --- a/libraries/networking/src/SequenceNumberStats.h +++ b/libraries/networking/src/SequenceNumberStats.h @@ -21,29 +21,27 @@ const int MAX_REASONABLE_SEQUENCE_GAP = 1000; class PacketStreamStats { public: PacketStreamStats() - : _numReceived(0), - _numUnreasonable(0), - _numEarly(0), - _numLate(0), - _numLost(0), - _numRecovered(0), - _numDuplicate(0) + : _expectedReceived(0), + _unreasonable(0), + _early(0), + _late(0), + _lost(0), + _recovered(0), + _duplicate(0) {} - float getUnreasonableRate() const { return (float)_numUnreasonable / _numReceived; } - float getNumEaryRate() const { return (float)_numEarly / _numReceived; } - float getLateRate() const { return (float)_numLate / _numReceived; } - float getLostRate() const { return (float)_numLost / _numReceived; } - float getRecoveredRate() const { return (float)_numRecovered / _numReceived; } - float getDuplicateRate() const { return (float)_numDuplicate / _numReceived; } + float getUnreasonableRate() const { return (float)_unreasonable / _expectedReceived; } + float getEaryRate() const { return (float)_early / _expectedReceived; } + float getLateRate() const { return (float)_late / _expectedReceived; } + float getLostRate() const { return (float)_lost / _expectedReceived; } - quint32 _numReceived; - quint32 _numUnreasonable; - quint32 _numEarly; - quint32 _numLate; - quint32 _numLost; - quint32 _numRecovered; - quint32 _numDuplicate; + quint32 _expectedReceived; + quint32 _unreasonable; + quint32 _early; + quint32 _late; + quint32 _lost; + quint32 _recovered; + quint32 _duplicate; }; class SequenceNumberStats { @@ -70,20 +68,25 @@ public: void pruneMissingSet(const bool wantExtraDebugging = false); void pushStatsToHistory() { _statsHistory.insert(_stats); } - quint32 getNumReceived() const { return _stats._numReceived; } - quint32 getNumUnreasonable() const { return _stats._numUnreasonable; } - quint32 getNumOutOfOrder() const { return _stats._numEarly + _stats._numLate; } - quint32 getNumEarly() const { return _stats._numEarly; } - quint32 getNumLate() const { return _stats._numLate; } - quint32 getNumLost() const { return _stats._numLost; } - quint32 getNumRecovered() const { return _stats._numRecovered; } - quint32 getNumDuplicate() const { return _stats._numDuplicate; } + quint32 getReceived() const { return _received; } + + quint32 getExpectedReceived() const { return _stats._expectedReceived; } + quint32 getUnreasonable() const { return _stats._unreasonable; } + quint32 getOutOfOrder() const { return _stats._early + _stats._late; } + quint32 getEarly() const { return _stats._early; } + quint32 getLate() const { return _stats._late; } + quint32 getLost() const { return _stats._lost; } + quint32 getRecovered() const { return _stats._recovered; } + quint32 getDuplicate() const { return _stats._duplicate; } + const PacketStreamStats& getStats() const { return _stats; } PacketStreamStats getStatsForHistoryWindow() const; const QSet& getMissingSet() const { return _missingSet; } private: - quint16 _lastReceived; + int _received; + + quint16 _lastReceivedSequence; QSet _missingSet; PacketStreamStats _stats;