From 2fcd3d443e343ea3df3f06a7edcfad1a920f431d Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Mon, 17 Apr 2023 16:56:15 +0200 Subject: [PATCH 01/37] [core] Fix crypto mode auto for listener sender (#2711). Co-authored-by: oviano --- srtcore/core.cpp | 51 ++++++++++++++++++++++++++++++++++++------------ srtcore/core.h | 6 +++++- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index abede00808..7205f85740 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -4179,7 +4179,7 @@ EConnectStatus srt::CUDT::processRendezvous( // The CryptoControl must be created by the prepareConnectionObjects() before interpreting and creating HSv5 extensions // because the it will be used there. - if (!prepareConnectionObjects(m_ConnRes, m_SrtHsSide, NULL)) + if (!prepareConnectionObjects(m_ConnRes, m_SrtHsSide, NULL) || !prepareBuffers(NULL)) { // m_RejectReason already handled HLOGC(cnlog.Debug, @@ -4734,6 +4734,7 @@ EConnectStatus srt::CUDT::postConnect(const CPacket* pResponse, bool rendezvous, // In this situation the interpretation of handshake was already done earlier. ok = ok && pResponse->isControl(); ok = ok && interpretSrtHandshake(m_ConnRes, *pResponse, 0, 0); + ok = ok && prepareBuffers(eout); if (!ok) { @@ -5548,7 +5549,7 @@ bool srt::CUDT::prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd // code with HSv5 rendezvous, in which this will be run // in a little bit "randomly selected" moment, but must // be run once in the whole connection process. - if (m_pSndBuffer) + if (m_pCryptoControl) { HLOGC(rslog.Debug, log << CONID() << "prepareConnectionObjects: (lazy) already created."); return true; @@ -5573,9 +5574,29 @@ bool srt::CUDT::prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd } } + if (!createCrypter(hsd, bidirectional)) // Make sure CC is created (lazy) + { + if (eout) + *eout = CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0); + m_RejectReason = SRT_REJ_RESOURCE; + return false; + } + + return true; +} + +bool srt::CUDT::prepareBuffers(CUDTException* eout) +{ + if (m_pSndBuffer) + { + HLOGC(rslog.Debug, log << CONID() << "prepareBuffers: (lazy) already created."); + return true; + } + try { - const int authtag = m_config.iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM ? HAICRYPT_AUTHTAG_MAX : 0; + // CryptoControl has to be initialized and in case of RESPONDER the KM REQ must be processed (interpretSrtHandshake(..)) for the crypto mode to be deduced. + const int authtag = (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) ? HAICRYPT_AUTHTAG_MAX : 0; m_pSndBuffer = new CSndBuffer(32, m_iMaxSRTPayloadSize, authtag); SRT_ASSERT(m_iISN != -1); m_pRcvBuffer = new srt::CRcvBuffer(m_iISN, m_config.iRcvBufSize, m_pRcvQueue->m_pUnitQueue, m_config.bMessageAPI); @@ -5587,19 +5608,10 @@ bool srt::CUDT::prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd { // Simply reject. if (eout) - { *eout = CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0); - } m_RejectReason = SRT_REJ_RESOURCE; return false; } - - if (!createCrypter(hsd, bidirectional)) // Make sure CC is created (lazy) - { - m_RejectReason = SRT_REJ_RESOURCE; - return false; - } - return true; } @@ -5710,6 +5722,19 @@ void srt::CUDT::acceptAndRespond(const sockaddr_any& agent, const sockaddr_any& throw CUDTException(MJ_SETUP, MN_REJECTED, 0); } + if (!prepareBuffers(NULL)) + { + HLOGC(cnlog.Debug, + log << CONID() << "acceptAndRespond: prepareConnectionObjects failed - responding with REJECT."); + // If the SRT buffers failed to be allocated, + // the connection must be rejected. + // + // Respond with the rejection message and exit with exception + // so that the caller will know that this new socket should be deleted. + w_hs.m_iReqType = URQFailure(m_RejectReason); + throw CUDTException(MJ_SETUP, MN_REJECTED, 0); + } + // Synchronize the time NOW because the following function is about // to use the start time to pass it to the receiver buffer data. bool have_group = false; @@ -9196,7 +9221,7 @@ int srt::CUDT::packLostData(CPacket& w_packet) // The packet has been ecrypted, thus the authentication tag is expected to be stored // in the SND buffer as well right after the payload. - if (m_config.iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM) + if (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) { w_packet.setLength(w_packet.getLength() + HAICRYPT_AUTHTAG_MAX); } diff --git a/srtcore/core.h b/srtcore/core.h index ce0e539587..e7ca57c508 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -487,10 +487,14 @@ class CUDT EConnectStatus processRendezvous(const CPacket* response, const sockaddr_any& serv_addr, EReadStatus, CPacket& reqpkt); void sendRendezvousRejection(const sockaddr_any& serv_addr, CPacket& request); - /// Create the CryptoControl object based on the HS packet. Allocates sender and receiver buffers and loss lists. + /// Create the CryptoControl object based on the HS packet. SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) bool prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd, CUDTException *eout); + /// Allocates sender and receiver buffers and loss lists. + SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) + bool prepareBuffers(CUDTException* eout); + SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) EConnectStatus postConnect(const CPacket* response, bool rendezvous, CUDTException* eout) ATR_NOEXCEPT; From 9fc5c0924c70d4d15b28379e49a7f1d643856d5f Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Tue, 18 Apr 2023 12:00:54 +0200 Subject: [PATCH 02/37] [build] Upgraded CI: ubuntu to version 20.04 (#2682). --- .github/workflows/android.yaml | 2 +- .github/workflows/cxx11-ubuntu.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/android.yaml b/.github/workflows/android.yaml index 713c3f331c..0af85fda38 100644 --- a/.github/workflows/android.yaml +++ b/.github/workflows/android.yaml @@ -9,7 +9,7 @@ on: jobs: build: name: NDK-R23 - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - name: Setup Android NDK R23 diff --git a/.github/workflows/cxx11-ubuntu.yaml b/.github/workflows/cxx11-ubuntu.yaml index 751b3fda12..500ff1beb5 100644 --- a/.github/workflows/cxx11-ubuntu.yaml +++ b/.github/workflows/cxx11-ubuntu.yaml @@ -9,7 +9,7 @@ on: jobs: build: name: ubuntu - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v3 From 4c9a4178d77543aa816cdb09b382e5163bc40a18 Mon Sep 17 00:00:00 2001 From: Maria Sharabayko <41019697+mbakholdina@users.noreply.github.com> Date: Tue, 18 Apr 2023 14:28:49 +0200 Subject: [PATCH 03/37] [docs] Added the link for registration in slack to the getting started table (#2721). --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 85a8417c20..8b263aebb2 100644 --- a/README.md +++ b/README.md @@ -144,14 +144,14 @@ In live streaming configurations, the SRT protocol maintains a constant end-to-e ## Getting Started with SRT -| | | | -|:-----------------------------------------------------------------------------------------------------------------------------:|:----------------------------------------------------------------------------------:|:---------------------------------------------------------------------------------:| -| [The SRT API](./docs#srt-api-documents) | [IETF Internet Draft](https://datatracker.ietf.org/doc/html/draft-sharabayko-srt-01) | [Sample Apps](./docs#sample-applications) | -| Reference documentation for the SRT library API | The SRT Protocol Internet Draft | Instructions for using test apps (`srt-live-transmit`, `srt-file-transmit`, etc.) | -| [SRT Technical Overview](https://github.com/Haivision/srt/files/2489142/SRT_Protocol_TechnicalOverview_DRAFT_2018-10-17.pdf) | [SRT Deployment Guide](https://www.srtalliance.org/srt-deployment-guide/) | [SRT CookBook](https://srtlab.github.io/srt-cookbook) | -| Early draft technical overview (precursor to the Internet Draft) | A comprehensive overview of the protocol with deployment guidelines | Development notes on the SRT protocol | -| [Innovation Labs Blog](https://medium.com/innovation-labs-blog/tagged/secure-reliable-transport) | [SRTLab YouTube Channel](https://www.youtube.com/channel/UCr35JJ32jKKWIYymR1PTdpA) | [Slack](https://srtalliance.slack.com) | -| The blog on Medium with SRT-related technical articles | Technical YouTube channel with useful videos | Slack channels to get the latest updates and ask questions | +| | | | +|:-----------------------------------------------------------------------------------------------------------------------------:|:------------------------------------------------------------------------------------:|:---------------------------------------------------------------------------------:| +| [The SRT API](./docs#srt-api-documents) | [IETF Internet Draft](https://datatracker.ietf.org/doc/html/draft-sharabayko-srt-01) | [Sample Apps](./docs#sample-applications) | +| Reference documentation for the SRT library API | The SRT Protocol Internet Draft | Instructions for using test apps (`srt-live-transmit`, `srt-file-transmit`, etc.) | +| [SRT Technical Overview](https://github.com/Haivision/srt/files/2489142/SRT_Protocol_TechnicalOverview_DRAFT_2018-10-17.pdf) | [SRT Deployment Guide](https://www.srtalliance.org/srt-deployment-guide/) | [SRT CookBook](https://srtlab.github.io/srt-cookbook) | +| Early draft technical overview (precursor to the Internet Draft) | A comprehensive overview of the protocol with deployment guidelines | Development notes on the SRT protocol | +| [Innovation Labs Blog](https://medium.com/innovation-labs-blog/tagged/secure-reliable-transport) | [SRTLab YouTube Channel](https://www.youtube.com/channel/UCr35JJ32jKKWIYymR1PTdpA) | [Slack](https://srtalliance.slack.com) | +| The blog on Medium with SRT-related technical articles | Technical YouTube channel with useful videos | Slack channels to get the latest updates and ask questions
[Join SRT Alliance on Slack](https://slackin-srtalliance.azurewebsites.net/) | ### Additional Documentation From 59cde5394510d5dc36828b04ddf8b9bc50b5ea9d Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 19 Apr 2023 16:35:36 +0200 Subject: [PATCH 04/37] [core] Fixed FEC Emergency resize crash (#2717). Fixed minimum history condition. --- srtcore/fec.cpp | 36 ++++++++++++++++++++++++++++-------- srtcore/fec.h | 2 +- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/srtcore/fec.cpp b/srtcore/fec.cpp index ad794e1f5a..2eea1ca5e9 100644 --- a/srtcore/fec.cpp +++ b/srtcore/fec.cpp @@ -1569,11 +1569,10 @@ size_t FECFilterBuiltin::ExtendRows(size_t rowx) const size_t size_in_packets = rowx * numberCols(); const int n_series = int(rowx / numberRows()); - if (size_in_packets > rcvBufferSize() && n_series > 2) + if (CheckEmergencyShrink(n_series, size_in_packets)) { - HLOGC(pflog.Debug, log << "FEC: Emergency resize, rowx=" << rowx << " series=" << n_series + HLOGC(pflog.Debug, log << "FEC: DONE Emergency resize, rowx=" << rowx << " series=" << n_series << "npackets=" << size_in_packets << " exceeds buf=" << rcvBufferSize()); - EmergencyShrink(n_series); } // Create and configure next groups. @@ -1701,8 +1700,27 @@ bool FECFilterBuiltin::IsLost(int32_t seq) const return rcv.cells[offset]; } -void FECFilterBuiltin::EmergencyShrink(size_t n_series) +bool FECFilterBuiltin::CheckEmergencyShrink(size_t n_series, size_t size_in_packets) { + // The minimum required size of the covered sequence range must be such + // that groups for packets from the previous range must be still reachable. + // It's then "this and previous" series in case of even arrangement. + // + // For staircase arrangement the potential range for a single column series + // (number of columns equal to a row size) spans for 2 matrices (rows * cols) + // minus one row. As dismissal is only allowed to be done by one full series + // of rows and columns, the history must keep as many groups as needed to reach + // out for this very packet of this group and all packets in the same row. + // Hence we need two series of columns to cover a similar range as two row, twice. + + const size_t min_series_history = m_arrangement_staircase ? 4 : 2; + + if (n_series <= min_series_history) + return false; + + if (size_in_packets < rcvBufferSize() && n_series < SRT_FEC_MAX_RCV_HISTORY) + return false; + // Shrink is required in order to prepare place for // either vertical or horizontal group in series `n_series`. @@ -1783,7 +1801,7 @@ void FECFilterBuiltin::EmergencyShrink(size_t n_series) else { HLOGC(pflog.Debug, log << "FEC: Shifting rcv row %" << oldbase << " -> %" << newbase); - rcv.rowq.erase(rcv.rowq.begin(), rcv.rowq.end() + shift_rows); + rcv.rowq.erase(rcv.rowq.begin(), rcv.rowq.begin() + shift_rows); } const size_t shift_cols = shift_series * numberCols(); @@ -1817,6 +1835,8 @@ void FECFilterBuiltin::EmergencyShrink(size_t n_series) rcv.cells.push_back(false); } rcv.cell_base = newbase; + + return true; } FECFilterBuiltin::EHangStatus FECFilterBuiltin::HangVertical(const CPacket& rpkt, signed char fec_col, loss_seqs_t& irrecover) @@ -2490,11 +2510,11 @@ size_t FECFilterBuiltin::ExtendColumns(size_t colgx) // of packets as many as the number of rows, so simply multiply this. const size_t size_in_packets = colgx * numberRows(); const size_t n_series = colgx / numberCols(); - if (size_in_packets > rcvBufferSize()/2 || n_series > SRT_FEC_MAX_RCV_HISTORY) + + if (CheckEmergencyShrink(n_series, size_in_packets)) { - HLOGC(pflog.Debug, log << "FEC: Emergency resize, colgx=" << colgx << " series=" << n_series + HLOGC(pflog.Debug, log << "FEC: DONE Emergency resize, colgx=" << colgx << " series=" << n_series << "npackets=" << size_in_packets << " exceeds buf=" << rcvBufferSize()); - EmergencyShrink(n_series); } else { diff --git a/srtcore/fec.h b/srtcore/fec.h index 23b5a58f11..0297094752 100644 --- a/srtcore/fec.h +++ b/srtcore/fec.h @@ -222,7 +222,7 @@ class FECFilterBuiltin: public SrtPacketFilterBase void RcvRebuild(Group& g, int32_t seqno, Group::Type tp); int32_t RcvGetLossSeqHoriz(Group& g); int32_t RcvGetLossSeqVert(Group& g); - void EmergencyShrink(size_t n_series); + bool CheckEmergencyShrink(size_t n_series, size_t size_in_packets); static void TranslateLossRecords(const std::set& loss, loss_seqs_t& irrecover); void RcvCheckDismissColumn(int32_t seqno, int colgx, loss_seqs_t& irrecover); From 6fcff6dfe4eed9df17a05168b88b5b069808c91c Mon Sep 17 00:00:00 2001 From: oviano Date: Fri, 21 Apr 2023 10:06:26 +0100 Subject: [PATCH 05/37] [core] Fixed various compiler warnings on various platforms (#2679). --- apps/apputil.cpp | 7 +++++-- apps/srt-live-transmit.cpp | 10 ++++++++-- apps/srt-tunnel.cpp | 18 +++++++++--------- apps/transmitmedia.cpp | 4 ++-- apps/uriparser.cpp | 6 +++--- haicrypt/cryspr-mbedtls.c | 1 - haicrypt/cryspr-openssl-evp.c | 10 +++++----- haicrypt/cryspr-openssl.c | 6 +++--- haicrypt/hcrypt.c | 2 +- srtcore/common.h | 6 ++++-- srtcore/core.cpp | 16 ++++++++-------- srtcore/epoll.h | 5 ++++- srtcore/fec.cpp | 12 ++++++------ srtcore/logging.h | 18 ++++++++++++++---- srtcore/platform_sys.h | 1 - srtcore/socketconfig.h | 10 ++++++++++ srtcore/srt_compat.c | 4 ++++ testing/srt-test-live.cpp | 10 ++++++++-- 18 files changed, 94 insertions(+), 52 deletions(-) diff --git a/apps/apputil.cpp b/apps/apputil.cpp index e5290408a1..22c31521bf 100644 --- a/apps/apputil.cpp +++ b/apps/apputil.cpp @@ -49,9 +49,12 @@ int inet_pton(int af, const char * src, void * dst) ZeroMemory(&ss, sizeof(ss)); // work around non-const API - strncpy(srcCopy, src, INET6_ADDRSTRLEN + 1); +#ifdef _MSC_VER + strncpy_s(srcCopy, INET6_ADDRSTRLEN + 1, src, _TRUNCATE); +#else + strncpy(srcCopy, src, INET6_ADDRSTRLEN); srcCopy[INET6_ADDRSTRLEN] = '\0'; - +#endif if (WSAStringToAddress( srcCopy, af, NULL, (struct sockaddr *)&ss, &ssSize) != 0) { diff --git a/apps/srt-live-transmit.cpp b/apps/srt-live-transmit.cpp index 40e1e32ff3..24ed33be7f 100644 --- a/apps/srt-live-transmit.cpp +++ b/apps/srt-live-transmit.cpp @@ -852,8 +852,14 @@ int main(int argc, char** argv) void TestLogHandler(void* opaque, int level, const char* file, int line, const char* area, const char* message) { char prefix[100] = ""; - if ( opaque ) - strncpy(prefix, (char*)opaque, 99); + if ( opaque ) { +#ifdef _MSC_VER + strncpy_s(prefix, sizeof(prefix), (char*)opaque, _TRUNCATE); +#else + strncpy(prefix, (char*)opaque, sizeof(prefix) - 1); + prefix[sizeof(prefix) - 1] = '\0'; +#endif + } time_t now; time(&now); char buf[1024]; diff --git a/apps/srt-tunnel.cpp b/apps/srt-tunnel.cpp index d00c42f4ce..569a0c26e1 100644 --- a/apps/srt-tunnel.cpp +++ b/apps/srt-tunnel.cpp @@ -299,7 +299,7 @@ class Tunnel Tunnel(Tunnelbox* m, std::unique_ptr&& acp, std::unique_ptr&& clr): parent_box(m), - med_acp(move(acp)), med_clr(move(clr)), + med_acp(std::move(acp)), med_clr(std::move(clr)), acp_to_clr(this, med_acp.get(), med_clr.get(), med_acp->id() + ">" + med_clr->id()), clr_to_acp(this, med_clr.get(), med_acp.get(), med_clr->id() + ">" + med_acp->id()) { @@ -649,7 +649,7 @@ void TcpMedium::CreateListener() sockaddr_any sa = CreateAddr(m_uri.host(), m_uri.portno()); - m_socket = socket(sa.get()->sa_family, SOCK_STREAM, IPPROTO_TCP); + m_socket = (int)socket(sa.get()->sa_family, SOCK_STREAM, IPPROTO_TCP); ConfigurePre(); int stat = ::bind(m_socket, sa.get(), sa.size()); @@ -694,7 +694,7 @@ unique_ptr SrtMedium::Accept() unique_ptr TcpMedium::Accept() { sockaddr_any sa; - int s = ::accept(m_socket, (sa.get()), (&sa.syslen())); + int s = (int)::accept(m_socket, (sa.get()), (&sa.syslen())); if (s == -1) { Error(errno, "accept"); @@ -726,7 +726,7 @@ void SrtMedium::CreateCaller() void TcpMedium::CreateCaller() { - m_socket = ::socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + m_socket = (int)::socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); ConfigurePre(); } @@ -850,7 +850,7 @@ Medium::ReadStatus Medium::Read(bytevector& w_output) size_t pred_size = shift + m_chunk; w_output.resize(pred_size); - int st = ReadInternal((w_output.data() + shift), m_chunk); + int st = ReadInternal((w_output.data() + shift), (int)m_chunk); if (st == -1) { if (IsErrorAgain()) @@ -884,7 +884,7 @@ Medium::ReadStatus Medium::Read(bytevector& w_output) void SrtMedium::Write(bytevector& w_buffer) { - int st = srt_send(m_socket, w_buffer.data(), w_buffer.size()); + int st = srt_send(m_socket, w_buffer.data(), (int)w_buffer.size()); if (st == SRT_ERROR) { Error(UDT::getlasterror(), "srt_send"); @@ -907,7 +907,7 @@ void SrtMedium::Write(bytevector& w_buffer) void TcpMedium::Write(bytevector& w_buffer) { - int st = ::send(m_socket, w_buffer.data(), w_buffer.size(), DEF_SEND_FLAG); + int st = ::send(m_socket, w_buffer.data(), (int)w_buffer.size(), DEF_SEND_FLAG); if (st == -1) { Error(errno, "send"); @@ -971,7 +971,7 @@ struct Tunnelbox lock_guard lk(access); Verb() << "Tunnelbox: Starting tunnel: " << acp->uri() << " <-> " << clr->uri(); - tunnels.emplace_back(new Tunnel(this, move(acp), move(clr))); + tunnels.emplace_back(new Tunnel(this, std::move(acp), std::move(clr))); // Note: after this instruction, acp and clr are no longer valid! auto& it = tunnels.back(); @@ -1191,7 +1191,7 @@ int main( int argc, char** argv ) Verb() << "Connected. Establishing pipe."; // No exception, we are free to pass :) - g_tunnels.install(move(accepted), move(caller)); + g_tunnels.install(std::move(accepted), std::move(caller)); } catch (...) { diff --git a/apps/transmitmedia.cpp b/apps/transmitmedia.cpp index 260323ef3b..87899e01dd 100644 --- a/apps/transmitmedia.cpp +++ b/apps/transmitmedia.cpp @@ -805,7 +805,7 @@ class UdpCommon void Setup(string host, int port, map attr) { - m_sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + m_sock = (int)socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (m_sock == -1) Error(SysError(), "UdpCommon::Setup: socket"); @@ -1144,7 +1144,7 @@ extern unique_ptr CreateMedium(const string& uri) } if (ptr.get()) - ptr->uri = move(u); + ptr->uri = std::move(u); return ptr; } diff --git a/apps/uriparser.cpp b/apps/uriparser.cpp index d962d1bb30..4f6bcc2c6e 100644 --- a/apps/uriparser.cpp +++ b/apps/uriparser.cpp @@ -156,7 +156,7 @@ void UriParser::Parse(const string& strUrl, DefaultExpect exp) if (idx != string::npos) { m_host = strUrl.substr(0, idx); - iQueryStart = idx + 1; + iQueryStart = (int)(idx + 1); } else { @@ -297,12 +297,12 @@ void UriParser::Parse(const string& strUrl, DefaultExpect exp) if (idx != string::npos) { strQueryPair = strUrl.substr(iQueryStart, idx - iQueryStart); - iQueryStart = idx + 1; + iQueryStart = (int)(idx + 1); } else { strQueryPair = strUrl.substr(iQueryStart, strUrl.size() - iQueryStart); - iQueryStart = idx; + iQueryStart = (int)idx; } idx = strQueryPair.find("="); diff --git a/haicrypt/cryspr-mbedtls.c b/haicrypt/cryspr-mbedtls.c index 7697e63b33..a961cca61e 100644 --- a/haicrypt/cryspr-mbedtls.c +++ b/haicrypt/cryspr-mbedtls.c @@ -160,7 +160,6 @@ int crysprMbedtls_AES_CtrCipher( /* AES-CTR128 Encryption */ static CRYSPR_cb *crysprMbedtls_Open(CRYSPR_methods *cryspr, size_t max_len) { crysprMbedtls_cb *aes_data; - CRYSPR_cb *cryspr_cb; aes_data = (crysprMbedtls_cb *)crysprHelper_Open(cryspr, sizeof(crysprMbedtls_cb), max_len); if (NULL == aes_data) { diff --git a/haicrypt/cryspr-openssl-evp.c b/haicrypt/cryspr-openssl-evp.c index 0d52509036..3e87e9a5ef 100644 --- a/haicrypt/cryspr-openssl-evp.c +++ b/haicrypt/cryspr-openssl-evp.c @@ -49,7 +49,7 @@ int crysprOpenSSL_EVP_AES_SetKey( CRYSPR_AESCTX* aes_key) /* CRYpto Service PRovider AES Key context */ { const EVP_CIPHER* cipher = NULL; - int idxKlen = (kstr_len / 8) - 2; /* key_len index in cipher_fnptr array in [0,1,2] range */ + int idxKlen = (int)((kstr_len / 8) - 2); /* key_len index in cipher_fnptr array in [0,1,2] range */ switch (cipher_type) { @@ -143,7 +143,7 @@ int crysprOpenSSL_EVP_AES_EcbCipher(bool bEncrypt, /* true:encry size_t* outlen_p) /* in/out dst len */ { int nmore = inlen % CRYSPR_AESBLKSZ; /* bytes in last incomplete block */ - int nblk = inlen / CRYSPR_AESBLKSZ + (nmore ? 1 : 0); /* blocks including incomplete */ + int nblk = (int)(inlen / CRYSPR_AESBLKSZ + (nmore ? 1 : 0)); /* blocks including incomplete */ size_t outsiz = (outlen_p ? *outlen_p : 0); int c_len = 0, f_len = 0; @@ -174,7 +174,7 @@ int crysprOpenSSL_EVP_AES_EcbCipher(bool bEncrypt, /* true:encry /* update ciphertext, c_len is filled with the length of ciphertext generated, * cryptoPtr->cipher_in_len is the size of plain/cipher text in bytes */ - if (!EVP_CipherUpdate(aes_key, out_txt, &c_len, indata, inlen)) + if (!EVP_CipherUpdate(aes_key, out_txt, &c_len, indata, (int)inlen)) { HCRYPT_LOG(LOG_ERR, "EVP_CipherUpdate(%p, out, %d, in, %d) failed\n", aes_key, c_len, inlen); return -1; @@ -227,7 +227,7 @@ int crysprOpenSSL_EVP_AES_CtrCipher(bool bEncrypt, /* true:encry /* update ciphertext, c_len is filled with the length of ciphertext generated, * cryptoPtr->cipher_in_len is the size of plain/cipher text in bytes */ - if (!EVP_CipherUpdate(aes_key, out_txt, &c_len, indata, inlen)) + if (!EVP_CipherUpdate(aes_key, out_txt, &c_len, indata, (int)inlen)) { HCRYPT_LOG(LOG_ERR, "%s\n", "EVP_CipherUpdate() failed"); return -1; @@ -341,7 +341,7 @@ int crysprOpenSSL_EVP_KmPbkdf2(CRYSPR_cb* cryspr_cb, unsigned char* out) /* derived key */ { (void)cryspr_cb; - int rc = PKCS5_PBKDF2_HMAC_SHA1(passwd, passwd_len, salt, salt_len, itr, key_len, out); + int rc = PKCS5_PBKDF2_HMAC_SHA1(passwd, (int)passwd_len, salt, (int)salt_len, itr, (int)key_len, out); return (rc == 1 ? 0 : -1); } diff --git a/haicrypt/cryspr-openssl.c b/haicrypt/cryspr-openssl.c index 3af907d242..87eb535ec5 100644 --- a/haicrypt/cryspr-openssl.c +++ b/haicrypt/cryspr-openssl.c @@ -48,12 +48,12 @@ int crysprOpenSSL_AES_SetKey( (void)cipher_type; if (bEncrypt) { /* Encrypt key */ - if (AES_set_encrypt_key(kstr, kstr_len * 8, aes_key)) { + if (AES_set_encrypt_key(kstr, (int)(kstr_len * 8), aes_key)) { HCRYPT_LOG(LOG_ERR, "%s", "AES_set_encrypt_key(kek) failed\n"); return(-1); } } else { /* Decrypt key */ - if (AES_set_decrypt_key(kstr, kstr_len * 8, aes_key)) { + if (AES_set_decrypt_key(kstr, (int)(kstr_len * 8), aes_key)) { HCRYPT_LOG(LOG_ERR, "%s", "AES_set_decrypt_key(kek) failed\n"); return(-1); } @@ -163,7 +163,7 @@ int crysprOpenSSL_KmPbkdf2( unsigned char *out) /* derived key */ { (void)cryspr_cb; - int rc = PKCS5_PBKDF2_HMAC_SHA1(passwd,passwd_len,salt,salt_len,itr,key_len,out); + int rc = PKCS5_PBKDF2_HMAC_SHA1(passwd,(int)passwd_len,salt,(int)salt_len,itr,(int)key_len,out); return(rc == 1? 0 : -1); } diff --git a/haicrypt/hcrypt.c b/haicrypt/hcrypt.c index d5178ccdd6..3b4b98ece1 100644 --- a/haicrypt/hcrypt.c +++ b/haicrypt/hcrypt.c @@ -339,7 +339,7 @@ int HaiCrypt_Close(HaiCrypt_Handle hhc) return rc; } -int HaiCrypt_IsAESGCM_Supported() +int HaiCrypt_IsAESGCM_Supported(void) { #if CRYSPR_HAS_AESGCM return 1; diff --git a/srtcore/common.h b/srtcore/common.h index 5f3dd7f181..5021fa5a88 100644 --- a/srtcore/common.h +++ b/srtcore/common.h @@ -53,7 +53,6 @@ modified by #ifndef INC_SRT_COMMON_H #define INC_SRT_COMMON_H -#define _CRT_SECURE_NO_WARNINGS 1 // silences windows complaints for sscanf #include #include #include @@ -1393,8 +1392,11 @@ inline ATR_CONSTEXPR uint32_t SrtVersion(int major, int minor, int patch) inline int32_t SrtParseVersion(const char* v) { int major, minor, patch; +#if defined(_MSC_VER) + int result = sscanf_s(v, "%d.%d.%d", &major, &minor, &patch); +#else int result = sscanf(v, "%d.%d.%d", &major, &minor, &patch); - +#endif if (result != 3) { return 0; diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 7205f85740..1386b97a20 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -614,8 +614,8 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen) } // Fallback: return from internal data - strcpy(((char*)optval), m_config.sBindToDevice.c_str()); - optlen = m_config.sBindToDevice.size(); + optlen = (int)m_config.sBindToDevice.copy((char*)optval, (size_t)optlen - 1); + ((char*)optval)[optlen] = '\0'; #else LOGC(smlog.Error, log << "SRTO_BINDTODEVICE is not supported on that platform"); throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); @@ -737,16 +737,16 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen) if (size_t(optlen) < m_config.sStreamName.size() + 1) throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); - strcpy((char *)optval, m_config.sStreamName.c_str()); - optlen = (int) m_config.sStreamName.size(); + optlen = (int)m_config.sStreamName.copy((char*)optval, (size_t)optlen - 1); + ((char*)optval)[optlen] = '\0'; break; case SRTO_CONGESTION: if (size_t(optlen) < m_config.sCongestion.size() + 1) throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); - strcpy((char *)optval, m_config.sCongestion.c_str()); - optlen = (int) m_config.sCongestion.size(); + optlen = (int)m_config.sCongestion.copy((char*)optval, (size_t)optlen - 1); + ((char*)optval)[optlen] = '\0'; break; case SRTO_MESSAGEAPI: @@ -805,8 +805,8 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen) if (size_t(optlen) < m_config.sPacketFilterConfig.size() + 1) throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); - strcpy((char *)optval, m_config.sPacketFilterConfig.c_str()); - optlen = (int) m_config.sPacketFilterConfig.size(); + optlen = (int)m_config.sPacketFilterConfig.copy((char*)optval, (size_t)optlen - 1); + ((char*)optval)[optlen] = '\0'; break; case SRTO_RETRANSMITALGO: diff --git a/srtcore/epoll.h b/srtcore/epoll.h index 7b0d941c8d..00d46ceb41 100644 --- a/srtcore/epoll.h +++ b/srtcore/epoll.h @@ -69,8 +69,11 @@ class CUDTGroup; class CEPollDesc { +#ifdef __GNUG__ const int m_iID; // epoll ID - +#else + const int m_iID SRT_ATR_UNUSED; // epoll ID +#endif struct Wait; struct Notice: public SRT_EPOLL_EVENT diff --git a/srtcore/fec.cpp b/srtcore/fec.cpp index 2eea1ca5e9..a41e3a33bb 100644 --- a/srtcore/fec.cpp +++ b/srtcore/fec.cpp @@ -345,13 +345,13 @@ void FECFilterBuiltin::ConfigureColumns(Container& which, int32_t isn) { offset = col + 1; // +1 because we want it for the next column HLOGC(pflog.Debug, log << "ConfigureColumns: [" << (col+1) << "]... (resetting to row 0: +" - << offset << " %" << CSeqNo::incseq(isn, offset) << ")"); + << offset << " %" << CSeqNo::incseq(isn, (int32_t)offset) << ")"); } else { offset += 1 + sizeRow(); HLOGC(pflog.Debug, log << "ConfigureColumns: [" << (col+1) << "] ... (continue +" - << offset << " %" << CSeqNo::incseq(isn, offset) << ")"); + << offset << " %" << CSeqNo::incseq(isn, (int32_t)offset) << ")"); } } } @@ -1129,10 +1129,10 @@ static void DebugPrintCells(int32_t base, const std::deque& cells, size_t for ( ; i < cells.size(); i += row_size ) { std::ostringstream os; - os << "cell[" << i << "-" << (i+row_size-1) << "] %" << CSeqNo::incseq(base, i) << ":"; + os << "cell[" << i << "-" << (i+row_size-1) << "] %" << CSeqNo::incseq(base, (int32_t)i) << ":"; for (size_t y = 0; y < row_size; ++y) { - os << " " << CellMark(cells, i+y); + os << " " << CellMark(cells, (int)(i+y)); } LOGP(pflog.Debug, os.str()); } @@ -1953,7 +1953,7 @@ void FECFilterBuiltin::RcvCheckDismissColumn(int32_t seq, int colgx, loss_seqs_t { HLOGC(pflog.Debug, log << "FEC/V: ... [" << i << "] base=%" << pg.base << " TOO EARLY (last=%" - << CSeqNo::incseq(pg.base, (sizeCol()-1)*sizeRow()) + << CSeqNo::incseq(pg.base, (int32_t)((sizeCol()-1)*sizeRow())) << ")"); continue; } @@ -1964,7 +1964,7 @@ void FECFilterBuiltin::RcvCheckDismissColumn(int32_t seq, int colgx, loss_seqs_t HLOGC(pflog.Debug, log << "FEC/V: ... [" << i << "] base=%" << pg.base << " - PAST last=%" - << CSeqNo::incseq(pg.base, (sizeCol()-1)*sizeRow()) + << CSeqNo::incseq(pg.base, (int32_t)((sizeCol()-1)*sizeRow())) << " - collecting losses."); pg.dismissed = true; // mark irrecover already collected diff --git a/srtcore/logging.h b/srtcore/logging.h index 91fbc5c318..e90ad4ac21 100644 --- a/srtcore/logging.h +++ b/srtcore/logging.h @@ -166,14 +166,24 @@ struct SRT_API LogDispatcher // See Logger::Logger; we know this has normally 2 characters, // except !!FATAL!!, which has 9. Still less than 32. - strcpy(prefix, your_pfx); - // If the size of the FA name together with severity exceeds the size, // just skip the former. if (logger_pfx && strlen(prefix) + strlen(logger_pfx) + 1 < MAX_PREFIX_SIZE) { - strcat(prefix, ":"); - strcat(prefix, logger_pfx); +#if defined(_MSC_VER) && _MSC_VER < 1900 + _snprintf(prefix, MAX_PREFIX_SIZE, "%s:%s", your_pfx, logger_pfx); +#else + snprintf(prefix, MAX_PREFIX_SIZE + 1, "%s:%s", your_pfx, logger_pfx); +#endif + } + else + { +#ifdef _MSC_VER + strncpy_s(prefix, MAX_PREFIX_SIZE + 1, your_pfx, _TRUNCATE); +#else + strncpy(prefix, your_pfx, MAX_PREFIX_SIZE); + prefix[MAX_PREFIX_SIZE] = '\0'; +#endif } } diff --git a/srtcore/platform_sys.h b/srtcore/platform_sys.h index fae2cc07cd..e2f0aa4d9c 100644 --- a/srtcore/platform_sys.h +++ b/srtcore/platform_sys.h @@ -24,7 +24,6 @@ #ifdef _WIN32 - #define _CRT_SECURE_NO_WARNINGS 1 // silences windows complaints for sscanf #include #include #include diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index 488d12fb10..be4520cd7f 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -158,6 +158,16 @@ class StringStorage return set(s.c_str(), s.size()); } + size_t copy(char* s, size_t length) const + { + if (!s) + return 0; + + size_t copy_len = std::min((size_t)len, length); + memcpy(s, stor, copy_len); + return copy_len; + } + std::string str() const { return len == 0 ? std::string() : std::string(stor); diff --git a/srtcore/srt_compat.c b/srtcore/srt_compat.c index 8241fadc42..fbf4859ae2 100644 --- a/srtcore/srt_compat.c +++ b/srtcore/srt_compat.c @@ -87,8 +87,12 @@ extern const char * SysStrError(int errnum, char * buf, size_t buflen) if (lpMsgBuf) { +#ifdef _MSC_VER + strncpy_s(buf, buflen, lpMsgBuf, _TRUNCATE); +#else strncpy(buf, lpMsgBuf, buflen-1); buf[buflen-1] = 0; +#endif LocalFree((HLOCAL)lpMsgBuf); } else diff --git a/testing/srt-test-live.cpp b/testing/srt-test-live.cpp index 7bf6bb51cb..e1fd2bad6f 100644 --- a/testing/srt-test-live.cpp +++ b/testing/srt-test-live.cpp @@ -994,8 +994,14 @@ int main( int argc, char** argv ) void TestLogHandler(void* opaque, int level, const char* file, int line, const char* area, const char* message) { char prefix[100] = ""; - if ( opaque ) - strncpy(prefix, (char*)opaque, 99); + if ( opaque ) { +#ifdef _MSC_VER + strncpy_s(prefix, sizeof(prefix), (char*)opaque, _TRUNCATE); +#else + strncpy(prefix, (char*)opaque, sizeof(prefix) - 1); + prefix[sizeof(prefix) - 1] = '\0'; +#endif + } time_t now; time(&now); char buf[1024]; From 30e7ccdf1f6bd55651c2c508ed0a0e7d34787c05 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Mon, 24 Apr 2023 12:41:16 +0200 Subject: [PATCH 06/37] [core] Minor fix of variable shadowing. --- srtcore/api.cpp | 2 +- srtcore/core.cpp | 2 -- srtcore/epoll.cpp | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 63dab3927d..7ec8ff5708 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -674,7 +674,7 @@ int srt::CUDTUnited::newConnection(const SRTSOCKET listen, // XXX this might require another check of group type. // For redundancy group, at least, update the status in the group CUDTGroup* g = ns->m_GroupOf; - ScopedLock glock(g->m_GroupLock); + ScopedLock grlock(g->m_GroupLock); if (g->m_bClosing) { error = 1; // "INTERNAL REJECTION" diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 1386b97a20..0e3cce0ee5 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -9726,8 +9726,6 @@ void srt::CUDT::processClose() void srt::CUDT::sendLossReport(const std::vector > &loss_seqs) { - typedef vector > loss_seqs_t; - vector seqbuffer; seqbuffer.reserve(2 * loss_seqs.size()); // pessimistic for (loss_seqs_t::const_iterator i = loss_seqs.begin(); i != loss_seqs.end(); ++i) diff --git a/srtcore/epoll.cpp b/srtcore/epoll.cpp index 55211380bf..269b9ff593 100644 --- a/srtcore/epoll.cpp +++ b/srtcore/epoll.cpp @@ -215,8 +215,8 @@ void srt::CEPoll::clear_ready_usocks(CEPollDesc& d, int direction) } } - for (size_t i = 0; i < cleared.size(); ++i) - d.removeSubscription(cleared[i]); + for (size_t j = 0; j < cleared.size(); ++j) + d.removeSubscription(cleared[j]); } int srt::CEPoll::add_ssock(const int eid, const SYSSOCKET& s, const int* events) From be254e7d50e2eece6f7633ab7ccd09eb2da4af9e Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Mon, 24 Apr 2023 20:34:53 +0200 Subject: [PATCH 07/37] [tests] Minor fix of variable shadowing. --- test/test_bonding.cpp | 4 ++-- test/test_sync.cpp | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index 228e59d8da..1a66f04ef2 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -29,10 +29,10 @@ TEST(Bonding, SRTConnectGroup) targets.push_back(gd); } - std::future closing_promise = std::async(std::launch::async, [](int ss) { + std::future closing_promise = std::async(std::launch::async, [](int s) { std::this_thread::sleep_for(std::chrono::seconds(2)); std::cerr << "Closing group" << std::endl; - srt_close(ss); + srt_close(s); }, ss); std::cout << "srt_connect_group calling " << std::endl; diff --git a/test/test_sync.cpp b/test/test_sync.cpp index fa0248f089..844705ea65 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -385,9 +385,9 @@ TEST(SyncEvent, WaitForNotifyOne) const steady_clock::duration timeout = seconds_from(5); - auto wait_async = [](Condition* cond, Mutex* mutex, const steady_clock::duration& timeout) { - CUniqueSync cc (*mutex, *cond); - return cc.wait_for(timeout); + auto wait_async = [](Condition* cv, Mutex* m, const steady_clock::duration& tmo) { + CUniqueSync cc (*m, *cv); + return cc.wait_for(tmo); }; auto wait_async_res = async(launch::async, wait_async, &cond, &mutex, timeout); @@ -406,9 +406,9 @@ TEST(SyncEvent, WaitNotifyOne) Condition cond; cond.init(); - auto wait_async = [](Condition* cond, Mutex* mutex) { - UniqueLock lock(*mutex); - return cond->wait(lock); + auto wait_async = [](Condition* cv, Mutex* m) { + UniqueLock lock(*m); + return cv->wait(lock); }; auto wait_async_res = async(launch::async, wait_async, &cond, &mutex); @@ -432,9 +432,9 @@ TEST(SyncEvent, WaitForTwoNotifyOne) srt::sync::atomic resource_ready(true); - auto wait_async = [&](Condition* cond, Mutex* mutex, const steady_clock::duration& timeout, int id) { - UniqueLock lock(*mutex); - if (cond->wait_for(lock, timeout) && resource_ready) + auto wait_async = [&](Condition* cv, Mutex* m, const steady_clock::duration& tmo, int id) { + UniqueLock lock(*m); + if (cv->wait_for(lock, tmo) && resource_ready) { notified_clients.push_back(id); resource_ready = false; @@ -536,9 +536,9 @@ TEST(SyncEvent, WaitForTwoNotifyAll) cond.init(); const steady_clock::duration timeout = seconds_from(3); - auto wait_async = [](Condition* cond, Mutex* mutex, const steady_clock::duration& timeout) { - UniqueLock lock(*mutex); - return cond->wait_for(lock, timeout); + auto wait_async = [](Condition* cv, Mutex* m, const steady_clock::duration& tmo) { + UniqueLock lock(*m); + return cv->wait_for(lock, tmo); }; auto wait_async1_res = async(launch::async, wait_async, &cond, &mutex, timeout); auto wait_async2_res = async(launch::async, wait_async, &cond, &mutex, timeout); @@ -565,9 +565,9 @@ TEST(SyncEvent, WaitForNotifyAll) cond.init(); const steady_clock::duration timeout = seconds_from(5); - auto wait_async = [](Condition* cond, Mutex* mutex, const steady_clock::duration& timeout) { - UniqueLock lock(*mutex); - return cond->wait_for(lock, timeout); + auto wait_async = [](Condition* cv, Mutex* m, const steady_clock::duration& tmo) { + UniqueLock lock(*m); + return cv->wait_for(lock, tmo); }; auto wait_async_res = async(launch::async, wait_async, &cond, &mutex, timeout); From 66c86b1806badf04230ac006ae10a66af116c17d Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Tue, 25 Apr 2023 09:41:48 +0200 Subject: [PATCH 08/37] [build] Add -Wshadow=local to CMake build flags. Supported since GCC 7.0. --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c48cb6900d..1d83189493 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -642,6 +642,9 @@ endif() # add extra warning flags for gccish compilers if (HAVE_COMPILER_GNU_COMPAT) set (SRT_GCC_WARN "-Wall -Wextra") + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 7.0 AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set (SRT_GCC_WARN "${SRT_GCC_WARN} -Wshadow=local") + endif() else() # cpp debugging on Windows :D #set (SRT_GCC_WARN "/showIncludes") From 3cefedefe91fca543083d260d1ed32efd2e7cba5 Mon Sep 17 00:00:00 2001 From: matoro Date: Sat, 29 Apr 2023 02:30:20 -0400 Subject: [PATCH 09/37] [core] Correct remaining endianness issues Fixes the last two remaining test failures on big-endian. These operations were all already no-ops on little-endian, and unnecessarily byteswapped the IP addresses on big-endian. Closes: https://github.com/Haivision/srt/issues/2697 --- srtcore/common.cpp | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/srtcore/common.cpp b/srtcore/common.cpp index 53148136ad..b621c8025e 100644 --- a/srtcore/common.cpp +++ b/srtcore/common.cpp @@ -168,11 +168,7 @@ void srt::CIPAddress::ntop(const sockaddr_any& addr, uint32_t ip[4]) } else { - const sockaddr_in6* a = &addr.sin6; - ip[3] = (a->sin6_addr.s6_addr[15] << 24) + (a->sin6_addr.s6_addr[14] << 16) + (a->sin6_addr.s6_addr[13] << 8) + a->sin6_addr.s6_addr[12]; - ip[2] = (a->sin6_addr.s6_addr[11] << 24) + (a->sin6_addr.s6_addr[10] << 16) + (a->sin6_addr.s6_addr[9] << 8) + a->sin6_addr.s6_addr[8]; - ip[1] = (a->sin6_addr.s6_addr[7] << 24) + (a->sin6_addr.s6_addr[6] << 16) + (a->sin6_addr.s6_addr[5] << 8) + a->sin6_addr.s6_addr[4]; - ip[0] = (a->sin6_addr.s6_addr[3] << 24) + (a->sin6_addr.s6_addr[2] << 16) + (a->sin6_addr.s6_addr[1] << 8) + a->sin6_addr.s6_addr[0]; + std::memcpy(ip, addr.sin6.sin6_addr.s6_addr, 16); } } @@ -223,18 +219,7 @@ void srt::CIPAddress::pton(sockaddr_any& w_addr, const uint32_t ip[4], const soc // Here both agent and peer use IPv6, in which case // `ip` contains the full IPv6 address, so just copy // it as is. - - // XXX Possibly, a simple - // memcpy( (a->sin6_addr.s6_addr), ip, 16); - // would do the same thing, and faster. The address in `ip`, - // even though coded here as uint32_t, is still big endian. - for (int i = 0; i < 4; ++ i) - { - a->sin6_addr.s6_addr[i * 4 + 0] = ip[i] & 0xFF; - a->sin6_addr.s6_addr[i * 4 + 1] = (unsigned char)((ip[i] & 0xFF00) >> 8); - a->sin6_addr.s6_addr[i * 4 + 2] = (unsigned char)((ip[i] & 0xFF0000) >> 16); - a->sin6_addr.s6_addr[i * 4 + 3] = (unsigned char)((ip[i] & 0xFF000000) >> 24); - } + std::memcpy(a->sin6_addr.s6_addr, ip, 16); return; // The address is written, nothing left to do. } From 9448e26fcd7602098b4bf9cd7fe535136e89e10b Mon Sep 17 00:00:00 2001 From: Maria Sharabayko Date: Fri, 2 Jun 2023 16:25:41 +0200 Subject: [PATCH 10/37] [docs] Minor updates to AEAD docs plus changed v1.6.0 to 1.5.2 in some files --- docs/API/API-functions.md | 2 +- docs/API/API-socket-options.md | 4 ++-- docs/build/build-options.md | 3 ++- srtcore/srt.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/API/API-functions.md b/docs/API/API-functions.md index 4292a99b55..042ece34cd 100644 --- a/docs/API/API-functions.md +++ b/docs/API/API-functions.md @@ -171,7 +171,7 @@ Since SRT v1.5.0. | [SRT_REJ_FILTER](#SRT_REJ_FILTER) | 1.3.4 | The [`SRTO_PACKETFILTER`](API-socket-options.md#SRTO_PACKETFILTER) option has been set differently on both connection parties | | [SRT_REJ_GROUP](#SRT_REJ_GROUP) | 1.4.2 | The group type or some group settings are incompatible for both connection parties | | [SRT_REJ_TIMEOUT](#SRT_REJ_TIMEOUT) | 1.4.2 | The connection wasn't rejected, but it timed out | -| [SRT_REJ_CRYPTO](#SRT_REJ_CRYPTO) | 1.6.0-dev | The connection was rejected due to an unsupported or mismatching encryption mode | +| [SRT_REJ_CRYPTO](#SRT_REJ_CRYPTO) | 1.5.2 | The connection was rejected due to an unsupported or mismatching encryption mode | | | | |

Error Codes

diff --git a/docs/API/API-socket-options.md b/docs/API/API-socket-options.md index 5d32db4ca1..10e732b0b6 100644 --- a/docs/API/API-socket-options.md +++ b/docs/API/API-socket-options.md @@ -204,7 +204,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`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 | pre | `int32_t` | ms | 3000 | 0.. | W | GSD+ | -| [`SRTO_CRYPTOMODE`](#SRTO_CRYPTOMODE) | 1.6.0-dev | pre | `int32_t` | | 0 (Auto) | [0, 2] | W | GSD | +| [`SRTO_CRYPTOMODE`](#SRTO_CRYPTOMODE) | 1.5.2 | pre | `int32_t` | | 0 (Auto) | [0, 2] | 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 | @@ -327,7 +327,7 @@ will be 10 times the value set with `SRTO_CONNTIMEO`. | OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | | ------------------ | --------- | -------- | --------- | ------ | -------- | ------ | --- | ------ | -| `SRTO_CRYPTOMODE` | 1.6.0-dev | pre | `int32_t` | | 0 (Auto) | [0, 2] | RW | GSD | +| `SRTO_CRYPTOMODE` | 1.5.2 | pre | `int32_t` | | 0 (Auto) | [0, 2] | RW | GSD | The encryption mode to be used if the [`SRTO_PASSPHRASE`](#SRTO_PASSPHRASE) is set. diff --git a/docs/build/build-options.md b/docs/build/build-options.md index a2fcd20cd8..25ef1f7d3a 100644 --- a/docs/build/build-options.md +++ b/docs/build/build-options.md @@ -275,7 +275,8 @@ use encryption for the connection. When ON, the AEAD API is enabled. The `ENABLE_ENCRYPTION` must be enabled as well. The AEAD functionality is only available if OpenSSL EVP is selected as the crypto provider: -build option `-DUSE_ENCLIB=openssl-evp`. +build option should be set to `USE_ENCLIB=openssl-evp`. + The AEAD API is to be official in SRT v1.6.0. diff --git a/srtcore/srt.h b/srtcore/srt.h index 39a90ce71f..c30169f05c 100644 --- a/srtcore/srt.h +++ b/srtcore/srt.h @@ -646,7 +646,7 @@ enum SRT_KM_STATE SRT_KM_S_NOSECRET = 3, // Stream encrypted and no secret to decrypt Keying Material SRT_KM_S_BADSECRET = 4 // Stream encrypted and wrong secret is used, cannot decrypt Keying Material #ifdef ENABLE_AEAD_API_PREVIEW - ,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong cryptographic mode is used, cannot decrypt. Since v1.6.0. + ,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong cryptographic mode is used, cannot decrypt. Since v1.5.2. #endif }; From 421f4e16b993519a834dcfc625e03f7f1ae8033c Mon Sep 17 00:00:00 2001 From: Steve Lhomme Date: Thu, 29 Jun 2023 12:30:17 +0200 Subject: [PATCH 11/37] [build] Fix downversioning of _WIN32_WINNT (#2754). --- CMakeLists.txt | 8 +++++++- scripts/test_vista.c | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 scripts/test_vista.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d83189493..bdd421e46e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -299,7 +299,13 @@ if(WIN32) if(ENABLE_INET_PTON) set(CMAKE_REQUIRED_LIBRARIES ws2_32) check_function_exists(inet_pton HAVE_INET_PTON) - add_definitions(-D_WIN32_WINNT=0x0600) + try_compile(AT_LEAST_VISTA + ${CMAKE_BINARY_DIR} + "${CMAKE_CURRENT_SOURCE_DIR}/scripts/test_vista.c") + if(NOT AT_LEAST_VISTA) + # force targeting Vista + add_definitions(-D_WIN32_WINNT=0x0600) + endif() else() add_definitions(-D_WIN32_WINNT=0x0501) endif() diff --git a/scripts/test_vista.c b/scripts/test_vista.c new file mode 100644 index 0000000000..833a4d373b --- /dev/null +++ b/scripts/test_vista.c @@ -0,0 +1,10 @@ +/* Copyright © 2023 Steve Lhomme */ +/* SPDX-License-Identifier: ISC */ +#include +#if !defined(_WIN32_WINNT) || _WIN32_WINNT < 0x0600 /* _WIN32_WINNT_VISTA */ +#error NOPE +#endif +int main(void) +{ + return 0; +} From 10e71a673873bb8d9612392b503d628419f5d938 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Thu, 29 Jun 2023 12:45:33 +0200 Subject: [PATCH 12/37] [core] Fixed unhandled error in haicrypt (#2685). --- haicrypt/hcrypt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/haicrypt/hcrypt.c b/haicrypt/hcrypt.c index 3b4b98ece1..2568654b1e 100644 --- a/haicrypt/hcrypt.c +++ b/haicrypt/hcrypt.c @@ -240,7 +240,8 @@ int HaiCrypt_Clone(HaiCrypt_Handle hhcSrc, HaiCrypt_CryptoDir tx, HaiCrypt_Handl if (tx) { HaiCrypt_Cfg crypto_config; - HaiCrypt_ExtractConfig(hhcSrc, &crypto_config); + if (-1 == HaiCrypt_ExtractConfig(hhcSrc, &crypto_config)) + return -1; /* * Just invert the direction written in flags and use the From 61c7bedf27a2edbcf6e6fdc179612980bae53c6a Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Thu, 29 Jun 2023 17:30:20 +0200 Subject: [PATCH 13/37] [core] Use overlapped WSASendTo to avoid loss in UDP sending (#2632). --- srtcore/channel.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/srtcore/channel.cpp b/srtcore/channel.cpp index 31a29092f9..091adf1159 100644 --- a/srtcore/channel.cpp +++ b/srtcore/channel.cpp @@ -774,8 +774,19 @@ int srt::CChannel::sendto(const sockaddr_any& addr, CPacket& packet, const socka #else DWORD size = (DWORD)(CPacket::HDR_SIZE + packet.getLength()); int addrsize = addr.size(); - int res = ::WSASendTo(m_iSocket, (LPWSABUF)packet.m_PacketVector, 2, &size, 0, addr.get(), addrsize, NULL, NULL); - res = (0 == res) ? size : -1; + WSAOVERLAPPED overlapped; + SecureZeroMemory((PVOID)&overlapped, sizeof(WSAOVERLAPPED)); + int res = ::WSASendTo(m_iSocket, (LPWSABUF)packet.m_PacketVector, 2, &size, 0, addr.get(), addrsize, &overlapped, NULL); + + if (res == SOCKET_ERROR && NET_ERROR == WSA_IO_PENDING) + { + DWORD dwFlags = 0; + const bool bCompleted = WSAGetOverlappedResult(m_iSocket, &overlapped, &size, true, &dwFlags); + WSACloseEvent(overlapped.hEvent); + res = bCompleted ? 0 : -1; + } + + res = (0 == res) ? size : -1; #endif packet.toHL(); From 1737e96a0fa12fcd619b7818e959bef53b56a5b7 Mon Sep 17 00:00:00 2001 From: Aaron Jencks <32805004+aaron-jencks@users.noreply.github.com> Date: Thu, 13 Jul 2023 04:02:14 -0500 Subject: [PATCH 14/37] [core] Add volatile keyword to asm block in rdtsc (#2759). --- srtcore/sync_posix.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srtcore/sync_posix.cpp b/srtcore/sync_posix.cpp index c44fe86c27..8cb475ea76 100644 --- a/srtcore/sync_posix.cpp +++ b/srtcore/sync_posix.cpp @@ -52,7 +52,7 @@ static void rdtsc(uint64_t& x) asm("mov %0=ar.itc" : "=r"(x)::"memory"); #elif SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_AMD64_RDTSC uint32_t lval, hval; - asm("rdtsc" : "=a"(lval), "=d"(hval)); + asm volatile("rdtsc" : "=a"(lval), "=d"(hval)); x = hval; x = (x << 32) | lval; #elif SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_WINQPC From 9f4e9b67433f6480e681ac962dbd4f0160aa0f95 Mon Sep 17 00:00:00 2001 From: Guangqing Chen Date: Thu, 1 Jun 2023 16:17:57 +0800 Subject: [PATCH 15/37] [core] Fixed srctime from closing socket was mistakenly cleared --- srtcore/core.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 0e3cce0ee5..a5d32d9e3b 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -6848,7 +6848,6 @@ int srt::CUDT::receiveMessage(char* data, int len, SRT_MSGCTRL& w_mctrl, int by_ ? m_pRcvBuffer->readMessage(data, len, &w_mctrl) : 0; leaveCS(m_RcvBufferLock); - w_mctrl.srctime = 0; // Kick TsbPd thread to schedule next wakeup (if running) if (m_bTsbPd) From c639310bdfd5352f7cbd2d6f322bb452ee8b009c Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Fri, 4 Aug 2023 16:49:46 +0200 Subject: [PATCH 16/37] [core] Fixed group read-ready epoll events. --- srtcore/group.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/srtcore/group.cpp b/srtcore/group.cpp index f4dfba1ba4..0855196a36 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -2310,13 +2310,19 @@ int CUDTGroup::recv(char* buf, int len, SRT_MSGCTRL& w_mc) } } } + bool canReadFurther = false; for (vector::const_iterator si = aliveMembers.begin(); si != aliveMembers.end(); ++si) { CUDTSocket* ps = *si; if (!ps->core().isRcvBufferReady()) m_Global.m_EPoll.update_events(ps->m_SocketID, ps->core().m_sPollID, SRT_EPOLL_IN, false); + else + canReadFurther = true; } + if (!canReadFurther) + m_Global.m_EPoll.update_events(id(), m_sPollID, SRT_EPOLL_IN, false); + return res; } LOGC(grlog.Error, log << "grp/recv: UNEXPECTED RUN PATH, ABANDONING."); From 31294e391c7383c766336c2c82b9bce1260b458d Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Tue, 18 Jul 2023 12:43:56 +0200 Subject: [PATCH 17/37] [core] Removed unused CUDTGroup::m_Positions. --- srtcore/group.cpp | 38 -------------------------------------- srtcore/group.h | 17 ----------------- 2 files changed, 55 deletions(-) diff --git a/srtcore/group.cpp b/srtcore/group.cpp index 0855196a36..fedfdcf765 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -2330,44 +2330,6 @@ int CUDTGroup::recv(char* buf, int len, SRT_MSGCTRL& w_mc) throw CUDTException(MJ_AGAIN, MN_RDAVAIL, 0); } -// [[using locked(m_GroupLock)]] -CUDTGroup::ReadPos* CUDTGroup::checkPacketAhead() -{ - typedef map::iterator pit_t; - ReadPos* out = 0; - - // This map no longer maps only ahead links. - // Here are all links, and whether ahead, it's defined by the sequence. - for (pit_t i = m_Positions.begin(); i != m_Positions.end(); ++i) - { - // i->first: socket ID - // i->second: ReadPos { sequence, packet } - // We are not interested with the socket ID because we - // aren't going to read from it - we have the packet already. - ReadPos& a = i->second; - - const int seqdiff = CSeqNo::seqcmp(a.mctrl.pktseq, m_RcvBaseSeqNo); - if (seqdiff == 1) - { - // The very next packet. Return it. - HLOGC(grlog.Debug, - log << "group/recv: Base %" << m_RcvBaseSeqNo << " ahead delivery POSSIBLE %" << a.mctrl.pktseq - << " #" << a.mctrl.msgno << " from @" << i->first << ")"); - out = &a; - } - else if (seqdiff < 1 && !a.packet.empty()) - { - HLOGC(grlog.Debug, - log << "group/recv: @" << i->first << " dropping collected ahead %" << a.mctrl.pktseq << "#" - << a.mctrl.msgno << " with base %" << m_RcvBaseSeqNo); - a.packet.clear(); - } - // In case when it's >1, keep it in ahead - } - - return out; -} - const char* CUDTGroup::StateStr(CUDTGroup::GroupState st) { static const char* const states[] = {"PENDING", "IDLE", "RUNNING", "BROKEN"}; diff --git a/srtcore/group.h b/srtcore/group.h index 09e0722671..c2863b44e7 100644 --- a/srtcore/group.h +++ b/srtcore/group.h @@ -194,9 +194,6 @@ class CUDTGroup m_bConnected = false; } - // XXX BUGFIX - m_Positions.erase(id); - return !empty; } @@ -646,20 +643,6 @@ class CUDTGroup time_point m_tsStartTime; time_point m_tsRcvPeerStartTime; - struct ReadPos - { - std::vector packet; - SRT_MSGCTRL mctrl; - ReadPos(int32_t s) - : mctrl(srt_msgctrl_default) - { - mctrl.pktseq = s; - } - }; - std::map m_Positions; - - ReadPos* checkPacketAhead(); - void recv_CollectAliveAndBroken(std::vector& w_alive, std::set& w_broken); /// The function polls alive member sockets and retrieves a list of read-ready. From 88ca9ccca4984dbf61a5e1a06ac551b4dead5304 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Wed, 19 Jul 2023 11:52:07 +0200 Subject: [PATCH 18/37] [core] Perf improvement of group reading. --- srtcore/buffer_rcv.cpp | 12 ++++++++---- srtcore/core.cpp | 5 +++++ srtcore/core.h | 3 +++ srtcore/group.cpp | 11 ++++------- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/srtcore/buffer_rcv.cpp b/srtcore/buffer_rcv.cpp index 3098407a6d..8774d4d1da 100644 --- a/srtcore/buffer_rcv.cpp +++ b/srtcore/buffer_rcv.cpp @@ -236,10 +236,14 @@ int CRcvBuffer::dropUpTo(int32_t seqno) m_iStartSeqNo = seqno; // Move forward if there are "read/drop" entries. releaseNextFillerEntries(); - // Set nonread position to the starting position before updating, - // because start position was increased, and preceding packets are invalid. - m_iFirstNonreadPos = m_iStartPos; - updateNonreadPos(); + + // If the nonread position is now behind the starting position, set it to the starting position and update. + // Preceding packets were likely missing, and the non read position can probably be moved further now. + if (CSeqNo::seqcmp(m_iFirstNonreadPos, m_iStartPos) < 0) + { + m_iFirstNonreadPos = m_iStartPos; + updateNonreadPos(); + } if (!m_tsbpd.isEnabled() && m_bMessageAPI) updateFirstReadableOutOfOrder(); return iDropCnt; diff --git a/srtcore/core.cpp b/srtcore/core.cpp index a5d32d9e3b..752eb51b64 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -6804,6 +6804,11 @@ bool srt::CUDT::isRcvBufferReady() const return m_pRcvBuffer->isRcvDataReady(steady_clock::now()); } +bool srt::CUDT::isRcvBufferReadyNoLock() const +{ + return m_pRcvBuffer->isRcvDataReady(steady_clock::now()); +} + // int by_exception: accepts values of CUDTUnited::ErrorHandling: // - 0 - by return value // - 1 - by exception diff --git a/srtcore/core.h b/srtcore/core.h index e7ca57c508..b83dec6e5e 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -720,6 +720,9 @@ class CUDT SRT_ATTR_EXCLUDES(m_RcvBufferLock) bool isRcvBufferReady() const; + SRT_ATTR_REQUIRES2(m_RcvBufferLock) + bool isRcvBufferReadyNoLock() const; + // TSBPD thread main function. static void* tsbpd(void* param); diff --git a/srtcore/group.cpp b/srtcore/group.cpp index fedfdcf765..2952d8c6b9 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -2295,13 +2295,14 @@ int CUDTGroup::recv(char* buf, int len, SRT_MSGCTRL& w_mc) m_stats.recv.count(res); updateAvgPayloadSize(res); + bool canReadFurther = false; for (vector::const_iterator si = aliveMembers.begin(); si != aliveMembers.end(); ++si) { CUDTSocket* ps = *si; ScopedLock lg(ps->core().m_RcvBufferLock); if (m_RcvBaseSeqNo != SRT_SEQNO_NONE) { - int cnt = ps->core().rcvDropTooLateUpTo(CSeqNo::incseq(m_RcvBaseSeqNo)); + const int cnt = ps->core().rcvDropTooLateUpTo(CSeqNo::incseq(m_RcvBaseSeqNo)); if (cnt > 0) { HLOGC(grlog.Debug, @@ -2309,12 +2310,8 @@ int CUDTGroup::recv(char* buf, int len, SRT_MSGCTRL& w_mc) << " packets after reading: m_RcvBaseSeqNo=" << m_RcvBaseSeqNo); } } - } - bool canReadFurther = false; - for (vector::const_iterator si = aliveMembers.begin(); si != aliveMembers.end(); ++si) - { - CUDTSocket* ps = *si; - if (!ps->core().isRcvBufferReady()) + + if (!ps->core().isRcvBufferReadyNoLock()) m_Global.m_EPoll.update_events(ps->m_SocketID, ps->core().m_sPollID, SRT_EPOLL_IN, false); else canReadFurther = true; From 69c23766255237f6b88b092f67e61ee07109e03f Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Fri, 4 Aug 2023 13:30:25 +0200 Subject: [PATCH 19/37] [core] Fixed RCV buffer initialization in Rendezvous. --- srtcore/core.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 752eb51b64..1a2b8d2fc9 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -5598,9 +5598,9 @@ bool srt::CUDT::prepareBuffers(CUDTException* eout) // CryptoControl has to be initialized and in case of RESPONDER the KM REQ must be processed (interpretSrtHandshake(..)) for the crypto mode to be deduced. const int authtag = (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) ? HAICRYPT_AUTHTAG_MAX : 0; m_pSndBuffer = new CSndBuffer(32, m_iMaxSRTPayloadSize, authtag); - SRT_ASSERT(m_iISN != -1); - m_pRcvBuffer = new srt::CRcvBuffer(m_iISN, m_config.iRcvBufSize, m_pRcvQueue->m_pUnitQueue, m_config.bMessageAPI); - // after introducing lite ACK, the sndlosslist may not be cleared in time, so it requires twice space. + SRT_ASSERT(m_iPeerISN != -1); + m_pRcvBuffer = new srt::CRcvBuffer(m_iPeerISN, m_config.iRcvBufSize, m_pRcvQueue->m_pUnitQueue, m_config.bMessageAPI); + // After introducing lite ACK, the sndlosslist may not be cleared in time, so it requires twice a space. m_pSndLossList = new CSndLossList(m_iFlowWindowSize * 2); m_pRcvLossList = new CRcvLossList(m_config.iFlightFlagSize); } From d06331382862c32fb9606887aeccd67e5c209f99 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Mon, 7 Aug 2023 17:23:53 +0200 Subject: [PATCH 20/37] [docs] Updating the explicit information for binding to IPv6 wildcard (#2765). --- docs/API/API-functions.md | 8 ++++++-- docs/API/API-socket-options.md | 20 ++++++++++++++++---- docs/apps/srt-live-transmit.md | 17 +++++++++-------- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/docs/API/API-functions.md b/docs/API/API-functions.md index 042ece34cd..f897ffcc2b 100644 --- a/docs/API/API-functions.md +++ b/docs/API/API-functions.md @@ -359,8 +359,12 @@ int srt_bind(SRTSOCKET u, const struct sockaddr* name, int namelen); Binds a socket to a local address and port. Binding specifies the local network interface and the UDP port number to be used for the socket. When the local address is a wildcard (`INADDR_ANY` for IPv4 or `in6addr_any` for IPv6), then -it's bound to all interfaces (although see `SRTO_IPV6ONLY` and additional -information below for details about the wildcard address in IPv6). +it's bound to all interfaces. + +**IMPORTANT**: When you bind an IPv6 wildcard address, note that the +`SRTO_IPV6ONLY` option must be set on the socket explicitly to 1 or 0 prior to +calling this function. See +[`SRTO_IPV6ONLY`](API-socket-options.md#SRTO_IPV6ONLY) for more details. Binding is necessary for every socket to be used for communication. If the socket is to be used to initiate a connection to a listener socket, which can be done, diff --git a/docs/API/API-socket-options.md b/docs/API/API-socket-options.md index 10e732b0b6..df4772d550 100644 --- a/docs/API/API-socket-options.md +++ b/docs/API/API-socket-options.md @@ -647,10 +647,22 @@ and the actual value for connected sockets. Set system socket option level `IPPROTO_IPV6` named `IPV6_V6ONLY`. This is meaningful only when the socket is going to be bound to the IPv6 wildcard address `in6addr_any` -(known also as `::`). In this case this option must be also set explicitly to 0 or 1 -before binding, otherwise binding will fail (this is because it is not possible to -determine the default value of this above-mentioned system option in any portable or -reliable way). Possible values are: +(known also as `::`). If you bind to a wildcard address, you have the following +possibilities: + +* IPv4 only: bind to an IPv4 wildcard address +* IPv6 only: bind to an IPv6 wildcard address and set this option to 1 +* IPv4 and IPv6: bind to an IPv6 wildcard address and set this option to 0 + +This option's default value is -1 because it is not possible to determine the default +value on the current platform, and if you bind to an IPv6 wildcard address, this value +is required prior to binding. When you bind implicitly by calling `srt_connect` on the +socket, this isn't a problem -- binding will be done using the system-default value and then +extracted afterwards. But if you want to bind explicitly using `srt_bind`, this +option must be set explicitly to 0 or 1 because this information is vital for +determining any potential bind conflicts with other sockets. + +Possible values are: * -1: (default) use system-default value (can be used when not binding to IPv6 wildcard `::`) * 0: The binding to `in6addr_any` will bind to both IPv6 and IPv4 wildcard address diff --git a/docs/apps/srt-live-transmit.md b/docs/apps/srt-live-transmit.md index cc8a25e225..f29e1ed7a8 100644 --- a/docs/apps/srt-live-transmit.md +++ b/docs/apps/srt-live-transmit.md @@ -285,11 +285,13 @@ specify the host as `::`. NOTE: Don't use square brackets syntax in the **adapter** parameter specification, as in this case only the host is expected. -3. If you want to listen for connections from both IPv4 and IPv6, mind the -`ipv6only` option. The default value for this option is system default (see -system manual for `IPV6_V6ONLY` socket option); if unsure, you might want to -enforce `ipv6only=0` in order to be able to accept both IPv4 and IPv6 -connections by the same listener, or set `ipv6only=1` to accept exclusively IPv6. +3. If you bind to an IPv6 wildcard address (with listener mode, or when using the `bind` +option), setting the `ipv6only` option to 0 or 1 is obligatory, as it is a part +of the binding definition. If you set it to 1, the binding will apply only to +IPv6 local addresses, and if you set it to 0, it will apply to both IPv4 and +IPv6 local addresses. See the +[`SRTO_IPV6ONLY`](../API/API-socket-options.md#SRTO_IPV6ONLY) option +description for details. 4. In rendezvous mode you may only interconnect both parties using IPv4, or both using IPv6. Unlike listener mode, if you want to leave the socket @@ -303,9 +305,8 @@ Examples: * `srt://[::]:5000` defines caller mode (!) with IPv6. -* `srt://[::]:5000?mode=listener` defines listener mode with IPv6. If the - default value for `IPV6_V6ONLY` system socket option is 0, it will accept - also IPv4 connections. +* `srt://[::]:5000?mode=listener&ipv6only=1` defines listener mode with IPv6. + Only connections from IPv6 callers will be accepted. * `srt://192.168.0.5:5000?mode=rendezvous` will make a rendezvous connection with local address `INADDR_ANY` (IPv4) and port 5000 to a destination with From 9f414377ed02a33606fc563e7cb93b2d4af18874 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Tue, 8 Aug 2023 10:21:18 +0200 Subject: [PATCH 21/37] [tests] Added custom main with transparent parameters for tests (#2681). --- .travis.yml | 3 +- test/TESTS_HOWTO.md | 107 ++++++++++++++++++ test/filelist.maf | 2 + test/test_bonding.cpp | 18 +-- test/test_common.cpp | 6 +- test/test_connection_timeout.cpp | 14 +-- test/test_enforced_encryption.cpp | 16 ++- test/test_env.h | 89 +++++++++++++++ test/test_epoll.cpp | 55 +++------ test/test_fec_rebuilding.cpp | 41 +++---- test/test_file_transmission.cpp | 4 +- test/test_ipv6.cpp | 17 +-- test/test_listen_callback.cpp | 17 ++- test/test_losslist_rcv.cpp | 2 + test/test_main.cpp | 180 ++++++++++++++++++++++++++++++ test/test_many_connections.cpp | 13 +-- test/test_muxer.cpp | 13 ++- test/test_reuseaddr.cpp | 99 +++------------- test/test_socket_options.cpp | 17 ++- test/test_socketdata.cpp | 6 +- test/test_unitqueue.cpp | 4 + 21 files changed, 504 insertions(+), 219 deletions(-) create mode 100644 test/TESTS_HOWTO.md create mode 100644 test/test_env.h create mode 100644 test/test_main.cpp diff --git a/.travis.yml b/.travis.yml index eaca3a5717..72df01bf15 100644 --- a/.travis.yml +++ b/.travis.yml @@ -71,7 +71,6 @@ matrix: - BUILD_TYPE=Release - BUILD_OPTS='-DENABLE_MONOTONIC_CLOCK=ON' script: - - TESTS_IPv6="TestMuxer.IPv4_and_IPv6:TestIPv6.v6_calls_v6*:ReuseAddr.ProtocolVersion:ReuseAddr.*6" ; # Tests to skip due to lack of IPv6 support - if [ "$TRAVIS_COMPILER" == "x86_64-w64-mingw32-g++" ]; then export CC="x86_64-w64-mingw32-gcc"; export CXX="x86_64-w64-mingw32-g++"; @@ -95,7 +94,7 @@ script: fi - if [ "$TRAVIS_COMPILER" != "x86_64-w64-mingw32-g++" ]; then ulimit -c unlimited; - ./test-srt --gtest_filter="-$TESTS_IPv6"; + ./test-srt -disable-ipv6; SUCCESS=$?; if [ -f core ]; then gdb -batch ./test-srt -c core -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; test $SUCCESS == 0; diff --git a/test/TESTS_HOWTO.md b/test/TESTS_HOWTO.md new file mode 100644 index 0000000000..e12d91d6e6 --- /dev/null +++ b/test/TESTS_HOWTO.md @@ -0,0 +1,107 @@ +# Rules for writing tests for SRT + +## 1. Use automatic startup/cleanup management + +Note that most of the test require SRT library to be initialized for the +time of running the test. There are two methods how you can do it: + +* In a free test (`TEST` macro), declare this in the beginning: +`srt::TestInit srtinit;` + +* In a fixture (`TEST_F` macro), draw your class off `srt::Test` +(instead of `testing::Test`) + +In the fixture case you should also use names `setup/teardown` instead of +`SetUp/TearDown`. Both these things will properly initialize and destroy the +library resources. + +## 2. Do not misuse ASSERT macros + +**Be careful** where you are using `ASSERT_*` macros. In distinction to +`EXPECT_*` macros, they interrupt the testing procedure by throwing an exception. +This means that if this fires, nothing will be executed up to the end of the +current testing procedure, unless it's a destructor of some object constructed +inside the procedure. + +This means, however, that if you have any resource deallocation procedures, which +must be placed there for completion regardless of the test result, the call to +`ASSERT_*` macro will skip them, which may often lead to misexecution of the +remaining tests and have them falsely failed. If this interruption is necessary, +there are the following methods you can use to prevent skipping resource cleanup: + +* Do not cleanup anything in the testing procedure. Use the fixture's teardown +method for any cleaning. Remember also that it is not allowed to use `ASSERT_*` +macros in the teardown procedure, should you need to test additionally to the +cleanup. + +* You can also use a local class with a destructor so that cleanups will execute +no matter what happened inside the procedure + +* Last resort, keep the code that might use `ASSERT_*` macro in the try-catch +block and free the resources in the `catch` clause, then rethrow the exception. +A disadvantage of this solution is that you'll have to repeat the cleanup +procedure outside the try-catch block. + +* Use `EXPECT_` macros, but still check the condition again and skip required +parts of the test that could not be done without this resource. + +# Useful SRT test features + +## Test command line parameters + +The SRT tests support command-line parameters. They are available in test +procedures, startups, and through this you can control some execution +aspects. The gtest-specific options are being removed from the command +line by the gtest library itself; all other parameters are available for +the user. The main API access function for this is `srt::TestEnv`. This +is a fixed singleton object accessed through `srt::TestEnv::me` pointer. +These arguments are accessible through two fields: + +* `TestEnv::args`: a plain vector with all the command line arguments +* `TestEnv::argmap`: a map of arguments parsed according to the option syntax + +The option syntax is the following: + +* `-option` : single option without argument; can be tested for presence +* `-option param1 param2 param3` : multiple parameters assigned to an option + +Special markers: + +* `--`: end of options +* `-/`: end of parameters for the current option + +To specify free parameters after an option (and possibly its own parameters), +end the parameter list with the `-/` phrase. The `--` phrase means that the +rest of command line parameters are arguments for the last specified option, +even if they start with a dash. Note that a single dash has no special meaning. + +The `TestEnv::argmap` is using option names (except the initial dash) as keys +and the value is a vector of the parameters specified after the option. Free +parameters are collected under an empty string key. For convenience you can +also use two `TestEnv` helper methods: + +* `OptionPresent(name)`: returns true if the option of `name` is present in the +map (note that options without parameters have simply an empty vector assigned) + +* `OptionValue(name)`: returns a string that contains all parameters for that +option separated by a space (note that the value type in the map is a vector +of strings) + +## Test environment feature checks + +The macro `SRTST_REQUIRE` can be used to check if particular feature of the +test environment is available. This binds to the `TestEnv::Available_FEATURE` +option if used as `SRTST_REQUIRE(FEATURE)`. This macro makes the test function +exit immediately with success. The checking function should take care of +printing appropriate information about that the test was forcefully passed. + +To add more environment availability features, add more `TestEnv::Available_*` +methods. Methods must return `bool`, but may have parameters, which are passed +next to the first argument in the macro transparently. Availability can be +tested internally, or taken as a good deal basing on options, as it is +currently done with the IPv6 feature - it is declared as not available when the +test application gets the `-disable-ipv6` option. + +It is unknown what future tests could require particular system features, +so this solution is open for further extensions. + diff --git a/test/filelist.maf b/test/filelist.maf index 6da0508a14..876439a83c 100644 --- a/test/filelist.maf +++ b/test/filelist.maf @@ -1,7 +1,9 @@ HEADERS any.hpp +test_env.h SOURCES +test_main.cpp test_buffer_rcv.cpp test_common.cpp test_connection_timeout.cpp diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index 1a66f04ef2..a03b7c5a03 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -5,16 +5,16 @@ #include #include "gtest/gtest.h" +#include "test_env.h" #include "srt.h" #include "netinet_any.h" TEST(Bonding, SRTConnectGroup) { + srt::TestInit srtinit; struct sockaddr_in sa; - srt_startup(); - const int ss = srt_create_group(SRT_GTYPE_BROADCAST); ASSERT_NE(ss, SRT_ERROR); @@ -54,8 +54,6 @@ TEST(Bonding, SRTConnectGroup) { std::cerr << "srt_close: " << srt_getlasterror_str() << std::endl; } - - srt_cleanup(); } #define ASSERT_SRT_SUCCESS(callform) ASSERT_NE(callform, -1) << "SRT ERROR: " << srt_getlasterror_str() @@ -133,8 +131,8 @@ void ConnectCallback(void* /*opaq*/, SRTSOCKET sock, int error, const sockaddr* TEST(Bonding, NonBlockingGroupConnect) { - srt_startup(); - + srt::TestInit srtinit; + const int ss = srt_create_group(SRT_GTYPE_BROADCAST); ASSERT_NE(ss, SRT_ERROR); std::cout << "Created group socket: " << ss << '\n'; @@ -207,8 +205,6 @@ TEST(Bonding, NonBlockingGroupConnect) listen_promise.wait(); EXPECT_EQ(srt_close(ss), 0) << "srt_close: %s\n" << srt_getlasterror_str(); - - srt_cleanup(); } void ConnectCallback_Close(void* /*opaq*/, SRTSOCKET sock, int error, const sockaddr* /*peer*/, int token) @@ -226,8 +222,8 @@ void ConnectCallback_Close(void* /*opaq*/, SRTSOCKET sock, int error, const sock TEST(Bonding, CloseGroupAndSocket) { - srt_startup(); - + srt::TestInit srtinit; + const int ss = srt_create_group(SRT_GTYPE_BROADCAST); ASSERT_NE(ss, SRT_ERROR); std::cout << "Created group socket: " << ss << '\n'; @@ -332,7 +328,5 @@ TEST(Bonding, CloseGroupAndSocket) std::cout << "CLOSED GROUP. Now waiting for sender to exit...\n"; sender.join(); listen_promise.wait(); - - srt_cleanup(); } diff --git a/test/test_common.cpp b/test/test_common.cpp index 901083dfa5..1a8cca061a 100644 --- a/test/test_common.cpp +++ b/test/test_common.cpp @@ -2,6 +2,7 @@ #include #include "gtest/gtest.h" +#include "test_env.h" #include "utilities.h" #include "common.h" @@ -43,6 +44,7 @@ void test_cipaddress_pton(const char* peer_ip, int family, const uint32_t (&ip)[ // Example IPv4 address: 192.168.0.1 TEST(CIPAddress, IPv4_pton) { + srt::TestInit srtinit; const char* peer_ip = "192.168.0.1"; const uint32_t ip[4] = {htobe32(0xC0A80001), 0, 0, 0}; test_cipaddress_pton(peer_ip, AF_INET, ip); @@ -51,6 +53,7 @@ TEST(CIPAddress, IPv4_pton) // Example IPv6 address: 2001:db8:85a3:8d3:1319:8a2e:370:7348 TEST(CIPAddress, IPv6_pton) { + srt::TestInit srtinit; const char* peer_ip = "2001:db8:85a3:8d3:1319:8a2e:370:7348"; const uint32_t ip[4] = {htobe32(0x20010db8), htobe32(0x85a308d3), htobe32(0x13198a2e), htobe32(0x03707348)}; @@ -59,9 +62,10 @@ TEST(CIPAddress, IPv6_pton) // Example IPv4 address: 192.168.0.1 // Maps to IPv6 address: 0:0:0:0:0:FFFF:192.168.0.1 -// Simplified: ::FFFF:192.168.0.1 +// Simplified: ::FFFF:192.168.0.1 TEST(CIPAddress, IPv4_in_IPv6_pton) { + srt::TestInit srtinit; const char* peer_ip = "::ffff:192.168.0.1"; const uint32_t ip[4] = {0, 0, htobe32(0x0000FFFF), htobe32(0xC0A80001)}; diff --git a/test/test_connection_timeout.cpp b/test/test_connection_timeout.cpp index 241c5f1a84..0b8bb78742 100644 --- a/test/test_connection_timeout.cpp +++ b/test/test_connection_timeout.cpp @@ -1,5 +1,6 @@ -#include #include +#include +#include "test_env.h" #ifdef _WIN32 #define INC_SRT_WIN_WINTIME // exclude gettimeofday from srt headers @@ -16,7 +17,7 @@ using namespace std; class TestConnectionTimeout - : public ::testing::Test + : public ::srt::Test { protected: TestConnectionTimeout() @@ -32,10 +33,8 @@ class TestConnectionTimeout protected: // SetUp() is run immediately before a test starts. - void SetUp() override + void setup() override { - ASSERT_EQ(srt_startup(), 0); - m_sa.sin_family = AF_INET; m_sa.sin_addr.s_addr = INADDR_ANY; m_udp_sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); @@ -60,12 +59,11 @@ class TestConnectionTimeout ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &m_sa.sin_addr), 1); } - void TearDown() override + void teardown() override { // Code here will be called just after the test completes. // OK to throw exceptions from here if needed. - ASSERT_NE(closesocket(m_udp_sock), -1); - srt_cleanup(); + EXPECT_NE(closesocket(m_udp_sock), -1); } protected: diff --git a/test/test_enforced_encryption.cpp b/test/test_enforced_encryption.cpp index bac8d147ed..d394692d6a 100644 --- a/test/test_enforced_encryption.cpp +++ b/test/test_enforced_encryption.cpp @@ -10,10 +10,11 @@ * Haivision Systems Inc. */ -#include #include #include #include +#include +#include "test_env.h" #include "srt.h" #include "sync.h" @@ -214,7 +215,7 @@ const TestCaseBlocking g_test_matrix_blocking[] = class TestEnforcedEncryption - : public ::testing::Test + : public srt::Test { protected: TestEnforcedEncryption() @@ -230,10 +231,8 @@ class TestEnforcedEncryption protected: // SetUp() is run immediately before a test starts. - void SetUp() + void setup() override { - ASSERT_EQ(srt_startup(), 0); - m_pollid = srt_epoll_create(); ASSERT_GE(m_pollid, 0); @@ -254,13 +253,12 @@ class TestEnforcedEncryption ASSERT_NE(srt_epoll_add_usock(m_pollid, m_listener_socket, &epoll_out), SRT_ERROR); } - void TearDown() + void teardown() override { // Code here will be called just after the test completes. // OK to throw exceptions from here if needed. - ASSERT_NE(srt_close(m_caller_socket), SRT_ERROR); - ASSERT_NE(srt_close(m_listener_socket), SRT_ERROR); - srt_cleanup(); + EXPECT_NE(srt_close(m_caller_socket), SRT_ERROR) << srt_getlasterror_str(); + EXPECT_NE(srt_close(m_listener_socket), SRT_ERROR) << srt_getlasterror_str(); } diff --git a/test/test_env.h b/test/test_env.h new file mode 100644 index 0000000000..2e2f78f7d9 --- /dev/null +++ b/test/test_env.h @@ -0,0 +1,89 @@ +#ifndef INC_SRT_TESTENV_H +#define INC_SRT_TESTENV_H + +#include +#include +#include +#include +#include "gtest/gtest.h" + + +namespace srt +{ +class TestEnv: public testing::Environment +{ +public: + static TestEnv* me; + std::vector args; + std::map> argmap; + + explicit TestEnv(int argc, char** argv) + : args(argv+1, argv+argc) + { + if (me) + throw std::invalid_argument("singleton"); + + me = this; + FillArgMap(); + } + + void FillArgMap(); + + bool OptionPresent(const std::string& key) + { + return argmap.count(key) > 0; + } + + std::string OptionValue(const std::string& key); + + // Specific test environment options + // All must be static, return bool. Arguments allowed. + // The name must start with Allowed_. + static bool Allowed_IPv6(); +}; + +#define SRTST_REQUIRES(feature,...) if (!srt::TestEnv::Allowed_##feature(__VA_ARGS__)) { return; } + + +class TestInit +{ +public: + int ninst; + + static void start(int& w_retstatus); + static void stop(); + + TestInit() { start((ninst)); } + ~TestInit() { stop(); } + + void HandlePerTestOptions(); + +}; + +class Test: public testing::Test +{ + std::unique_ptr init_holder; +public: + + virtual void setup() = 0; + virtual void teardown() = 0; + + void SetUp() override final + { + init_holder.reset(new TestInit); + init_holder->HandlePerTestOptions(); + setup(); + } + + void TearDown() override final + { + teardown(); + init_holder.reset(); + } +}; + +struct sockaddr_any CreateAddr(const std::string& name, unsigned short port, int pref_family); + +} //namespace + +#endif diff --git a/test/test_epoll.cpp b/test/test_epoll.cpp index f9dfc393c9..1552741d3f 100644 --- a/test/test_epoll.cpp +++ b/test/test_epoll.cpp @@ -4,6 +4,7 @@ #include #include #include "gtest/gtest.h" +#include "test_env.h" #include "api.h" #include "epoll.h" @@ -14,8 +15,7 @@ using namespace srt; TEST(CEPoll, InfiniteWait) { - ASSERT_EQ(srt_startup(), 0); - + srt::TestInit srtinit; const int epoll_id = srt_epoll_create(); ASSERT_GE(epoll_id, 0); @@ -25,13 +25,11 @@ TEST(CEPoll, InfiniteWait) 0, 0, 0, 0), SRT_ERROR); EXPECT_EQ(srt_epoll_release(epoll_id), 0); - - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, WaitNoSocketsInEpoll) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; const int epoll_id = srt_epoll_create(); ASSERT_GE(epoll_id, 0); @@ -47,12 +45,11 @@ TEST(CEPoll, WaitNoSocketsInEpoll) EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, WaitNoSocketsInEpoll2) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; const int epoll_id = srt_epoll_create(); ASSERT_GE(epoll_id, 0); @@ -63,12 +60,11 @@ TEST(CEPoll, WaitNoSocketsInEpoll2) EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, WaitEmptyCall) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); ASSERT_NE(client_sock, SRT_ERROR); @@ -87,12 +83,11 @@ TEST(CEPoll, WaitEmptyCall) -1, 0, 0, 0, 0), SRT_ERROR); EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, UWaitEmptyCall) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); ASSERT_NE(client_sock, SRT_ERROR); @@ -111,12 +106,11 @@ TEST(CEPoll, UWaitEmptyCall) EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, WaitAllSocketsInEpollReleased) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); ASSERT_NE(client_sock, SRT_ERROR); @@ -146,12 +140,11 @@ TEST(CEPoll, WaitAllSocketsInEpollReleased) EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, WaitAllSocketsInEpollReleased2) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); ASSERT_NE(client_sock, SRT_ERROR); @@ -176,12 +169,11 @@ TEST(CEPoll, WaitAllSocketsInEpollReleased2) EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, WrongEpoll_idOnAddUSock) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); ASSERT_NE(client_sock, SRT_ERROR); @@ -199,13 +191,12 @@ TEST(CEPoll, WrongEpoll_idOnAddUSock) EXPECT_EQ(srt_epoll_release(epoll_id), 0); - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, HandleEpollEvent) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); EXPECT_NE(client_sock, SRT_ERROR); @@ -256,7 +247,6 @@ TEST(CEPoll, HandleEpollEvent) throw; } - EXPECT_EQ(srt_cleanup(), 0); } @@ -266,7 +256,7 @@ TEST(CEPoll, HandleEpollEvent) // be notified about connection break via polling the accepted socket. TEST(CEPoll, NotifyConnectionBreak) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; // 1. Prepare client SRTSOCKET client_sock = srt_create_socket(); @@ -376,13 +366,12 @@ TEST(CEPoll, NotifyConnectionBreak) if (!state_valid) cerr << "socket state: " << state << endl; - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, HandleEpollEvent2) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); EXPECT_NE(client_sock, SRT_ERROR); @@ -438,13 +427,12 @@ TEST(CEPoll, HandleEpollEvent2) throw; } - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, HandleEpollNoEvent) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); EXPECT_NE(client_sock, SRT_ERROR); @@ -490,12 +478,11 @@ TEST(CEPoll, HandleEpollNoEvent) throw; } - EXPECT_EQ(srt_cleanup(), 0); } TEST(CEPoll, ThreadedUpdate) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; SRTSOCKET client_sock = srt_create_socket(); EXPECT_NE(client_sock, SRT_ERROR); @@ -556,13 +543,10 @@ TEST(CEPoll, ThreadedUpdate) cerr << ex.getErrorMessage() << endl; throw; } - - - EXPECT_EQ(srt_cleanup(), 0); } -class TestEPoll: public testing::Test +class TestEPoll: public srt::Test { protected: @@ -692,7 +676,7 @@ class TestEPoll: public testing::Test ASSERT_EQ(rlen, 1); // get exactly one read event without writes ASSERT_EQ(wlen, 0); // get exactly one read event without writes - ASSERT_EQ(read[0], servsock); // read event is for bind socket + ASSERT_EQ(read[0], servsock); // read event is for bind socket } sockaddr_in scl; @@ -750,10 +734,8 @@ class TestEPoll: public testing::Test srt_close(servsock); } - void SetUp() override + void setup() override { - ASSERT_EQ(srt_startup(), 0); - m_client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, m_client_pollid); @@ -762,11 +744,10 @@ class TestEPoll: public testing::Test } - void TearDown() override + void teardown() override { (void)srt_epoll_release(m_client_pollid); (void)srt_epoll_release(m_server_pollid); - srt_cleanup(); } }; diff --git a/test/test_fec_rebuilding.cpp b/test/test_fec_rebuilding.cpp index 46afd8981a..44b9804a51 100644 --- a/test/test_fec_rebuilding.cpp +++ b/test/test_fec_rebuilding.cpp @@ -3,6 +3,7 @@ #include #include "gtest/gtest.h" +#include "test_env.h" #include "packet.h" #include "fec.h" #include "core.h" @@ -15,7 +16,7 @@ using namespace std; using namespace srt; -class TestFECRebuilding: public testing::Test +class TestFECRebuilding: public srt::Test { protected: FECFilterBuiltin* fec = nullptr; @@ -31,7 +32,7 @@ class TestFECRebuilding: public testing::Test PacketFilter::globalInit(); } - void SetUp() override + void setup() override { int timestamp = 10; @@ -86,7 +87,7 @@ class TestFECRebuilding: public testing::Test } } - void TearDown() override + void teardown() override { delete fec; } @@ -207,7 +208,7 @@ bool filterConfigSame(const string& config1, const string& config2) TEST(TestFEC, ConfigExchange) { - srt_startup(); + srt::TestInit srtinit; CUDTSocket* s1; @@ -234,12 +235,11 @@ TEST(TestFEC, ConfigExchange) string exp_config = "fec,cols:10,rows:10,arq:never,layout:staircase"; EXPECT_TRUE(filterConfigSame(fec_configback, exp_config)); - srt_cleanup(); } TEST(TestFEC, ConfigExchangeFaux) { - srt_startup(); + srt::TestInit srtinit; CUDTSocket* s1; @@ -273,12 +273,11 @@ TEST(TestFEC, ConfigExchangeFaux) cout << "(NOTE: expecting a failure message)\n"; EXPECT_FALSE(m1.checkApplyFilterConfig("fec,cols:10,arq:never")); - srt_cleanup(); } TEST(TestFEC, Connection) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -328,12 +327,11 @@ TEST(TestFEC, Connection) EXPECT_TRUE(filterConfigSame(caller_config, fec_config_final)); EXPECT_TRUE(filterConfigSame(accept_config, fec_config_final)); - srt_cleanup(); } TEST(TestFEC, ConnectionReorder) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -381,12 +379,11 @@ TEST(TestFEC, ConnectionReorder) EXPECT_TRUE(filterConfigSame(caller_config, fec_config_final)); EXPECT_TRUE(filterConfigSame(accept_config, fec_config_final)); - srt_cleanup(); } TEST(TestFEC, ConnectionFull1) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -434,11 +431,11 @@ TEST(TestFEC, ConnectionFull1) EXPECT_TRUE(filterConfigSame(caller_config, fec_config_final)); EXPECT_TRUE(filterConfigSame(accept_config, fec_config_final)); - srt_cleanup(); } + TEST(TestFEC, ConnectionFull2) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -486,12 +483,11 @@ TEST(TestFEC, ConnectionFull2) EXPECT_TRUE(filterConfigSame(caller_config, fec_config_final)); EXPECT_TRUE(filterConfigSame(accept_config, fec_config_final)); - srt_cleanup(); } TEST(TestFEC, ConnectionMess) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -539,12 +535,11 @@ TEST(TestFEC, ConnectionMess) EXPECT_TRUE(filterConfigSame(caller_config, fec_config_final)); EXPECT_TRUE(filterConfigSame(accept_config, fec_config_final)); - srt_cleanup(); } TEST(TestFEC, ConnectionForced) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -586,12 +581,11 @@ TEST(TestFEC, ConnectionForced) EXPECT_TRUE(filterConfigSame(result_config1, fec_config_final)); EXPECT_TRUE(filterConfigSame(result_config2, fec_config_final)); - srt_cleanup(); } TEST(TestFEC, RejectionConflict) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -629,12 +623,11 @@ TEST(TestFEC, RejectionConflict) int sclen = sizeof scl; EXPECT_EQ(srt_accept(l, (sockaddr*)& scl, &sclen), SRT_ERROR); - srt_cleanup(); } TEST(TestFEC, RejectionIncompleteEmpty) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -669,13 +662,12 @@ TEST(TestFEC, RejectionIncompleteEmpty) int sclen = sizeof scl; EXPECT_EQ(srt_accept(l, (sockaddr*)& scl, &sclen), SRT_ERROR); - srt_cleanup(); } TEST(TestFEC, RejectionIncomplete) { - srt_startup(); + srt::TestInit srtinit; SRTSOCKET s = srt_create_socket(); SRTSOCKET l = srt_create_socket(); @@ -713,7 +705,6 @@ TEST(TestFEC, RejectionIncomplete) int sclen = sizeof scl; EXPECT_EQ(srt_accept(l, (sockaddr*)& scl, &sclen), SRT_ERROR); - srt_cleanup(); } TEST_F(TestFECRebuilding, Prepare) diff --git a/test/test_file_transmission.cpp b/test/test_file_transmission.cpp index 5a646fb7dc..8ed56ff361 100644 --- a/test/test_file_transmission.cpp +++ b/test/test_file_transmission.cpp @@ -11,6 +11,7 @@ */ #include +#include "test_env.h" #ifdef _WIN32 #define INC_SRT_WIN_WINTIME // exclude gettimeofday from srt headers @@ -29,7 +30,7 @@ TEST(Transmission, FileUpload) { - srt_startup(); + srt::TestInit srtinit; // Generate the source file // We need a file that will contain more data @@ -195,5 +196,4 @@ TEST(Transmission, FileUpload) remove("file.source"); remove("file.target"); - (void)srt_cleanup(); } diff --git a/test/test_ipv6.cpp b/test/test_ipv6.cpp index 4dd235e06f..ee11292b09 100644 --- a/test/test_ipv6.cpp +++ b/test/test_ipv6.cpp @@ -1,13 +1,15 @@ -#include "gtest/gtest.h" #include #include +#include "gtest/gtest.h" +#include "test_env.h" + #include "srt.h" #include "netinet_any.h" using srt::sockaddr_any; class TestIPv6 - : public ::testing::Test + : public srt::Test { protected: int yes = 1; @@ -25,10 +27,8 @@ class TestIPv6 protected: // SetUp() is run immediately before a test starts. - void SetUp() + void setup() override { - ASSERT_GE(srt_startup(), 0); - m_caller_sock = srt_create_socket(); ASSERT_NE(m_caller_sock, SRT_ERROR); // IPv6 calling IPv4 would otherwise fail if the system-default net.ipv6.bindv6only=1. @@ -38,13 +38,12 @@ class TestIPv6 ASSERT_NE(m_listener_sock, SRT_ERROR); } - void TearDown() + void teardown() override { // Code here will be called just after the test completes. // OK to throw exceptions from here if needed. srt_close(m_listener_sock); srt_close(m_caller_sock); - srt_cleanup(); } public: @@ -140,6 +139,8 @@ TEST_F(TestIPv6, v4_calls_v6_mapped) TEST_F(TestIPv6, v6_calls_v6_mapped) { + SRTST_REQUIRES(IPv6); + sockaddr_any sa (AF_INET6); sa.hport(m_listen_port); @@ -157,6 +158,8 @@ TEST_F(TestIPv6, v6_calls_v6_mapped) TEST_F(TestIPv6, v6_calls_v6) { + SRTST_REQUIRES(IPv6); + sockaddr_any sa (AF_INET6); sa.hport(m_listen_port); diff --git a/test/test_listen_callback.cpp b/test/test_listen_callback.cpp index a3bdb3003d..c0609372e5 100644 --- a/test/test_listen_callback.cpp +++ b/test/test_listen_callback.cpp @@ -1,8 +1,9 @@ -#include #include #include #include #include +#include +#include "test_env.h" #ifdef _WIN32 #define INC_SRT_WIN_WINTIME // exclude gettimeofday from srt headers @@ -15,7 +16,7 @@ srt_listen_callback_fn SrtTestListenCallback; class ListenerCallback - : public testing::Test + : public srt::Test { protected: ListenerCallback() @@ -34,10 +35,8 @@ class ListenerCallback sockaddr_in sa; sockaddr* psa; - void SetUp() + void setup() { - ASSERT_EQ(srt_startup(), 0); - // Create server on 127.0.0.1:5555 server_sock = srt_create_socket(); @@ -124,7 +123,7 @@ class ListenerCallback srt_epoll_release(eid); } - void TearDown() + void teardown() { std::cout << "TeadDown: closing all sockets\n"; // Close the socket @@ -132,11 +131,9 @@ class ListenerCallback EXPECT_EQ(srt_close(server_sock), SRT_SUCCESS); // After that, the thread should exit - std::cout << "TearDown: joining accept thread\n"; + std::cout << "teardown: joining accept thread\n"; accept_thread.join(); - std::cout << "TearDown: SRT exit\n"; - - srt_cleanup(); + std::cout << "teardown: SRT exit\n"; } }; diff --git a/test/test_losslist_rcv.cpp b/test/test_losslist_rcv.cpp index 03a3088298..9ab4f382b1 100644 --- a/test/test_losslist_rcv.cpp +++ b/test/test_losslist_rcv.cpp @@ -1,5 +1,6 @@ #include #include "gtest/gtest.h" +#include "test_env.h" #include "common.h" #include "list.h" @@ -72,6 +73,7 @@ TEST_F(CRcvLossListTest, InsertTwoElemsEdge) TEST(CRcvFreshLossListTest, CheckFreshLossList) { + srt::TestInit srtinit; std::deque floss { CRcvFreshLoss (10, 15, 5), CRcvFreshLoss (25, 29, 10), diff --git a/test/test_main.cpp b/test/test_main.cpp new file mode 100644 index 0000000000..47c18c42fd --- /dev/null +++ b/test/test_main.cpp @@ -0,0 +1,180 @@ +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "test_env.h" + +#include "srt.h" +#include "netinet_any.h" + +using namespace std; + +int main(int argc, char **argv) +{ + string command_line_arg(argc == 2 ? argv[1] : ""); + testing::InitGoogleTest(&argc, argv); + testing::AddGlobalTestEnvironment(new srt::TestEnv(argc, argv)); + return RUN_ALL_TESTS(); +} + +namespace srt +{ + +TestEnv* TestEnv::me = 0; + +void TestEnv::FillArgMap() +{ + // The rule is: + // - first arguments go to an empty string key + // - if an argument has - in the beginning, name the key + // - key followed by args collected in a list + // - double dash prevents interpreting further args as option keys + + string key; + bool expectkey = true; + + argmap[""]; + + for (auto& a: args) + { + if (a.size() > 1) + { + if (expectkey && a[0] == '-') + { + if (a[1] == '-') + expectkey = false; + else if (a[1] == '/') + key = ""; + else + { + key = a.substr(1); + argmap[key]; // Make sure it exists even empty + } + + continue; + } + } + argmap[key].push_back(a); + } + + return; +} + +std::string TestEnv::OptionValue(const std::string& key) +{ + std::ostringstream out; + + auto it = argmap.find(key); + if (it != argmap.end() && !it->second.empty()) + { + auto iv = it->second.begin(); + out << (*iv); + while (++iv != it->second.end()) + { + out << " " << (*iv); + } + } + + return out.str(); +} + +// Specific functions +bool TestEnv::Allowed_IPv6() +{ + if (TestEnv::me->OptionPresent("disable-ipv6")) + { + std::cout << "TEST: IPv6 testing disabled, FORCED PASS\n"; + return false; + } + return true; +} + + +void TestInit::start(int& w_retstatus) +{ + ASSERT_GE(w_retstatus = srt_startup(), 0); +} + +void TestInit::stop() +{ + EXPECT_NE(srt_cleanup(), -1); +} + +// This function finds some interesting options among command +// line arguments and does specific things. +void TestInit::HandlePerTestOptions() +{ + // As a short example: + // use '-logdebug' option to turn on debug logging. + + if (TestEnv::me->OptionPresent("logdebug")) + { + srt_setloglevel(LOG_DEBUG); + } +} + +// Copied from ../apps/apputil.cpp, can't really link this file here. +sockaddr_any CreateAddr(const std::string& name, unsigned short port, int pref_family) +{ + using namespace std; + + // Handle empty name. + // If family is specified, empty string resolves to ANY of that family. + // If not, it resolves to IPv4 ANY (to specify IPv6 any, use [::]). + if (name == "") + { + sockaddr_any result(pref_family == AF_INET6 ? pref_family : AF_INET); + result.hport(port); + return result; + } + + bool first6 = pref_family != AF_INET; + int families[2] = {AF_INET6, AF_INET}; + if (!first6) + { + families[0] = AF_INET; + families[1] = AF_INET6; + } + + for (int i = 0; i < 2; ++i) + { + int family = families[i]; + sockaddr_any result (family); + + // Try to resolve the name by pton first + if (inet_pton(family, name.c_str(), result.get_addr()) == 1) + { + result.hport(port); // same addr location in ipv4 and ipv6 + return result; + } + } + + // If not, try to resolve by getaddrinfo + // This time, use the exact value of pref_family + + sockaddr_any result; + addrinfo fo = { + 0, + pref_family, + 0, 0, + 0, 0, + NULL, NULL + }; + + addrinfo* val = nullptr; + int erc = getaddrinfo(name.c_str(), nullptr, &fo, &val); + if (erc == 0) + { + result.set(val->ai_addr); + result.len = result.size(); + result.hport(port); // same addr location in ipv4 and ipv6 + } + freeaddrinfo(val); + + return result; +} + + +} diff --git a/test/test_many_connections.cpp b/test/test_many_connections.cpp index 0dbc62b4c2..20deccf70c 100644 --- a/test/test_many_connections.cpp +++ b/test/test_many_connections.cpp @@ -1,10 +1,11 @@ #define _CRT_RAND_S // For Windows, rand_s -#include #include #include #include #include +#include +#include "test_env.h" #ifdef _WIN32 #include @@ -26,7 +27,7 @@ using srt::sockaddr_any; class TestConnection - : public ::testing::Test + : public ::srt::Test { protected: TestConnection() @@ -45,11 +46,8 @@ class TestConnection static const size_t NSOCK = 60; protected: - // SetUp() is run immediately before a test starts. - void SetUp() override + void setup() override { - ASSERT_EQ(srt_startup(), 0); - m_sa.sin_family = AF_INET; m_sa.sin_addr.s_addr = INADDR_ANY; @@ -83,9 +81,8 @@ class TestConnection ASSERT_NE(srt_listen(m_server_sock, NSOCK), -1); } - void TearDown() override + void teardown() override { - srt_cleanup(); } void AcceptLoop() diff --git a/test/test_muxer.cpp b/test/test_muxer.cpp index f7efc85175..e85a12118c 100644 --- a/test/test_muxer.cpp +++ b/test/test_muxer.cpp @@ -1,9 +1,11 @@ #include "gtest/gtest.h" +#include "test_env.h" + #include #include "srt.h" class TestMuxer - : public ::testing::Test + : public srt::Test { protected: TestMuxer() @@ -18,10 +20,8 @@ class TestMuxer protected: // SetUp() is run immediately before a test starts. - void SetUp() + void setup() override { - ASSERT_GE(srt_startup(), 0); - m_caller_sock = srt_create_socket(); ASSERT_NE(m_caller_sock, SRT_ERROR); @@ -41,7 +41,7 @@ class TestMuxer srt_epoll_add_usock(m_client_pollid, m_caller_sock, &epoll_out); } - void TearDown() + void teardown() override { // Code here will be called just after the test completes. // OK to throw exceptions from here if needed. @@ -49,7 +49,6 @@ class TestMuxer srt_epoll_release(m_server_pollid); srt_close(m_listener_sock_ipv4); srt_close(m_listener_sock_ipv6); - srt_cleanup(); } public: @@ -100,6 +99,8 @@ class TestMuxer TEST_F(TestMuxer, IPv4_and_IPv6) { + SRTST_REQUIRES(IPv6); + int yes = 1; int no = 0; diff --git a/test/test_reuseaddr.cpp b/test/test_reuseaddr.cpp index 60532eb806..fe90273116 100644 --- a/test/test_reuseaddr.cpp +++ b/test/test_reuseaddr.cpp @@ -1,9 +1,10 @@ -#include "gtest/gtest.h" #include #include #ifndef _WIN32 #include #endif +#include "gtest/gtest.h" +#include "test_env.h" #include "common.h" #include "srt.h" @@ -23,67 +24,6 @@ struct AtReturnJoin } }; -// Copied from ../apps/apputil.cpp, can't really link this file here. -sockaddr_any CreateAddr(const std::string& name, unsigned short port, int pref_family = AF_INET) -{ - using namespace std; - - // Handle empty name. - // If family is specified, empty string resolves to ANY of that family. - // If not, it resolves to IPv4 ANY (to specify IPv6 any, use [::]). - if (name == "") - { - sockaddr_any result(pref_family == AF_INET6 ? pref_family : AF_INET); - result.hport(port); - return result; - } - - bool first6 = pref_family != AF_INET; - int families[2] = {AF_INET6, AF_INET}; - if (!first6) - { - families[0] = AF_INET; - families[1] = AF_INET6; - } - - for (int i = 0; i < 2; ++i) - { - int family = families[i]; - sockaddr_any result (family); - - // Try to resolve the name by pton first - if (inet_pton(family, name.c_str(), result.get_addr()) == 1) - { - result.hport(port); // same addr location in ipv4 and ipv6 - return result; - } - } - - // If not, try to resolve by getaddrinfo - // This time, use the exact value of pref_family - - sockaddr_any result; - addrinfo fo = { - 0, - pref_family, - 0, 0, - 0, 0, - NULL, NULL - }; - - addrinfo* val = nullptr; - int erc = getaddrinfo(name.c_str(), nullptr, &fo, &val); - if (erc == 0) - { - result.set(val->ai_addr); - result.len = result.size(); - result.hport(port); // same addr location in ipv4 and ipv6 - } - freeaddrinfo(val); - - return result; -} - #ifdef _WIN32 // On Windows there's a function for it, but it requires an extra @@ -191,7 +131,7 @@ void clientSocket(std::string ip, int port, bool expect_success) int epoll_out = SRT_EPOLL_OUT; srt_epoll_add_usock(client_pollid, g_client_sock, &epoll_out); - sockaddr_any sa = CreateAddr(ip, port, family); + sockaddr_any sa = srt::CreateAddr(ip, port, family); std::cout << "[T/C] Connecting to: " << sa.str() << " (" << famname << ")" << std::endl; @@ -274,7 +214,7 @@ SRTSOCKET prepareSocket() bool bindSocket(SRTSOCKET bindsock, std::string ip, int port, bool expect_success) { - sockaddr_any sa = CreateAddr(ip, port); + sockaddr_any sa = srt::CreateAddr(ip, port, AF_INET); std::string fam = (sa.family() == AF_INET) ? "IPv4" : "IPv6"; @@ -361,7 +301,7 @@ void testAccept(SRTSOCKET bindsock, std::string ip, int port, bool expect_succes ASSERT_EQ(rlen, 1); // get exactly one read event without writes ASSERT_EQ(wlen, 0); // get exactly one read event without writes - ASSERT_EQ(read[0], bindsock); // read event is for bind socket + ASSERT_EQ(read[0], bindsock); // read event is for bind socket } sockaddr_any scl; @@ -454,7 +394,7 @@ void shutdownListener(SRTSOCKET bindsock) TEST(ReuseAddr, SameAddr1) { - ASSERT_EQ(srt_startup(), 0); + srt::TestInit srtinit; client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -475,16 +415,15 @@ TEST(ReuseAddr, SameAddr1) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, SameAddr2) { + srt::TestInit srtinit; std::string localip = GetLocalIP(AF_INET); if (localip == "") return; // DISABLE TEST if this doesn't work. - ASSERT_EQ(srt_startup(), 0); client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -509,12 +448,12 @@ TEST(ReuseAddr, SameAddr2) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, SameAddrV6) { - ASSERT_EQ(srt_startup(), 0); + SRTST_REQUIRES(IPv6); + srt::TestInit srtinit; client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -539,17 +478,16 @@ TEST(ReuseAddr, SameAddrV6) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, DiffAddr) { + srt::TestInit srtinit; std::string localip = GetLocalIP(AF_INET); if (localip == "") return; // DISABLE TEST if this doesn't work. - ASSERT_EQ(srt_startup(), 0); client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -570,11 +508,11 @@ TEST(ReuseAddr, DiffAddr) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, Wildcard) { + srt::TestInit srtinit; #if defined(_WIN32) || defined(CYGWIN) std::cout << "!!!WARNING!!!: On Windows connection to localhost this way isn't possible.\n" "Forcing test to pass, PLEASE FIX.\n"; @@ -587,7 +525,6 @@ TEST(ReuseAddr, Wildcard) if (localip == "") return; // DISABLE TEST if this doesn't work. - ASSERT_EQ(srt_startup(), 0); client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -607,11 +544,12 @@ TEST(ReuseAddr, Wildcard) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, Wildcard6) { + SRTST_REQUIRES(IPv6); + srt::TestInit srtinit; #if defined(_WIN32) || defined(CYGWIN) std::cout << "!!!WARNING!!!: On Windows connection to localhost this way isn't possible.\n" "Forcing test to pass, PLEASE FIX.\n"; @@ -629,7 +567,6 @@ TEST(ReuseAddr, Wildcard6) // performed there. std::string localip_v4 = GetLocalIP(AF_INET); - ASSERT_EQ(srt_startup(), 0); client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -686,17 +623,18 @@ TEST(ReuseAddr, Wildcard6) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, ProtocolVersion6) { + SRTST_REQUIRES(IPv6); + + srt::TestInit srtinit; #if defined(_WIN32) || defined(CYGWIN) std::cout << "!!!WARNING!!!: On Windows connection to localhost this way isn't possible.\n" "Forcing test to pass, PLEASE FIX.\n"; return; #endif - ASSERT_EQ(srt_startup(), 0); client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -724,17 +662,17 @@ TEST(ReuseAddr, ProtocolVersion6) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } TEST(ReuseAddr, ProtocolVersionFaux6) { + SRTST_REQUIRES(IPv6); + srt::TestInit srtinit; #if defined(_WIN32) || defined(CYGWIN) std::cout << "!!!WARNING!!!: On Windows connection to localhost this way isn't possible.\n" "Forcing test to pass, PLEASE FIX.\n"; return; #endif - ASSERT_EQ(srt_startup(), 0); client_pollid = srt_epoll_create(); ASSERT_NE(SRT_ERROR, client_pollid); @@ -761,5 +699,4 @@ TEST(ReuseAddr, ProtocolVersionFaux6) (void)srt_epoll_release(client_pollid); (void)srt_epoll_release(server_pollid); - srt_cleanup(); } diff --git a/test/test_socket_options.cpp b/test/test_socket_options.cpp index 9b63853441..3c9f8d6773 100644 --- a/test/test_socket_options.cpp +++ b/test/test_socket_options.cpp @@ -10,10 +10,11 @@ * Haivision Systems Inc. */ -#include #include #include #include +#include +#include "test_env.h" // SRT includes #include "any.hpp" @@ -25,7 +26,7 @@ using namespace srt; class TestSocketOptions - : public ::testing::Test + : public ::srt::Test { protected: TestSocketOptions() @@ -79,10 +80,9 @@ class TestSocketOptions } protected: - // SetUp() is run immediately before a test starts. - void SetUp() + // setup() is run immediately before a test starts. + void setup() { - ASSERT_GE(srt_startup(), 0); const int yes = 1; memset(&m_sa, 0, sizeof m_sa); @@ -101,13 +101,12 @@ class TestSocketOptions ASSERT_EQ(srt_setsockopt(m_listen_sock, 0, SRTO_SNDSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect } - void TearDown() + void teardown() { // Code here will be called just after the test completes. // OK to throw exceptions from here if needed. - ASSERT_NE(srt_close(m_caller_sock), SRT_ERROR); - ASSERT_NE(srt_close(m_listen_sock), SRT_ERROR); - srt_cleanup(); + EXPECT_NE(srt_close(m_caller_sock), SRT_ERROR); + EXPECT_NE(srt_close(m_listen_sock), SRT_ERROR); } protected: diff --git a/test/test_socketdata.cpp b/test/test_socketdata.cpp index ab733fa576..547a2ebc9f 100644 --- a/test/test_socketdata.cpp +++ b/test/test_socketdata.cpp @@ -4,10 +4,10 @@ #include #include "gtest/gtest.h" +#include "test_env.h" #include "srt.h" #include "netinet_any.h" -#include "apputil.hpp" using namespace std; using namespace std::chrono; @@ -15,6 +15,8 @@ using namespace srt; TEST(SocketData, PeerName) { + srt::TestInit srtinit; + // Single-threaded one-app connect/accept action int csock = srt_create_socket(); @@ -25,7 +27,7 @@ TEST(SocketData, PeerName) srt_setsockflag(csock, SRTO_RCVSYN, &rd_nonblocking, sizeof (rd_nonblocking)); //srt_setsockflag(lsock, SRTO_RCVSYN, &rd_nonblocking, sizeof (rd_nonblocking)); - sockaddr_any addr = CreateAddr("127.0.0.1", 5000, AF_INET); + sockaddr_any addr = srt::CreateAddr("127.0.0.1", 5000, AF_INET); ASSERT_NE(srt_bind(lsock, addr.get(), addr.size()), -1); diff --git a/test/test_unitqueue.cpp b/test/test_unitqueue.cpp index 39517c1342..c58242727c 100644 --- a/test/test_unitqueue.cpp +++ b/test/test_unitqueue.cpp @@ -1,6 +1,7 @@ #include #include #include "gtest/gtest.h" +#include "test_env.h" #include "queue.h" using namespace std; @@ -15,6 +16,7 @@ using namespace srt; /// the very last element of the queue (it was skipped). TEST(CUnitQueue, Increase) { + srt::TestInit srtinit; const int buffer_size_pkts = 4; CUnitQueue unit_queue(buffer_size_pkts, 1500); @@ -35,6 +37,7 @@ TEST(CUnitQueue, Increase) /// beginning of the same queue. TEST(CUnitQueue, IncreaseAndFree) { + srt::TestInit srtinit; const int buffer_size_pkts = 4; CUnitQueue unit_queue(buffer_size_pkts, 1500); @@ -59,6 +62,7 @@ TEST(CUnitQueue, IncreaseAndFree) /// Thus the test checks if TEST(CUnitQueue, IncreaseAndFreeGrouped) { + srt::TestInit srtinit; const int buffer_size_pkts = 4; CUnitQueue unit_queue(buffer_size_pkts, 1500); From 256244fb69d443b8f042913ac15ee51902a79685 Mon Sep 17 00:00:00 2001 From: john Date: Tue, 8 Aug 2023 16:32:00 +0800 Subject: [PATCH 22/37] [core] Fix memory leak when can't buffer a HS packet (#2757). --- srtcore/queue.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/srtcore/queue.cpp b/srtcore/queue.cpp index 4282965b45..9b13a624a9 100644 --- a/srtcore/queue.cpp +++ b/srtcore/queue.cpp @@ -1783,7 +1783,11 @@ void srt::CRcvQueue::storePkt(int32_t id, CPacket* pkt) { // avoid storing too many packets, in case of malfunction or attack if (i->second.size() > 16) + { + delete[] pkt->m_pcData; + delete pkt; return; + } i->second.push(pkt); } From c6572bf20b619992bcd56990b433278ca01afca2 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Tue, 8 Aug 2023 15:56:39 +0200 Subject: [PATCH 23/37] [core] Refactor CRcvQueue::storePkt(..) for better resource management (#2775). --- srtcore/core.cpp | 2 +- srtcore/packet.cpp | 11 +++++------ srtcore/queue.cpp | 25 ++++++++----------------- srtcore/queue.h | 2 +- 4 files changed, 15 insertions(+), 25 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 1a2b8d2fc9..d58aa2478f 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -4817,7 +4817,7 @@ EConnectStatus srt::CUDT::postConnect(const CPacket* pResponse, bool rendezvous, // XXX Problem around CONN_CONFUSED! // If some too-eager packets were received from a listener // that thinks it's connected, but his last handshake was missed, - // they are collected by CRcvQueue::storePkt. The removeConnector + // they are collected by CRcvQueue::storePktClone. The removeConnector // function will want to delete them all, so it would be nice // if these packets can be re-delivered. Of course the listener // should be prepared to resend them (as every packet can be lost diff --git a/srtcore/packet.cpp b/srtcore/packet.cpp index 33555e7bb7..fbb56a42c7 100644 --- a/srtcore/packet.cpp +++ b/srtcore/packet.cpp @@ -221,6 +221,7 @@ void CPacket::deallocate() if (m_data_owned) delete[](char*) m_PacketVector[PV_DATA].data(); m_PacketVector[PV_DATA].set(NULL, 0); + m_data_owned = false; } char* CPacket::release() @@ -241,8 +242,7 @@ CPacket::~CPacket() { // PV_HEADER is always owned, PV_DATA may use a "borrowed" buffer. // Delete the internal buffer only if it was declared as owned. - if (m_data_owned) - delete[](char*) m_PacketVector[PV_DATA].data(); + deallocate(); } size_t CPacket::getLength() const @@ -561,10 +561,9 @@ CPacket* CPacket::clone() const { CPacket* pkt = new CPacket; memcpy((pkt->m_nHeader), m_nHeader, HDR_SIZE); - pkt->m_pcData = new char[m_PacketVector[PV_DATA].size()]; - memcpy((pkt->m_pcData), m_pcData, m_PacketVector[PV_DATA].size()); - pkt->m_PacketVector[PV_DATA].setLength(m_PacketVector[PV_DATA].size()); - + pkt->allocate(this->getLength()); + SRT_ASSERT(this->getLength() == pkt->getLength()); + memcpy((pkt->m_pcData), m_pcData, this->getLength()); pkt->m_DestAddr = m_DestAddr; return pkt; diff --git a/srtcore/queue.cpp b/srtcore/queue.cpp index 9b13a624a9..863148b34a 100644 --- a/srtcore/queue.cpp +++ b/srtcore/queue.cpp @@ -1162,7 +1162,6 @@ srt::CRcvQueue::~CRcvQueue() while (!i->second.empty()) { CPacket* pkt = i->second.front(); - delete[] pkt->m_pcData; delete pkt; i->second.pop(); } @@ -1365,14 +1364,12 @@ srt::EReadStatus srt::CRcvQueue::worker_RetrieveUnit(int32_t& w_id, CUnit*& w_un { // no space, skip this packet CPacket temp; - temp.m_pcData = new char[m_szPayloadSize]; - temp.setLength(m_szPayloadSize); + temp.allocate(m_szPayloadSize); THREAD_PAUSED(); EReadStatus rst = m_pChannel->recvfrom((w_addr), (temp)); THREAD_RESUMED(); // Note: this will print nothing about the packet details unless heavy logging is on. LOGC(qrlog.Error, log << CONID() << "LOCAL STORAGE DEPLETED. Dropping 1 packet: " << temp.Info()); - delete[] temp.m_pcData; // Be transparent for RST_ERROR, but ignore the correct // data read and fake that the packet was dropped. @@ -1541,7 +1538,7 @@ srt::EConnectStatus srt::CRcvQueue::worker_TryAsyncRend_OrStore(int32_t id, CUni if (cst == CONN_CONFUSED) { LOGC(cnlog.Warn, log << "AsyncOrRND: PACKET NOT HANDSHAKE - re-requesting handshake from peer"); - storePkt(id, unit->m_Packet.clone()); + storePktClone(id, unit->m_Packet); if (!u->processAsyncConnectRequest(RST_AGAIN, CONN_CONTINUE, &unit->m_Packet, u->m_PeerAddr)) { // Reuse previous behavior to reject a packet @@ -1616,7 +1613,7 @@ srt::EConnectStatus srt::CRcvQueue::worker_TryAsyncRend_OrStore(int32_t id, CUni log << "AsyncOrRND: packet RESOLVED TO ID=" << id << " -- continuing through CENTRAL PACKET QUEUE"); // This is where also the packets for rendezvous connection will be landing, // in case of a synchronous connection. - storePkt(id, unit->m_Packet.clone()); + storePktClone(id, unit->m_Packet); return CONN_CONTINUE; } @@ -1680,7 +1677,6 @@ int srt::CRcvQueue::recvfrom(int32_t id, CPacket& w_packet) w_packet.setLength(newpkt->getLength()); w_packet.m_DestAddr = newpkt->m_DestAddr; - delete[] newpkt->m_pcData; delete newpkt; // remove this message from queue, @@ -1735,7 +1731,6 @@ void srt::CRcvQueue::removeConnector(const SRTSOCKET& id) log << "removeConnector: ... and its packet queue with " << i->second.size() << " packets collected"); while (!i->second.empty()) { - delete[] i->second.front()->m_pcData; delete i->second.front(); i->second.pop(); } @@ -1768,7 +1763,7 @@ srt::CUDT* srt::CRcvQueue::getNewEntry() return u; } -void srt::CRcvQueue::storePkt(int32_t id, CPacket* pkt) +void srt::CRcvQueue::storePktClone(int32_t id, const CPacket& pkt) { CUniqueSync passcond(m_BufferLock, m_BufferCond); @@ -1776,26 +1771,22 @@ void srt::CRcvQueue::storePkt(int32_t id, CPacket* pkt) if (i == m_mBuffer.end()) { - m_mBuffer[id].push(pkt); + m_mBuffer[id].push(pkt.clone()); passcond.notify_one(); } else { - // avoid storing too many packets, in case of malfunction or attack + // Avoid storing too many packets, in case of malfunction or attack. if (i->second.size() > 16) - { - delete[] pkt->m_pcData; - delete pkt; return; - } - i->second.push(pkt); + i->second.push(pkt.clone()); } } void srt::CMultiplexer::destroy() { - // Reverse order of the assigned + // Reverse order of the assigned. delete m_pRcvQueue; delete m_pSndQueue; delete m_pTimer; diff --git a/srtcore/queue.h b/srtcore/queue.h index f527780878..dd68a77214 100644 --- a/srtcore/queue.h +++ b/srtcore/queue.h @@ -551,7 +551,7 @@ class CRcvQueue bool ifNewEntry(); CUDT* getNewEntry(); - void storePkt(int32_t id, CPacket* pkt); + void storePktClone(int32_t id, const CPacket& pkt); private: sync::Mutex m_LSLock; From 744035b2a41d17793d1596c79dc60d8add9ca5d4 Mon Sep 17 00:00:00 2001 From: yomnes0 <127947185+yomnes0@users.noreply.github.com> Date: Tue, 8 Aug 2023 17:08:10 +0200 Subject: [PATCH 24/37] [core] Fix hang up on not enough space in the RCV buffer (#2745). When there is space available in the receiving buffer after it is full, send an ack to allow the sender to resume transmission. Reschedule sending if ACK decreases the flight span after sending is congested. Co-authored-by: Maxim Sharabayko --- srtcore/core.cpp | 51 +++++++++++++++++++++++++++--------------------- srtcore/core.h | 2 +- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index d58aa2478f..bcc1050015 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -7908,7 +7908,6 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) SRT_ASSERT(ctrlpkt.getMsgTimeStamp() != 0); int nbsent = 0; int local_prevack = 0; - #if ENABLE_HEAVY_LOGGING struct SaveBack { @@ -7931,21 +7930,22 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) // The TSBPD thread may change the first lost sequence record (TLPKTDROP). // To avoid it the m_RcvBufferLock has to be acquired. UniqueLock bufflock(m_RcvBufferLock); - + // The full ACK should be sent to indicate there is now available space in the RCV buffer + // since the last full ACK. It should unblock the sender to proceed further. + const bool bNeedFullAck = (m_bBufferWasFull && getAvailRcvBufferSizeNoLock() > 0); int32_t ack; // First unacknowledged packet sequence number (acknowledge up to ack). if (!getFirstNoncontSequence((ack), (reason))) return nbsent; - if (m_iRcvLastAckAck == ack) + if (m_iRcvLastAckAck == ack && !bNeedFullAck) { - HLOGC(xtlog.Debug, - log << CONID() << "sendCtrl(UMSG_ACK): last ACK %" << ack << "(" << reason << ") == last ACKACK"); - return nbsent; + HLOGC(xtlog.Debug, + log << CONID() << "sendCtrl(UMSG_ACK): last ACK %" << ack << "(" << reason << ") == last ACKACK"); + return nbsent; } - // send out a lite ACK // to save time on buffer processing and bandwidth/AS measurement, a lite ACK only feeds back an ACK number - if (size == SEND_LITE_ACK) + if (size == SEND_LITE_ACK && !bNeedFullAck) { bufflock.unlock(); ctrlpkt.pack(UMSG_ACK, NULL, &ack, size); @@ -8083,7 +8083,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) CGlobEvent::triggerEvent(); } } - else if (ack == m_iRcvLastAck) + else if (ack == m_iRcvLastAck && !bNeedFullAck) { // If the ACK was just sent already AND elapsed time did not exceed RTT, if ((steady_clock::now() - m_tsLastAckTime) < @@ -8094,7 +8094,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) return nbsent; } } - else + else if (!bNeedFullAck) { // Not possible (m_iRcvCurrSeqNo+1 <% m_iRcvLastAck ?) LOGC(xtlog.Error, log << CONID() << "sendCtrl(UMSG_ACK): IPE: curr %" << ack << " <% last %" << m_iRcvLastAck); @@ -8105,7 +8105,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) // [[using locked(m_RcvBufferLock)]]; // Send out the ACK only if has not been received by the sender before - if (CSeqNo::seqcmp(m_iRcvLastAck, m_iRcvLastAckAck) > 0) + if (CSeqNo::seqcmp(m_iRcvLastAck, m_iRcvLastAckAck) > 0 || bNeedFullAck) { // NOTE: The BSTATS feature turns on extra fields above size 6 // also known as ACKD_TOTAL_SIZE_VER100. @@ -8121,10 +8121,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) data[ACKD_RTT] = m_iSRTT; data[ACKD_RTTVAR] = m_iRTTVar; data[ACKD_BUFFERLEFT] = (int) getAvailRcvBufferSizeNoLock(); - // a minimum flow window of 2 is used, even if buffer is full, to break potential deadlock - if (data[ACKD_BUFFERLEFT] < 2) - data[ACKD_BUFFERLEFT] = 2; - + m_bBufferWasFull = data[ACKD_BUFFERLEFT] == 0; if (steady_clock::now() - m_tsLastAckTime > m_tdACKInterval) { int rcvRate; @@ -8299,7 +8296,6 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ m_tsLastRspAckTime = currtime; m_iReXmitCount = 1; // Reset re-transmit count since last ACK } - return; } @@ -8340,14 +8336,25 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ return; } - if (CSeqNo::seqcmp(ackdata_seqno, m_iSndLastAck) >= 0) + if (CSeqNo::seqcmp(ackdata_seqno, m_iSndLastAck) >= 0) + { + const int cwnd1 = std::min(int(m_iFlowWindowSize), int(m_dCongestionWindow)); + const bool bWasStuck = cwnd1<= getFlightSpan(); + // Update Flow Window Size, must update before and together with m_iSndLastAck + m_iFlowWindowSize = ackdata[ACKD_BUFFERLEFT]; + m_iSndLastAck = ackdata_seqno; + m_tsLastRspAckTime = currtime; + m_iReXmitCount = 1; // Reset re-transmit count since last ACK + + const int cwnd = std::min(int(m_iFlowWindowSize), int(m_dCongestionWindow)); + if (bWasStuck && cwnd > getFlightSpan()) { - // Update Flow Window Size, must update before and together with m_iSndLastAck - m_iFlowWindowSize = ackdata[ACKD_BUFFERLEFT]; - m_iSndLastAck = ackdata_seqno; - m_tsLastRspAckTime = currtime; - m_iReXmitCount = 1; // Reset re-transmit count since last ACK + m_pSndQueue->m_pSndUList->update(this, CSndUList::DONT_RESCHEDULE); + HLOGC(gglog.Debug, + log << CONID() << "processCtrlAck: could reschedule SND. iFlowWindowSize " << m_iFlowWindowSize + << " SPAN " << getFlightSpan() << " ackdataseqno %" << ackdata_seqno); } + } /* * We must not ignore full ack received by peer diff --git a/srtcore/core.h b/srtcore/core.h index b83dec6e5e..c1d8e64e87 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -937,7 +937,7 @@ class CUDT int32_t m_iAckSeqNo; // Last ACK sequence number sync::atomic m_iRcvCurrSeqNo; // (RCV) Largest received sequence number. RcvQTh, TSBPDTh. int32_t m_iRcvCurrPhySeqNo; // Same as m_iRcvCurrSeqNo, but physical only (disregarding a filter) - + bool m_bBufferWasFull; // Indicate that RX buffer was full last time a ack was sent int32_t m_iPeerISN; // Initial Sequence Number of the peer side uint32_t m_uPeerSrtVersion; From 7cfe12b7e026e396c97c46b83a09c409d0378b93 Mon Sep 17 00:00:00 2001 From: Guangqing Chen Date: Wed, 17 May 2023 19:18:59 +0800 Subject: [PATCH 25/37] [core] fix tsbpd() may deadlock with processCtrlShutdown() --- srtcore/core.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index bcc1050015..2d3189bc92 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -5456,6 +5456,10 @@ void * srt::CUDT::tsbpd(void* param) tsNextDelivery = steady_clock::time_point(); // Ready to read, nothing to wait for. } + // We may just briefly unlocked the m_RecvLock, so we need to check m_bClosing again to avoid deadlock. + if (self->m_bClosing) + break; + if (!is_zero(tsNextDelivery)) { IF_HEAVY_LOGGING(const steady_clock::duration timediff = tsNextDelivery - tnow); From 46b0579a82e8aa4e544f74a0346c151688f1b060 Mon Sep 17 00:00:00 2001 From: Guangqing Chen Date: Wed, 9 Aug 2023 19:30:49 +0800 Subject: [PATCH 26/37] [core] Slightly optimize the RCV drop by message number (#2686). Some minor improvements of logs and comments. --- srtcore/buffer_rcv.cpp | 33 ++++++++++++++++++--------------- srtcore/core.cpp | 8 ++++---- srtcore/group.cpp | 10 ++++------ 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/srtcore/buffer_rcv.cpp b/srtcore/buffer_rcv.cpp index 8774d4d1da..1f46788aab 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 2d3189bc92..724b6a0840 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 2952d8c6b9..001dd4802d 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. From d039fe6bef16574f1ee239dcacdbdc056fe6fba3 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Thu, 10 Aug 2023 09:13:48 +0200 Subject: [PATCH 27/37] [core] Rejection not undertaken in rendezvous after KMX failure (#2692). --- srtcore/core.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 724b6a0840..ebba384fb2 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -4276,6 +4276,7 @@ EConnectStatus srt::CUDT::processRendezvous( { // m_RejectReason is already set, so set the reqtype accordingly m_ConnReq.m_iReqType = URQFailure(m_RejectReason); + return CONN_REJECT; } } // This should be false, make a kinda assert here. From 50619bd8a6cd62e3a2b3271f05f19714d9f6c172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Ma=C5=82ecki?= Date: Mon, 7 Aug 2023 17:09:49 +0200 Subject: [PATCH 28/37] [core] Fix: In rendezvous when processing resulted in ACCEPT it was still sending rejection --- srtcore/core.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index ebba384fb2..0fb31795af 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -3689,14 +3689,20 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) if (cst == CONN_CONTINUE) continue; - // Just in case it wasn't set, set this as a fallback - if (m_RejectReason == SRT_REJ_UNKNOWN) - m_RejectReason = SRT_REJ_ROGUE; + HLOGC(cnlog.Debug, + log << CONID() << "startConnect: processRendezvous returned cst=" << ConnectStatusStr(cst)); - // rejection or erroneous code. - reqpkt.setLength(m_iMaxSRTPayloadSize); - reqpkt.setControl(UMSG_HANDSHAKE); - sendRendezvousRejection(serv_addr, (reqpkt)); + if (cst == CONN_REJECT) + { + // Just in case it wasn't set, set this as a fallback + if (m_RejectReason == SRT_REJ_UNKNOWN) + m_RejectReason = SRT_REJ_ROGUE; + + // rejection or erroneous code. + reqpkt.setLength(m_iMaxSRTPayloadSize); + reqpkt.setControl(UMSG_HANDSHAKE); + sendRendezvousRejection(serv_addr, (reqpkt)); + } } if (cst == CONN_REJECT) @@ -4680,7 +4686,8 @@ bool srt::CUDT::applyResponseSettings(const CPacket* pHspkt /*[[nullable]]*/) AT HLOGC(cnlog.Debug, log << CONID() << "applyResponseSettings: HANSHAKE CONCLUDED. SETTING: payload-size=" << m_iMaxSRTPayloadSize - << " mss=" << m_ConnRes.m_iMSS << " flw=" << m_ConnRes.m_iFlightFlagSize << " isn=" << m_ConnRes.m_iISN + << " mss=" << m_ConnRes.m_iMSS << " flw=" << m_ConnRes.m_iFlightFlagSize << " peer-ISN=" << m_ConnRes.m_iISN + << " local-ISN=" << m_iISN << " peerID=" << m_ConnRes.m_iID << " sourceIP=" << m_SourceAddr.str()); return true; From 54ef85fc5a8eeae914d8b9e113959da6efb57bc7 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Mon, 14 Aug 2023 11:05:19 +0200 Subject: [PATCH 29/37] [core] Minor code clean up in CRateEstimator. --- srtcore/buffer_tools.cpp | 26 +++++++++++++------------- srtcore/buffer_tools.h | 4 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/srtcore/buffer_tools.cpp b/srtcore/buffer_tools.cpp index 1cdc72d588..3f5045e27a 100644 --- a/srtcore/buffer_tools.cpp +++ b/srtcore/buffer_tools.cpp @@ -138,20 +138,20 @@ void CRateEstimator::updateInputRate(const time_point& time, int pkts, int bytes const bool early_update = (m_InRatePeriod < INPUTRATE_RUNNING_US) && (m_iInRatePktsCount > INPUTRATE_MAX_PACKETS); const uint64_t period_us = count_microseconds(time - m_tsInRateStartTime); - if (early_update || period_us > m_InRatePeriod) - { - // Required Byte/sec rate (payload + headers) - m_iInRateBytesCount += (m_iInRatePktsCount * CPacket::SRT_DATA_HDR_SIZE); - m_iInRateBps = (int)(((int64_t)m_iInRateBytesCount * 1000000) / period_us); - HLOGC(bslog.Debug, - log << "updateInputRate: pkts:" << m_iInRateBytesCount << " bytes:" << m_iInRatePktsCount - << " rate=" << (m_iInRateBps * 8) / 1000 << "kbps interval=" << period_us); - m_iInRatePktsCount = 0; - m_iInRateBytesCount = 0; - m_tsInRateStartTime = time; + if (!early_update && period_us <= m_InRatePeriod) + return; - setInputRateSmpPeriod(INPUTRATE_RUNNING_US); - } + // Required Byte/sec rate (payload + headers) + m_iInRateBytesCount += (m_iInRatePktsCount * CPacket::SRT_DATA_HDR_SIZE); + m_iInRateBps = (int)(((int64_t)m_iInRateBytesCount * 1000000) / period_us); + HLOGC(bslog.Debug, + log << "updateInputRate: pkts:" << m_iInRateBytesCount << " bytes:" << m_iInRatePktsCount + << " rate=" << (m_iInRateBps * 8) / 1000 << "kbps interval=" << period_us); + m_iInRatePktsCount = 0; + m_iInRateBytesCount = 0; + m_tsInRateStartTime = time; + + setInputRateSmpPeriod(INPUTRATE_RUNNING_US); } diff --git a/srtcore/buffer_tools.h b/srtcore/buffer_tools.h index a6d81a3565..2131ff868d 100644 --- a/srtcore/buffer_tools.h +++ b/srtcore/buffer_tools.h @@ -120,8 +120,8 @@ class CRateEstimator static const int INPUTRATE_INITIAL_BYTESPS = BW_INFINITE; private: - int m_iInRatePktsCount; // number of payload bytes added since InRateStartTime - int m_iInRateBytesCount; // number of payload bytes added since InRateStartTime + int m_iInRatePktsCount; // number of payload packets added since InRateStartTime. + int m_iInRateBytesCount; // number of payload bytes added since InRateStartTime. time_point m_tsInRateStartTime; uint64_t m_InRatePeriod; // usec int m_iInRateBps; // Input Rate in Bytes/sec From f9e36db8fbf963f1def561d21e1424939ba2198a Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Mon, 14 Aug 2023 11:08:51 +0200 Subject: [PATCH 30/37] [core] Initialize ISN and PeerISN in CUDT. --- srtcore/core.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 0fb31795af..b2265cb9b0 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -305,7 +305,10 @@ void srt::CUDT::construct() // m_cbPacketArrival.set(this, &CUDT::defaultPacketArrival); } -srt::CUDT::CUDT(CUDTSocket* parent): m_parent(parent) +srt::CUDT::CUDT(CUDTSocket* parent) + : m_parent(parent) + , m_iISN(-1) + , m_iPeerISN(-1) { construct(); @@ -328,7 +331,10 @@ srt::CUDT::CUDT(CUDTSocket* parent): m_parent(parent) } -srt::CUDT::CUDT(CUDTSocket* parent, const CUDT& ancestor): m_parent(parent) +srt::CUDT::CUDT(CUDTSocket* parent, const CUDT& ancestor) + : m_parent(parent) + , m_iISN(-1) + , m_iPeerISN(-1) { construct(); From 033dc9f3bf1b149489c3758d2d798494e27eeac3 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Tue, 15 Aug 2023 14:18:06 +0200 Subject: [PATCH 31/37] [core] Drop unencrypted packets in GCM mode. --- srtcore/core.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index b2265cb9b0..765520f599 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -10021,6 +10021,22 @@ int srt::CUDT::handleSocketPacketReception(const vector& incoming, bool& } } } + else if (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) + { + // Unencrypted packets are not allowed. + const int iDropCnt = m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE, CRcvBuffer::DROP_EXISTING); + + const steady_clock::time_point tnow = steady_clock::now(); + ScopedLock lg(m_StatsLock); + m_stats.rcvr.dropped.count(stats::BytesPackets(iDropCnt* rpkt.getLength(), iDropCnt)); + m_stats.rcvr.undecrypted.count(stats::BytesPackets(rpkt.getLength(), 1)); + if (frequentLogAllowed(tnow)) + { + LOGC(qrlog.Warn, log << CONID() << "Packet not encrypted (seqno %" << u->m_Packet.getSeqNo() << "), dropped " + << iDropCnt << ". pktRcvUndecryptTotal=" << m_stats.rcvr.undecrypted.total.count() << "."); + m_tsLogSlowDown = tnow; + } + } } if (adding_successful) From e9eb8b330a25b443b3e009091616ffa8aa30752b Mon Sep 17 00:00:00 2001 From: Jose Santiago Date: Wed, 16 Aug 2023 01:20:33 -0500 Subject: [PATCH 32/37] [apps] Fix the build for target without IP_ADD_SOURCE_MEMBERSHIP (#2779). --- apps/transmitmedia.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/transmitmedia.cpp b/apps/transmitmedia.cpp index 87899e01dd..8509927d3d 100644 --- a/apps/transmitmedia.cpp +++ b/apps/transmitmedia.cpp @@ -848,7 +848,6 @@ class UdpCommon if (is_multicast) { - ip_mreq_source mreq_ssm; ip_mreq mreq; sockaddr_any maddr (AF_INET); int opt_name; @@ -872,6 +871,7 @@ class UdpCommon if (attr.count("source")) { #ifdef IP_ADD_SOURCE_MEMBERSHIP + ip_mreq_source mreq_ssm; /* this is an ssm. we need to use the right struct and opt */ opt_name = IP_ADD_SOURCE_MEMBERSHIP; mreq_ssm.imr_multiaddr.s_addr = sadr.sin.sin_addr.s_addr; From 78a102052f7457f4d304851953a4760e66129958 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Wed, 16 Aug 2023 08:43:28 +0200 Subject: [PATCH 33/37] [core] Added maximum BW limit for retransmissions (#2714). --- CMakeLists.txt | 8 +++ apps/socketoptions.hpp | 3 + docs/API/API-socket-options.md | 17 +++++ docs/build/build-options.md | 6 ++ srtcore/buffer_tools.cpp | 119 ++++++++++++++++++++++++++++++- srtcore/buffer_tools.h | 83 +++++++++++++++++++-- srtcore/core.cpp | 22 ++++++ srtcore/core.h | 5 +- srtcore/socketconfig.cpp | 18 +++++ srtcore/socketconfig.h | 6 ++ srtcore/srt.h | 3 + test/filelist.maf | 1 + test/test_snd_rate_estimator.cpp | 113 +++++++++++++++++++++++++++++ 13 files changed, 394 insertions(+), 10 deletions(-) create mode 100644 test/test_snd_rate_estimator.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index bdd421e46e..473d2d473c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -153,6 +153,7 @@ option(ENABLE_GETNAMEINFO "In-logs sockaddr-to-string should do rev-dns" OFF) option(ENABLE_UNITTESTS "Enable unit tests" OFF) option(ENABLE_ENCRYPTION "Enable encryption in SRT" ON) option(ENABLE_AEAD_API_PREVIEW "Enable AEAD API preview in SRT" Off) +option(ENABLE_MAXREXMITBW "Enable SRTO_MAXREXMITBW (v1.6.0 API preview)" Off) option(ENABLE_CXX_DEPS "Extra library dependencies in srt.pc for the CXX libraries useful with C language" ON) option(USE_STATIC_LIBSTDCXX "Should use static rather than shared libstdc++" OFF) option(ENABLE_INET_PTON "Set to OFF to prevent usage of inet_pton when building against modern SDKs while still requiring compatibility with older Windows versions, such as Windows XP, Windows Server 2003 etc." ON) @@ -466,6 +467,13 @@ if (USE_GNUSTL) set (SRT_LIBS_PRIVATE ${SRT_LIBS_PRIVATE} ${GNUSTL_LIBRARIES} ${GNUSTL_LDFLAGS}) endif() +if (ENABLE_MAXREXMITBW) + add_definitions(-DENABLE_MAXREXMITBW) + message(STATUS "MAXREXMITBW API: ENABLED") +else() + message(STATUS "MAXREXMITBW API: DISABLED") +endif() + if (USING_DEFAULT_COMPILER_PREFIX) # Detect if the compiler is GNU compatible for flags if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Intel|Clang|AppleClang") diff --git a/apps/socketoptions.hpp b/apps/socketoptions.hpp index 80f93767e2..b8aa67b861 100644 --- a/apps/socketoptions.hpp +++ b/apps/socketoptions.hpp @@ -258,6 +258,9 @@ const SocketOption srt_options [] { #ifdef ENABLE_AEAD_API_PREVIEW ,{ "cryptomode", 0, SRTO_CRYPTOMODE, SocketOption::PRE, SocketOption::INT, nullptr } #endif +#ifdef ENABLE_MAXREXMITBW + ,{ "maxrexmitbw", 0, SRTO_MAXREXMITBW, SocketOption::POST, SocketOption::INT64, nullptr } +#endif }; } diff --git a/docs/API/API-socket-options.md b/docs/API/API-socket-options.md index df4772d550..19d3019f9e 100644 --- a/docs/API/API-socket-options.md +++ b/docs/API/API-socket-options.md @@ -224,6 +224,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`SRTO_LINGER`](#SRTO_LINGER) | | post | `linger` | s | off \* | 0.. | RW | GSD | | [`SRTO_LOSSMAXTTL`](#SRTO_LOSSMAXTTL) | 1.2.0 | post | `int32_t` | packets | 0 | 0.. | RW | GSD+ | | [`SRTO_MAXBW`](#SRTO_MAXBW) | | post | `int64_t` | B/s | -1 | -1.. | RW | GSD | +| [`SRTO_MAXREXMITBW`](#SRTO_MAXREXMITBW) | 1.5.3 | post | `int64_t` | B/s | -1 | -1.. | RW | GSD | | [`SRTO_MESSAGEAPI`](#SRTO_MESSAGEAPI) | 1.3.0 | pre | `bool` | | true | | W | GSD | | [`SRTO_MININPUTBW`](#SRTO_MININPUTBW) | 1.4.3 | post | `int64_t` | B/s | 0 | 0.. | RW | GSD | | [`SRTO_MINVERSION`](#SRTO_MINVERSION) | 1.3.0 | pre | `int32_t` | version | 0x010000 | \* | RW | GSD | @@ -846,6 +847,22 @@ therefore the default -1 remains even in live mode. --- +#### SRTO_MAXREXMITBW + +| OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | +| -------------------- | ----- | -------- | ---------- | ------- | -------- | ------ | --- | ------ | +| `SRTO_MAXREXMITBW` | 1.5.3 | post | `int64_t` | B/s | -1 | -1.. | RW | GSD | + +Maximum BW limit for retransmissions: + +- `-1`: unlimited; +- `0`: do not allow retransmissions. +- `>0`: BW usage limit in Bytes/sec for packet retransmissions (including 16 bytes of SRT header). + +[Return to list](#list-of-options) + +--- + #### SRTO_MESSAGEAPI | OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | diff --git a/docs/build/build-options.md b/docs/build/build-options.md index 25ef1f7d3a..c97111ec70 100644 --- a/docs/build/build-options.md +++ b/docs/build/build-options.md @@ -35,6 +35,7 @@ Option details are given further below. | [`ENABLE_DEBUG`](#enable_debug) | 1.2.0 | `INT` | ON | Allows release/debug control through the `CMAKE_BUILD_TYPE` variable. | | [`ENABLE_ENCRYPTION`](#enable_encryption) | 1.3.3 | `BOOL` | ON | Enables encryption feature, with dependency on an external encryption library. | | [`ENABLE_AEAD_API_PREVIEW`](#enable_aead_api_preview) | 1.5.2 | `BOOL` | OFF | Enables AEAD preview API (encryption with integrity check). | +| [`ENABLE_MAXREXMITBW`](#enable_maxrexmitbw) | 1.5.3 | `BOOL` | OFF | Enables SRTO_MAXREXMITBW (v1.6.0 API). | | [`ENABLE_GETNAMEINFO`](#enable_getnameinfo) | 1.3.0 | `BOOL` | OFF | Enables the use of `getnameinfo` to allow using reverse DNS to resolve an internal IP address into a readable internet domain name. | | [`ENABLE_HAICRYPT_LOGGING`](#enable_haicrypt_logging) | 1.3.1 | `BOOL` | OFF | Enables logging in the *haicrypt* module, which serves as a connector to an encryption library. | | [`ENABLE_HEAVY_LOGGING`](#enable_heavy_logging) | 1.3.0 | `BOOL` | OFF | Enables heavy logging instructions in the code that occur often and cover many detailed aspects of library behavior. Default: OFF in release mode. | @@ -279,6 +280,11 @@ build option should be set to `USE_ENCLIB=openssl-evp`. The AEAD API is to be official in SRT v1.6.0. +#### ENABLE_MAXREXMITBW +**`--enable-maxrexmitbw`** (default: OFF) + +When ON, the `SRTO_MAXREXMITBW` is enabled (to become official in SRT v1.6.0). + #### ENABLE_GETNAMEINFO **`--enable-getnameinfo`** (default: OFF) diff --git a/srtcore/buffer_tools.cpp b/srtcore/buffer_tools.cpp index 3f5045e27a..0dcf2547fd 100644 --- a/srtcore/buffer_tools.cpp +++ b/srtcore/buffer_tools.cpp @@ -145,8 +145,8 @@ void CRateEstimator::updateInputRate(const time_point& time, int pkts, int bytes m_iInRateBytesCount += (m_iInRatePktsCount * CPacket::SRT_DATA_HDR_SIZE); m_iInRateBps = (int)(((int64_t)m_iInRateBytesCount * 1000000) / period_us); HLOGC(bslog.Debug, - log << "updateInputRate: pkts:" << m_iInRateBytesCount << " bytes:" << m_iInRatePktsCount - << " rate=" << (m_iInRateBps * 8) / 1000 << "kbps interval=" << period_us); + log << "updateInputRate: pkts:" << m_iInRateBytesCount << " bytes:" << m_iInRatePktsCount + << " rate=" << (m_iInRateBps * 8) / 1000 << "kbps interval=" << period_us); m_iInRatePktsCount = 0; m_iInRateBytesCount = 0; m_tsInRateStartTime = time; @@ -154,7 +154,122 @@ void CRateEstimator::updateInputRate(const time_point& time, int pkts, int bytes setInputRateSmpPeriod(INPUTRATE_RUNNING_US); } +CSndRateEstimator::CSndRateEstimator(const time_point& tsNow) + : m_tsFirstSampleTime(tsNow) + , m_iFirstSampleIdx(0) + , m_iCurSampleIdx(0) + , m_iRateBps(0) +{ + +} + +void CSndRateEstimator::addSample(const time_point& ts, int pkts, size_t bytes) +{ + const int iSampleDeltaIdx = (int) count_milliseconds(ts - m_tsFirstSampleTime) / SAMPLE_DURATION_MS; + const int delta = NUM_PERIODS - iSampleDeltaIdx; + + // TODO: -delta <= NUM_PERIODS, then just reset the state on the estimator. + + if (iSampleDeltaIdx >= 2 * NUM_PERIODS) + { + // Just reset the estimator and start like if new. + for (int i = 0; i < NUM_PERIODS; ++i) + { + const int idx = incSampleIdx(m_iFirstSampleIdx, i); + m_Samples[idx].reset(); + + if (idx == m_iCurSampleIdx) + break; + } + + m_iFirstSampleIdx = 0; + m_iCurSampleIdx = 0; + m_iRateBps = 0; + m_tsFirstSampleTime += milliseconds_from(iSampleDeltaIdx * SAMPLE_DURATION_MS); + } + else if (iSampleDeltaIdx > NUM_PERIODS) + { + // In run-time a constant flow of samples is expected. Once all periods are filled (after 1 second of sampling), + // the iSampleDeltaIdx should be either (NUM_PERIODS - 1), + // or NUM_PERIODS. In the later case it means the start of a new sampling period. + int d = delta; + while (d < 0) + { + m_Samples[m_iFirstSampleIdx].reset(); + m_iFirstSampleIdx = incSampleIdx(m_iFirstSampleIdx); + m_tsFirstSampleTime += milliseconds_from(SAMPLE_DURATION_MS); + m_iCurSampleIdx = incSampleIdx(m_iCurSampleIdx); + ++d; + } + } + + // Check if the new sample period has started. + const int iNewDeltaIdx = (int) count_milliseconds(ts - m_tsFirstSampleTime) / SAMPLE_DURATION_MS; + if (incSampleIdx(m_iFirstSampleIdx, iNewDeltaIdx) != m_iCurSampleIdx) + { + // Now there should be some periods (at most last NUM_PERIODS) ready to be summed, + // rate estimation updated, after which all the new entry should be added. + Sample sum; + int iNumPeriods = 0; + bool bMetNonEmpty = false; + for (int i = 0; i < NUM_PERIODS; ++i) + { + const int idx = incSampleIdx(m_iFirstSampleIdx, i); + const Sample& s = m_Samples[idx]; + sum += s; + if (bMetNonEmpty || !s.empty()) + { + ++iNumPeriods; + bMetNonEmpty = true; + } + + if (idx == m_iCurSampleIdx) + break; + } + + if (iNumPeriods == 0) + { + m_iRateBps = 0; + } + else + { + m_iRateBps = sum.m_iBytesCount * 1000 / (iNumPeriods * SAMPLE_DURATION_MS); + } + + HLOGC(bslog.Note, + log << "CSndRateEstimator: new rate estimation :" << (m_iRateBps * 8) / 1000 << " kbps. Based on " + << iNumPeriods << " periods, " << sum.m_iPktsCount << " packets, " << sum.m_iBytesCount << " bytes."); + // Shift one sampling period to start collecting the new one. + m_iCurSampleIdx = incSampleIdx(m_iCurSampleIdx); + m_Samples[m_iCurSampleIdx].reset(); + + // If all NUM_SAMPLES are recorded, the first position has to be shifted as well. + if (delta <= 0) + { + m_iFirstSampleIdx = incSampleIdx(m_iFirstSampleIdx); + m_tsFirstSampleTime += milliseconds_from(SAMPLE_DURATION_MS); + } + } + + m_Samples[m_iCurSampleIdx].m_iBytesCount += bytes; + m_Samples[m_iCurSampleIdx].m_iPktsCount += pkts; +} + +int CSndRateEstimator::getCurrentRate() const +{ + SRT_ASSERT(m_iCurSampleIdx >= 0 && m_iCurSampleIdx < NUM_PERIODS); + return (int) avg_iir<16, unsigned long long>(m_iRateBps, m_Samples[m_iCurSampleIdx].m_iBytesCount * 1000 / SAMPLE_DURATION_MS); +} + +int CSndRateEstimator::incSampleIdx(int val, int inc) const +{ + SRT_ASSERT(inc >= 0 && inc <= NUM_PERIODS); + val += inc; + while (val >= NUM_PERIODS) + val -= NUM_PERIODS; + return val; +} } diff --git a/srtcore/buffer_tools.h b/srtcore/buffer_tools.h index 2131ff868d..e6ce89d0de 100644 --- a/srtcore/buffer_tools.h +++ b/srtcore/buffer_tools.h @@ -55,7 +55,8 @@ modified by #include "common.h" -namespace srt { +namespace srt +{ /// The AvgBufSize class is used to calculate moving average of the buffer (RCV or SND) class AvgBufSize @@ -104,11 +105,9 @@ class CRateEstimator void setInputRateSmpPeriod(int period); /// Update input rate calculation. - /// @param [in] time current time in microseconds + /// @param [in] time current time /// @param [in] pkts number of packets newly added to the buffer /// @param [in] bytes number of payload bytes in those newly added packets - /// - /// @return Current size of the data in the sending list. void updateInputRate(const time_point& time, int pkts = 0, int bytes = 0); void resetInputRateSmpPeriod(bool disable = false) { setInputRateSmpPeriod(disable ? 0 : INPUTRATE_FAST_START_US); } @@ -123,10 +122,80 @@ class CRateEstimator int m_iInRatePktsCount; // number of payload packets added since InRateStartTime. int m_iInRateBytesCount; // number of payload bytes added since InRateStartTime. time_point m_tsInRateStartTime; - uint64_t m_InRatePeriod; // usec - int m_iInRateBps; // Input Rate in Bytes/sec + uint64_t m_InRatePeriod; // usec + int m_iInRateBps; // Input Rate in Bytes/sec +}; + + +class CSndRateEstimator +{ + typedef sync::steady_clock::time_point time_point; + +public: + CSndRateEstimator(const time_point& tsNow); + + /// Add sample. + /// @param [in] time sample (sending) time. + /// @param [in] pkts number of packets in the sample. + /// @param [in] bytes number of payload bytes in the sample. + void addSample(const time_point& time, int pkts = 0, size_t bytes = 0); + + /// Retrieve estimated bitrate in bytes per second + int getRate() const { return m_iRateBps; } + + /// Retrieve estimated bitrate in bytes per second inluding the current sampling interval. + int getCurrentRate() const; + +private: + static const int NUM_PERIODS = 10; + static const int SAMPLE_DURATION_MS = 100; // 100 ms + struct Sample + { + int m_iPktsCount; // number of payload packets + int m_iBytesCount; // number of payload bytes + + void reset() + { + m_iPktsCount = 0; + m_iBytesCount = 0; + } + + Sample() + : m_iPktsCount(0) + , m_iBytesCount(0) + { + } + + Sample(int iPkts, int iBytes) + : m_iPktsCount(iPkts) + , m_iBytesCount(iBytes) + { + } + + Sample operator+(const Sample& other) + { + return Sample(m_iPktsCount + other.m_iPktsCount, m_iBytesCount + other.m_iBytesCount); + } + + Sample& operator+=(const Sample& other) + { + *this = *this + other; + return *this; + } + + bool empty() const { return m_iPktsCount == 0; } + }; + + int incSampleIdx(int val, int inc = 1) const; + + Sample m_Samples[NUM_PERIODS]; + + time_point m_tsFirstSampleTime; //< Start time of the first sameple. + int m_iFirstSampleIdx; //< Index of the first sample. + int m_iCurSampleIdx; //< Index of the current sample being collected. + int m_iRateBps; // Input Rate in Bytes/sec }; -} +} // namespace srt #endif diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 765520f599..084cad670f 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -307,6 +307,9 @@ void srt::CUDT::construct() srt::CUDT::CUDT(CUDTSocket* parent) : m_parent(parent) +#ifdef ENABLE_MAXREXMITBW + , m_SndRexmitRate(sync::steady_clock::now()) +#endif , m_iISN(-1) , m_iPeerISN(-1) { @@ -333,6 +336,9 @@ srt::CUDT::CUDT(CUDTSocket* parent) srt::CUDT::CUDT(CUDTSocket* parent, const CUDT& ancestor) : m_parent(parent) +#ifdef ENABLE_MAXREXMITBW + , m_SndRexmitRate(sync::steady_clock::now()) +#endif , m_iISN(-1) , m_iPeerISN(-1) { @@ -9274,6 +9280,10 @@ int srt::CUDT::packLostData(CPacket& w_packet) } setDataPacketTS(w_packet, tsOrigin); +#ifdef ENABLE_MAXREXMITBW + m_SndRexmitRate.addSample(time_now, 1, w_packet.getLength()); +#endif + return payload; } @@ -9435,6 +9445,18 @@ bool srt::CUDT::isRetransmissionAllowed(const time_point& tnow SRT_ATR_UNUSED) return false; } +#ifdef ENABLE_MAXREXMITBW + m_SndRexmitRate.addSample(tnow, 0, 0); // Update the estimation. + const int64_t iRexmitRateBps = m_SndRexmitRate.getRate(); + const int64_t iRexmitRateLimitBps = m_config.llMaxRexmitBW; + if (iRexmitRateLimitBps >= 0 && iRexmitRateBps > iRexmitRateLimitBps) + { + // Too many retransmissions, so don't send anything. + // TODO: When to wake up next time? + return false; + } +#endif + #if SRT_DEBUG_TRACE_SND g_snd_logger.state.canRexmit = true; #endif diff --git a/srtcore/core.h b/srtcore/core.h index c1d8e64e87..611642d7aa 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -489,7 +489,7 @@ class CUDT /// Create the CryptoControl object based on the HS packet. SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) - bool prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd, CUDTException *eout); + bool prepareConnectionObjects(const CHandShake &hs, HandshakeSide hsd, CUDTException* eout); /// Allocates sender and receiver buffers and loss lists. SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) @@ -822,6 +822,9 @@ class CUDT CSndBuffer* m_pSndBuffer; // Sender buffer CSndLossList* m_pSndLossList; // Sender loss list CPktTimeWindow<16, 16> m_SndTimeWindow; // Packet sending time window +#ifdef ENABLE_MAXREXMITBW + CSndRateEstimator m_SndRexmitRate; // Retransmission retae estimation. +#endif atomic_duration m_tdSendInterval; // Inter-packet time, in CPU clock cycles diff --git a/srtcore/socketconfig.cpp b/srtcore/socketconfig.cpp index 4761620012..5c5b8e0903 100644 --- a/srtcore/socketconfig.cpp +++ b/srtcore/socketconfig.cpp @@ -236,6 +236,21 @@ struct CSrtConfigSetter } }; +#ifdef ENABLE_MAXREXMITBW +template<> +struct CSrtConfigSetter +{ + static void set(CSrtConfig& co, const void* optval, int optlen) + { + const int64_t val = cast_optval(optval, optlen); + if (val < -1) + throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); + + co.llMaxRexmitBW = val; + } +}; +#endif + template<> struct CSrtConfigSetter { @@ -997,6 +1012,9 @@ int dispatchSet(SRT_SOCKOPT optName, CSrtConfig& co, const void* optval, int opt #ifdef ENABLE_AEAD_API_PREVIEW DISPATCH(SRTO_CRYPTOMODE); #endif +#ifdef ENABLE_MAXREXMITBW + DISPATCH(SRTO_MAXREXMITBW); +#endif #undef DISPATCH default: diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index be4520cd7f..403616edfe 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -227,6 +227,9 @@ struct CSrtConfig: CSrtMuxerConfig int iSndTimeOut; // sending timeout in milliseconds int iRcvTimeOut; // receiving timeout in milliseconds int64_t llMaxBW; // maximum data transfer rate (threshold) +#ifdef ENABLE_MAXREXMITBW + int64_t llMaxRexmitBW; // maximum bandwidth limit for retransmissions (Bytes/s). +#endif // These fields keep the options for encryption // (SRTO_PASSPHRASE, SRTO_PBKEYLEN). Crypto object is @@ -289,6 +292,9 @@ struct CSrtConfig: CSrtMuxerConfig , iSndTimeOut(-1) , iRcvTimeOut(-1) , llMaxBW(-1) +#ifdef ENABLE_MAXREXMITBW + , llMaxRexmitBW(-1) +#endif , bDataSender(false) , bMessageAPI(true) , bTSBPD(true) diff --git a/srtcore/srt.h b/srtcore/srt.h index c30169f05c..53b6fd274c 100644 --- a/srtcore/srt.h +++ b/srtcore/srt.h @@ -242,6 +242,9 @@ typedef enum SRT_SOCKOPT { #ifdef ENABLE_AEAD_API_PREVIEW SRTO_CRYPTOMODE = 62, // Encryption cipher mode (AES-CTR, AES-GCM, ...). #endif +#ifdef ENABLE_MAXREXMITBW + SRTO_MAXREXMITBW = 63, // Maximum bandwidth limit for retransmision (Bytes/s) +#endif SRTO_E_SIZE // Always last element, not a valid option. } SRT_SOCKOPT; diff --git a/test/filelist.maf b/test/filelist.maf index 876439a83c..7ae5693b19 100644 --- a/test/filelist.maf +++ b/test/filelist.maf @@ -28,6 +28,7 @@ test_unitqueue.cpp test_utilities.cpp test_reuseaddr.cpp test_socketdata.cpp +test_snd_rate_estimator.cpp # Tests for bonding only - put here! diff --git a/test/test_snd_rate_estimator.cpp b/test/test_snd_rate_estimator.cpp new file mode 100644 index 0000000000..e3d512424c --- /dev/null +++ b/test/test_snd_rate_estimator.cpp @@ -0,0 +1,113 @@ +#include +#include +#include "gtest/gtest.h" +#include "buffer_tools.h" +#include "sync.h" + +using namespace srt; +using namespace std; + +#ifdef ENABLE_MAXREXMITBW + +class CSndRateEstFixture + : public ::testing::Test +{ +protected: + CSndRateEstFixture() + : m_tsStart(sync::steady_clock::now()) + , m_rateEst(m_tsStart) + { + // initialization code here + } + + virtual ~CSndRateEstFixture() + { + // cleanup any pending stuff, but no exceptions allowed + } + +protected: + // SetUp() is run immediately before a test starts. + void SetUp() override + { + // make_unique is unfortunatelly C++14 + + } + + void TearDown() override + { + // Code here will be called just after the test completes. + // OK to throw exceptions from here if needed. + } + + const sync::steady_clock::time_point m_tsStart; + CSndRateEstimator m_rateEst; +}; + +// Check the available size of the receiver buffer. +TEST_F(CSndRateEstFixture, Empty) +{ + //EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1); + EXPECT_EQ(m_rateEst.getRate(), 0); +} + + +TEST_F(CSndRateEstFixture, CBRSending) +{ + // Generate CBR sending for 2.1 seconds to wrap the buffer around. + for (int i = 0; i < 2100; ++i) + { + const auto t = m_tsStart + sync::milliseconds_from(i); + m_rateEst.addSample(t, 1, 1316); + + const auto rate = m_rateEst.getRate(); + if (i >= 100) + EXPECT_EQ(rate, 1316000) << "i=" << i; + else + EXPECT_EQ(rate, 0) << "i=" << i; + } + +} + +// Make a 1 second long pause and check that the rate is 0 again +// only for one sampling period. +TEST_F(CSndRateEstFixture, CBRSendingAfterPause) +{ + // Send 100 packets with 1000 bytes each + for (int i = 0; i < 3100; ++i) + { + if (i >= 1000 && i < 2000) + continue; + const auto t = m_tsStart + sync::milliseconds_from(i); + m_rateEst.addSample(t, 1, 1316); + + const auto rate = m_rateEst.getRate(); + if (i >= 100 && !(i >= 2000 && i < 2100)) + EXPECT_EQ(rate, 1316000) << "i=" << i; + else + EXPECT_EQ(rate, 0) << "i=" << i; + } +} + +// Make a short 0.5 second pause and check the bitrate goes down, but not to 0. +// Those empty samples should be included in bitrate estimation. +TEST_F(CSndRateEstFixture, CBRSendingShortPause) +{ + // Send 100 packets with 1000 bytes each + for (int i = 0; i < 3100; ++i) + { + if (i >= 1000 && i < 1500) + continue; + const auto t = m_tsStart + sync::milliseconds_from(i); + m_rateEst.addSample(t, 1, 1316); + + const auto rate = m_rateEst.getRate(); + if (i >= 1500 && i < 2000) + EXPECT_EQ(rate, 658000) << "i=" << i; + else if (i >= 100) + EXPECT_EQ(rate, 1316000) << "i=" << i; + else + EXPECT_EQ(rate, 0) << "i=" << i; + } +} + +#endif // ENABLE_MAXREXMITBW From 4682646e1212679298b9c3e1218cddf1ae3330c6 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Wed, 16 Aug 2023 08:45:28 +0200 Subject: [PATCH 34/37] [API] SRT version raised to 1.5.3. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 473d2d473c..1492306a28 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ # cmake_minimum_required (VERSION 2.8.12 FATAL_ERROR) -set (SRT_VERSION 1.5.2) +set (SRT_VERSION 1.5.3) set (CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/scripts") include(haiUtil) # needed for set_version_variables From 33a620bb3eb56ebcb14755b62ffb92466563aae5 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 16 Aug 2023 15:45:28 +0200 Subject: [PATCH 35/37] [apps] Fixed conditional IP_ADD_SOURCE_MEMBERSHIP in testmedia (#2780). --- testing/testmedia.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/testing/testmedia.cpp b/testing/testmedia.cpp index 34d2bdc9b9..a4e74fffb0 100755 --- a/testing/testmedia.cpp +++ b/testing/testmedia.cpp @@ -2754,9 +2754,8 @@ void UdpCommon::Setup(string host, int port, map attr) if (is_multicast) { - ip_mreq_source mreq_ssm; ip_mreq mreq; - sockaddr_any maddr; + sockaddr_any maddr (AF_INET); int opt_name; void* mreq_arg_ptr; socklen_t mreq_arg_size; @@ -2777,6 +2776,8 @@ void UdpCommon::Setup(string host, int port, map attr) if (attr.count("source")) { +#ifdef IP_ADD_SOURCE_MEMBERSHIP + ip_mreq_source mreq_ssm; /* this is an ssm. we need to use the right struct and opt */ opt_name = IP_ADD_SOURCE_MEMBERSHIP; mreq_ssm.imr_multiaddr.s_addr = sadr.sin.sin_addr.s_addr; @@ -2784,6 +2785,9 @@ void UdpCommon::Setup(string host, int port, map attr) inet_pton(AF_INET, attr.at("source").c_str(), &mreq_ssm.imr_sourceaddr); mreq_arg_size = sizeof(mreq_ssm); mreq_arg_ptr = &mreq_ssm; +#else + throw std::runtime_error("UdpCommon: source-filter multicast not supported by OS"); +#endif } else { From b1d8b0482bace6d10655092b8add30272d7d6039 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Mon, 21 Aug 2023 08:25:54 +0200 Subject: [PATCH 36/37] [core] Fixed SRT_ATTR_REQUIRES use. --- srtcore/core.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/srtcore/core.h b/srtcore/core.h index 611642d7aa..71c955c331 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -720,13 +720,13 @@ class CUDT SRT_ATTR_EXCLUDES(m_RcvBufferLock) bool isRcvBufferReady() const; - SRT_ATTR_REQUIRES2(m_RcvBufferLock) + SRT_ATTR_REQUIRES(m_RcvBufferLock) bool isRcvBufferReadyNoLock() const; // TSBPD thread main function. static void* tsbpd(void* param); - /// Drop too late packets (receiver side). Updaet loss lists and ACK positions. + /// Drop too late packets (receiver side). Update loss lists and ACK positions. /// The @a seqno packet itself is not dropped. /// @param seqno [in] The sequence number of the first packets following those to be dropped. /// @return The number of packets dropped. @@ -823,7 +823,7 @@ class CUDT CSndLossList* m_pSndLossList; // Sender loss list CPktTimeWindow<16, 16> m_SndTimeWindow; // Packet sending time window #ifdef ENABLE_MAXREXMITBW - CSndRateEstimator m_SndRexmitRate; // Retransmission retae estimation. + CSndRateEstimator m_SndRexmitRate; // Retransmission rate estimation. #endif atomic_duration m_tdSendInterval; // Inter-packet time, in CPU clock cycles @@ -866,7 +866,7 @@ class CUDT // and this is the sequence number that refers to the block at position [0]. Upon acknowledgement, // this value is shifted to the acknowledged position, and the blocks are removed from the // m_pSndBuffer buffer up to excluding this sequence number. - // XXX CONSIDER removing this field and give up the maintenance of this sequence number + // XXX CONSIDER removing this field and giving up the maintenance of this sequence number // to the sending buffer. This way, extraction of an old packet for retransmission should // require only the lost sequence number, and how to find the packet with this sequence // will be up to the sending buffer. From 51e3d0d50ba281ce985a1712785b5cb34f447b92 Mon Sep 17 00:00:00 2001 From: Thierry Lelegard Date: Mon, 21 Aug 2023 18:45:55 +0200 Subject: [PATCH 37/37] [build] Added missing public header files in Windows binary installer (#2784). The header file access_control.h was added to the source tree at some point but was not added to the Windows installer. --- scripts/win-installer/libsrt.nsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/win-installer/libsrt.nsi b/scripts/win-installer/libsrt.nsi index 4628642cc8..1dffc3fd17 100644 --- a/scripts/win-installer/libsrt.nsi +++ b/scripts/win-installer/libsrt.nsi @@ -125,9 +125,11 @@ Section "Install" ; Header files. CreateDirectory "$INSTDIR\include\srt" SetOutPath "$INSTDIR\include\srt" + File "${RepoDir}\srtcore\access_control.h" File "${RepoDir}\srtcore\logging_api.h" File "${RepoDir}\srtcore\platform_sys.h" File "${RepoDir}\srtcore\srt.h" + File "${RepoDir}\srtcore\udt.h" File "${Build64Dir}\version.h" CreateDirectory "$INSTDIR\include\win"