Replace lock with SequenceChecker in PacketRouter
Since PacketRouter now is only used on the worker thread, there is no need for a lock.
Bug: webrtc:14502
Change-Id: I65778f68b7e211d7bc7388a4615888a49ceb5f59
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300964
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39848}
diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc
index b28d977..85490ef 100644
--- a/modules/pacing/packet_router.cc
+++ b/modules/pacing/packet_router.cc
@@ -19,12 +19,10 @@
#include "absl/types/optional.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtcp_packet.h"
-#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_interface.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/system/unused.h"
-#include "rtc_base/time_utils.h"
#include "rtc_base/trace_event.h"
namespace webrtc {
@@ -37,6 +35,7 @@
transport_seq_(start_transport_seq) {}
PacketRouter::~PacketRouter() {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_DCHECK(send_modules_map_.empty());
RTC_DCHECK(send_modules_list_.empty());
RTC_DCHECK(rtcp_feedback_senders_.empty());
@@ -47,7 +46,7 @@
void PacketRouter::AddSendRtpModule(RtpRtcpInterface* rtp_module,
bool remb_candidate) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
AddSendRtpModuleToMap(rtp_module, rtp_module->SSRC());
if (absl::optional<uint32_t> rtx_ssrc = rtp_module->RtxSsrc()) {
@@ -68,6 +67,7 @@
void PacketRouter::AddSendRtpModuleToMap(RtpRtcpInterface* rtp_module,
uint32_t ssrc) {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_DCHECK(send_modules_map_.find(ssrc) == send_modules_map_.end());
// Signal to module that the pacer thread is attached and can send packets.
@@ -86,6 +86,7 @@
}
void PacketRouter::RemoveSendRtpModuleFromMap(uint32_t ssrc) {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
auto it = send_modules_map_.find(ssrc);
RTC_DCHECK(it != send_modules_map_.end());
send_modules_list_.remove(it->second);
@@ -93,7 +94,7 @@
}
void PacketRouter::RemoveSendRtpModule(RtpRtcpInterface* rtp_module) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
MaybeRemoveRembModuleCandidate(rtp_module, /* media_sender = */ true);
RemoveSendRtpModuleFromMap(rtp_module->SSRC());
@@ -112,7 +113,7 @@
void PacketRouter::AddReceiveRtpModule(RtcpFeedbackSenderInterface* rtcp_sender,
bool remb_candidate) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_DCHECK(std::find(rtcp_feedback_senders_.begin(),
rtcp_feedback_senders_.end(),
rtcp_sender) == rtcp_feedback_senders_.end());
@@ -126,7 +127,7 @@
void PacketRouter::RemoveReceiveRtpModule(
RtcpFeedbackSenderInterface* rtcp_sender) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
MaybeRemoveRembModuleCandidate(rtcp_sender, /* media_sender = */ false);
auto it = std::find(rtcp_feedback_senders_.begin(),
rtcp_feedback_senders_.end(), rtcp_sender);
@@ -136,11 +137,11 @@
void PacketRouter::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
const PacedPacketInfo& cluster_info) {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc"), "PacketRouter::SendPacket",
"sequence_number", packet->SequenceNumber(), "rtp_timestamp",
packet->Timestamp());
- MutexLock lock(&modules_mutex_);
// With the new pacer code path, transport sequence numbers are only set here,
// on the pacer thread. Therefore we don't need atomics/synchronization.
bool assign_transport_sequence_number =
@@ -184,7 +185,7 @@
}
std::vector<std::unique_ptr<RtpPacketToSend>> PacketRouter::FetchFec() {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
std::move(pending_fec_packets_);
pending_fec_packets_.clear();
@@ -193,10 +194,10 @@
std::vector<std::unique_ptr<RtpPacketToSend>> PacketRouter::GeneratePadding(
DataSize size) {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("webrtc"),
"PacketRouter::GeneratePadding", "bytes", size.bytes());
- MutexLock lock(&modules_mutex_);
// First try on the last rtp module to have sent media. This increases the
// the chance that any payload based padding will be useful as it will be
// somewhat distributed over modules according the packet rate, even if it
@@ -238,7 +239,7 @@
void PacketRouter::OnAbortedRetransmissions(
uint32_t ssrc,
rtc::ArrayView<const uint16_t> sequence_numbers) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
auto it = send_modules_map_.find(ssrc);
if (it != send_modules_map_.end()) {
it->second->OnAbortedRetransmissions(sequence_numbers);
@@ -246,7 +247,7 @@
}
absl::optional<uint32_t> PacketRouter::GetRtxSsrcForMedia(uint32_t ssrc) const {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
auto it = send_modules_map_.find(ssrc);
if (it != send_modules_map_.end() && it->second->SSRC() == ssrc) {
// A module is registered with the given SSRC, and that SSRC is the main
@@ -257,12 +258,12 @@
}
uint16_t PacketRouter::CurrentTransportSequenceNumber() const {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
return transport_seq_ & 0xFFFF;
}
void PacketRouter::SendRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
if (!active_remb_module_) {
return;
@@ -275,7 +276,7 @@
void PacketRouter::SendCombinedRtcpPacket(
std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets) {
- MutexLock lock(&modules_mutex_);
+ RTC_DCHECK_RUN_ON(&thread_checker_);
// Prefer send modules.
for (RtpRtcpInterface* rtp_module : send_modules_list_) {
@@ -308,6 +309,7 @@
void PacketRouter::MaybeRemoveRembModuleCandidate(
RtcpFeedbackSenderInterface* candidate_module,
bool media_sender) {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_DCHECK(candidate_module);
std::vector<RtcpFeedbackSenderInterface*>& candidates =
media_sender ? sender_remb_candidates_ : receiver_remb_candidates_;
@@ -325,12 +327,14 @@
}
void PacketRouter::UnsetActiveRembModule() {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_CHECK(active_remb_module_);
active_remb_module_->UnsetRemb();
active_remb_module_ = nullptr;
}
void PacketRouter::DetermineActiveRembModule() {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
// Sender modules take precedence over receiver modules, because SRs (sender
// reports) are sent more frequently than RR (receiver reports).
// When adding the first sender module, we should change the active REMB
diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h
index 68b82c6..805f422 100644
--- a/modules/pacing/packet_router.h
+++ b/modules/pacing/packet_router.h
@@ -20,12 +20,12 @@
#include <utility>
#include <vector>
+#include "api/sequence_checker.h"
#include "api/transport/network_types.h"
#include "modules/pacing/pacing_controller.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtcp_packet.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
-#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/thread_annotations.h"
namespace webrtc {
@@ -74,45 +74,40 @@
private:
void AddRembModuleCandidate(RtcpFeedbackSenderInterface* candidate_module,
- bool media_sender)
- RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_mutex_);
+ bool media_sender);
void MaybeRemoveRembModuleCandidate(
RtcpFeedbackSenderInterface* candidate_module,
- bool media_sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_mutex_);
- void UnsetActiveRembModule() RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_mutex_);
- void DetermineActiveRembModule() RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_mutex_);
- void AddSendRtpModuleToMap(RtpRtcpInterface* rtp_module, uint32_t ssrc)
- RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_mutex_);
- void RemoveSendRtpModuleFromMap(uint32_t ssrc)
- RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_mutex_);
+ bool media_sender);
+ void UnsetActiveRembModule();
+ void DetermineActiveRembModule();
+ void AddSendRtpModuleToMap(RtpRtcpInterface* rtp_module, uint32_t ssrc);
+ void RemoveSendRtpModuleFromMap(uint32_t ssrc);
- mutable Mutex modules_mutex_;
+ SequenceChecker thread_checker_;
// Ssrc to RtpRtcpInterface module;
std::unordered_map<uint32_t, RtpRtcpInterface*> send_modules_map_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
std::list<RtpRtcpInterface*> send_modules_list_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
// The last module used to send media.
- RtpRtcpInterface* last_send_module_ RTC_GUARDED_BY(modules_mutex_);
+ RtpRtcpInterface* last_send_module_ RTC_GUARDED_BY(thread_checker_);
// Rtcp modules of the rtp receivers.
std::vector<RtcpFeedbackSenderInterface*> rtcp_feedback_senders_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
// Candidates for the REMB module can be RTP sender/receiver modules, with
// the sender modules taking precedence.
std::vector<RtcpFeedbackSenderInterface*> sender_remb_candidates_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
std::vector<RtcpFeedbackSenderInterface*> receiver_remb_candidates_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
RtcpFeedbackSenderInterface* active_remb_module_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
- uint64_t transport_seq_ RTC_GUARDED_BY(modules_mutex_);
+ uint64_t transport_seq_ RTC_GUARDED_BY(thread_checker_);
- // TODO(bugs.webrtc.org/10809): Replace lock with a sequence checker once the
- // process thread is gone.
std::vector<std::unique_ptr<RtpPacketToSend>> pending_fec_packets_
- RTC_GUARDED_BY(modules_mutex_);
+ RTC_GUARDED_BY(thread_checker_);
};
} // namespace webrtc
#endif // MODULES_PACING_PACKET_ROUTER_H_
