From b74aad32ede3cb9d9c65d538c28b6099ca216f2c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 10 Jul 2017 18:27:40 -0700 Subject: [PATCH 01/24] minor cleanup in OctreeSendThread::packetDistributor() --- .../src/octree/OctreeSendThread.cpp | 56 ++++++------------- 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 7db06f12c0..2a4e317670 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -334,8 +334,6 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* && ((!viewFrustumChanged && nodeData->getViewFrustumJustStoppedChanging()) || nodeData->hasLodChanged())); } - bool somethingToSend = true; // assume we have something - // If our packet already has content in it, then we must use the color choice of the waiting packet. // If we're starting a fresh packet, then... // If we're moving, and the client asked for low res, then we force monochrome, otherwise, use @@ -404,7 +402,6 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* // If we have something in our elementBag, then turn them into packets and send them out... if (!nodeData->elementBag.isEmpty()) { - int bytesWritten = 0; quint64 start = usecTimestampNow(); // TODO: add these to stats page @@ -414,6 +411,7 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* int extraPackingAttempts = 0; bool completedScene = false; + bool somethingToSend = true; // assume we have something while (somethingToSend && packetsSentThisInterval < maxPacketsPerInterval && !nodeData->isShuttingDown()) { float lockWaitElapsedUsec = OctreeServer::SKIP_TIME; float encodeElapsedUsec = OctreeServer::SKIP_TIME; @@ -461,7 +459,7 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* // are reported to client. Since you can encode without the lock nodeData->stats.encodeStarted(); - bytesWritten = _myServer->getOctree()->encodeTreeBitstream(subTree, &_packetData, nodeData->elementBag, params); + _myServer->getOctree()->encodeTreeBitstream(subTree, &_packetData, nodeData->elementBag, params); quint64 encodeEnd = usecTimestampNow(); encodeElapsedUsec = (float)(encodeEnd - encodeStart); @@ -479,45 +477,30 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* nodeData->stats.encodeStopped(); }); } else { - // If the bag was empty then we didn't even attempt to encode, and so we know the bytesWritten were 0 - bytesWritten = 0; somethingToSend = false; // this will cause us to drop out of the loop... } - // If the last node didn't fit, but we're in compressed mode, then we actually want to see if we can fit a - // little bit more in this packet. To do this we write into the packet, but don't send it yet, we'll - // keep attempting to write in compressed mode to add more compressed segments - - // We only consider sending anything if there is something in the _packetData to send... But - // if bytesWritten == 0 it means either the subTree couldn't fit or we had an empty bag... Both cases - // mean we should send the previous packet contents and reset it. if (completedScene || lastNodeDidntFit) { - + // we probably want to flush what has accumulated in nodeData but: + // do we have more data to send? and is there room? if (_packetData.hasContent()) { + // yes, more data to send quint64 compressAndWriteStart = usecTimestampNow(); - - // if for some reason the finalized size is greater than our available size, then probably the "compressed" - // form actually inflated beyond our padding, and in this case we will send the current packet, then - // write to out new packet... - unsigned int writtenSize = _packetData.getFinalizedSize() + sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); - - if (writtenSize > nodeData->getAvailable()) { + unsigned int additionalSize = _packetData.getFinalizedSize() + sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); + if (additionalSize > nodeData->getAvailable()) { + // no room --> flush what we've got packetsSentThisInterval += handlePacketSend(node, nodeData, trueBytesSent, truePacketsSent); } + // either there is room, or we've flushed and reset nodeData's data buffer + // so we can transfer whatever is in _packetData to nodeData nodeData->writeToPacket(_packetData.getFinalizedData(), _packetData.getFinalizedSize()); - quint64 compressAndWriteEnd = usecTimestampNow(); - compressAndWriteElapsedUsec = (float)(compressAndWriteEnd - compressAndWriteStart); + compressAndWriteElapsedUsec = (float)(usecTimestampNow()- compressAndWriteStart); } - // If we're not running compressed, then we know we can just send now. Or if we're running compressed, but - // the packet doesn't have enough space to bother attempting to pack more... - bool sendNow = true; - - if (!completedScene && (nodeData->getAvailable() >= MINIMUM_ATTEMPT_MORE_PACKING && - extraPackingAttempts <= REASONABLE_NUMBER_OF_PACKING_ATTEMPTS)) { - sendNow = false; // try to pack more - } + bool sendNow = completedScene || + nodeData->getAvailable() < MINIMUM_ATTEMPT_MORE_PACKING || + extraPackingAttempts > REASONABLE_NUMBER_OF_PACKING_ATTEMPTS; int targetSize = MAX_OCTREE_PACKET_DATA_SIZE; if (sendNow) { @@ -529,16 +512,13 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* targetSize = nodeData->getAvailable() - sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); extraPackingAttempts = 0; } else { - // If we're in compressed mode, then we want to see if we have room for more in this wire packet. - // but we've finalized the _packetData, so we want to start a new section, we will do that by - // resetting the packet settings with the max uncompressed size of our current available space - // in the wire packet. We also include room for our section header, and a little bit of padding - // to account for the fact that whenc compressing small amounts of data, we sometimes end up with - // a larger compressed size then uncompressed size + // We want to see if we have room for more in this wire packet but we've copied the _packetData, + // so we want to start a new section. We will do that by resetting the packet settings with the max + // size of our current available space in the wire packet plus room for our section header and a + // little bit of padding. targetSize = nodeData->getAvailable() - sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE) - COMPRESS_PADDING; } _packetData.changeSettings(true, targetSize); // will do reset - NOTE: Always compressed - } OctreeServer::trackTreeWaitTime(lockWaitElapsedUsec); OctreeServer::trackEncodeTime(encodeElapsedUsec); From 076ae28bed5d46df9d892bf831940b102845931c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 09:52:39 -0700 Subject: [PATCH 02/24] one less strand of spaghetti when counting packets --- .../src/octree/OctreeSendThread.cpp | 28 +++++++++++-------- .../src/octree/OctreeSendThread.h | 6 ++-- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 2a4e317670..028128e80b 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -130,7 +130,7 @@ AtomicUIntStat OctreeSendThread::_totalSpecialPackets { 0 }; int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, int& trueBytesSent, - int& truePacketsSent, bool dontSuppressDuplicate) { + bool dontSuppressDuplicate) { OctreeServer::didHandlePacketSend(this); // if we're shutting down, then exit early @@ -222,7 +222,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* } trueBytesSent += statsPacket.getDataSize(); - truePacketsSent++; packetsSent++; OctreeServer::didCallWriteDatagram(this); @@ -290,7 +289,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* if (packetSent) { nodeData->stats.packetSent(nodeData->getPacket().getPayloadSize()); trueBytesSent += nodeData->getPacket().getPayloadSize(); - truePacketsSent++; packetsSent++; nodeData->octreePacketSent(); nodeData->resetOctreePacket(); @@ -342,7 +340,9 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* // If we have a packet waiting, and our desired want color, doesn't match the current waiting packets color // then let's just send that waiting packet. if (nodeData->isPacketWaiting()) { - packetsSentThisInterval += handlePacketSend(node, nodeData, trueBytesSent, truePacketsSent); + int numPackets = handlePacketSend(node, nodeData, trueBytesSent); + truePacketsSent += numPackets; + packetsSentThisInterval += numPackets; } else { nodeData->resetOctreePacket(); } @@ -373,8 +373,9 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* //unsigned long encodeTime = nodeData->stats.getTotalEncodeTime(); //unsigned long elapsedTime = nodeData->stats.getElapsedTime(); - int packetsJustSent = handlePacketSend(node, nodeData, trueBytesSent, truePacketsSent, isFullScene); - packetsSentThisInterval += packetsJustSent; + int numPackets = handlePacketSend(node, nodeData, trueBytesSent, isFullScene); + truePacketsSent += numPackets; + packetsSentThisInterval += numPackets; // If we're starting a full scene, then definitely we want to empty the elementBag if (isFullScene) { @@ -489,7 +490,9 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* unsigned int additionalSize = _packetData.getFinalizedSize() + sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); if (additionalSize > nodeData->getAvailable()) { // no room --> flush what we've got - packetsSentThisInterval += handlePacketSend(node, nodeData, trueBytesSent, truePacketsSent); + int numPackets = handlePacketSend(node, nodeData, trueBytesSent); + truePacketsSent += numPackets; + packetsSentThisInterval += numPackets; } // either there is room, or we've flushed and reset nodeData's data buffer @@ -505,7 +508,9 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* int targetSize = MAX_OCTREE_PACKET_DATA_SIZE; if (sendNow) { quint64 packetSendingStart = usecTimestampNow(); - packetsSentThisInterval += handlePacketSend(node, nodeData, trueBytesSent, truePacketsSent); + int numPackets = handlePacketSend(node, nodeData, trueBytesSent); + truePacketsSent += numPackets; + packetsSentThisInterval += numPackets; quint64 packetSendingEnd = usecTimestampNow(); packetSendingElapsedUsec = (float)(packetSendingEnd - packetSendingStart); @@ -587,12 +592,11 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* // the clients will know the scene is stable if (isFullScene) { int thisTrueBytesSent = 0; - int thisTruePacketsSent = 0; nodeData->stats.sceneCompleted(); - int packetsJustSent = handlePacketSend(node, nodeData, thisTrueBytesSent, thisTruePacketsSent, true); + int numPackets = handlePacketSend(node, nodeData, thisTrueBytesSent, true); _totalBytes += thisTrueBytesSent; - _totalPackets += thisTruePacketsSent; - truePacketsSent += packetsJustSent; + _totalPackets += numPackets; + truePacketsSent += numPackets; } } diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 06c9b5f1d6..f1d153b6e3 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -34,7 +34,7 @@ public: void setIsShuttingDown(); bool isShuttingDown() { return _isShuttingDown; } - + QUuid getNodeUuid() const { return _nodeUuid; } static AtomicUIntStat _totalBytes; @@ -58,9 +58,9 @@ protected: QWeakPointer _node; private: - int handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, int& trueBytesSent, int& truePacketsSent, bool dontSuppressDuplicate = false); + int handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, int& trueBytesSent, bool dontSuppressDuplicate = false); int packetDistributor(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged); - + QUuid _nodeUuid; From ca3f0ceecbe5d57481000fc23a6ea14beaf0b619 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 10:02:18 -0700 Subject: [PATCH 03/24] remove crufty comments --- assignment-client/src/octree/OctreeSendThread.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 028128e80b..7c6478208e 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -332,13 +332,7 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* && ((!viewFrustumChanged && nodeData->getViewFrustumJustStoppedChanging()) || nodeData->hasLodChanged())); } - // If our packet already has content in it, then we must use the color choice of the waiting packet. - // If we're starting a fresh packet, then... - // If we're moving, and the client asked for low res, then we force monochrome, otherwise, use - // the clients requested color state. - - // If we have a packet waiting, and our desired want color, doesn't match the current waiting packets color - // then let's just send that waiting packet. + // send any waiting packet if (nodeData->isPacketWaiting()) { int numPackets = handlePacketSend(node, nodeData, trueBytesSent); truePacketsSent += numPackets; From a5cd11cea7d65de0a492aa53fcc4935f03861442 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 10:11:24 -0700 Subject: [PATCH 04/24] remove more crufty comments --- assignment-client/src/octree/OctreeSendThread.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 7c6478208e..e6449f9f72 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -399,10 +399,6 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* if (!nodeData->elementBag.isEmpty()) { quint64 start = usecTimestampNow(); - // TODO: add these to stats page - //quint64 startCompressTimeMsecs = OctreePacketData::getCompressContentTime() / 1000; - //quint64 startCompressCalls = OctreePacketData::getCompressContentCalls(); - int extraPackingAttempts = 0; bool completedScene = false; @@ -570,12 +566,6 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* int elapsedmsec = (end - start) / USECS_PER_MSEC; OctreeServer::trackLoopTime(elapsedmsec); - // TODO: add these to stats page - //quint64 endCompressCalls = OctreePacketData::getCompressContentCalls(); - //int elapsedCompressCalls = endCompressCalls - startCompressCalls; - //quint64 endCompressTimeMsecs = OctreePacketData::getCompressContentTime() / 1000; - //int elapsedCompressTimeMsecs = endCompressTimeMsecs - startCompressTimeMsecs; - // if after sending packets we've emptied our bag, then we want to remember that we've sent all // the octree elements from the current view frustum if (nodeData->elementBag.isEmpty()) { From 9d111d1f92126001265bb0af305030f4b217a968 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 13:42:35 -0700 Subject: [PATCH 05/24] remove unused OctreeSendThread::_nodeMissingCount --- assignment-client/src/octree/OctreeSendThread.cpp | 1 - assignment-client/src/octree/OctreeSendThread.h | 1 - 2 files changed, 2 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index e6449f9f72..f35fe9ef70 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -81,7 +81,6 @@ bool OctreeSendThread::process() { // don't do any send processing until the initial load of the octree is complete... if (_myServer->isInitialLoadComplete()) { if (auto node = _node.lock()) { - _nodeMissingCount = 0; OctreeQueryNode* nodeData = static_cast(node->getLinkedData()); // Sometimes the node data has not yet been linked, in which case we can't really do anything diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index f1d153b6e3..df1d5f3811 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -66,7 +66,6 @@ private: OctreePacketData _packetData; - int _nodeMissingCount { 0 }; bool _isShuttingDown { false }; }; From 15879b28324dcde7c0b28c89855b01093b42a6e3 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 13:53:08 -0700 Subject: [PATCH 06/24] + OctreeSendThread::traverseTreeAndSendContents() this abstracts a portion of OctreeSendThread::packetDistributor() which will make it easier to split apart the tree traversal from the sending of packets also cleaned up some of the packet stats tracking --- .../src/octree/OctreeSendThread.cpp | 370 +++++++++--------- .../src/octree/OctreeSendThread.h | 6 +- 2 files changed, 192 insertions(+), 184 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index f35fe9ef70..edf7b6da0b 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -128,8 +128,7 @@ AtomicUIntStat OctreeSendThread::_totalSpecialBytes { 0 }; AtomicUIntStat OctreeSendThread::_totalSpecialPackets { 0 }; -int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, int& trueBytesSent, - bool dontSuppressDuplicate) { +int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, bool dontSuppressDuplicate) { OctreeServer::didHandlePacketSend(this); // if we're shutting down, then exit early @@ -140,15 +139,14 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* bool debug = _myServer->wantsDebugSending(); quint64 now = usecTimestampNow(); - bool packetSent = false; // did we send a packet? - int packetsSent = 0; + int numPackets = 0; // Here's where we check to see if this packet is a duplicate of the last packet. If it is, we will silently // obscure the packet and not send it. This allows the callers and upper level logic to not need to know about // this rate control savings. if (!dontSuppressDuplicate && nodeData->shouldSuppressDuplicatePacket()) { nodeData->resetOctreePacket(); // we still need to reset it though! - return packetsSent; // without sending... + return numPackets; // without sending... } // If we've got a stats message ready to send, then see if we can piggyback them together @@ -165,9 +163,12 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // since a stats message is only included on end of scene, don't consider any of these bytes "wasted", since // there was nothing else to send. int thisWastedBytes = 0; + int numBytes = statsPacket.getDataSize(); _totalWastedBytes += thisWastedBytes; - _totalBytes += statsPacket.getDataSize(); + _totalBytes += numBytes; _totalPackets++; + _trueBytesSent += numBytes; + numPackets++; if (debug) { NLPacket& sentPacket = nodeData->getPacket(); @@ -190,18 +191,22 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // actually send it OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(statsPacket, *node); - packetSent = true; } else { // not enough room in the packet, send two packets + + // first packet OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(statsPacket, *node); // since a stats message is only included on end of scene, don't consider any of these bytes "wasted", since // there was nothing else to send. int thisWastedBytes = 0; + int numBytes = statsPacket.getDataSize(); _totalWastedBytes += thisWastedBytes; - _totalBytes += statsPacket.getDataSize(); + _totalBytes += numBytes; _totalPackets++; + _trueBytesSent += numBytes; + numPackets++; if (debug) { NLPacket& sentPacket = nodeData->getPacket(); @@ -220,18 +225,17 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* "] wasted bytes:" << thisWastedBytes << " [" << _totalWastedBytes << "]"; } - trueBytesSent += statsPacket.getDataSize(); - packetsSent++; - + // second packet OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(nodeData->getPacket(), *node); - packetSent = true; - int packetSizeWithHeader = nodeData->getPacket().getDataSize(); - thisWastedBytes = udt::MAX_PACKET_SIZE - packetSizeWithHeader; + numBytes = nodeData->getPacket().getDataSize(); + thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; _totalWastedBytes += thisWastedBytes; - _totalBytes += nodeData->getPacket().getDataSize(); + _totalBytes += numBytes; _totalPackets++; + _trueBytesSent += numBytes; + numPackets++; if (debug) { NLPacket& sentPacket = nodeData->getPacket(); @@ -257,13 +261,13 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // just send the octree packet OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(nodeData->getPacket(), *node); - packetSent = true; - int packetSizeWithHeader = nodeData->getPacket().getDataSize(); - int thisWastedBytes = udt::MAX_PACKET_SIZE - packetSizeWithHeader; + int numBytes = nodeData->getPacket().getDataSize(); + int thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; _totalWastedBytes += thisWastedBytes; - _totalBytes += packetSizeWithHeader; + _totalBytes += numBytes; _totalPackets++; + numPackets++; if (debug) { NLPacket& sentPacket = nodeData->getPacket(); @@ -278,22 +282,22 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* qDebug() << "Sending packet at " << now << " [" << _totalPackets <<"]: sequence: " << sequence << " timestamp: " << timestamp << - " size: " << packetSizeWithHeader << " [" << _totalBytes << + " size: " << numBytes << " [" << _totalBytes << "] wasted bytes:" << thisWastedBytes << " [" << _totalWastedBytes << "]"; } + _trueBytesSent += numBytes; } } // remember to track our stats - if (packetSent) { + if (numPackets > 0) { nodeData->stats.packetSent(nodeData->getPacket().getPayloadSize()); - trueBytesSent += nodeData->getPacket().getPayloadSize(); - packetsSent++; nodeData->octreePacketSent(); nodeData->resetOctreePacket(); } - return packetsSent; + _truePacketsSent += numPackets; + return numPackets; } /// Version of octree element distributor that sends the deepest LOD level at once @@ -312,13 +316,9 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* preDistributionProcessing(); } - // calculate max number of packets that can be sent during this interval - int clientMaxPacketsPerInterval = std::max(1, (nodeData->getMaxQueryPacketsPerSecond() / INTERVALS_PER_SECOND)); - int maxPacketsPerInterval = std::min(clientMaxPacketsPerInterval, _myServer->getPacketsPerClientPerInterval()); - - int truePacketsSent = 0; - int trueBytesSent = 0; - int packetsSentThisInterval = 0; + _truePacketsSent = 0; + _trueBytesSent = 0; + _packetsSentThisInterval = 0; bool isFullScene = nodeData->shouldForceFullScene(); if (isFullScene) { @@ -333,9 +333,7 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* // send any waiting packet if (nodeData->isPacketWaiting()) { - int numPackets = handlePacketSend(node, nodeData, trueBytesSent); - truePacketsSent += numPackets; - packetsSentThisInterval += numPackets; + _packetsSentThisInterval += handlePacketSend(node, nodeData); } else { nodeData->resetOctreePacket(); } @@ -366,9 +364,7 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* //unsigned long encodeTime = nodeData->stats.getTotalEncodeTime(); //unsigned long elapsedTime = nodeData->stats.getElapsedTime(); - int numPackets = handlePacketSend(node, nodeData, trueBytesSent, isFullScene); - truePacketsSent += numPackets; - packetsSentThisInterval += numPackets; + _packetsSentThisInterval += handlePacketSend(node, nodeData, isFullScene); // If we're starting a full scene, then definitely we want to empty the elementBag if (isFullScene) { @@ -398,165 +394,42 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* if (!nodeData->elementBag.isEmpty()) { quint64 start = usecTimestampNow(); - int extraPackingAttempts = 0; - bool completedScene = false; - - bool somethingToSend = true; // assume we have something - while (somethingToSend && packetsSentThisInterval < maxPacketsPerInterval && !nodeData->isShuttingDown()) { - float lockWaitElapsedUsec = OctreeServer::SKIP_TIME; - float encodeElapsedUsec = OctreeServer::SKIP_TIME; - float compressAndWriteElapsedUsec = OctreeServer::SKIP_TIME; - float packetSendingElapsedUsec = OctreeServer::SKIP_TIME; - - quint64 startInside = usecTimestampNow(); - - bool lastNodeDidntFit = false; // assume each node fits - if (!nodeData->elementBag.isEmpty()) { - - quint64 lockWaitStart = usecTimestampNow(); - _myServer->getOctree()->withReadLock([&]{ - quint64 lockWaitEnd = usecTimestampNow(); - lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart); - quint64 encodeStart = usecTimestampNow(); - - OctreeElementPointer subTree = nodeData->elementBag.extract(); - if (!subTree) { - return; - } - - float octreeSizeScale = nodeData->getOctreeSizeScale(); - int boundaryLevelAdjustClient = nodeData->getBoundaryLevelAdjust(); - - int boundaryLevelAdjust = boundaryLevelAdjustClient + - (viewFrustumChanged ? LOW_RES_MOVING_ADJUST : NO_BOUNDARY_ADJUST); - - EncodeBitstreamParams params(INT_MAX, WANT_EXISTS_BITS, DONT_CHOP, - viewFrustumChanged, boundaryLevelAdjust, octreeSizeScale, - isFullScene, _myServer->getJurisdiction(), nodeData); - nodeData->copyCurrentViewFrustum(params.viewFrustum); - if (viewFrustumChanged) { - nodeData->copyLastKnownViewFrustum(params.lastViewFrustum); - } - - // Our trackSend() function is implemented by the server subclass, and will be called back - // during the encodeTreeBitstream() as new entities/data elements are sent - params.trackSend = [this, node](const QUuid& dataID, quint64 dataEdited) { - _myServer->trackSend(dataID, dataEdited, node->getUUID()); - }; - - // TODO: should this include the lock time or not? This stat is sent down to the client, - // it seems like it may be a good idea to include the lock time as part of the encode time - // are reported to client. Since you can encode without the lock - nodeData->stats.encodeStarted(); - - _myServer->getOctree()->encodeTreeBitstream(subTree, &_packetData, nodeData->elementBag, params); - - quint64 encodeEnd = usecTimestampNow(); - encodeElapsedUsec = (float)(encodeEnd - encodeStart); - - // If after calling encodeTreeBitstream() there are no nodes left to send, then we know we've - // sent the entire scene. We want to know this below so we'll actually write this content into - // the packet and send it - completedScene = nodeData->elementBag.isEmpty(); - - if (params.stopReason == EncodeBitstreamParams::DIDNT_FIT) { - lastNodeDidntFit = true; - extraPackingAttempts++; - } - - nodeData->stats.encodeStopped(); - }); - } else { - somethingToSend = false; // this will cause us to drop out of the loop... - } - - if (completedScene || lastNodeDidntFit) { - // we probably want to flush what has accumulated in nodeData but: - // do we have more data to send? and is there room? - if (_packetData.hasContent()) { - // yes, more data to send - quint64 compressAndWriteStart = usecTimestampNow(); - unsigned int additionalSize = _packetData.getFinalizedSize() + sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); - if (additionalSize > nodeData->getAvailable()) { - // no room --> flush what we've got - int numPackets = handlePacketSend(node, nodeData, trueBytesSent); - truePacketsSent += numPackets; - packetsSentThisInterval += numPackets; - } - - // either there is room, or we've flushed and reset nodeData's data buffer - // so we can transfer whatever is in _packetData to nodeData - nodeData->writeToPacket(_packetData.getFinalizedData(), _packetData.getFinalizedSize()); - compressAndWriteElapsedUsec = (float)(usecTimestampNow()- compressAndWriteStart); - } - - bool sendNow = completedScene || - nodeData->getAvailable() < MINIMUM_ATTEMPT_MORE_PACKING || - extraPackingAttempts > REASONABLE_NUMBER_OF_PACKING_ATTEMPTS; - - int targetSize = MAX_OCTREE_PACKET_DATA_SIZE; - if (sendNow) { - quint64 packetSendingStart = usecTimestampNow(); - int numPackets = handlePacketSend(node, nodeData, trueBytesSent); - truePacketsSent += numPackets; - packetsSentThisInterval += numPackets; - quint64 packetSendingEnd = usecTimestampNow(); - packetSendingElapsedUsec = (float)(packetSendingEnd - packetSendingStart); - - targetSize = nodeData->getAvailable() - sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); - extraPackingAttempts = 0; - } else { - // We want to see if we have room for more in this wire packet but we've copied the _packetData, - // so we want to start a new section. We will do that by resetting the packet settings with the max - // size of our current available space in the wire packet plus room for our section header and a - // little bit of padding. - targetSize = nodeData->getAvailable() - sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE) - COMPRESS_PADDING; - } - _packetData.changeSettings(true, targetSize); // will do reset - NOTE: Always compressed - } - OctreeServer::trackTreeWaitTime(lockWaitElapsedUsec); - OctreeServer::trackEncodeTime(encodeElapsedUsec); - OctreeServer::trackCompressAndWriteTime(compressAndWriteElapsedUsec); - OctreeServer::trackPacketSendingTime(packetSendingElapsedUsec); - - quint64 endInside = usecTimestampNow(); - quint64 elapsedInsideUsecs = endInside - startInside; - OctreeServer::trackInsideTime((float)elapsedInsideUsecs); - } - - if (somethingToSend && _myServer->wantsVerboseDebug()) { - qCDebug(octree) << "Hit PPS Limit, packetsSentThisInterval =" << packetsSentThisInterval - << " maxPacketsPerInterval = " << maxPacketsPerInterval - << " clientMaxPacketsPerInterval = " << clientMaxPacketsPerInterval; - } + traverseTreeAndSendContents(node, nodeData, viewFrustumChanged, isFullScene); // Here's where we can/should allow the server to send other data... // send the environment packet // TODO: should we turn this into a while loop to better handle sending multiple special packets if (_myServer->hasSpecialPacketsToSend(node) && !nodeData->isShuttingDown()) { int specialPacketsSent = 0; - trueBytesSent += _myServer->sendSpecialPackets(node, nodeData, specialPacketsSent); + int specialBytesSent = _myServer->sendSpecialPackets(node, nodeData, specialPacketsSent); nodeData->resetOctreePacket(); // because nodeData's _sequenceNumber has changed - truePacketsSent += specialPacketsSent; - packetsSentThisInterval += specialPacketsSent; + _truePacketsSent += specialPacketsSent; + _trueBytesSent += specialBytesSent; + _packetsSentThisInterval += specialPacketsSent; _totalPackets += specialPacketsSent; - _totalBytes += trueBytesSent; + _totalBytes += specialBytesSent; _totalSpecialPackets += specialPacketsSent; - _totalSpecialBytes += trueBytesSent; + _totalSpecialBytes += specialBytesSent; } + // calculate max number of packets that can be sent during this interval + int clientMaxPacketsPerInterval = std::max(1, (nodeData->getMaxQueryPacketsPerSecond() / INTERVALS_PER_SECOND)); + int maxPacketsPerInterval = std::min(clientMaxPacketsPerInterval, _myServer->getPacketsPerClientPerInterval()); + // Re-send packets that were nacked by the client - while (nodeData->hasNextNackedPacket() && packetsSentThisInterval < maxPacketsPerInterval) { + while (nodeData->hasNextNackedPacket() && _packetsSentThisInterval < maxPacketsPerInterval) { const NLPacket* packet = nodeData->getNextNackedPacket(); if (packet) { DependencyManager::get()->sendUnreliablePacket(*packet, *node); - truePacketsSent++; - packetsSentThisInterval++; + int numBytes = packet->getDataSize(); + _truePacketsSent++; + _trueBytesSent += numBytes; + _packetsSentThisInterval++; - _totalBytes += packet->getDataSize(); _totalPackets++; + _totalBytes += numBytes; _totalWastedBytes += udt::MAX_PACKET_SIZE - packet->getDataSize(); } } @@ -574,16 +447,147 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* // If this was a full scene then make sure we really send out a stats packet at this point so that // the clients will know the scene is stable if (isFullScene) { - int thisTrueBytesSent = 0; nodeData->stats.sceneCompleted(); - int numPackets = handlePacketSend(node, nodeData, thisTrueBytesSent, true); - _totalBytes += thisTrueBytesSent; - _totalPackets += numPackets; - truePacketsSent += numPackets; + handlePacketSend(node, nodeData, true); } } } // end if bag wasn't empty, and so we sent stuff... - return truePacketsSent; + return _truePacketsSent; +} + +void OctreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene) { + // calculate max number of packets that can be sent during this interval + int clientMaxPacketsPerInterval = std::max(1, (nodeData->getMaxQueryPacketsPerSecond() / INTERVALS_PER_SECOND)); + int maxPacketsPerInterval = std::min(clientMaxPacketsPerInterval, _myServer->getPacketsPerClientPerInterval()); + + int extraPackingAttempts = 0; + bool completedScene = false; + + bool somethingToSend = true; // assume we have something + while (somethingToSend && _packetsSentThisInterval < maxPacketsPerInterval && !nodeData->isShuttingDown()) { + float lockWaitElapsedUsec = OctreeServer::SKIP_TIME; + float encodeElapsedUsec = OctreeServer::SKIP_TIME; + float compressAndWriteElapsedUsec = OctreeServer::SKIP_TIME; + float packetSendingElapsedUsec = OctreeServer::SKIP_TIME; + + quint64 startInside = usecTimestampNow(); + + bool lastNodeDidntFit = false; // assume each node fits + if (!nodeData->elementBag.isEmpty()) { + + quint64 lockWaitStart = usecTimestampNow(); + _myServer->getOctree()->withReadLock([&]{ + quint64 lockWaitEnd = usecTimestampNow(); + lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart); + quint64 encodeStart = usecTimestampNow(); + + OctreeElementPointer subTree = nodeData->elementBag.extract(); + if (!subTree) { + return; + } + + float octreeSizeScale = nodeData->getOctreeSizeScale(); + int boundaryLevelAdjustClient = nodeData->getBoundaryLevelAdjust(); + + int boundaryLevelAdjust = boundaryLevelAdjustClient + + (viewFrustumChanged ? LOW_RES_MOVING_ADJUST : NO_BOUNDARY_ADJUST); + + EncodeBitstreamParams params(INT_MAX, WANT_EXISTS_BITS, DONT_CHOP, + viewFrustumChanged, boundaryLevelAdjust, octreeSizeScale, + isFullScene, _myServer->getJurisdiction(), nodeData); + nodeData->copyCurrentViewFrustum(params.viewFrustum); + if (viewFrustumChanged) { + nodeData->copyLastKnownViewFrustum(params.lastViewFrustum); + } + + // Our trackSend() function is implemented by the server subclass, and will be called back + // during the encodeTreeBitstream() as new entities/data elements are sent + params.trackSend = [this, node](const QUuid& dataID, quint64 dataEdited) { + _myServer->trackSend(dataID, dataEdited, node->getUUID()); + }; + + // TODO: should this include the lock time or not? This stat is sent down to the client, + // it seems like it may be a good idea to include the lock time as part of the encode time + // are reported to client. Since you can encode without the lock + nodeData->stats.encodeStarted(); + + // NOTE: this is where the tree "contents" are actaully packed + _myServer->getOctree()->encodeTreeBitstream(subTree, &_packetData, nodeData->elementBag, params); + + quint64 encodeEnd = usecTimestampNow(); + encodeElapsedUsec = (float)(encodeEnd - encodeStart); + + // If after calling encodeTreeBitstream() there are no nodes left to send, then we know we've + // sent the entire scene. We want to know this below so we'll actually write this content into + // the packet and send it + completedScene = nodeData->elementBag.isEmpty(); + + if (params.stopReason == EncodeBitstreamParams::DIDNT_FIT) { + lastNodeDidntFit = true; + extraPackingAttempts++; + } + + nodeData->stats.encodeStopped(); + }); + } else { + somethingToSend = false; // this will cause us to drop out of the loop... + } + + if (completedScene || lastNodeDidntFit) { + // we probably want to flush what has accumulated in nodeData but: + // do we have more data to send? and is there room? + if (_packetData.hasContent()) { + // yes, more data to send + quint64 compressAndWriteStart = usecTimestampNow(); + unsigned int additionalSize = _packetData.getFinalizedSize() + sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); + if (additionalSize > nodeData->getAvailable()) { + // no room --> flush what we've got + _packetsSentThisInterval += handlePacketSend(node, nodeData); + } + + // either there is room, or we've flushed and reset nodeData's data buffer + // so we can transfer whatever is in _packetData to nodeData + nodeData->writeToPacket(_packetData.getFinalizedData(), _packetData.getFinalizedSize()); + compressAndWriteElapsedUsec = (float)(usecTimestampNow()- compressAndWriteStart); + } + + bool sendNow = completedScene || + nodeData->getAvailable() < MINIMUM_ATTEMPT_MORE_PACKING || + extraPackingAttempts > REASONABLE_NUMBER_OF_PACKING_ATTEMPTS; + + int targetSize = MAX_OCTREE_PACKET_DATA_SIZE; + if (sendNow) { + quint64 packetSendingStart = usecTimestampNow(); + _packetsSentThisInterval += handlePacketSend(node, nodeData); + quint64 packetSendingEnd = usecTimestampNow(); + packetSendingElapsedUsec = (float)(packetSendingEnd - packetSendingStart); + + targetSize = nodeData->getAvailable() - sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE); + extraPackingAttempts = 0; + } else { + // We want to see if we have room for more in this wire packet but we've copied the _packetData, + // so we want to start a new section. We will do that by resetting the packet settings with the max + // size of our current available space in the wire packet plus room for our section header and a + // little bit of padding. + targetSize = nodeData->getAvailable() - sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE) - COMPRESS_PADDING; + } + _packetData.changeSettings(true, targetSize); // will do reset - NOTE: Always compressed + } + OctreeServer::trackTreeWaitTime(lockWaitElapsedUsec); + OctreeServer::trackEncodeTime(encodeElapsedUsec); + OctreeServer::trackCompressAndWriteTime(compressAndWriteElapsedUsec); + OctreeServer::trackPacketSendingTime(packetSendingElapsedUsec); + + quint64 endInside = usecTimestampNow(); + quint64 elapsedInsideUsecs = endInside - startInside; + OctreeServer::trackInsideTime((float)elapsedInsideUsecs); + } + + if (somethingToSend && _myServer->wantsVerboseDebug()) { + qCDebug(octree) << "Hit PPS Limit, packetsSentThisInterval =" << _packetsSentThisInterval + << " maxPacketsPerInterval = " << maxPacketsPerInterval + << " clientMaxPacketsPerInterval = " << clientMaxPacketsPerInterval; + } } diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index df1d5f3811..d158539f57 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -53,12 +53,13 @@ protected: /// Called before a packetDistributor pass to allow for pre-distribution processing virtual void preDistributionProcessing() {}; + virtual void traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene); OctreeServer* _myServer { nullptr }; QWeakPointer _node; private: - int handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, int& trueBytesSent, bool dontSuppressDuplicate = false); + int handlePacketSend(SharedNodePointer node, OctreeQueryNode* nodeData, bool dontSuppressDuplicate = false); int packetDistributor(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged); @@ -66,6 +67,9 @@ private: OctreePacketData _packetData; + int _truePacketsSent { 0 }; // available for debug stats + int _trueBytesSent { 0 }; // available for debug stats + int _packetsSentThisInterval { 0 }; // used for bandwidth throttle condition bool _isShuttingDown { false }; }; From 3226d33830d6ffaff5d728ed91c0b746f8116f1e Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 14:02:16 -0700 Subject: [PATCH 07/24] use cached OctreeSendThread::_nodeUuid --- assignment-client/src/octree/OctreeSendThread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index edf7b6da0b..2766d5e2e6 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -504,8 +504,8 @@ void OctreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, Octre // Our trackSend() function is implemented by the server subclass, and will be called back // during the encodeTreeBitstream() as new entities/data elements are sent - params.trackSend = [this, node](const QUuid& dataID, quint64 dataEdited) { - _myServer->trackSend(dataID, dataEdited, node->getUUID()); + params.trackSend = [this](const QUuid& dataID, quint64 dataEdited) { + _myServer->trackSend(dataID, dataEdited, _nodeUuid); }; // TODO: should this include the lock time or not? This stat is sent down to the client, From badbe980970311cfdfa5e4b39ea42d43f5918696 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 14:21:49 -0700 Subject: [PATCH 08/24] minor cleanup and constency for packet stats --- .../src/octree/OctreeSendThread.cpp | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 2766d5e2e6..46439b74df 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -160,13 +160,13 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // copy octree message to back of stats message statsPacket.write(nodeData->getPacket().getData(), nodeData->getPacket().getDataSize()); - // since a stats message is only included on end of scene, don't consider any of these bytes "wasted", since - // there was nothing else to send. - int thisWastedBytes = 0; int numBytes = statsPacket.getDataSize(); - _totalWastedBytes += thisWastedBytes; _totalBytes += numBytes; _totalPackets++; + // since a stats message is only included on end of scene, don't consider any of these bytes "wasted" + // there was nothing else to send. + int thisWastedBytes = 0; + //_totalWastedBytes += 0; _trueBytesSent += numBytes; numPackets++; @@ -198,13 +198,13 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(statsPacket, *node); - // since a stats message is only included on end of scene, don't consider any of these bytes "wasted", since - // there was nothing else to send. - int thisWastedBytes = 0; int numBytes = statsPacket.getDataSize(); - _totalWastedBytes += thisWastedBytes; _totalBytes += numBytes; _totalPackets++; + // since a stats message is only included on end of scene, don't consider any of these bytes "wasted" + // there was nothing else to send. + int thisWastedBytes = 0; + //_totalWastedBytes += 0; _trueBytesSent += numBytes; numPackets++; @@ -230,10 +230,11 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* DependencyManager::get()->sendUnreliablePacket(nodeData->getPacket(), *node); numBytes = nodeData->getPacket().getDataSize(); - thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; - _totalWastedBytes += thisWastedBytes; _totalBytes += numBytes; _totalPackets++; + // since a stats message is only included on end of scene, don't consider any of these bytes "wasted" + // there was nothing else to send. + //_totalWastedBytes += 0; _trueBytesSent += numBytes; numPackets++; @@ -263,11 +264,12 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* DependencyManager::get()->sendUnreliablePacket(nodeData->getPacket(), *node); int numBytes = nodeData->getPacket().getDataSize(); - int thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; - _totalWastedBytes += thisWastedBytes; _totalBytes += numBytes; _totalPackets++; + int thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; + _totalWastedBytes += thisWastedBytes; numPackets++; + _trueBytesSent += numBytes; if (debug) { NLPacket& sentPacket = nodeData->getPacket(); @@ -285,7 +287,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* " size: " << numBytes << " [" << _totalBytes << "] wasted bytes:" << thisWastedBytes << " [" << _totalWastedBytes << "]"; } - _trueBytesSent += numBytes; } } @@ -331,8 +332,8 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* && ((!viewFrustumChanged && nodeData->getViewFrustumJustStoppedChanging()) || nodeData->hasLodChanged())); } - // send any waiting packet if (nodeData->isPacketWaiting()) { + // send the waiting packet _packetsSentThisInterval += handlePacketSend(node, nodeData); } else { nodeData->resetOctreePacket(); From 3670a04d8e73343a090cd9bcd22e2b8fa57c7f87 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 14:24:58 -0700 Subject: [PATCH 09/24] switch bare pointers to AudioInjector objects to be QSharedPointers --- interface/src/Application.h | 2 +- interface/src/avatar/AvatarManager.cpp | 3 +- interface/src/avatar/AvatarManager.h | 4 +- libraries/audio-client/src/AudioClient.cpp | 64 +++++++++---------- libraries/audio-client/src/AudioClient.h | 4 +- libraries/audio/src/AbstractAudioInterface.h | 3 +- libraries/audio/src/AudioInjector.cpp | 34 +++++++--- libraries/audio/src/AudioInjector.h | 46 +++++++------ libraries/audio/src/AudioInjectorManager.cpp | 58 ++++++++--------- libraries/audio/src/AudioInjectorManager.h | 25 ++++---- .../script-engine/src/ScriptAudioInjector.cpp | 6 +- .../script-engine/src/ScriptAudioInjector.h | 16 ++--- libraries/ui/src/ui/types/SoundEffect.h | 3 +- 13 files changed, 148 insertions(+), 120 deletions(-) diff --git a/interface/src/Application.h b/interface/src/Application.h index cf0ae91a0f..ce27c4a70a 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -678,7 +678,7 @@ private: QTimer _addAssetToWorldErrorTimer; FileScriptingInterface* _fileDownload; - AudioInjector* _snapshotSoundInjector { nullptr }; + AudioInjectorPointer _snapshotSoundInjector; SharedSoundPointer _snapshotSound; DisplayPluginPointer _autoSwitchDisplayModeSupportedHMDPlugin; diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index c46d61cf68..b63816eb4f 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -434,8 +434,7 @@ void AvatarManager::handleCollisionEvents(const CollisionEvents& collisionEvents // but most avatars are roughly the same size, so let's not be so fancy yet. const float AVATAR_STRETCH_FACTOR = 1.0f; - - _collisionInjectors.remove_if([](QPointer& injector) { + _collisionInjectors.remove_if([](AudioInjectorPointer injector) { return !injector || injector->isFinished(); }); diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index f1e71f7367..d6f140ea74 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -25,8 +25,8 @@ #include "AvatarMotionState.h" #include "MyAvatar.h" +#include "AudioInjector.h" -class AudioInjector; class AvatarManager : public AvatarHashMap { Q_OBJECT @@ -104,7 +104,7 @@ private: std::shared_ptr _myAvatar; quint64 _lastSendAvatarDataTime = 0; // Controls MyAvatar send data rate. - std::list> _collisionInjectors; + std::list _collisionInjectors; RateCounter<> _myAvatarSendRate; int _numAvatarsUpdated { 0 }; diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index c630fe09e4..d532b18836 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -210,9 +210,9 @@ AudioClient::AudioClient() : connect(&_receivedAudioStream, &MixedProcessedAudioStream::processSamples, this, &AudioClient::processReceivedSamples, Qt::DirectConnection); - connect(this, &AudioClient::changeDevice, this, [=](const QAudioDeviceInfo& outputDeviceInfo) { + connect(this, &AudioClient::changeDevice, this, [=](const QAudioDeviceInfo& outputDeviceInfo) { qCDebug(audioclient) << "got AudioClient::changeDevice signal, about to call switchOutputToAudioDevice() outputDeviceInfo: [" << outputDeviceInfo.deviceName() << "]"; - switchOutputToAudioDevice(outputDeviceInfo); + switchOutputToAudioDevice(outputDeviceInfo); }); connect(&_receivedAudioStream, &InboundAudioStream::mismatchedAudioCodec, this, &AudioClient::handleMismatchAudioFormat); @@ -261,10 +261,10 @@ void AudioClient::cleanupBeforeQuit() { // so this must be explicitly, synchronously stopped static ConditionalGuard guard; if (QThread::currentThread() != thread()) { - // This will likely be called from the main thread, but we don't want to do blocking queued calls - // from the main thread, so we use a normal auto-connection invoke, and then use a conditional to wait + // This will likely be called from the main thread, but we don't want to do blocking queued calls + // from the main thread, so we use a normal auto-connection invoke, and then use a conditional to wait // for completion - // The effect is the same, yes, but we actually want to avoid the use of Qt::BlockingQueuedConnection + // The effect is the same, yes, but we actually want to avoid the use of Qt::BlockingQueuedConnection // in the code QMetaObject::invokeMethod(this, "cleanupBeforeQuit"); guard.wait(); @@ -630,7 +630,7 @@ void AudioClient::handleAudioEnvironmentDataPacket(QSharedPointerreadPrimitive(&bitset); bool hasReverb = oneAtBit(bitset, HAS_REVERB_BIT); - + if (hasReverb) { float reverbTime, wetLevel; message->readPrimitive(&reverbTime); @@ -728,7 +728,7 @@ void AudioClient::Gate::flush() { void AudioClient::handleNoisyMutePacket(QSharedPointer message) { if (!_muted) { toggleMute(); - + // have the audio scripting interface emit a signal to say we were muted by the mixer emit mutedByMixer(); } @@ -737,7 +737,7 @@ void AudioClient::handleNoisyMutePacket(QSharedPointer message) void AudioClient::handleMuteEnvironmentPacket(QSharedPointer message) { glm::vec3 position; float radius; - + message->readPrimitive(&position); message->readPrimitive(&radius); @@ -770,7 +770,7 @@ void AudioClient::handleSelectedAudioFormat(QSharedPointer mess } void AudioClient::selectAudioFormat(const QString& selectedCodecName) { - + _selectedCodecName = selectedCodecName; qCDebug(audioclient) << "Selected Codec:" << _selectedCodecName; @@ -787,7 +787,7 @@ void AudioClient::selectAudioFormat(const QString& selectedCodecName) { for (auto& plugin : codecPlugins) { if (_selectedCodecName == plugin->getName()) { _codec = plugin; - _receivedAudioStream.setupCodec(plugin, _selectedCodecName, AudioConstants::STEREO); + _receivedAudioStream.setupCodec(plugin, _selectedCodecName, AudioConstants::STEREO); _encoder = plugin->createEncoder(AudioConstants::SAMPLE_RATE, AudioConstants::MONO); qCDebug(audioclient) << "Selected Codec Plugin:" << _codec.get(); break; @@ -795,7 +795,7 @@ void AudioClient::selectAudioFormat(const QString& selectedCodecName) { } } - + bool AudioClient::switchAudioDevice(QAudio::Mode mode, const QAudioDeviceInfo& deviceInfo) { auto device = deviceInfo; @@ -1203,11 +1203,11 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { // lock the injectors Lock lock(_injectorsMutex); - QVector injectorsToRemove; + QVector injectorsToRemove; memset(mixBuffer, 0, AudioConstants::NETWORK_FRAME_SAMPLES_STEREO * sizeof(float)); - for (AudioInjector* injector : _activeLocalAudioInjectors) { + for (AudioInjectorPointer injector : _activeLocalAudioInjectors) { // the lock guarantees that injectorBuffer, if found, is invariant AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer(); if (injectorBuffer) { @@ -1220,7 +1220,7 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { // get one frame from the injector memset(_localScratchBuffer, 0, bytesToRead); if (0 < injectorBuffer->readData((char*)_localScratchBuffer, bytesToRead)) { - + if (injector->isAmbisonic()) { // no distance attenuation @@ -1249,36 +1249,36 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { for (int i = 0; i < AudioConstants::NETWORK_FRAME_SAMPLES_STEREO; i++) { mixBuffer[i] += convertToFloat(_localScratchBuffer[i]) * gain; } - + } else { // calculate distance, gain and azimuth for hrtf glm::vec3 relativePosition = injector->getPosition() - _positionGetter(); float distance = glm::max(glm::length(relativePosition), EPSILON); - float gain = gainForSource(distance, injector->getVolume()); + float gain = gainForSource(distance, injector->getVolume()); float azimuth = azimuthForSource(relativePosition); - + // mono gets spatialized into mixBuffer - injector->getLocalHRTF().render(_localScratchBuffer, mixBuffer, HRTF_DATASET_INDEX, + injector->getLocalHRTF().render(_localScratchBuffer, mixBuffer, HRTF_DATASET_INDEX, azimuth, distance, gain, AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL); } - + } else { - + qCDebug(audioclient) << "injector has no more data, marking finished for removal"; injector->finishLocalInjection(); injectorsToRemove.append(injector); } } else { - + qCDebug(audioclient) << "injector has no local buffer, marking as finished for removal"; injector->finishLocalInjection(); injectorsToRemove.append(injector); } } - - for (AudioInjector* injector : injectorsToRemove) { + + for (AudioInjectorPointer injector : injectorsToRemove) { qCDebug(audioclient) << "removing injector"; _activeLocalAudioInjectors.removeOne(injector); } @@ -1369,7 +1369,7 @@ void AudioClient::setIsStereoInput(bool isStereoInput) { } } -bool AudioClient::outputLocalInjector(AudioInjector* injector) { +bool AudioClient::outputLocalInjector(AudioInjectorPointer injector) { AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer(); if (injectorBuffer) { // local injectors are on the AudioInjectorsThread, so we must guard access @@ -1711,9 +1711,9 @@ int AudioClient::calculateNumberOfFrameSamples(int numBytes) const { float AudioClient::azimuthForSource(const glm::vec3& relativePosition) { glm::quat inverseOrientation = glm::inverse(_orientationGetter()); - + glm::vec3 rotatedSourcePosition = inverseOrientation * relativePosition; - + // project the rotated source position vector onto the XZ plane rotatedSourcePosition.y = 0.0f; @@ -1721,15 +1721,15 @@ float AudioClient::azimuthForSource(const glm::vec3& relativePosition) { float rotatedSourcePositionLength2 = glm::length2(rotatedSourcePosition); if (rotatedSourcePositionLength2 > SOURCE_DISTANCE_THRESHOLD) { - + // produce an oriented angle about the y-axis glm::vec3 direction = rotatedSourcePosition * (1.0f / fastSqrtf(rotatedSourcePositionLength2)); float angle = fastAcosf(glm::clamp(-direction.z, -1.0f, 1.0f)); // UNIT_NEG_Z is "forward" return (direction.x < 0.0f) ? -angle : angle; - } else { + } else { // no azimuth if they are in same spot - return 0.0f; + return 0.0f; } } @@ -1869,9 +1869,9 @@ void AudioClient::startThread() { moveToNewNamedThread(this, "Audio Thread", [this] { start(); }); } -void AudioClient::setInputVolume(float volume) { - if (_audioInput && volume != (float)_audioInput->volume()) { - _audioInput->setVolume(volume); +void AudioClient::setInputVolume(float volume) { + if (_audioInput && volume != (float)_audioInput->volume()) { + _audioInput->setVolume(volume); emit inputVolumeChanged(_audioInput->volume()); } } diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 62b99d2443..7fbfcc25d6 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -143,7 +143,7 @@ public: Q_INVOKABLE void setAvatarBoundingBoxParameters(glm::vec3 corner, glm::vec3 scale); - bool outputLocalInjector(AudioInjector* injector) override; + bool outputLocalInjector(AudioInjectorPointer injector) override; QAudioDeviceInfo getActiveAudioDevice(QAudio::Mode mode) const; QList getAudioDevices(QAudio::Mode mode) const; @@ -380,7 +380,7 @@ private: bool _hasReceivedFirstPacket { false }; - QVector _activeLocalAudioInjectors; + QVector _activeLocalAudioInjectors; bool _isPlayingBackRecording { false }; diff --git a/libraries/audio/src/AbstractAudioInterface.h b/libraries/audio/src/AbstractAudioInterface.h index 2e14b9956b..314884a67a 100644 --- a/libraries/audio/src/AbstractAudioInterface.h +++ b/libraries/audio/src/AbstractAudioInterface.h @@ -18,6 +18,7 @@ #include #include "AudioInjectorOptions.h" +#include "AudioInjector.h" class AudioInjector; class AudioInjectorLocalBuffer; @@ -35,7 +36,7 @@ public: // threadsafe // moves injector->getLocalBuffer() to another thread (so removes its parent) // take care to delete it when ~AudioInjector, as parenting Qt semantics will not work - virtual bool outputLocalInjector(AudioInjector* injector) = 0; + virtual bool outputLocalInjector(AudioInjectorPointer injector) = 0; public slots: virtual bool shouldLoopbackInjectors() { return false; } diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 47e6c98144..5b42168bdd 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -132,7 +132,7 @@ void AudioInjector::restart() { } } -bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjector*)) { +bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjectorPointer)) { _state = AudioInjectorState::NotFinished; int byteOffset = 0; @@ -150,7 +150,7 @@ bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjector* bool success = true; if (!_options.localOnly) { auto injectorManager = DependencyManager::get(); - if (!(*injectorManager.*injection)(this)) { + if (!(*injectorManager.*injection)(getThisPointer())) { success = false; finishNetworkInjection(); } @@ -173,7 +173,7 @@ bool AudioInjector::injectLocally() { // call this function on the AudioClient's thread // this will move the local buffer's thread to the LocalInjectorThread - success = _localAudioInterface->outputLocalInjector(this); + success = _localAudioInterface->outputLocalInjector(getThisPointer()); if (!success) { qCDebug(audio) << "AudioInjector::injectLocally could not output locally via _localAudioInterface"; @@ -429,7 +429,8 @@ void AudioInjector::stopAndDeleteLater() { QMetaObject::invokeMethod(this, "deleteLater", Qt::QueuedConnection); } -AudioInjector* AudioInjector::playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position) { +AudioInjectorPointer AudioInjector::playSound(SharedSoundPointer sound, const float volume, + const float stretchFactor, const glm::vec3 position) { if (!sound || !sound->isReady()) { return nullptr; } @@ -462,8 +463,8 @@ AudioInjector* AudioInjector::playSound(SharedSoundPointer sound, const float vo return playSoundAndDelete(resampled, options); } -AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options) { - AudioInjector* sound = playSound(buffer, options); +AudioInjectorPointer AudioInjector::playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options) { + AudioInjectorPointer sound = playSound(buffer, options); if (sound) { sound->_state |= AudioInjectorState::PendingDelete; @@ -473,10 +474,27 @@ AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const } -AudioInjector* AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) { - AudioInjector* injector = new AudioInjector(buffer, options); +AudioInjectorPointer AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) { + AudioInjectorPointer injector = AudioInjectorPointer(new AudioInjector(buffer, options)); + injector->setThisPointer(injector); + if (!injector->inject(&AudioInjectorManager::threadInjector)) { qWarning() << "AudioInjector::playSound failed to thread injector"; } return injector; } + +AudioInjectorPointer AudioInjector::getThisPointer() { + std::lock_guard lock(_refLock); + QSharedPointer this_ref(_self); + if (this_ref.isNull()) { + this_ref = QSharedPointer(this); + _self = this_ref.toWeakRef(); + } + return this_ref; +} + +void AudioInjector::setThisPointer(AudioInjectorPointer self) { + std::lock_guard lock(_refLock); + _self = self.toWeakRef(); +} diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index a901c2520f..056f498b3a 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -32,6 +32,8 @@ class AbstractAudioInterface; class AudioInjectorManager; +class AudioInjector; +using AudioInjectorPointer = QSharedPointer; enum class AudioInjectorState : uint8_t { @@ -46,19 +48,19 @@ AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs); AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs); // In order to make scripting cleaner for the AudioInjector, the script now holds on to the AudioInjector object -// until it dies. +// until it dies. class AudioInjector : public QObject { Q_OBJECT public: AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions); AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions); ~AudioInjector(); - + bool isFinished() const { return (stateHas(AudioInjectorState::Finished)); } - + int getCurrentSendOffset() const { return _currentSendOffset; } void setCurrentSendOffset(int currentSendOffset) { _currentSendOffset = currentSendOffset; } - + AudioInjectorLocalBuffer* getLocalBuffer() const { return _localBuffer; } AudioHRTF& getLocalHRTF() { return _localHRTF; } AudioFOA& getLocalFOA() { return _localFOA; } @@ -72,36 +74,41 @@ public: bool stateHas(AudioInjectorState state) const ; static void setLocalAudioInterface(AbstractAudioInterface* audioInterface) { _localAudioInterface = audioInterface; } - static AudioInjector* playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options); - static AudioInjector* playSound(const QByteArray& buffer, const AudioInjectorOptions options); - static AudioInjector* playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position); + static AudioInjectorPointer playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options); + static AudioInjectorPointer playSound(const QByteArray& buffer, const AudioInjectorOptions options); + static AudioInjectorPointer playSound(SharedSoundPointer sound, const float volume, + const float stretchFactor, const glm::vec3 position); + + QWeakPointer weakRef() const { return _self; } + AudioInjectorPointer getThisPointer(); + void setThisPointer(AudioInjectorPointer self); public slots: void restart(); - + void stop(); void triggerDeleteAfterFinish(); void stopAndDeleteLater(); - + const AudioInjectorOptions& getOptions() const { return _options; } void setOptions(const AudioInjectorOptions& options); - + float getLoudness() const { return _loudness; } bool isPlaying() const { return !stateHas(AudioInjectorState::Finished); } void finish(); void finishLocalInjection(); void finishNetworkInjection(); - + signals: void finished(); void restarting(); - + private: int64_t injectNextFrame(); - bool inject(bool(AudioInjectorManager::*injection)(AudioInjector*)); + bool inject(bool(AudioInjectorManager::*injection)(AudioInjectorPointer)); bool injectLocally(); void deleteLocalBuffer(); - + static AbstractAudioInterface* _localAudioInterface; QByteArray _audioData; @@ -112,17 +119,20 @@ private: int _currentSendOffset { 0 }; std::unique_ptr _currentPacket { nullptr }; AudioInjectorLocalBuffer* _localBuffer { nullptr }; - + int64_t _nextFrame { 0 }; std::unique_ptr _frameTimer { nullptr }; quint16 _outgoingSequenceNumber { 0 }; - + // when the injector is local, we need this AudioHRTF _localHRTF; AudioFOA _localFOA; friend class AudioInjectorManager; + + QWeakPointer _self; + mutable std::mutex _refLock; }; -Q_DECLARE_METATYPE(AudioInjector*) - +Q_DECLARE_METATYPE(AudioInjectorPointer) + #endif // hifi_AudioInjector_h diff --git a/libraries/audio/src/AudioInjectorManager.cpp b/libraries/audio/src/AudioInjectorManager.cpp index c66e209ea9..f9f45987d4 100644 --- a/libraries/audio/src/AudioInjectorManager.cpp +++ b/libraries/audio/src/AudioInjectorManager.cpp @@ -21,26 +21,26 @@ AudioInjectorManager::~AudioInjectorManager() { _shouldStop = true; - + Lock lock(_injectorsMutex); - + // make sure any still living injectors are stopped and deleted while (!_injectors.empty()) { // grab the injector at the front auto& timePointerPair = _injectors.top(); - + // ask it to stop and be deleted timePointerPair.second->stopAndDeleteLater(); - + _injectors.pop(); } - + // get rid of the lock now that we've stopped all living injectors lock.unlock(); - + // in case the thread is waiting for injectors wake it up now _injectorReady.notify_one(); - + // quit and wait on the manager thread, if we ever created it if (_thread) { _thread->quit(); @@ -51,10 +51,10 @@ AudioInjectorManager::~AudioInjectorManager() { void AudioInjectorManager::createThread() { _thread = new QThread; _thread->setObjectName("Audio Injector Thread"); - + // when the thread is started, have it call our run to handle injection of audio connect(_thread, &QThread::started, this, &AudioInjectorManager::run, Qt::DirectConnection); - + // start the thread _thread->start(); } @@ -63,20 +63,20 @@ void AudioInjectorManager::run() { while (!_shouldStop) { // wait until the next injector is ready, or until we get a new injector given to us Lock lock(_injectorsMutex); - + if (_injectors.size() > 0) { // when does the next injector need to send a frame? // do we get to wait or should we just go for it now? - + auto timeInjectorPair = _injectors.top(); - + auto nextTimestamp = timeInjectorPair.first; int64_t difference = int64_t(nextTimestamp - usecTimestampNow()); - + if (difference > 0) { _injectorReady.wait_for(lock, std::chrono::microseconds(difference)); } - + if (_injectors.size() > 0) { // loop through the injectors in the map and send whatever frames need to go out auto front = _injectors.top(); @@ -90,7 +90,7 @@ void AudioInjectorManager::run() { // either way we're popping this injector off - get a copy first auto injector = front.second; _injectors.pop(); - + if (!injector.isNull()) { // this is an injector that's ready to go, have it send a frame now auto nextCallDelta = injector->injectNextFrame(); @@ -100,7 +100,7 @@ void AudioInjectorManager::run() { heldInjectors.emplace(heldInjectors.end(), usecTimestampNow() + nextCallDelta, injector); } } - + if (_injectors.size() > 0) { front = _injectors.top(); } else { @@ -120,10 +120,10 @@ void AudioInjectorManager::run() { // we have no current injectors, wait until we get at least one before we do anything _injectorReady.wait(lock); } - + // unlock the lock in case something in process events needs to modify the queue lock.unlock(); - + QCoreApplication::processEvents(); } } @@ -139,36 +139,36 @@ bool AudioInjectorManager::wouldExceedLimits() { // Should be called inside of a return false; } -bool AudioInjectorManager::threadInjector(AudioInjector* injector) { +bool AudioInjectorManager::threadInjector(AudioInjectorPointer injector) { if (_shouldStop) { qCDebug(audio) << "AudioInjectorManager::threadInjector asked to thread injector but is shutting down."; return false; } - + // guard the injectors vector with a mutex Lock lock(_injectorsMutex); - + if (wouldExceedLimits()) { return false; } else { if (!_thread) { createThread(); } - + // move the injector to the QThread injector->moveToThread(_thread); - + // add the injector to the queue with a send timestamp of now - _injectors.emplace(usecTimestampNow(), InjectorQPointer { injector }); - + _injectors.emplace(usecTimestampNow(), AudioInjectorPointer { injector }); + // notify our wait condition so we can inject two frames for this injector immediately _injectorReady.notify_one(); - + return true; } } -bool AudioInjectorManager::restartFinishedInjector(AudioInjector* injector) { +bool AudioInjectorManager::restartFinishedInjector(AudioInjectorPointer injector) { if (_shouldStop) { qCDebug(audio) << "AudioInjectorManager::threadInjector asked to thread injector but is shutting down."; return false; @@ -181,8 +181,8 @@ bool AudioInjectorManager::restartFinishedInjector(AudioInjector* injector) { return false; } else { // add the injector to the queue with a send timestamp of now - _injectors.emplace(usecTimestampNow(), InjectorQPointer { injector }); - + _injectors.emplace(usecTimestampNow(), injector->getThisPointer()); + // notify our wait condition so we can inject two frames for this injector immediately _injectorReady.notify_one(); } diff --git a/libraries/audio/src/AudioInjectorManager.h b/libraries/audio/src/AudioInjectorManager.h index de5537856e..ac1d364b86 100644 --- a/libraries/audio/src/AudioInjectorManager.h +++ b/libraries/audio/src/AudioInjectorManager.h @@ -23,7 +23,7 @@ #include -class AudioInjector; +#include "AudioInjector.h" class AudioInjectorManager : public QObject, public Dependency { Q_OBJECT @@ -33,39 +33,38 @@ public: private slots: void run(); private: - - using InjectorQPointer = QPointer; - using TimeInjectorPointerPair = std::pair; - + + using TimeInjectorPointerPair = std::pair; + struct greaterTime { bool operator() (const TimeInjectorPointerPair& x, const TimeInjectorPointerPair& y) const { return x.first > y.first; } }; - + using InjectorQueue = std::priority_queue, greaterTime>; using Mutex = std::mutex; using Lock = std::unique_lock; - - bool threadInjector(AudioInjector* injector); - bool restartFinishedInjector(AudioInjector* injector); + + bool threadInjector(AudioInjectorPointer injector); + bool restartFinishedInjector(AudioInjectorPointer injector); void notifyInjectorReadyCondition() { _injectorReady.notify_one(); } bool wouldExceedLimits(); - + AudioInjectorManager() {}; AudioInjectorManager(const AudioInjectorManager&) = delete; AudioInjectorManager& operator=(const AudioInjectorManager&) = delete; - + void createThread(); - + QThread* _thread { nullptr }; bool _shouldStop { false }; InjectorQueue _injectors; Mutex _injectorsMutex; std::condition_variable _injectorReady; - + friend class AudioInjector; }; diff --git a/libraries/script-engine/src/ScriptAudioInjector.cpp b/libraries/script-engine/src/ScriptAudioInjector.cpp index c0ad2debd9..37a9ad65d7 100644 --- a/libraries/script-engine/src/ScriptAudioInjector.cpp +++ b/libraries/script-engine/src/ScriptAudioInjector.cpp @@ -21,7 +21,7 @@ QScriptValue injectorToScriptValue(QScriptEngine* engine, ScriptAudioInjector* c // when the script goes down we want to cleanup the injector QObject::connect(engine, &QScriptEngine::destroyed, in, &ScriptAudioInjector::stopInjectorImmediately, Qt::DirectConnection); - + return engine->newQObject(in, QScriptEngine::ScriptOwnership); } @@ -29,10 +29,10 @@ void injectorFromScriptValue(const QScriptValue& object, ScriptAudioInjector*& o out = qobject_cast(object.toQObject()); } -ScriptAudioInjector::ScriptAudioInjector(AudioInjector* injector) : +ScriptAudioInjector::ScriptAudioInjector(AudioInjectorPointer injector) : _injector(injector) { - QObject::connect(injector, &AudioInjector::finished, this, &ScriptAudioInjector::finished); + QObject::connect(injector.data(), &AudioInjector::finished, this, &ScriptAudioInjector::finished); } ScriptAudioInjector::~ScriptAudioInjector() { diff --git a/libraries/script-engine/src/ScriptAudioInjector.h b/libraries/script-engine/src/ScriptAudioInjector.h index 4de12af62c..018fbe5110 100644 --- a/libraries/script-engine/src/ScriptAudioInjector.h +++ b/libraries/script-engine/src/ScriptAudioInjector.h @@ -18,31 +18,31 @@ class ScriptAudioInjector : public QObject { Q_OBJECT - + Q_PROPERTY(bool playing READ isPlaying) Q_PROPERTY(float loudness READ getLoudness) Q_PROPERTY(AudioInjectorOptions options WRITE setOptions READ getOptions) public: - ScriptAudioInjector(AudioInjector* injector); + ScriptAudioInjector(AudioInjectorPointer injector); ~ScriptAudioInjector(); public slots: void restart() { _injector->restart(); } void stop() { _injector->stop(); } - + const AudioInjectorOptions& getOptions() const { return _injector->getOptions(); } void setOptions(const AudioInjectorOptions& options) { _injector->setOptions(options); } - + float getLoudness() const { return _injector->getLoudness(); } bool isPlaying() const { return _injector->isPlaying(); } - + signals: void finished(); - + protected slots: void stopInjectorImmediately(); private: - QPointer _injector; - + AudioInjectorPointer _injector; + friend QScriptValue injectorToScriptValue(QScriptEngine* engine, ScriptAudioInjector* const& in); }; diff --git a/libraries/ui/src/ui/types/SoundEffect.h b/libraries/ui/src/ui/types/SoundEffect.h index 656f98dd8d..a7e29d86f9 100644 --- a/libraries/ui/src/ui/types/SoundEffect.h +++ b/libraries/ui/src/ui/types/SoundEffect.h @@ -15,6 +15,7 @@ #include class AudioInjector; +using AudioInjectorPointer = QSharedPointer; // SoundEffect object, exposed to qml only, not interface JavaScript. // This is used to play spatial sound effects on tablets/web entities from within QML. @@ -38,7 +39,7 @@ protected: QUrl _url; float _volume { 1.0f }; SharedSoundPointer _sound; - AudioInjector* _injector { nullptr }; + AudioInjectorPointer _injector; }; #endif // hifi_SoundEffect_h From 1fe7718b9f720b48c13016134eec2f7e3689427b Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Jul 2017 15:02:45 -0700 Subject: [PATCH 10/24] restore wasted byte count when unable to fit stats --- assignment-client/src/octree/OctreeSendThread.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 46439b74df..868b377ced 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -232,9 +232,9 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* numBytes = nodeData->getPacket().getDataSize(); _totalBytes += numBytes; _totalPackets++; - // since a stats message is only included on end of scene, don't consider any of these bytes "wasted" - // there was nothing else to send. - //_totalWastedBytes += 0; + // we count wasted bytes here because we were unable to fit the stats packet + thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; + _totalWastedBytes += thisWastedBytes; _trueBytesSent += numBytes; numPackets++; From 3c8c87cff582757814c0e146877b16e99272ccd8 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 16:48:28 -0700 Subject: [PATCH 11/24] i hate you, milkman dan --- libraries/audio/src/AudioInjector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 5b42168bdd..b1b5fdc316 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -432,7 +432,7 @@ void AudioInjector::stopAndDeleteLater() { AudioInjectorPointer AudioInjector::playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position) { if (!sound || !sound->isReady()) { - return nullptr; + return AudioInjectorPointer(nullptr); } AudioInjectorOptions options; From 59c586bc5d870d08a1017c76191bd9d8c9115d4c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 16:58:19 -0700 Subject: [PATCH 12/24] remove unused code --- libraries/audio/src/AudioInjector.h | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index 056f498b3a..c53a65ecd9 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -79,7 +79,6 @@ public: static AudioInjectorPointer playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position); - QWeakPointer weakRef() const { return _self; } AudioInjectorPointer getThisPointer(); void setThisPointer(AudioInjectorPointer self); From 0b2f13dacc210e59aad8a58a94f28e9fd90f3281 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 17:07:44 -0700 Subject: [PATCH 13/24] don't use deleteLater on smartpointered object --- libraries/audio/src/AudioInjector.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index b1b5fdc316..1362d4850e 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -426,7 +426,6 @@ void AudioInjector::triggerDeleteAfterFinish() { void AudioInjector::stopAndDeleteLater() { stop(); - QMetaObject::invokeMethod(this, "deleteLater", Qt::QueuedConnection); } AudioInjectorPointer AudioInjector::playSound(SharedSoundPointer sound, const float volume, From 81489ea6c419fdfb9af8f9968f4e557c3bce4a43 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 18:44:37 -0700 Subject: [PATCH 14/24] use QEnableSharedFromThis --- libraries/audio/src/AudioInjector.cpp | 20 ++------------------ libraries/audio/src/AudioInjector.h | 8 +------- libraries/audio/src/AudioInjectorManager.cpp | 2 +- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 1362d4850e..b6c159ec90 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -150,7 +150,7 @@ bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjectorP bool success = true; if (!_options.localOnly) { auto injectorManager = DependencyManager::get(); - if (!(*injectorManager.*injection)(getThisPointer())) { + if (!(*injectorManager.*injection)(sharedFromThis())) { success = false; finishNetworkInjection(); } @@ -173,7 +173,7 @@ bool AudioInjector::injectLocally() { // call this function on the AudioClient's thread // this will move the local buffer's thread to the LocalInjectorThread - success = _localAudioInterface->outputLocalInjector(getThisPointer()); + success = _localAudioInterface->outputLocalInjector(sharedFromThis()); if (!success) { qCDebug(audio) << "AudioInjector::injectLocally could not output locally via _localAudioInterface"; @@ -475,25 +475,9 @@ AudioInjectorPointer AudioInjector::playSoundAndDelete(const QByteArray& buffer, AudioInjectorPointer AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) { AudioInjectorPointer injector = AudioInjectorPointer(new AudioInjector(buffer, options)); - injector->setThisPointer(injector); if (!injector->inject(&AudioInjectorManager::threadInjector)) { qWarning() << "AudioInjector::playSound failed to thread injector"; } return injector; } - -AudioInjectorPointer AudioInjector::getThisPointer() { - std::lock_guard lock(_refLock); - QSharedPointer this_ref(_self); - if (this_ref.isNull()) { - this_ref = QSharedPointer(this); - _self = this_ref.toWeakRef(); - } - return this_ref; -} - -void AudioInjector::setThisPointer(AudioInjectorPointer self) { - std::lock_guard lock(_refLock); - _self = self.toWeakRef(); -} diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index c53a65ecd9..89320ecc7a 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -49,7 +49,7 @@ AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs) // In order to make scripting cleaner for the AudioInjector, the script now holds on to the AudioInjector object // until it dies. -class AudioInjector : public QObject { +class AudioInjector : public QObject, public QEnableSharedFromThis { Q_OBJECT public: AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions); @@ -79,9 +79,6 @@ public: static AudioInjectorPointer playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position); - AudioInjectorPointer getThisPointer(); - void setThisPointer(AudioInjectorPointer self); - public slots: void restart(); @@ -127,9 +124,6 @@ private: AudioHRTF _localHRTF; AudioFOA _localFOA; friend class AudioInjectorManager; - - QWeakPointer _self; - mutable std::mutex _refLock; }; Q_DECLARE_METATYPE(AudioInjectorPointer) diff --git a/libraries/audio/src/AudioInjectorManager.cpp b/libraries/audio/src/AudioInjectorManager.cpp index f9f45987d4..e1465252b5 100644 --- a/libraries/audio/src/AudioInjectorManager.cpp +++ b/libraries/audio/src/AudioInjectorManager.cpp @@ -181,7 +181,7 @@ bool AudioInjectorManager::restartFinishedInjector(AudioInjectorPointer injector return false; } else { // add the injector to the queue with a send timestamp of now - _injectors.emplace(usecTimestampNow(), injector->getThisPointer()); + _injectors.emplace(usecTimestampNow(), injector); // notify our wait condition so we can inject two frames for this injector immediately _injectorReady.notify_one(); From c52e7e180a47b3c20511b533671a85ca4614a327 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 19:42:03 -0700 Subject: [PATCH 15/24] use const AudioInjectorPointer& in some places --- interface/src/avatar/AvatarManager.cpp | 2 +- libraries/audio-client/src/AudioClient.cpp | 2 +- libraries/audio/src/AudioInjector.cpp | 4 ++-- libraries/audio/src/AudioInjector.h | 2 +- libraries/audio/src/AudioInjectorManager.cpp | 4 ++-- libraries/audio/src/AudioInjectorManager.h | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index b63816eb4f..03def29b8a 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -434,7 +434,7 @@ void AvatarManager::handleCollisionEvents(const CollisionEvents& collisionEvents // but most avatars are roughly the same size, so let's not be so fancy yet. const float AVATAR_STRETCH_FACTOR = 1.0f; - _collisionInjectors.remove_if([](AudioInjectorPointer injector) { + _collisionInjectors.remove_if([](const AudioInjectorPointer& injector) { return !injector || injector->isFinished(); }); diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index d532b18836..f1a69441f1 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1207,7 +1207,7 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { memset(mixBuffer, 0, AudioConstants::NETWORK_FRAME_SAMPLES_STEREO * sizeof(float)); - for (AudioInjectorPointer injector : _activeLocalAudioInjectors) { + for (const AudioInjectorPointer& injector : _activeLocalAudioInjectors) { // the lock guarantees that injectorBuffer, if found, is invariant AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer(); if (injectorBuffer) { diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index b6c159ec90..82e3a3e5b5 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -132,7 +132,7 @@ void AudioInjector::restart() { } } -bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjectorPointer)) { +bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(const AudioInjectorPointer&)) { _state = AudioInjectorState::NotFinished; int byteOffset = 0; @@ -474,7 +474,7 @@ AudioInjectorPointer AudioInjector::playSoundAndDelete(const QByteArray& buffer, AudioInjectorPointer AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) { - AudioInjectorPointer injector = AudioInjectorPointer(new AudioInjector(buffer, options)); + AudioInjectorPointer injector = AudioInjectorPointer::create<>(buffer, options); if (!injector->inject(&AudioInjectorManager::threadInjector)) { qWarning() << "AudioInjector::playSound failed to thread injector"; diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index 89320ecc7a..09ce1d32b8 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -101,7 +101,7 @@ signals: private: int64_t injectNextFrame(); - bool inject(bool(AudioInjectorManager::*injection)(AudioInjectorPointer)); + bool inject(bool(AudioInjectorManager::*injection)(const AudioInjectorPointer&)); bool injectLocally(); void deleteLocalBuffer(); diff --git a/libraries/audio/src/AudioInjectorManager.cpp b/libraries/audio/src/AudioInjectorManager.cpp index e1465252b5..e8a46ba2b4 100644 --- a/libraries/audio/src/AudioInjectorManager.cpp +++ b/libraries/audio/src/AudioInjectorManager.cpp @@ -139,7 +139,7 @@ bool AudioInjectorManager::wouldExceedLimits() { // Should be called inside of a return false; } -bool AudioInjectorManager::threadInjector(AudioInjectorPointer injector) { +bool AudioInjectorManager::threadInjector(const AudioInjectorPointer& injector) { if (_shouldStop) { qCDebug(audio) << "AudioInjectorManager::threadInjector asked to thread injector but is shutting down."; return false; @@ -168,7 +168,7 @@ bool AudioInjectorManager::threadInjector(AudioInjectorPointer injector) { } } -bool AudioInjectorManager::restartFinishedInjector(AudioInjectorPointer injector) { +bool AudioInjectorManager::restartFinishedInjector(const AudioInjectorPointer& injector) { if (_shouldStop) { qCDebug(audio) << "AudioInjectorManager::threadInjector asked to thread injector but is shutting down."; return false; diff --git a/libraries/audio/src/AudioInjectorManager.h b/libraries/audio/src/AudioInjectorManager.h index ac1d364b86..9aca3014e3 100644 --- a/libraries/audio/src/AudioInjectorManager.h +++ b/libraries/audio/src/AudioInjectorManager.h @@ -48,8 +48,8 @@ private: using Mutex = std::mutex; using Lock = std::unique_lock; - bool threadInjector(AudioInjectorPointer injector); - bool restartFinishedInjector(AudioInjectorPointer injector); + bool threadInjector(const AudioInjectorPointer& injector); + bool restartFinishedInjector(const AudioInjectorPointer& injector); void notifyInjectorReadyCondition() { _injectorReady.notify_one(); } bool wouldExceedLimits(); From 972dc4d1b29cb8d13f5b07493b875c59b270846b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 19:56:34 -0700 Subject: [PATCH 16/24] more const ref --- libraries/audio-client/src/AudioClient.cpp | 4 ++-- libraries/audio-client/src/AudioClient.h | 2 +- libraries/audio/src/AbstractAudioInterface.h | 2 +- libraries/script-engine/src/ScriptAudioInjector.cpp | 2 +- libraries/script-engine/src/ScriptAudioInjector.h | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index f1a69441f1..3d9b1de10f 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1278,7 +1278,7 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { } } - for (AudioInjectorPointer injector : injectorsToRemove) { + for (const AudioInjectorPointer& injector : injectorsToRemove) { qCDebug(audioclient) << "removing injector"; _activeLocalAudioInjectors.removeOne(injector); } @@ -1369,7 +1369,7 @@ void AudioClient::setIsStereoInput(bool isStereoInput) { } } -bool AudioClient::outputLocalInjector(AudioInjectorPointer injector) { +bool AudioClient::outputLocalInjector(const AudioInjectorPointer& injector) { AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer(); if (injectorBuffer) { // local injectors are on the AudioInjectorsThread, so we must guard access diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 7fbfcc25d6..31e36671c7 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -143,7 +143,7 @@ public: Q_INVOKABLE void setAvatarBoundingBoxParameters(glm::vec3 corner, glm::vec3 scale); - bool outputLocalInjector(AudioInjectorPointer injector) override; + bool outputLocalInjector(const AudioInjectorPointer& injector) override; QAudioDeviceInfo getActiveAudioDevice(QAudio::Mode mode) const; QList getAudioDevices(QAudio::Mode mode) const; diff --git a/libraries/audio/src/AbstractAudioInterface.h b/libraries/audio/src/AbstractAudioInterface.h index 314884a67a..8b48b55206 100644 --- a/libraries/audio/src/AbstractAudioInterface.h +++ b/libraries/audio/src/AbstractAudioInterface.h @@ -36,7 +36,7 @@ public: // threadsafe // moves injector->getLocalBuffer() to another thread (so removes its parent) // take care to delete it when ~AudioInjector, as parenting Qt semantics will not work - virtual bool outputLocalInjector(AudioInjectorPointer injector) = 0; + virtual bool outputLocalInjector(const AudioInjectorPointer& injector) = 0; public slots: virtual bool shouldLoopbackInjectors() { return false; } diff --git a/libraries/script-engine/src/ScriptAudioInjector.cpp b/libraries/script-engine/src/ScriptAudioInjector.cpp index 37a9ad65d7..1b734e8cdf 100644 --- a/libraries/script-engine/src/ScriptAudioInjector.cpp +++ b/libraries/script-engine/src/ScriptAudioInjector.cpp @@ -29,7 +29,7 @@ void injectorFromScriptValue(const QScriptValue& object, ScriptAudioInjector*& o out = qobject_cast(object.toQObject()); } -ScriptAudioInjector::ScriptAudioInjector(AudioInjectorPointer injector) : +ScriptAudioInjector::ScriptAudioInjector(const AudioInjectorPointer& injector) : _injector(injector) { QObject::connect(injector.data(), &AudioInjector::finished, this, &ScriptAudioInjector::finished); diff --git a/libraries/script-engine/src/ScriptAudioInjector.h b/libraries/script-engine/src/ScriptAudioInjector.h index 018fbe5110..4c2871dd34 100644 --- a/libraries/script-engine/src/ScriptAudioInjector.h +++ b/libraries/script-engine/src/ScriptAudioInjector.h @@ -23,7 +23,7 @@ class ScriptAudioInjector : public QObject { Q_PROPERTY(float loudness READ getLoudness) Q_PROPERTY(AudioInjectorOptions options WRITE setOptions READ getOptions) public: - ScriptAudioInjector(AudioInjectorPointer injector); + ScriptAudioInjector(const AudioInjectorPointer& injector); ~ScriptAudioInjector(); public slots: void restart() { _injector->restart(); } From 3897ab2723c9210de9abd298d04bb753343515b5 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 20:16:00 -0700 Subject: [PATCH 17/24] don't call deleteLater on smartpointered object --- libraries/audio/src/AudioInjector.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 82e3a3e5b5..d7cb77bc7f 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -92,11 +92,6 @@ void AudioInjector::finish() { emit finished(); deleteLocalBuffer(); - - if (stateHas(AudioInjectorState::PendingDelete)) { - // we've been asked to delete after finishing, trigger a deleteLater here - deleteLater(); - } } void AudioInjector::restart() { From 084a989a7acdaf0313361ff4d36420c4db41df00 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 11 Jul 2017 21:21:42 -0700 Subject: [PATCH 18/24] stopAndDeleteLater is now just stop --- libraries/audio/src/AudioInjector.cpp | 6 +----- libraries/audio/src/AudioInjector.h | 1 - libraries/audio/src/AudioInjectorManager.cpp | 2 +- libraries/script-engine/src/ScriptAudioInjector.cpp | 2 +- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index d7cb77bc7f..1b16a28546 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -413,16 +413,12 @@ void AudioInjector::triggerDeleteAfterFinish() { } if (stateHas(AudioInjectorState::Finished)) { - stopAndDeleteLater(); + stop(); } else { _state |= AudioInjectorState::PendingDelete; } } -void AudioInjector::stopAndDeleteLater() { - stop(); -} - AudioInjectorPointer AudioInjector::playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position) { if (!sound || !sound->isReady()) { diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index 09ce1d32b8..aed51c5f85 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -84,7 +84,6 @@ public slots: void stop(); void triggerDeleteAfterFinish(); - void stopAndDeleteLater(); const AudioInjectorOptions& getOptions() const { return _options; } void setOptions(const AudioInjectorOptions& options); diff --git a/libraries/audio/src/AudioInjectorManager.cpp b/libraries/audio/src/AudioInjectorManager.cpp index e8a46ba2b4..a2f8de8da9 100644 --- a/libraries/audio/src/AudioInjectorManager.cpp +++ b/libraries/audio/src/AudioInjectorManager.cpp @@ -30,7 +30,7 @@ AudioInjectorManager::~AudioInjectorManager() { auto& timePointerPair = _injectors.top(); // ask it to stop and be deleted - timePointerPair.second->stopAndDeleteLater(); + timePointerPair.second->stop(); _injectors.pop(); } diff --git a/libraries/script-engine/src/ScriptAudioInjector.cpp b/libraries/script-engine/src/ScriptAudioInjector.cpp index 1b734e8cdf..516f62401f 100644 --- a/libraries/script-engine/src/ScriptAudioInjector.cpp +++ b/libraries/script-engine/src/ScriptAudioInjector.cpp @@ -44,5 +44,5 @@ ScriptAudioInjector::~ScriptAudioInjector() { void ScriptAudioInjector::stopInjectorImmediately() { qCDebug(scriptengine) << "ScriptAudioInjector::stopInjectorImmediately called to stop audio injector immediately."; - _injector->stopAndDeleteLater(); + _injector->stop(); } From b556ddc0ef80a582922201a7eea59de0591356a4 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 12 Jul 2017 07:24:19 -0700 Subject: [PATCH 19/24] remove unneeded AudioInjectorPointer constructor --- libraries/audio/src/AudioInjectorManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/audio/src/AudioInjectorManager.cpp b/libraries/audio/src/AudioInjectorManager.cpp index a2f8de8da9..f30d3093ec 100644 --- a/libraries/audio/src/AudioInjectorManager.cpp +++ b/libraries/audio/src/AudioInjectorManager.cpp @@ -159,7 +159,7 @@ bool AudioInjectorManager::threadInjector(const AudioInjectorPointer& injector) injector->moveToThread(_thread); // add the injector to the queue with a send timestamp of now - _injectors.emplace(usecTimestampNow(), AudioInjectorPointer { injector }); + _injectors.emplace(usecTimestampNow(), injector); // notify our wait condition so we can inject two frames for this injector immediately _injectorReady.notify_one(); From 364600c447340998aead63f93caeef45d735fb15 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 12 Jul 2017 11:12:47 -0700 Subject: [PATCH 20/24] fix rendering of simulation ownership debug icons --- interface/src/Menu.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 8b616c3029..c7223be208 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -680,7 +680,7 @@ Menu::Menu() { // Developer > Physics >>> MenuWrapper* physicsOptionsMenu = developerMenu->addMenu("Physics"); { - auto drawStatusConfig = qApp->getRenderEngine()->getConfiguration()->getConfig(); + auto drawStatusConfig = qApp->getRenderEngine()->getConfiguration()->getConfig("RenderMainView.DrawStatus"); addCheckableActionToQMenuAndActionHash(physicsOptionsMenu, MenuOption::PhysicsShowOwned, 0, false, drawStatusConfig, SLOT(setShowNetwork(bool))); } From ddf2312dc6db21a13f8f260b242291d7d07fd80c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 12 Jul 2017 12:45:52 -0700 Subject: [PATCH 21/24] code review --- interface/src/avatar/AvatarManager.h | 2 +- libraries/audio/src/AudioInjector.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index d6f140ea74..30801807d6 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -22,10 +22,10 @@ #include #include #include +#include #include "AvatarMotionState.h" #include "MyAvatar.h" -#include "AudioInjector.h" class AvatarManager : public AvatarHashMap { diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 1b16a28546..9276461783 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -422,7 +422,7 @@ void AudioInjector::triggerDeleteAfterFinish() { AudioInjectorPointer AudioInjector::playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position) { if (!sound || !sound->isReady()) { - return AudioInjectorPointer(nullptr); + return AudioInjectorPointer(); } AudioInjectorOptions options; From 06618e81d01766270010ac448763f4461b498977 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 12 Jul 2017 13:14:32 -0700 Subject: [PATCH 22/24] remove unneeded <> --- libraries/audio/src/AudioInjector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 9276461783..ee57e42e77 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -465,7 +465,7 @@ AudioInjectorPointer AudioInjector::playSoundAndDelete(const QByteArray& buffer, AudioInjectorPointer AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) { - AudioInjectorPointer injector = AudioInjectorPointer::create<>(buffer, options); + AudioInjectorPointer injector = AudioInjectorPointer::create(buffer, options); if (!injector->inject(&AudioInjectorManager::threadInjector)) { qWarning() << "AudioInjector::playSound failed to thread injector"; From 44de1dd2be479cb4f4f4f53a1abc4cead23796fe Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 12 Jul 2017 19:25:39 -0700 Subject: [PATCH 23/24] put result of Avatar::getJointNames back in index-order --- .../avatars-renderer/src/avatars-renderer/Avatar.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 6e8f0f01e7..7097d01226 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -1048,11 +1048,17 @@ int Avatar::getJointIndex(const QString& name) const { } QStringList Avatar::getJointNames() const { - QStringList result; + QVector result; withValidJointIndicesCache([&]() { - result = _modelJointIndicesCache.keys(); + QHashIterator i(_modelJointIndicesCache); + while (i.hasNext()) { + i.next(); + int index = _modelJointIndicesCache[i.key()]; + result.resize(index); + result[index] = i.value(); + } }); - return result; + return result.toList(); } glm::vec3 Avatar::getJointPosition(int index) const { From 85d0b682842189b62604a0bc05bd26c97ab3947a Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 12 Jul 2017 20:36:37 -0700 Subject: [PATCH 24/24] try again on putting result of Avatar::getJointNames in index order --- .../src/avatars-renderer/Avatar.cpp | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 7097d01226..4016592d0a 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -1048,17 +1048,38 @@ int Avatar::getJointIndex(const QString& name) const { } QStringList Avatar::getJointNames() const { - QVector result; + QStringList result; withValidJointIndicesCache([&]() { + // find out how large the vector needs to be + int maxJointIndex = -1; + QHashIterator k(_modelJointIndicesCache); + while (k.hasNext()) { + k.next(); + int index = k.value(); + if (index > maxJointIndex) { + maxJointIndex = index; + } + } + // iterate through the hash and put joint names + // into the vector at their indices + QVector resultVector(maxJointIndex+1); QHashIterator i(_modelJointIndicesCache); while (i.hasNext()) { i.next(); - int index = _modelJointIndicesCache[i.key()]; - result.resize(index); - result[index] = i.value(); + int index = i.value(); + resultVector[index] = i.key(); + } + // convert to QList and drop out blanks + result = resultVector.toList(); + QMutableListIterator j(result); + while (j.hasNext()) { + QString jointName = j.next(); + if (jointName.isEmpty()) { + j.remove(); + } } }); - return result.toList(); + return result; } glm::vec3 Avatar::getJointPosition(int index) const {