[core] Repurpose SRTO_GROUPMINSTABLETIMEO

apps/socketoptions.hpp

@@ -249,12 +249,10 @@ const SocketOption srt_options [] {
     { "packetfilter", 0, SRTO_PACKETFILTER, SocketOption::PRE, SocketOption::STRING, nullptr },
 #if ENABLE_EXPERIMENTAL_BONDING
     { "groupconnect", 0, SRTO_GROUPCONNECT, SocketOption::PRE, SocketOption::INT, nullptr},
+    { "groupminstabletimeo", 0, SRTO_GROUPMINSTABLETIMEO, SocketOption::PRE, SocketOption::INT, nullptr},
 #endif
 #ifdef SRT_ENABLE_BINDTODEVICE
     { "bindtodevice", 0, SRTO_BINDTODEVICE, SocketOption::PRE, SocketOption::STRING, nullptr},
-#endif
-#if ENABLE_EXPERIMENTAL_BONDING
-    { "groupstabtimeo", 0, SRTO_GROUPSTABTIMEO, SocketOption::POST, SocketOption::INT, nullptr},
 #endif
     { "retransmitalgo", 0, SRTO_RETRANSMITALGO, SocketOption::PRE, SocketOption::INT, nullptr }
 };

docs/API/API-socket-options.md

