[core] Drop msg by TTL even if hasn't ever been sent (#2068)

Author: gou4shi1

docs/API/API-functions.md

@@ -1703,11 +1703,8 @@ called function should work.
 - `msgttl`: [IN]. In **message** and **live mode** only, specifies the TTL for 
 sending messages (in `[ms]`). Not used for receiving messages. If this value
 is not negative, it defines the maximum time up to which this message should
-stay scheduled for sending for the sake of later retransmission. A message
-is always sent for the first time, but the UDP packet carrying it may be
-(also partially) lost, and if so, lacking packets will be retransmitted. If
-the message is not successfully resent before TTL expires, further retransmission
-is given up and the message is discarded.
+stay scheduled for sending. If TTL has expired, the message sending and further retransmissions are discarded, even
+if it has never been sent so far.
 
 - `inorder`: [IN]. In **message mode** only, specifies that sent messages should 
 be extracted by the receiver in the order of sending. This can be meaningful if 

srtcore/buffer.cpp

@@ -397,57 +397,70 @@ int CSndBuffer::addBufferFromFile(fstream& ifs, int len)
     return total;
 }
 
-int CSndBuffer::readData(CPacket& w_packet, steady_clock::time_point& w_srctime, int kflgs)
+int CSndBuffer::readData(CPacket& w_packet, steady_clock::time_point& w_srctime, int kflgs, int& w_seqnoinc)
 {
-    // No data to read
-    if (m_pCurrBlock == m_pLastBlock)
-        return 0;
+    int readlen = 0;
+    w_seqnoinc = 0;
+
+    while (m_pCurrBlock != m_pLastBlock)
+    {
+        // Make the packet REFLECT the data stored in the buffer.
+        w_packet.m_pcData = m_pCurrBlock->m_pcData;
+        readlen = m_pCurrBlock->m_iLength;
+        w_packet.setLength(readlen);
+        w_packet.m_iSeqNo = m_pCurrBlock->m_iSeqNo;
+
+        // XXX This is probably done because the encryption should happen
+        // just once, and so this sets the encryption flags to both msgno bitset
+        // IN THE PACKET and IN THE BLOCK. This is probably to make the encryption
+        // happen at the time when scheduling a new packet to send, but the packet
+        // must remain in the send buffer until it's ACKed. For the case of rexmit
+        // the packet will be taken "as is" (that is, already encrypted).
+        //
+        // The problem is in the order of things:
+        // 0. When the application stores the data, some of the flags for PH_MSGNO are set.
+        // 1. The readData() is called to get the original data sent by the application.
+        // 2. The data are original and must be encrypted. They WILL BE encrypted, later.
+        // 3. So far we are in readData() so the encryption flags must be updated NOW because
+        //    later we won't have access to the block's data.
+        // 4. After exiting from readData(), the packet is being encrypted. It's immediately
+        //    sent, however the data must remain in the sending buffer until they are ACKed.
+        // 5. In case when rexmission is needed, the second overloaded version of readData
+        //    is being called, and the buffer + PH_MSGNO value is extracted. All interesting
+        //    flags must be present and correct at that time.
+        //
+        // The only sensible way to fix this problem is to encrypt the packet not after
+        // extracting from here, but when the packet is stored into CSndBuffer. The appropriate
+        // flags for PH_MSGNO will be applied directly there. Then here the value for setting
+        // PH_MSGNO will be set as is.
+
+        if (kflgs == -1)
+        {
+            HLOGC(bslog.Debug, log << CONID() << " CSndBuffer: ERROR: encryption required and not possible. NOT SENDING.");
+            readlen = 0;
+        }
+        else
+        {
+            m_pCurrBlock->m_iMsgNoBitset |= MSGNO_ENCKEYSPEC::wrap(kflgs);
+        }
 
-    // Make the packet REFLECT the data stored in the buffer.
-    w_packet.m_pcData = m_pCurrBlock->m_pcData;
-    int readlen       = m_pCurrBlock->m_iLength;
-    w_packet.setLength(readlen);
-    w_packet.m_iSeqNo = m_pCurrBlock->m_iSeqNo;
-
-    // XXX This is probably done because the encryption should happen
-    // just once, and so this sets the encryption flags to both msgno bitset
-    // IN THE PACKET and IN THE BLOCK. This is probably to make the encryption
-    // happen at the time when scheduling a new packet to send, but the packet
-    // must remain in the send buffer until it's ACKed. For the case of rexmit
-    // the packet will be taken "as is" (that is, already encrypted).
-    //
-    // The problem is in the order of things:
-    // 0. When the application stores the data, some of the flags for PH_MSGNO are set.
-    // 1. The readData() is called to get the original data sent by the application.
-    // 2. The data are original and must be encrypted. They WILL BE encrypted, later.
-    // 3. So far we are in readData() so the encryption flags must be updated NOW because
-    //    later we won't have access to the block's data.
-    // 4. After exiting from readData(), the packet is being encrypted. It's immediately
-    //    sent, however the data must remain in the sending buffer until they are ACKed.
-    // 5. In case when rexmission is needed, the second overloaded version of readData
-    //    is being called, and the buffer + PH_MSGNO value is extracted. All interesting
-    //    flags must be present and correct at that time.
-    //
-    // The only sensible way to fix this problem is to encrypt the packet not after
-    // extracting from here, but when the packet is stored into CSndBuffer. The appropriate
-    // flags for PH_MSGNO will be applied directly there. Then here the value for setting
-    // PH_MSGNO will be set as is.
+        Block* p = m_pCurrBlock;
+        w_packet.m_iMsgNo = m_pCurrBlock->m_iMsgNoBitset;
+        w_srctime = m_pCurrBlock->m_tsOriginTime;
+        m_pCurrBlock = m_pCurrBlock->m_pNext;
 
-    if (kflgs == -1)
-    {
-        HLOGC(bslog.Debug, log << CONID() << " CSndBuffer: ERROR: encryption required and not possible. NOT SENDING.");
-        readlen = 0;
-    }
-    else
-    {
-        m_pCurrBlock->m_iMsgNoBitset |= MSGNO_ENCKEYSPEC::wrap(kflgs);
-    }
-
-    w_packet.m_iMsgNo = m_pCurrBlock->m_iMsgNoBitset;
-    w_srctime         = m_pCurrBlock->m_tsOriginTime;
-    m_pCurrBlock      = m_pCurrBlock->m_pNext;
+        if ((p->m_iTTL >= 0) && (count_milliseconds(steady_clock::now() - w_srctime) > p->m_iTTL))
+        {
+            LOGC(bslog.Warn, log << CONID() << "CSndBuffer: skipping packet %" << p->m_iSeqNo << " #" << p->getMsgSeq() << " with TTL=" << p->m_iTTL);
+            // Skip this packet due to TTL expiry.
+            readlen = 0;
+            ++w_seqnoinc;
+            continue;
+        }
 
-    HLOGC(bslog.Debug, log << CONID() << "CSndBuffer: extracting packet size=" << readlen << " to send");
+        HLOGC(bslog.Debug, log << CONID() << "CSndBuffer: extracting packet size=" << readlen << " to send");
+        break;
+    }
 
     return readlen;
 }

