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}
This commit is contained in:
Per K
WebRTC LUCI CQ
parent
df0862c7ec
commit
1b77daea81
@ -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 @@ PacketRouter::PacketRouter(uint16_t start_transport_seq)
|
||||
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 @@ PacketRouter::~PacketRouter() {
|
||||
|
||||
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::AddSendRtpModule(RtpRtcpInterface* rtp_module,
|
||||
|
||||
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::AddSendRtpModuleToMap(RtpRtcpInterface* rtp_module,
|
||||
}
|
||||
|
||||
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::RemoveSendRtpModuleFromMap(uint32_t ssrc) {
|
||||
}
|
||||
|
||||
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::RemoveSendRtpModule(RtpRtcpInterface* rtp_module) {
|
||||
|
||||
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::AddReceiveRtpModule(RtcpFeedbackSenderInterface* rtcp_sender,
|
||||
|
||||
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::RemoveReceiveRtpModule(
|
||||
|
||||
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 @@ void PacketRouter::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
|
||||
}
|
||||
|
||||
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::FetchFec() {
|
||||
|
||||
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 @@ std::vector<std::unique_ptr<RtpPacketToSend>> PacketRouter::GeneratePadding(
|
||||
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 @@ void PacketRouter::OnAbortedRetransmissions(
|
||||
}
|
||||
|
||||
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 @@ absl::optional<uint32_t> PacketRouter::GetRtxSsrcForMedia(uint32_t ssrc) const {
|
||||
}
|
||||
|
||||
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::SendRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs) {
|
||||
|
||||
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::AddRembModuleCandidate(
|
||||
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::MaybeRemoveRembModuleCandidate(
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
@ -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 @@ class PacketRouter : public PacingController::PacketSender {
|
||||
|
||||
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_
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user