@@ -205,7 +205,7 @@ The following table lists SRT API socket options in alphabetical order. Option d
 | [`SRTO_EVENT`](#SRTO_EVENT)                            |       |          | `int32_t` | flags   |                   |          | R   | S     |
 | [`SRTO_FC`](#SRTO_FC)                                  |       | pre      | `int32_t` | pkts    | 25600             | 32..     | RW  | GSD   |
 | [`SRTO_GROUPCONNECT`](#SRTO_GROUPCONNECT)              | 1.5.0 | pre      | `int32_t` |         | 0                 | 0...1    | W   | S     |
-| [`SRTO_GROUPSTABTIMEO`](#SRTO_GROUPSTABTIMEO)          | 1.5.0 | pre      | `int32_t` | ms      | 80                | 10-...   | W   | GSD+  |
+| [`SRTO_GROUPMINSTABLETIMEO`](#SRTO_GROUPMINSTABLETIMEO) | 1.4.5 | pre     | `int32_t` | ms      | 60                | 60-...   | W   | GDI+  |
 | [`SRTO_GROUPTYPE`](#SRTO_GROUPTYPE)                    | 1.5.0 |          | `int32_t` | enum    |                   |          | R   | S     |
 | [`SRTO_INPUTBW`](#SRTO_INPUTBW)                        | 1.0.5 | post     | `int64_t` | B/s     | 0                 | 0..      | RW  | GSD   |
 | [`SRTO_IPTOS`](#SRTO_IPTOS)                            | 1.0.5 | pre-bind | `int32_t` |         | (system)          | 0..255   | RW  | GSD   |
@@ -455,46 +455,35 @@ function will return the group, not this socket ID.
 
 ---
 
-#### SRTO_GROUPSTABTIMEO
+#### SRTO_GROUPMINSTABLETIMEO
 
 | OptName               | Since | Restrict | Type       | Units  | Default  | Range  | Dir | Entity |
 | --------------------- | ----- | -------- | ---------- | ------ | -------- | ------ | --- | ------ |
-| `SRTO_GROUPSTABTIMEO` | 1.5.0 | pre      | `int32_t`  | ms     | 80       | 10-... | W   | GSD+   |
-
-**Not in use at the moment. Is to be repurposed in SRT v1.4.3!**
-
-This setting is used for groups of type `SRT_GTYPE_BACKUP`. It defines the stability
-timeout, which is the maximum interval between two consecutive packets retrieved from
-the peer on the currently active link. These two packets can be of any type,
-but this setting usually refers to control packets while the agent is a sender.
-Idle links exchange only keepalive messages once per second, so they do not
-count. Note that this option is meaningless on sockets that are not members of
-the Backup-type group.
-
-This value should be set with a thoroughly selected balance and correspond to
-the maximum stretched response time between two consecutive ACK messages. By default
-ACK messages are sent every 10ms (so this interval is not dependent on the network
-latency), and so should be the interval between two consecutive received ACK
-messages. Note, however, that the network jitter on the public internet causes
-these intervals to be stretched, even to multiples of that interval. Both large
-and small values of this option have consequences:
-
-Large values of this option prevent overreaction on highly stretched response
-times, but introduce a latency penalty - the latency must be greater
-than this value (otherwise switching to another link won't preserve
-smooth signal sending). Large values will also contribute to higher packet
-bursts sent at the moment when an idle link is activated.
-
-Smaller values of this option respect low latency requirements very
-well, but may cause overreaction on even slightly stretched response times. This is
-unwanted, as a link switch should ideally happen only when the currently active
-link is really broken, as every link switch costs extra overhead (it counts
-for 100% for a time of one ACK interval).
+| `SRTO_GROUPMINSTABLETIMEO` | 1.4.5 | pre | `int32_t`  | ms     | 60       | 60-... | W   | GDI+   |
+
+The option is used for groups of type `SRT_GTYPE_BACKUP`. It defines the **minimum** value of the stability
+timeout for all active member sockets in a group.
+The actual timeout value is determined in runtime based on the RTT estimate of an individual member socket.
+If there is no response from the peer for the calculated timeout,
+the member is considered unstable, triggering activation of an idle backup member.
+
+The samller the value is, the earlier a backup member might be activated to prepare transision to that path.
+However, it may also lead to spurious activations of backup paths.
+The higher the value is, the later would a backup link be activated. All unacknowledged payload packets
+have to be retransmitted through the backup path. If they don't reach the receiver in time, they would be dropped.
+Therefore, an appropriate adjustment of the SRT buffering delay
+(`SRTO_PEERLATENCY` on sender, `SRTO_RCVLATENCY` on receiver) should also be considered.
+
+Normally the receiver should send an ACK back to sender every 10 ms. In case of congestion,
+in the live streaming configuration of SRT a loss report is expected to be sent every RTT/2.
+The network jitter and increase of RTT on the public internet causes
+these intervals to be stretched.
+The default minimum value of 60 ms is selected as a general fit for most of the use cases.
+
+Please refer to the [SRT Connection Bonding: Main/Backup](../features/bonding-main-backup.md) document for more details.
 
 Note that the value of this option is not allowed to exceed the value of
-`SRTO_PEERIDLETIMEO`. Usually it is only meaningful if you change the latter
-option, as the default value of it is way above any sensible value of
-`SRTO_GROUPSTABTIMEO`.
+`SRTO_PEERIDLETIMEO`, which determines the timeout to actually break an idle (irresponsive) connection.
 
 [Return to list](#list-of-options)
 

srtcore/core.cpp

@@ -199,6 +199,7 @@ struct SrtOptionAction
 #endif
 #if ENABLE_EXPERIMENTAL_BONDING
         flags[SRTO_GROUPCONNECT]       = SRTO_R_PRE;
+        flags[SRTO_GROUPMINSTABLETIMEO]= SRTO_R_PRE;
 #endif
         flags[SRTO_PACKETFILTER]       = SRTO_R_PRE;
         flags[SRTO_RETRANSMITALGO]     = SRTO_R_PRE;
@@ -767,10 +768,15 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen)
 
 #if ENABLE_EXPERIMENTAL_BONDING
     case SRTO_GROUPCONNECT:
-        optlen         = sizeof (int);
+        optlen        = sizeof (int);
         *(int*)optval = m_config.iGroupConnect;
         break;
 
+    case SRTO_GROUPMINSTABLETIMEO:
+        optlen = sizeof(int);
+        *(int*)optval = (int)m_config.uMinStabilityTimeout_ms;
+        break;
+
     case SRTO_GROUPTYPE:
         optlen         = sizeof (int);
         *(int*)optval = m_HSGroupType;
@@ -788,7 +794,7 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen)
         break;
 
     case SRTO_PEERIDLETIMEO:
-        *(int *)optval = m_config.iPeerIdleTimeout;
+        *(int *)optval = m_config.iPeerIdleTimeout_ms;
         optlen         = sizeof(int);
         break;
 
@@ -9698,7 +9704,7 @@ bool srt::CUDT::overrideSndSeqNo(int32_t seq)
     const int diff = CSeqNo(seq) - CSeqNo(m_iSndCurrSeqNo);
     if (diff < 0 || diff > CSeqNo::m_iSeqNoTH)
     {
-        LOGC(gslog.Error, log << CONID() << "IPE: Overridding with seq %" << seq << " DISCREPANCY against current %"
+        LOGC(gslog.Error, log << CONID() << "IPE: Overriding with seq %" << seq << " DISCREPANCY against current %"
                 << m_iSndCurrSeqNo << " and next sched %" << m_iSndNextSeqNo << " - diff=" << diff);
         return false;
     }
@@ -11191,7 +11197,7 @@ bool srt::CUDT::checkExpTimer(const steady_clock::time_point& currtime, int chec
         return false;
 
     // ms -> us
-    const int PEER_IDLE_TMO_US = m_config.iPeerIdleTimeout * 1000;
+    const int PEER_IDLE_TMO_US = m_config.iPeerIdleTimeout_ms * 1000;
     // Haven't received any information from the peer, is it dead?!
     // timeout: at least 16 expirations and must be greater than 5 seconds
     time_point last_rsp_time = m_tsLastRspTime.load();

srtcore/core.h

@@ -327,7 +327,7 @@ class CUDT
     int         MSS()               const { return m_config.iMSS; }
 
     uint32_t        peerLatency_us()        const { return m_iPeerTsbPdDelay_ms * 1000; }
-    int             peerIdleTimeout_ms()    const { return m_config.iPeerIdleTimeout; }
+    int             peerIdleTimeout_ms()    const { return m_config.iPeerIdleTimeout_ms; }
     size_t          maxPayloadSize()        const { return m_iMaxSRTPayloadSize; }
     size_t          OPT_PayloadSize()       const { return m_config.zExpPayloadSize; }
     int             sndLossLength()               { return m_pSndLossList->getLossLength(); }

srtcore/group.cpp

@@ -258,7 +258,7 @@ CUDTGroup::CUDTGroup(SRT_GROUP_TYPE gtype)
     , m_iBusy()
     , m_iSndOldestMsgNo(SRT_MSGNO_NONE)
     , m_iSndAckedMsgNo(SRT_MSGNO_NONE)
-    , m_uOPT_StabilityTimeout(CSrtConfig::COMM_DEF_STABILITY_TIMEOUT_US)
+    , m_uOPT_MinStabilityTimeout_us(1000 * CSrtConfig::COMM_DEF_MIN_STABILITY_TIMEOUT_MS)
     // -1 = "undefined"; will become defined with first added socket
     , m_iMaxPayloadSize(-1)
     , m_bSynRecving(true)
@@ -387,41 +387,43 @@ void CUDTGroup::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
         m_iRcvTimeOut = cast_optval<int>(optval, optlen);
         break;
 
-    case SRTO_GROUPSTABTIMEO:
+    case SRTO_GROUPMINSTABLETIMEO:
     {
-        const int val = cast_optval<int>(optval, optlen);
+        const int val_ms = cast_optval<int>(optval, optlen);
+        const int min_timeo_ms = (int) CSrtConfig::COMM_DEF_MIN_STABILITY_TIMEOUT_MS;
+        if (val_ms < min_timeo_ms)
+        {
+            LOGC(qmlog.Error,
+                 log << "group option: SRTO_GROUPMINSTABLETIMEO min allowed value is " << min_timeo_ms << " ms.");
+            throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
+        }
 
         // Search if you already have SRTO_PEERIDLETIMEO set
-        int                          idletmo = CSrtConfig::COMM_RESPONSE_TIMEOUT_MS;
+        int idletmo = CSrtConfig::COMM_RESPONSE_TIMEOUT_MS;
         vector<ConfigItem>::iterator f =
             find_if(m_config.begin(), m_config.end(), ConfigItem::OfType(SRTO_PEERIDLETIMEO));
         if (f != m_config.end())
         {
             f->get(idletmo); // worst case, it will leave it unchanged.
         }
 
-        if (val >= idletmo)
+        if (val_ms > idletmo)
         {
             LOGC(qmlog.Error,
-                 log << "group option: SRTO_GROUPSTABTIMEO(" << val << ") exceeds SRTO_PEERIDLETIMEO(" << idletmo
-                     << ")");
+                 log << "group option: SRTO_GROUPMINSTABLETIMEO=" << val_ms << " exceeds SRTO_PEERIDLETIMEO=" << idletmo);
             throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
         }
 
-        m_uOPT_StabilityTimeout = val * 1000;
+        m_uOPT_MinStabilityTimeout_us = 1000 * val_ms;
     }
 
     break;
 
-        // XXX Currently no socket groups allow any other
-        // congestion control mode other than live.
     case SRTO_CONGESTION:
-    {
+        // Currently no socket groups allow any other
+        // congestion control mode other than live.
         LOGP(gmlog.Error, "group option: SRTO_CONGESTION is only allowed as 'live' and cannot be changed");
         throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
-    }
-
-        // Other options to be specifically interpreted by group may follow.
 
     default:
         break;
@@ -430,7 +432,6 @@ void CUDTGroup::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
     // All others must be simply stored for setting on a socket.
     // If the group is already open and any post-option is about
     // to be modified, it must be allowed and applied on all sockets.
-
     if (m_bOpened)
     {
         // There's at least one socket in the group, so only
@@ -515,6 +516,9 @@ void CUDTGroup::deriveSettings(CUDT* u)
     // SRTO_SNDTIMEO
     m_iSndTimeOut = u->m_config.iSndTimeOut;
 
+    // SRTO_GROUPMINSTABLETIMEO
+    m_uOPT_MinStabilityTimeout_us = 1000 * u->m_config.uMinStabilityTimeout_ms;
+
     // Ok, this really is disgusting, but there's only one way
     // to properly do it. Would be nice to have some more universal
     // connection between an option symbolic name and the internals
@@ -564,8 +568,7 @@ void CUDTGroup::deriveSettings(CUDT* u)
     IM(SRTO_MINVERSION, uMinimumPeerSrtVersion);
     IM(SRTO_ENFORCEDENCRYPTION, bEnforcedEnc);
     IM(SRTO_IPV6ONLY, iIpV6Only);
-    IM(SRTO_PEERIDLETIMEO, iPeerIdleTimeout);
-    IM(SRTO_GROUPSTABTIMEO, uStabilityTimeout);
+    IM(SRTO_PEERIDLETIMEO, iPeerIdleTimeout_ms);
 
     importOption(m_config, SRTO_PACKETFILTER, u->m_config.sPacketFilterConfig.str());
 
@@ -760,6 +763,8 @@ static bool getOptDefault(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen)
         RD(true);
     case SRTO_PAYLOADSIZE:
         RD(0);
+    case SRTO_GROUPMINSTABLETIMEO:
+        RD(CSrtConfig::COMM_DEF_MIN_STABILITY_TIMEOUT_MS);
     }
 
 #undef RD
@@ -3328,7 +3333,8 @@ CUDTGroup::BackupMemberState CUDTGroup::sendBackup_QualifyActiveState(const gli_
     const CUDT& u = d->ps->core();
 
     const uint32_t latency_us = u.peerLatency_us();
-    const int32_t min_stability_us = 60000; // Minimum Link Stability Timeout: 60ms.
+
+    const int32_t min_stability_us = m_uOPT_MinStabilityTimeout_us;
     const int64_t initial_stabtout_us = max<int64_t>(min_stability_us, latency_us);
     const int64_t probing_period_us = initial_stabtout_us + 5 * CUDT::COMM_SYN_INTERVAL_US;
 

srtcore/group.h

@@ -614,7 +614,7 @@ class CUDTGroup
     senderBuffer_t   m_SenderBuffer;
     int32_t          m_iSndOldestMsgNo; // oldest position in the sender buffer
     volatile int32_t m_iSndAckedMsgNo;
-    uint32_t         m_uOPT_StabilityTimeout;
+    uint32_t         m_uOPT_MinStabilityTimeout_us;
 
     // THIS function must be called only in a function for a group type
     // that does use sender buffer.

srtcore/socketconfig.cpp

@@ -263,7 +263,6 @@ struct CSrtConfigSetter<SRTO_BINDTODEVICE>
         using namespace srt_logging;
 #ifdef SRT_ENABLE_BINDTODEVICE
         using namespace std;
-        using namespace srt_logging;
 
         string val;
         if (optlen == -1)
@@ -782,7 +781,7 @@ struct CSrtConfigSetter<SRTO_PEERIDLETIMEO>
         if (val < 0)
             throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
 
-        co.iPeerIdleTimeout = val;
+        co.iPeerIdleTimeout_ms = val;
     }
 };
 
@@ -835,33 +834,37 @@ struct CSrtConfigSetter<SRTO_PACKETFILTER>
 
 #if ENABLE_EXPERIMENTAL_BONDING
 template<>
-struct CSrtConfigSetter<SRTO_GROUPSTABTIMEO>
+struct CSrtConfigSetter<SRTO_GROUPMINSTABLETIMEO>
 {
     static void set(CSrtConfig& co, const void* optval, int optlen)
     {
         using namespace srt_logging;
         // This option is meaningless for the socket itself.
         // It's set here just for the sake of setting it on a listener
         // socket so that it is then applied on the group when a
-        // group connection is configuired.
-        const int val = cast_optval<int>(optval, optlen);
+        // group connection is configured.
+        const int val_ms = cast_optval<int>(optval, optlen);
+        const int min_timeo_ms = (int) CSrtConfig::COMM_DEF_MIN_STABILITY_TIMEOUT_MS;
 
-        // Search if you already have SRTO_PEERIDLETIMEO set
+        if (val_ms < min_timeo_ms)
+        {
+            LOGC(qmlog.Error,
+                log << "group option: SRTO_GROUPMINSTABLETIMEO min allowed value is "
+                    << min_timeo_ms << " ms.");
+            throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
+        }
 
-        const int idletmo = co.iPeerIdleTimeout;
+        const int idletmo_ms = co.iPeerIdleTimeout_ms;
 
-        // Both are in milliseconds.
-        // This option is RECORDED in microseconds, while
-        // idletmo is recorded in milliseconds, only translated to
-        // microseconds directly before use.
-        if (val >= idletmo)
+        if (val_ms > idletmo_ms)
         {
-            LOGC(aclog.Error, log << "group option: SRTO_GROUPSTABTIMEO(" << val
-                                  << ") exceeds SRTO_PEERIDLETIMEO(" << idletmo << ")");
+            LOGC(aclog.Error, log << "group option: SRTO_GROUPMINSTABLETIMEO(" << val_ms
+                                  << ") exceeds SRTO_PEERIDLETIMEO(" << idletmo_ms << ")");
             throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
         }
 
-        co.uStabilityTimeout = val * 1000;
+        co.uMinStabilityTimeout_ms = val_ms;
+        LOGC(smlog.Error, log << "SRTO_GROUPMINSTABLETIMEO set " << val_ms);
     }
 };
 #endif
@@ -927,16 +930,14 @@ int dispatchSet(SRT_SOCKOPT optName, CSrtConfig& co, const void* optval, int opt
         DISPATCH(SRTO_TRANSTYPE);
 #if ENABLE_EXPERIMENTAL_BONDING
         DISPATCH(SRTO_GROUPCONNECT);
+        DISPATCH(SRTO_GROUPMINSTABLETIMEO);
 #endif
         DISPATCH(SRTO_KMREFRESHRATE);
         DISPATCH(SRTO_KMPREANNOUNCE);
         DISPATCH(SRTO_ENFORCEDENCRYPTION);
         DISPATCH(SRTO_PEERIDLETIMEO);
         DISPATCH(SRTO_IPV6ONLY);
         DISPATCH(SRTO_PACKETFILTER);
-#if ENABLE_EXPERIMENTAL_BONDING
-        DISPATCH(SRTO_GROUPSTABTIMEO);
-#endif
         DISPATCH(SRTO_RETRANSMITALGO);
 
 #undef DISPATCH
@@ -964,7 +965,7 @@ bool SRT_SocketOptionObject::add(SRT_SOCKOPT optname, const void* optval, size_t
     case SRTO_CONNTIMEO:
     case SRTO_DRIFTTRACER:
         //SRTO_FC - not allowed to be different among group members
-    case SRTO_GROUPSTABTIMEO:
+    case SRTO_GROUPMINSTABLETIMEO:
         //SRTO_INPUTBW - per transmission setting
     case SRTO_IPTOS:
     case SRTO_IPTTL: