[core] Fixed overrideSndSeqNo() not clear buffer.

[gou4shi1]

D:SRT.gs: grp/sendBackup: ... trying @362808828 - resending 11 collected messages...
D:SRT.gs: @362808828: overrideSndSeqNo: sched-seq=1540037828 send-seq=1540037827 (unchanged)
D:SRT.as: @362808828: SND-DROP: %(1540037828-1540037910) n=83pkt 108778B, span=1427 ms, FIRST #160424
D:SRT.as: @362808828: buf:SENDING (BEFORE) srctime:01:38:51.051264000 [STDY] DATA SIZE: 892 sched-SEQUENCE: 1540037828 STAMP: D8C417EF
D:SRT.as: @362808828: buf:SENDING srctime:01:38:51.051264000 [STDY] size=892 #160569 SCHED %1540037828(>> %1540037828) !D8C417EF
D:SRT.as: @362808828: sock:SENDING (END): success, size=892
D:SRT.as: @362808828: buf:SENDING (BEFORE) srctime:01:38:51.133805000 [STDY] DATA SIZE: 1398 sched-SEQUENCE: 1540037829 STAMP: C26BC454
D:SRT.as: @362808828: buf:SENDING srctime:01:38:51.133805000 [STDY] size=1398 #160570 SCHED %1540037829(>> %1540037829) !C26BC454
D:SRT.as: @362808828: sock:SENDING (END): success, size=1398
/SRT:SndQ:w3133*E:SRT.qs: @362808828: IPE: packData: SCHEDULING sequence 1540037828 is behind of EXTRACTION sequence 1540037911, dropping this packet: DIFF=-83 STAMP=D8C417EF

Note the IPE log, the SCHEDULING sequence 1540037828 is behind of EXTRACTION sequence 1540037911, because of the SND-DROP: %(1540037828-1540037910).
In sndDropTooLate(), the m_iSndLastAck was mistakenly updated to 1540037911 by m_pSndBuffer->dropLateData(), so the issue is that m_pSndBuffer was not cleared by overrideSndSeqNo() in main-backup mode.

[codecov-commenter]

Codecov Report

Merging #2753 (2a5bb72) into master (3cefede) will decrease coverage by 0.75%.
The diff coverage is 0.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #2753      +/-   ##
==========================================
- Coverage   67.51%   66.77%   -0.75%     
==========================================
  Files          99       99              
  Lines       20166    20178      +12     
==========================================
- Hits        13616    13473     -143     
- Misses       6550     6705     +155     

Impacted Files	Coverage Δ
srtcore/buffer_snd.cpp	66.77% <0.00%> (-1.94%)	⬇️
srtcore/buffer_snd.h	50.00% <ø> (ø)
srtcore/core.cpp	60.74% <0.00%> (-1.49%)	⬇️
srtcore/group.cpp	37.76% <0.00%> (ø)

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ethouris]

Well, ok, but why is everything dropped from the sender buffer instead of only sequences up to the scheduling sequence?

[gou4shi1]

why is everything dropped from the sender buffer instead of only sequences up to the scheduling sequence?

Correct me if I"m wrong, the scheduling sequence should be the last seq in sender buffer? If no other IPE, "everything" should be same as "only sequences up to the scheduling sequence"?

[ethouris]

Depends on which sender buffer. We are with backup groups here, but sender buffering is separate for a socket and for a group. Every socket still has its own sender buffering and there's an extra sender buffer for the group, which are stored when group sending happens. The sender buffer for an idle member socket is not filled on regular sending, only when the backup-activation process starts and packets are being resent from the group sender buffer.

The problem is that as the idle link doesn't send packets, it also doesn't store anything in the socket's sender buffer and hence doesn't increase the sending sequence. This gets "tricked", that is, when ACK comes, the anchor sequence of the sender buffer must be shifted to the position of the active link. When the backup activation happens, all packets since the last ACK should be resent, and the sender buffer of the socket should be prepared for this by getting shifted. However, ACK handler and backup activation happen in different threads, so there's no guarantee that the sender buffer will have the right anchor sequence number at the moment of the backup activation. The code must be then prepared for even a worst case scenario, that is, when the anchor sequence of the socket's sender buffer has to be fixed before sending the first packet from the group's sender buffer. It is, however, predicted that the sequence can be shifted forwards, not backwards. The need of backward fixing should be impossible because the anchor sequence either stays unchanged and is then behind the last sent, or it is shifted together with ACK and is equal to the anchor sequence of all other sockets.

Another small problem is that at the moment of backup activation there shouldn't be any packets in the sender buffers of idle member sockets. I don't exactly remember how it is implemented, but anyway: it should catch the empty buffer and only make sure that the anchor sequence of the sender buffer is equal to the last ACK received. ACK should cause dismissal of packets from the group sender buffer as well by obvious reasons - so there shouldn't be also a situation when the sequence number of the packet picked up from the group sender buffer predates the activated member socket's sender buffer's anchor sequence.