[core] Fixed DROPREQ by TTL packet drop (#2003)

The sequence range to drop in the sender's drop request was 1 packet wider than needed.
Haivision · May 18, 2021 · d9150ea · d9150ea
1 parent 7e5b3ee
commit d9150ea
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 5 deletions.
diff --git a/srtcore/buffer.cpp b/srtcore/buffer.cpp
@@ -523,6 +523,10 @@ int CSndBuffer::readData(const int offset, CPacket& w_packet, steady_clock::time
     // by sequence number. Consider using some circular buffer.
     for (int i = 0; i < offset; ++i)
         p = p->m_pNext;
+#if ENABLE_HEAVY_LOGGING
+    const int32_t first_seq = p->m_iSeqNo;
+    int32_t last_seq = p->m_iSeqNo;
+#endif
 
     // Check if the block that is the next candidate to send (m_pCurrBlock pointing) is stale.
 
@@ -546,6 +550,9 @@ int CSndBuffer::readData(const int offset, CPacket& w_packet, steady_clock::time
         bool move     = false;
         while (msgno == p->getMsgSeq())
         {
+#if ENABLE_HEAVY_LOGGING
+            last_seq = p->m_iSeqNo;
+#endif
             if (p == m_pCurrBlock)
                 move = true;
             p = p->m_pNext;
@@ -555,7 +562,8 @@ int CSndBuffer::readData(const int offset, CPacket& w_packet, steady_clock::time
         }
 
         HLOGC(qslog.Debug,
-              log << "CSndBuffer::readData: due to TTL exceeded, " << w_msglen << " messages to drop, up to " << msgno);
+              log << "CSndBuffer::readData: due to TTL exceeded, SEQ " << first_seq << " - " << last_seq << ", "
+                  << w_msglen << " packets to drop, msgno=" << msgno);
 
         // If readData returns -1, then msgno_bitset is understood as a Message ID to drop.
         // This means that in this case it should be written by the message sequence value only

diff --git a/srtcore/core.cpp b/srtcore/core.cpp
@@ -8881,18 +8881,19 @@ int CUDT::packLostData(CPacket& w_packet, steady_clock::time_point& w_origintime
         {
             int32_t seqpair[2];
             seqpair[0] = w_packet.m_iSeqNo;
-            seqpair[1] = CSeqNo::incseq(seqpair[0], msglen);
+            SRT_ASSERT(msglen >= 1);
+            seqpair[1] = CSeqNo::incseq(seqpair[0], msglen - 1);
 
             HLOGC(qrlog.Debug, log << "IPE: loss-reported packets not found in SndBuf - requesting DROP: "
-                    << "msg=" << MSGNO_SEQ::unwrap(w_packet.m_iMsgNo) << " SEQ:"
+                    << "msg=" << MSGNO_SEQ::unwrap(w_packet.m_iMsgNo) << " msglen=" << msglen << " SEQ:"
                     << seqpair[0] << " - " << seqpair[1] << "(" << (-offset) << " packets)");
             sendCtrl(UMSG_DROPREQ, &w_packet.m_iMsgNo, seqpair, sizeof(seqpair));
 
             // only one msg drop request is necessary
             m_pSndLossList->removeUpTo(seqpair[1]);
 
             // skip all dropped packets
-            m_iSndCurrSeqNo = CSeqNo::maxseq(m_iSndCurrSeqNo, CSeqNo::incseq(seqpair[1]));
+            m_iSndCurrSeqNo = CSeqNo::maxseq(m_iSndCurrSeqNo, seqpair[1]);
 
             continue;
         }
@@ -11288,4 +11289,3 @@ void CUDT::handleKeepalive(const char* /*data*/, size_t /*size*/)
     }
 #endif
 }
-