srtcore/buffer.h

@@ -142,17 +142,18 @@ class CSndBuffer
     int addBufferFromFile(std::fstream& ifs, int len);
 
     /// Find data position to pack a DATA packet from the furthest reading point.
-    /// @param [out] w_packet data packet buffer to fill.
-    /// @param [out] w_origintime origin time stamp of the message.
-    /// @param [in] kflags Odd|Even crypto key flag.
+    /// @param [out] packet the packet to read.
+    /// @param [out] origintime origin time stamp of the message
+    /// @param [in] kflags Odd|Even crypto key flag
+    /// @param [out] seqnoinc the number of packets skipped due to TTL, so that seqno should be incremented.
     /// @return Actual length of data read.
-    int readData(CPacket& w_packet, time_point& w_origintime, int kflgs);
+    int readData(CPacket& w_packet, time_point& w_origintime, int kflgs, int& w_seqnoinc);
 
     /// Find data position to pack a DATA packet for a retransmission.
     /// @param [in] offset offset from the last ACK point (backward sequence number difference)
-    /// @param [out] w_packet data packet buffer to fill.
-    /// @param [out] w_origintime origin time stamp of the message
-    /// @param [out] w_msglen length of the message
+    /// @param [out] packet the packet to read.
+    /// @param [out] origintime origin time stamp of the message
+    /// @param [out] msglen length of the message
     /// @return Actual length of data read (return 0 if offset too large, -1 if TTL exceeded).
     int readData(const int offset, CPacket& w_packet, time_point& w_origintime, int& w_msglen);
 

srtcore/core.cpp

@@ -9219,17 +9219,15 @@ int srt::CUDT::packLostData(CPacket& w_packet, steady_clock::time_point& w_origi
             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) << " msglen=" << msglen << " SEQ:"
-                    << seqpair[0] << " - " << seqpair[1] << "(" << (-offset) << " packets)");
+            HLOGC(qrlog.Debug,
+                  log << "loss-reported packets expired in SndBuf - requesting DROP: "
+                      << "msgno=" << MSGNO_SEQ::unwrap(w_packet.m_iMsgNo) << " msglen=" << msglen
+                      << " SEQ:" << seqpair[0] << " - " << seqpair[1]);
             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_pSndLossList->removeUpTo(seqpair[1]);
             m_iSndCurrSeqNo = CSeqNo::maxseq(m_iSndCurrSeqNo, seqpair[1]);
-
             continue;
         }
         else if (payload == 0)
@@ -9327,7 +9325,14 @@ std::pair<int, steady_clock::time_point> srt::CUDT::packData(CPacket& w_packet)
             // It would be nice to research as to whether CSndBuffer::Block::m_iMsgNoBitset field
             // isn't a useless redundant state copy. If it is, then taking the flags here can be removed.
             kflg    = m_pCryptoControl->getSndCryptoFlags();
-            payload = m_pSndBuffer->readData((w_packet), (origintime), kflg);
+            int pktskipseqno = 0;
+            payload = m_pSndBuffer->readData((w_packet), (origintime), kflg, (pktskipseqno));
+            if (pktskipseqno)
+            {
+                // Some packets were skipped due to TTL expiry.
+                m_iSndCurrSeqNo = CSeqNo::incseq(m_iSndCurrSeqNo, pktskipseqno);
+            }
+
             if (payload)
             {
                 // A CHANGE. The sequence number is currently added to the packet