[core] Slightly optimize the RCV drop by message number (#2686).

Some minor improvements of logs and comments.

srtcore/buffer_rcv.cpp

@@ -261,8 +261,9 @@ int CRcvBuffer::dropAll()
 int CRcvBuffer::dropMessage(int32_t seqnolo, int32_t seqnohi, int32_t msgno, DropActionIfExists actionOnExisting)
 {
     IF_RCVBUF_DEBUG(ScopedLog scoped_log);
-    IF_RCVBUF_DEBUG(scoped_log.ss << "CRcvBuffer::dropMessage: seqnolo " << seqnolo << " seqnohi " << seqnohi
-        << ", msgno " << msgno << " m_iStartSeqNo " << m_iStartSeqNo);
+    IF_RCVBUF_DEBUG(scoped_log.ss << "CRcvBuffer::dropMessage(): %(" << seqnolo << " - " << seqnohi << ")"
+                                  << " #" << msgno << " actionOnExisting=" << actionOnExisting << " m_iStartSeqNo=%"
+                                  << m_iStartSeqNo);
 
     // Drop by packet seqno range to also wipe those packets that do not exist in the buffer.
     const int offset_a = CSeqNo::seqoff(m_iStartSeqNo, seqnolo);
@@ -297,7 +298,9 @@ int CRcvBuffer::dropMessage(int32_t seqnolo, int32_t seqnohi, int32_t msgno, Dro
             if (bKeepExisting && bnd == PB_SOLO)
             {
                 bDropByMsgNo = false; // Solo packet, don't search for the rest of the message.
-                LOGC(rbuflog.Debug, log << "CRcvBuffer.dropMessage(): Skipped dropping an exising SOLO packet %" << packetAt(i).getSeqNo() << ".");
+                LOGC(rbuflog.Debug,
+                     log << "CRcvBuffer::dropMessage(): Skipped dropping an existing SOLO packet %"
+                         << packetAt(i).getSeqNo() << ".");
                 continue;
             }
 
@@ -323,13 +326,15 @@ int CRcvBuffer::dropMessage(int32_t seqnolo, int32_t seqnohi, int32_t msgno, Dro
 
     if (bDropByMsgNo)
     {
-        // First try to drop by message number in case the message starts earlier thtan @a seqnolo.
-        // The sender should have the last packet of the message it is requesting to be dropped,
-        // therefore we don't search forward.
+        // If msgno is specified, potentially not the whole message was dropped using seqno range.
+        // The sender might have removed the first packets of the message, and thus @a seqnolo may point to a packet in the middle.
+        // The sender should have the last packet of the message it is requesting to be dropped.
+        // Therefore we don't search forward, but need to check earlier packets in the RCV buffer.
+        // Try to drop by the message number in case the message starts earlier than @a seqnolo.
         const int stop_pos = decPos(m_iStartPos);
         for (int i = start_pos; i != stop_pos; i = decPos(i))
         {
-            // Can't drop is message number is not known.
+            // Can't drop if message number is not known.
             if (!m_entries[i].pUnit) // also dropped earlier.
                 continue;
 
@@ -340,21 +345,19 @@ int CRcvBuffer::dropMessage(int32_t seqnolo, int32_t seqnohi, int32_t msgno, Dro
 
             if (bKeepExisting && bnd == PB_SOLO)
             {
-                LOGC(rbuflog.Debug, log << "CRcvBuffer.dropMessage(): Skipped dropping an exising SOLO message packet %"
-                    << packetAt(i).getSeqNo() << ".");
+                LOGC(rbuflog.Debug,
+                     log << "CRcvBuffer::dropMessage(): Skipped dropping an existing SOLO message packet %"
+                         << packetAt(i).getSeqNo() << ".");
                 break;
             }
 
             ++iDropCnt;
             dropUnitInPos(i);
             m_entries[i].status = EntryState_Drop;
+            // As the search goes backward, i is always earlier than minDroppedOffset.
+            minDroppedOffset = offPos(m_iStartPos, i);
 
-            if (minDroppedOffset == -1)
-                minDroppedOffset = offPos(m_iStartPos, i);
-            else
-                minDroppedOffset = min(offPos(m_iStartPos, i), minDroppedOffset);
-
-            // Break the loop if the start of message has been found. No need to search further.
+            // Break the loop if the start of the message has been found. No need to search further.
             if (bnd == PB_FIRST)
                 break;
         }

srtcore/core.cpp

@@ -9769,7 +9769,7 @@ void srt::CUDT::sendLossReport(const std::vector<std::pair<int32_t, int32_t> > &
 bool srt::CUDT::overrideSndSeqNo(int32_t seq)
 {
     // This function is intended to be called from the socket
-    // group managmenet functions to synchronize the sequnece in
+    // group management functions to synchronize the sequnece in
     // all sockes in the bonding group. THIS sequence given
     // here is the sequence TO BE STAMPED AT THE EXACTLY NEXT
     // sent payload. Therefore, screw up the ISN to exactly this
@@ -9811,9 +9811,9 @@ bool srt::CUDT::overrideSndSeqNo(int32_t seq)
     // the latter is ahead with the number of packets already scheduled, but
     // not yet sent.
 
-    HLOGC(gslog.Debug, log << CONID() << "overrideSndSeqNo: sched-seq=" << m_iSndNextSeqNo << " send-seq=" << m_iSndCurrSeqNo
-        << " (unchanged)"
-        );
+    HLOGC(gslog.Debug,
+          log << CONID() << "overrideSndSeqNo: sched-seq=" << m_iSndNextSeqNo << " send-seq=" << m_iSndCurrSeqNo
+              << " (unchanged)");
     return true;
 }
 

srtcore/group.cpp

@@ -1223,12 +1223,10 @@ int CUDTGroup::sendBroadcast(const char* buf, int len, SRT_MSGCTRL& w_mc)
     // and therefore take over the leading role in setting the ISN. If the
     // second one fails, too, then the only remaining idle link will simply
     // go with its own original sequence.
-    //
-    // On the opposite side the reader should know that the link is inactive
-    // so the first received payload activates it. Activation of an idle link
-    // means that the very first packet arriving is TAKEN AS A GOOD DEAL, that is,
-    // no LOSSREPORT is sent even if the sequence looks like a "jumped over".
-    // Only for activated links is the LOSSREPORT sent upon seqhole detection.
+
+    // On the opposite side, if the first packet arriving looks like a jump over,
+    // the corresponding LOSSREPORT is sent. For packets that are truly lost,
+    // the sender retransmits them, for packets that before ISN, DROPREQ is sent.
 
     // Now we can go to the idle links and attempt to send the payload
     // also over them.