From b38bb7f507201bbd8b20faee6bbfb4859dcaac92 Mon Sep 17 00:00:00 2001 From: quink-black Date: Thu, 18 Mar 2021 20:16:22 +0800 Subject: [PATCH] [core] Changed SRTO_CONNTIMEO restriction to PRE (#1864) * Changed SRTO_CONNTIMEO to be pre-restricted * Added timeout check to TestConnectionTimeout.BlockingLoop with 200ms tolerance. GitHub CI (macOS) gave up to 141 ms extra delay. Co-authored-by: Maxim Sharabayko --- docs/APISocketOptions.md | 4 ++-- srtcore/core.cpp | 2 +- srtcore/core.h | 2 +- test/test_connection_timeout.cpp | 12 ++++++++---- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/APISocketOptions.md b/docs/APISocketOptions.md index 1d37ad738..149241a73 100644 --- a/docs/APISocketOptions.md +++ b/docs/APISocketOptions.md @@ -202,7 +202,7 @@ The following table lists SRT socket options in alphabetical order. Option detai | :----------------------------------------------------- | :---: | :------: | :-------: | :-----: | :-----------: | :------: |:---:|:-----:| | [`SRTO_BINDTODEVICE`](#SRTO_BINDTODEVICE) | 1.4.2 | pre-bind | `string` | | | | RW | GSD+ | | [`SRTO_CONGESTION`](#SRTO_CONGESTION) | 1.3.0 | pre | `string` | | "live" | * | W | S | -| [`SRTO_CONNTIMEO`](#SRTO_CONNTIMEO) | 1.1.2 | post | `int32_t` | ms | 3000 | 0.. | W | GSD+ | +| [`SRTO_CONNTIMEO`](#SRTO_CONNTIMEO) | 1.1.2 | pre | `int32_t` | ms | 3000 | 0.. | W | GSD+ | | [`SRTO_DRIFTTRACER`](#SRTO_DRIFTTRACER) | 1.4.2 | post | `bool` | | true | | RW | GSD | | [`SRTO_ENFORCEDENCRYPTION`](#SRTO_ENFORCEDENCRYPTION) | 1.3.2 | pre | `bool` | | true | | W | GSD | | [`SRTO_EVENT`](#SRTO_EVENT) | | | `int32_t` | flags | | | R | S | @@ -311,7 +311,7 @@ rather change the whole set of options using the [`SRTO_TRANSTYPE`](#SRTO_TRANST | OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | | ------------------ | ----- | -------- | --------- | ------ | -------- | ------ | --- | ------ | -| `SRTO_CONNTIMEO` | 1.1.2 | post | `int32_t` | msec | 3000 | 0.. | W | GSD+ | +| `SRTO_CONNTIMEO` | 1.1.2 | pre | `int32_t` | msec | 3000 | 0.. | W | GSD+ | Connect timeout. This option applies to the caller and rendezvous connection modes. For the rendezvous mode (see `SRTO_RENDEZVOUS`) the effective connection timeout diff --git a/srtcore/core.cpp b/srtcore/core.cpp index e91763f9d..da4f07668 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -125,7 +125,6 @@ extern const SRT_SOCKOPT srt_post_opt_list [SRT_SOCKOPT_NPOST] = { SRTO_MININPUTBW, SRTO_OHEADBW, SRTO_SNDDROPDELAY, - SRTO_CONNTIMEO, SRTO_DRIFTTRACER, SRTO_LOSSMAXTTL }; @@ -172,6 +171,7 @@ struct SrtOptionAction flags[SRTO_SNDDROPDELAY] = SRTO_R_PRE; flags[SRTO_NAKREPORT] = SRTO_R_PRE; flags[SRTO_VERSION] = SRTO_R_PRE; + flags[SRTO_CONNTIMEO] = SRTO_R_PRE; flags[SRTO_LOSSMAXTTL] = 0 | SRTO_POST_SPEC; flags[SRTO_RCVLATENCY] = SRTO_R_PRE; flags[SRTO_PEERLATENCY] = SRTO_R_PRE; diff --git a/srtcore/core.h b/srtcore/core.h index ef395fd2f..0a4c2d86d 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -116,7 +116,7 @@ enum AckDataItem }; const size_t ACKD_FIELD_SIZE = sizeof(int32_t); -static const size_t SRT_SOCKOPT_NPOST = 13; +static const size_t SRT_SOCKOPT_NPOST = 12; extern const SRT_SOCKOPT srt_post_opt_list []; enum GroupDataItem diff --git a/test/test_connection_timeout.cpp b/test/test_connection_timeout.cpp index bbd87e145..8b9bd31f8 100644 --- a/test/test_connection_timeout.cpp +++ b/test/test_connection_timeout.cpp @@ -126,7 +126,6 @@ TEST_F(TestConnectionTimeout, Nonblocking) { int wlen = 2; SRTSOCKET write[2]; - using namespace std; const chrono::steady_clock::time_point chrono_ts_start = chrono::steady_clock::now(); // Here we check the connection timeout. @@ -143,9 +142,8 @@ TEST_F(TestConnectionTimeout, Nonblocking) { const chrono::steady_clock::time_point chrono_ts_end = chrono::steady_clock::now(); const auto delta_ms = chrono::duration_cast(chrono_ts_end - chrono_ts_start).count(); // Confidence interval border : +/-80 ms - EXPECT_LE(delta_ms, connection_timeout_ms + 80); - EXPECT_GE(delta_ms, connection_timeout_ms - 80); - cerr << "Timeout was: " << delta_ms << "\n"; + EXPECT_LE(delta_ms, connection_timeout_ms + 80) << "Timeout was: " << delta_ms; + EXPECT_GE(delta_ms, connection_timeout_ms - 80) << "Timeout was: " << delta_ms; EXPECT_EQ(rlen, 1); EXPECT_EQ(read[0], client_sock); @@ -186,8 +184,14 @@ TEST_F(TestConnectionTimeout, BlockingLoop) const sockaddr* psa = reinterpret_cast(&m_sa); for (int i = 0; i < 10; ++i) { + const chrono::steady_clock::time_point chrono_ts_start = chrono::steady_clock::now(); EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR); + const auto delta_ms = chrono::duration_cast(chrono::steady_clock::now() - chrono_ts_start).count(); + // Confidence interval border : +/-200 ms + EXPECT_LE(delta_ms, connection_timeout_ms + 200) << "Timeout was: " << delta_ms; + EXPECT_GE(delta_ms, connection_timeout_ms - 200) << "Timeout was: " << delta_ms; + const int error_code = srt_getlasterror(nullptr); EXPECT_EQ(error_code, SRT_ENOSERVER); if (error_code != SRT_ENOSERVER)