diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 9101356fb1..84928fa298 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -409,7 +409,14 @@ bool Connection::processReceivedSequenceNumber(SequenceNumber sequenceNumber, in if (((uint32_t) sequenceNumber & 0xF) == 0) { _receiveWindow.onProbePair1Arrival(); } else if (((uint32_t) sequenceNumber & 0xF) == 1) { - _receiveWindow.onProbePair2Arrival(); + // only use this packet for bandwidth estimation if we didn't just receive a control packet in its place + if (!_receivedControlProbeTail) { + _receiveWindow.onProbePair2Arrival(); + } else { + // reset our control probe tail marker so the next probe that comes with data can be used + _receivedControlProbeTail = false; + } + } _receiveWindow.onPacketArrival(); @@ -510,6 +517,11 @@ void Connection::processControl(std::unique_ptr controlPacket) { case ControlPacket::HandshakeACK: processHandshakeACK(move(controlPacket)); break; + case ControlPacket::ProbeTail: + if (_isReceivingData) { + processProbeTail(move(controlPacket)); + } + break; } } @@ -733,6 +745,23 @@ void Connection::processTimeoutNAK(std::unique_ptr controlPacket) _stats.record(ConnectionStats::Stats::ReceivedTimeoutNAK); } +void Connection::processProbeTail(std::unique_ptr controlPacket) { + if (((uint32_t) _lastReceivedSequenceNumber & 0xF) == 0) { + // this is the second packet in a probe set so we can estimate bandwidth + // the sender sent this to us in lieu of sending new data (because they didn't have any) + +#ifdef UDT_CONNECTION_DEBUG + qCDebug(networking) << "Processing second packet of probe from control packet instead of data packet"; +#endif + + _receiveWindow.onProbePair2Arrival(); + + // mark that we processed a control packet for the second in the pair and we should not mark + // the next data packet received + _receivedControlProbeTail = true; + } +} + void Connection::resetReceiveState() { // reset all SequenceNumber member variables back to default @@ -767,6 +796,7 @@ void Connection::resetReceiveState() { // clear the intervals in the receive window _receiveWindow.reset(); + _receivedControlProbeTail = false; // clear any pending received messages _pendingReceivedMessages.clear(); diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index eb5961695c..f10c9dd720 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -94,6 +94,7 @@ private: void processTimeoutNAK(std::unique_ptr controlPacket); void processHandshake(std::unique_ptr controlPacket); void processHandshakeACK(std::unique_ptr controlPacket); + void processProbeTail(std::unique_ptr controlPacket); void resetReceiveState(); void resetRTT(); @@ -145,7 +146,8 @@ private: Socket* _parentSocket { nullptr }; HifiSockAddr _destination; - PacketTimeWindow _receiveWindow { 16, 64 }; // Window of interval between packets (16) and probes (64) for bandwidth and receive speed + PacketTimeWindow _receiveWindow { 16, 64 }; // Window of interval between packets (16) and probes (64) for timing + bool _receivedControlProbeTail { false }; // Marker for receipt of control packet probe tail (in lieu of probe with data) std::unique_ptr _congestionControl; diff --git a/libraries/networking/src/udt/ControlPacket.cpp b/libraries/networking/src/udt/ControlPacket.cpp index 8c799f97a6..e672ab426b 100644 --- a/libraries/networking/src/udt/ControlPacket.cpp +++ b/libraries/networking/src/udt/ControlPacket.cpp @@ -100,7 +100,7 @@ void ControlPacket::readType() { Q_ASSERT_X(bitAndType & CONTROL_BIT_MASK, "ControlPacket::readHeader()", "This should be a control packet"); uint16_t packetType = (bitAndType & ~CONTROL_BIT_MASK) >> (8 * sizeof(Type)); - Q_ASSERT_X(packetType <= ControlPacket::Type::HandshakeACK, "ControlPacket::readType()", "Received a control packet with wrong type"); + Q_ASSERT_X(packetType <= ControlPacket::Type::ProbeTail, "ControlPacket::readType()", "Received a control packet with wrong type"); // read the type _type = (Type) packetType; diff --git a/libraries/networking/src/udt/ControlPacket.h b/libraries/networking/src/udt/ControlPacket.h index 1976899c14..fea8210ba0 100644 --- a/libraries/networking/src/udt/ControlPacket.h +++ b/libraries/networking/src/udt/ControlPacket.h @@ -33,7 +33,8 @@ public: NAK, TimeoutNAK, Handshake, - HandshakeACK + HandshakeACK, + ProbeTail }; static std::unique_ptr create(Type type, qint64 size = -1); diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index f63b251888..28f71ec93f 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -443,10 +443,12 @@ bool SendQueue::maybeSendNewPacket() { _packets.pop_front(); std::unique_ptr secondPacket; + bool shouldSendPairTail = false; if (((uint32_t) nextNumber & 0xF) == 0) { // the first packet is the first in a probe pair - every 16 (rightmost 16 bits = 0) packets // pull off a second packet if we can before we unlock + shouldSendPairTail = true; if (_packets.size() > 0) { secondPacket.swap(_packets.front()); @@ -463,6 +465,12 @@ bool SendQueue::maybeSendNewPacket() { // do we have a second in a pair to send as well? if (secondPacket) { sendNewPacketAndAddToSentList(move(secondPacket), getNextSequenceNumber()); + } else if (shouldSendPairTail) { + // we didn't get a second packet to send in the probe pair + // send a control packet of type ProbePairTail so the receiver can still do + // proper bandwidth estimation + static auto pairTailPacket = ControlPacket::create(ControlPacket::ProbeTail); + _socket->writeBasePacket(*pairTailPacket, _destination); } // We sent our packet(s), return here