[core] Pre-initialization of ISN for the group before connecting any …

…socket (#1438)

srtcore/core.cpp

@@ -4010,16 +4010,16 @@ void CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn)
 
     if (forced_isn == SRT_SEQNO_NONE)
     {
-        // Random Initial Sequence Number (normal mode)
-        srand(count_microseconds(steady_clock::now().time_since_epoch()));
-        m_iISN = m_ConnReq.m_iISN = (int32_t)(CSeqNo::m_iMaxSeqNo * (double(rand()) / RAND_MAX));
+        forced_isn = generateISN();
+        HLOGC(mglog.Debug, log << "startConnect: ISN generated = " << forced_isn);
     }
     else
     {
-        // Predefined ISN (for debug purposes)
-        m_iISN = m_ConnReq.m_iISN = forced_isn;
+        HLOGC(mglog.Debug, log << "startConnect: ISN forced = " << forced_isn);
     }
 
+    m_iISN = m_ConnReq.m_iISN = forced_isn;
+
     setInitialSndSeq(m_iISN);
     m_SndLastAck2Time = steady_clock::now();
 
@@ -11430,6 +11430,10 @@ CUDTGroup::CUDTGroup(SRT_GROUP_TYPE gtype)
     m_RcvEID = m_pGlobal->m_EPoll.create(&m_RcvEpolld);
     m_SndEID = m_pGlobal->m_EPoll.create(&m_SndEpolld);
 
+    // Set this data immediately during creation before
+    // two or more sockets start arguing about it.
+    m_iLastSchedSeqNo = CUDT::generateISN();
+
     // Configure according to type
     switch (gtype)
     {
@@ -11964,7 +11968,19 @@ void CUDTGroup::syncWithSocket(const CUDT& core)
     // [[using locked(m_GroupLock)]];
 
     set_currentSchedSequence(core.ISN());
-    setInitialRxSequence(core.m_iPeerISN);
+
+    // XXX
+    // Might need further investigation as to whether this isn't
+    // wrong for some cases. By having this -1 here the value will be
+    // laziliy set from the first reading one. It is believed that
+    // it covers all possible scenarios, that is:
+    //
+    // - no readers - no problem!
+    // - have some readers and a new is attached - this is set already
+    // - connect multiple links, but none has read yet - you'll be the first.
+    //
+    // Previous implementation used setting to: core.m_iPeerISN
+    resetInitialRxSequence();
 
     // Get the latency (possibly fixed against the opposite side)
     // from the first socket (core.m_iTsbPdDelay_ms),
@@ -14957,3 +14973,9 @@ int CUDTGroup::configure(const char* str)
 
     return 0;
 }
+
+// Forwarder needed due to class definition order
+int32_t CUDTGroup::generateISN()
+{
+    return CUDT::generateISN();
+}

srtcore/core.h

@@ -426,8 +426,11 @@ class CUDTGroup
             // that was disconnected other than immediately closing it.
             if (m_Group.empty())
             {
-                m_iLastSchedSeqNo = SRT_SEQNO_NONE;
-                setInitialRxSequence(SRT_SEQNO_NONE);
+                // When the group is empty, there's no danger that this
+                // number will collide with any ISN provided by a socket.
+                // Also since now every socket will derive this ISN.
+                m_iLastSchedSeqNo = generateISN();
+                resetInitialRxSequence();
             }
             s = true;
         }
@@ -457,6 +460,7 @@ class CUDTGroup
     int send(const char* buf, int len, SRT_MSGCTRL& w_mc);
     int sendBroadcast(const char* buf, int len, SRT_MSGCTRL& w_mc);
     int sendBackup(const char* buf, int len, SRT_MSGCTRL& w_mc);
+    static int32_t generateISN();
 
 private:
     // For Backup, sending all previous packet
@@ -835,7 +839,7 @@ class CUDTGroup
 #endif
     }
 
-    void setInitialRxSequence(int32_t)
+    void resetInitialRxSequence()
     {
         // The app-reader doesn't care about the real sequence number.
         // The first provided one will be taken as a good deal; even if
@@ -1113,6 +1117,14 @@ class CUDT
         m_pRcvQueue->removeListener(this);
     }
 
+    static int32_t generateISN()
+    {
+        using namespace srt::sync;
+        // Random Initial Sequence Number (normal mode)
+        srand(count_microseconds(steady_clock::now().time_since_epoch()));
+        return (int32_t)(CSeqNo::m_iMaxSeqNo * (double(rand()) / RAND_MAX));
+    }
+
     // XXX See CUDT::tsbpd() to see how to implement it. This should
     // do the same as TLPKTDROP feature when skipping packets that are agreed
     // to be lost. Note that this is predicted to be called with TSBPD off.