Merge pull request #42 from birarda/control-pair-probe

send a control packet as bandwidth probe

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> 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> controlPacket)
     _stats.record(ConnectionStats::Stats::ReceivedTimeoutNAK);
 }
 
+void Connection::processProbeTail(std::unique_ptr<ControlPacket> 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();

libraries/networking/src/udt/Connection.h

@@ -94,6 +94,7 @@ private:
     void processTimeoutNAK(std::unique_ptr<ControlPacket> controlPacket);
     void processHandshake(std::unique_ptr<ControlPacket> controlPacket);
     void processHandshakeACK(std::unique_ptr<ControlPacket> controlPacket);
+    void processProbeTail(std::unique_ptr<ControlPacket> 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> _congestionControl;
    

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;

libraries/networking/src/udt/ControlPacket.h

@@ -33,7 +33,8 @@ public:
         NAK,
         TimeoutNAK,
         Handshake,
-        HandshakeACK
+        HandshakeACK,
+        ProbeTail
     };
     
     static std::unique_ptr<ControlPacket> create(Type type, qint64 size = -1);

libraries/networking/src/udt/SendQueue.cpp

@@ -443,10 +443,12 @@ bool SendQueue::maybeSendNewPacket() {
         _packets.pop_front();
         
         std::unique_ptr<Packet> 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