From 58384dcd5e981fbb4295525c7e7d91ceecfdcb3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Tue, 5 Mar 2024 13:47:16 +0100 Subject: [PATCH] Test usrsctp with usrsctp_get_timeout() ### Details - Related task: https://github.com/versatica/mediasoup/issues/1011. Note that usrsctp author never cared about the existing PR so we are on our own. - So I've forked usrctp, added the `usrsctp_get_timeout()` feature and released version 0.9.6.0: https://github.com/versatica/usrsctp/pull/1 - And I've made `usrsctp.wrap` point to it. ### Notes - It would be great to have a way to pass `sctp_debug=true` when building usrsctp Meson subproject so it defines `SCTP_DEBUG` (needed in `DepUsrSCTP.cpp` to show SCTP debug). An manual alternative is to edit `meson.build` of usrsctp and add it unconditionally. - I'm calling `HandleUsrSctpTimers()` everytime the timer fires (of course), also in `onSendSctpData` callback and also when SCTP data is received (in `SctpAssociation::ProcessSctpData()`. Not sure if correct. --- worker/include/DepUsrSCTP.hpp | 2 ++ worker/src/DepUsrSCTP.cpp | 39 +++++++++++++++++++++++++++--- worker/src/RTC/SctpAssociation.cpp | 4 +++ worker/subprojects/usrsctp.wrap | 8 +++--- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/worker/include/DepUsrSCTP.hpp b/worker/include/DepUsrSCTP.hpp index 122cb63c47..8ee5ab0ebe 100644 --- a/worker/include/DepUsrSCTP.hpp +++ b/worker/include/DepUsrSCTP.hpp @@ -18,6 +18,7 @@ class DepUsrSCTP public: void Start(); void Stop(); + void HandleUsrSctpTimers(); /* Pure virtual methods inherited from TimerHandle::Listener. */ public: @@ -37,6 +38,7 @@ class DepUsrSCTP static void RegisterSctpAssociation(RTC::SctpAssociation* sctpAssociation); static void DeregisterSctpAssociation(RTC::SctpAssociation* sctpAssociation); static RTC::SctpAssociation* RetrieveSctpAssociation(uintptr_t id); + static void HandleUsrSctpTimers(); private: thread_local static Checker* checker; diff --git a/worker/src/DepUsrSCTP.cpp b/worker/src/DepUsrSCTP.cpp index 700bac3348..c5a2ae1198 100644 --- a/worker/src/DepUsrSCTP.cpp +++ b/worker/src/DepUsrSCTP.cpp @@ -13,20 +13,26 @@ /* Static. */ -static constexpr size_t CheckerInterval{ 10u }; // In ms. static std::mutex GlobalSyncMutex; static size_t GlobalInstances{ 0u }; /* Static methods for usrsctp global callbacks. */ -inline static int onSendSctpData(void* addr, void* data, size_t len, uint8_t /*tos*/, uint8_t /*setDf*/) +static int onSendSctpData(void* addr, void* data, size_t len, uint8_t /*tos*/, uint8_t /*setDf*/) { + MS_TRACE(); + auto* sctpAssociation = DepUsrSCTP::RetrieveSctpAssociation(reinterpret_cast(addr)); if (!sctpAssociation) { MS_WARN_TAG(sctp, "no SctpAssociation found"); + DepUsrSCTP::HandleUsrSctpTimers(); + + // TODO: Why are we returning -1 here? What does it tell usrsctp? + // If the SctpAssociation doesn't exist anymore we don't want that usrscp + // insists. return -1; } @@ -34,12 +40,15 @@ inline static int onSendSctpData(void* addr, void* data, size_t len, uint8_t /*t // NOTE: Must not free data, usrsctp lib does it. + DepUsrSCTP::HandleUsrSctpTimers(); + return 0; } // Static method for printing usrsctp debug. -inline static void sctpDebug(const char* format, ...) +static void sctpDebug(const char* format, ...) { + MS_TRACE(); char buffer[10000]; va_list ap; @@ -207,6 +216,13 @@ RTC::SctpAssociation* DepUsrSCTP::RetrieveSctpAssociation(uintptr_t id) return it->second; } +void DepUsrSCTP::HandleUsrSctpTimers() +{ + MS_TRACE(); + + DepUsrSCTP::checker->HandleUsrSctpTimers(); +} + /* DepUsrSCTP::Checker instance methods. */ DepUsrSCTP::Checker::Checker() : timer(new TimerHandle(this)) @@ -229,7 +245,7 @@ void DepUsrSCTP::Checker::Start() this->lastCalledAtMs = 0u; - this->timer->Start(CheckerInterval, CheckerInterval); + HandleUsrSctpTimers(); } void DepUsrSCTP::Checker::Stop() @@ -243,10 +259,23 @@ void DepUsrSCTP::Checker::Stop() this->timer->Stop(); } +void DepUsrSCTP::Checker::HandleUsrSctpTimers() +{ + MS_TRACE(); + + auto timeout = static_cast(usrsctp_get_timeout()); + + MS_DUMP("------ starting timer with usrsctp timeout: %" PRIu64 " ms", timeout); + + this->timer->Start(timeout); +} + void DepUsrSCTP::Checker::OnTimer(TimerHandle* /*timer*/) { MS_TRACE(); + MS_DUMP("------ onTimer!"); + auto nowMs = DepLibUV::GetTimeMs(); const int elapsedMs = this->lastCalledAtMs ? static_cast(nowMs - this->lastCalledAtMs) : 0; @@ -267,4 +296,6 @@ void DepUsrSCTP::Checker::OnTimer(TimerHandle* /*timer*/) #endif this->lastCalledAtMs = nowMs; + + HandleUsrSctpTimers(); } diff --git a/worker/src/RTC/SctpAssociation.cpp b/worker/src/RTC/SctpAssociation.cpp index 499ec2a053..07e21f0bba 100644 --- a/worker/src/RTC/SctpAssociation.cpp +++ b/worker/src/RTC/SctpAssociation.cpp @@ -390,6 +390,10 @@ namespace RTC #endif usrsctp_conninput(reinterpret_cast(this->id), data, len, 0); + + // TODO: IMHO it makes sense that we handle/refresh usrsctp timers when + // SCTP data is received. + DepUsrSCTP::HandleUsrSctpTimers(); } void SctpAssociation::SendSctpMessage( diff --git a/worker/subprojects/usrsctp.wrap b/worker/subprojects/usrsctp.wrap index 3cd7061b98..3f85e6d12d 100644 --- a/worker/subprojects/usrsctp.wrap +++ b/worker/subprojects/usrsctp.wrap @@ -1,8 +1,8 @@ [wrap-file] -directory = usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299 -source_url = https://github.com/sctplab/usrsctp/archive/ebb18adac6501bad4501b1f6dccb67a1c85cc299.zip -source_filename = ebb18adac6501bad4501b1f6dccb67a1c85cc299.zip -source_hash = e77a855ce395b877e9f2aa7ebe070dfaf5cb11cfecdeb424cc876fc0d98e5d11 +directory = usrsctp-0.9.6.0 +source_url = https://github.com/versatica/usrsctp/archive/refs/tags/0.9.6.0.zip +source_filename = usrsctp-0.9.6.0.zip +source_hash = 13518f1912631c796da167ba3c3e4fd7f80886de4afa75abc6fcf843160b65f8 [provide] usrsctp = usrsctp_dep