diff --git a/srtcore/buffer_rcv.cpp b/srtcore/buffer_rcv.cpp index 8774d4d1d..1f46788aa 100644 --- a/srtcore/buffer_rcv.cpp +++ b/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; } diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 2d3189bc9..724b6a084 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -9769,7 +9769,7 @@ void srt::CUDT::sendLossReport(const std::vector > & 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; } diff --git a/srtcore/group.cpp b/srtcore/group.cpp index 2952d8c6b..001dd4802 100644 --- a/srtcore/group.cpp +++ b/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.