Skip to content

Commit

Permalink
Avoid abseil containers with SeqManager compare functions (#1369)
Browse files Browse the repository at this point in the history
  • Loading branch information
ibc authored Apr 9, 2024
1 parent 1a507ec commit c0e3a62
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 9 deletions.
1 change: 1 addition & 0 deletions .github/workflows/codeql.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ jobs:
env:
MEDIASOUP_SKIP_WORKER_PREBUILT_DOWNLOAD: 'true'
MEDIASOUP_LOCAL_DEV: 'true'
MEDIASOUP_BUILDTYPE: 'Release'

steps:
- name: Checkout repository
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/mediasoup-node.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,16 @@ jobs:
node: 20
- os: windows-2022
node: 20
build-type:
- Release
- Debug

runs-on: ${{ matrix.ci.os }}

env:
MEDIASOUP_SKIP_WORKER_PREBUILT_DOWNLOAD: 'true'
MEDIASOUP_LOCAL_DEV: 'true'
MEDIASOUP_BUILDTYPE: ${{ matrix.build-type }}

steps:
- name: Checkout
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/mediasoup-rust.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ jobs:
- name: cargo clippy
run: cargo clippy --all-targets -- -D warnings

# NOTE: In Windows this will build and test libmediasoupworker in release
# mode twice since build.rs doesn't allow debug mode for Windows.
- name: cargo test
run: cargo test --verbose
run: |
cargo test --verbose
cargo test --release --verbose
- name: cargo doc
run: cargo doc --locked --all --no-deps --lib
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/mediasoup-worker-fuzzer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ jobs:
cc: clang
cxx: clang++
arch: arm64
build-type:
- Release
- Debug

runs-on: ${{ matrix.build.os }}

Expand All @@ -28,6 +31,7 @@ jobs:
CXX: ${{ matrix.build.cxx }}
MEDIASOUP_SKIP_WORKER_PREBUILT_DOWNLOAD: 'true'
MEDIASOUP_LOCAL_DEV: 'true'
MEDIASOUP_BUILDTYPE: ${{ matrix.build-type }}

steps:
- name: Checkout
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/mediasoup-worker-prebuild.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ jobs:
CXX: ${{ matrix.build.cxx }}
MEDIASOUP_SKIP_WORKER_PREBUILT_DOWNLOAD: 'true'
MEDIASOUP_LOCAL_DEV: 'true'
MEDIASOUP_BUILDTYPE: 'Release'

steps:
- name: Checkout
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/mediasoup-worker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ jobs:
# A single Node.js version should be fine for C++.
node:
- 20
build-type:
- Release
- Debug

runs-on: ${{ matrix.build.os }}

Expand All @@ -61,6 +64,7 @@ jobs:
CXX: ${{ matrix.build.cxx }}
MEDIASOUP_SKIP_WORKER_PREBUILT_DOWNLOAD: 'true'
MEDIASOUP_LOCAL_DEV: 'true'
MEDIASOUP_BUILDTYPE: ${{ matrix.build-type }}

steps:
- name: Checkout
Expand Down
8 changes: 3 additions & 5 deletions worker/include/RTC/NackGenerator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#include "RTC/RtpPacket.hpp"
#include "RTC/SeqManager.hpp"
#include "handles/TimerHandle.hpp"
#include <absl/container/btree_map.h>
#include <absl/container/btree_set.h>
#include <map>
#include <set>
#include <vector>
Expand Down Expand Up @@ -80,9 +78,9 @@ namespace RTC
// Allocated by this.
TimerHandle* timer{ nullptr };
// Others.
absl::btree_map<uint16_t, NackInfo, RTC::SeqManager<uint16_t>::SeqLowerThan> nackList;
absl::btree_set<uint16_t, RTC::SeqManager<uint16_t>::SeqLowerThan> keyFrameList;
absl::btree_set<uint16_t, RTC::SeqManager<uint16_t>::SeqLowerThan> recoveredList;
std::map<uint16_t, NackInfo, RTC::SeqManager<uint16_t>::SeqLowerThan> nackList;
std::set<uint16_t, RTC::SeqManager<uint16_t>::SeqLowerThan> keyFrameList;
std::set<uint16_t, RTC::SeqManager<uint16_t>::SeqLowerThan> recoveredList;
bool started{ false };
uint16_t lastSeq{ 0u }; // Seq number of last valid packet.
uint32_t rtt{ 0u }; // Round trip time (ms).
Expand Down
3 changes: 1 addition & 2 deletions worker/include/RTC/SenderBandwidthEstimator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "RTC/RateCalculator.hpp"
#include "RTC/SeqManager.hpp"
#include "RTC/TrendCalculator.hpp"
#include <absl/container/btree_map.h>
#include <map>

namespace RTC
Expand Down Expand Up @@ -103,7 +102,7 @@ namespace RTC
uint32_t initialAvailableBitrate{ 0u };
uint32_t availableBitrate{ 0u };
uint64_t lastAvailableBitrateEventAtMs{ 0u };
absl::btree_map<uint16_t, SentInfo, RTC::SeqManager<uint16_t>::SeqLowerThan> sentInfos;
std::map<uint16_t, SentInfo, RTC::SeqManager<uint16_t>::SeqLowerThan> sentInfos;
float rtt{ 0 }; // Round trip time in ms.
CummulativeResult cummulativeResult;
CummulativeResult probationCummulativeResult;
Expand Down
2 changes: 2 additions & 0 deletions worker/include/RTC/WebRtcServer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ namespace RTC
absl::flat_hash_map<std::string, RTC::WebRtcTransport*> mapLocalIceUsernameFragmentWebRtcTransport;
// Map of WebRtcTransports indexed by TransportTuple.hash.
absl::flat_hash_map<uint64_t, RTC::WebRtcTransport*> mapTupleWebRtcTransport;
// Whether the destructor has been called.
bool closing{ false };
};
} // namespace RTC

Expand Down
1 change: 1 addition & 0 deletions worker/src/RTC/AudioLevelObserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "RTC/RtpDictionaries.hpp"
#include <absl/container/btree_map.h>
#include <cmath> // std::lround()

namespace RTC
Expand Down
11 changes: 10 additions & 1 deletion worker/src/RTC/WebRtcServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ namespace RTC
{
MS_TRACE();

this->closing = true;

this->shared->channelMessageRegistrator->UnregisterHandler(this->id);

for (auto& item : this->udpSocketOrTcpServers)
Expand Down Expand Up @@ -497,7 +499,14 @@ namespace RTC
this->webRtcTransports.find(webRtcTransport) != this->webRtcTransports.end(),
"WebRtcTransport not handled");

this->webRtcTransports.erase(webRtcTransport);
// NOTE: If WebRtcServer is closing then do not remove the transport from
// the set since it would modify the set while the WebRtcServer destructor
// is iterating it.
// See: https://github.com/versatica/mediasoup/pull/1369#issuecomment-2044672247
if (!this->closing)
{
this->webRtcTransports.erase(webRtcTransport);
}
}

inline void WebRtcServer::OnWebRtcTransportLocalIceUsernameFragmentAdded(
Expand Down

0 comments on commit c0e3a62

Please sign in to comment.