RtcpSender: remove lock recursions.
This change removes lock recursions and adds thread annotations.
Bug: webrtc:11567
Change-Id: Idc872bda693776667f3b9126bd79589d74398b34
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175640
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31308}
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index f06d429..269a735 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -262,8 +262,8 @@
return 0;
}
- return SendCompoundRTCP(feedback_state,
- {RTCPPacketType::kRtcpLossNotification});
+ return SendCompoundRTCPLocked(
+ feedback_state, {RTCPPacketType::kRtcpLossNotification}, 0, nullptr);
}
void RTCPSender::SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs) {
@@ -712,76 +712,11 @@
{
rtc::CritScope lock(&critical_section_rtcp_sender_);
- if (method_ == RtcpMode::kOff) {
- RTC_LOG(LS_WARNING) << "Can't send rtcp if it is disabled.";
- return -1;
+ auto result = ComputeCompoundRTCPPacket(feedback_state, packet_types,
+ nack_size, nack_list, &container);
+ if (result) {
+ return *result;
}
- // Add all flags as volatile. Non volatile entries will not be overwritten.
- // All new volatile flags added will be consumed by the end of this call.
- SetFlags(packet_types, true);
-
- // Prevent sending streams to send SR before any media has been sent.
- const bool can_calculate_rtp_timestamp = (last_frame_capture_time_ms_ >= 0);
- if (!can_calculate_rtp_timestamp) {
- bool consumed_sr_flag = ConsumeFlag(kRtcpSr);
- bool consumed_report_flag = sending_ && ConsumeFlag(kRtcpReport);
- bool sender_report = consumed_report_flag || consumed_sr_flag;
- if (sender_report && AllVolatileFlagsConsumed()) {
- // This call was for Sender Report and nothing else.
- return 0;
- }
- if (sending_ && method_ == RtcpMode::kCompound) {
- // Not allowed to send any RTCP packet without sender report.
- return -1;
- }
- }
-
- if (packet_type_counter_.first_packet_time_ms == -1)
- packet_type_counter_.first_packet_time_ms = clock_->TimeInMilliseconds();
-
- // We need to send our NTP even if we haven't received any reports.
- RtcpContext context(feedback_state, nack_size, nack_list,
- clock_->TimeInMicroseconds());
-
- PrepareReport(feedback_state);
-
- std::unique_ptr<rtcp::RtcpPacket> packet_bye;
-
- auto it = report_flags_.begin();
- while (it != report_flags_.end()) {
- auto builder_it = builders_.find(it->type);
- RTC_DCHECK(builder_it != builders_.end())
- << "Could not find builder for packet type " << it->type;
- if (it->is_volatile) {
- report_flags_.erase(it++);
- } else {
- ++it;
- }
-
- BuilderFunc func = builder_it->second;
- std::unique_ptr<rtcp::RtcpPacket> packet = (this->*func)(context);
- if (packet == nullptr)
- return -1;
- // If there is a BYE, don't append now - save it and append it
- // at the end later.
- if (builder_it->first == kRtcpBye) {
- packet_bye = std::move(packet);
- } else {
- container.Append(packet.release());
- }
- }
-
- // Append the BYE now at the end
- if (packet_bye) {
- container.Append(packet_bye.release());
- }
-
- if (packet_type_counter_observer_ != nullptr) {
- packet_type_counter_observer_->RtcpPacketTypesCounterUpdated(
- remote_ssrc_, packet_type_counter_);
- }
-
- RTC_DCHECK(AllVolatileFlagsConsumed());
max_packet_size = max_packet_size_;
}
@@ -789,6 +724,100 @@
return bytes_sent == 0 ? -1 : 0;
}
+int32_t RTCPSender::SendCompoundRTCPLocked(
+ const FeedbackState& feedback_state,
+ const std::set<RTCPPacketType>& packet_types,
+ int32_t nack_size,
+ const uint16_t* nack_list) {
+ PacketContainer container(transport_, event_log_);
+ auto result = ComputeCompoundRTCPPacket(feedback_state, packet_types,
+ nack_size, nack_list, &container);
+ if (result) {
+ return *result;
+ }
+ size_t bytes_sent = container.SendPackets(max_packet_size_);
+ return bytes_sent == 0 ? -1 : 0;
+}
+
+absl::optional<int32_t> RTCPSender::ComputeCompoundRTCPPacket(
+ const FeedbackState& feedback_state,
+ const std::set<RTCPPacketType>& packet_types,
+ int32_t nack_size,
+ const uint16_t* nack_list,
+ rtcp::CompoundPacket* out_packet) {
+ if (method_ == RtcpMode::kOff) {
+ RTC_LOG(LS_WARNING) << "Can't send rtcp if it is disabled.";
+ return -1;
+ }
+ // Add all flags as volatile. Non volatile entries will not be overwritten.
+ // All new volatile flags added will be consumed by the end of this call.
+ SetFlags(packet_types, true);
+
+ // Prevent sending streams to send SR before any media has been sent.
+ const bool can_calculate_rtp_timestamp = (last_frame_capture_time_ms_ >= 0);
+ if (!can_calculate_rtp_timestamp) {
+ bool consumed_sr_flag = ConsumeFlag(kRtcpSr);
+ bool consumed_report_flag = sending_ && ConsumeFlag(kRtcpReport);
+ bool sender_report = consumed_report_flag || consumed_sr_flag;
+ if (sender_report && AllVolatileFlagsConsumed()) {
+ // This call was for Sender Report and nothing else.
+ return 0;
+ }
+ if (sending_ && method_ == RtcpMode::kCompound) {
+ // Not allowed to send any RTCP packet without sender report.
+ return -1;
+ }
+ }
+
+ if (packet_type_counter_.first_packet_time_ms == -1)
+ packet_type_counter_.first_packet_time_ms = clock_->TimeInMilliseconds();
+
+ // We need to send our NTP even if we haven't received any reports.
+ RtcpContext context(feedback_state, nack_size, nack_list,
+ clock_->TimeInMicroseconds());
+
+ PrepareReport(feedback_state);
+
+ std::unique_ptr<rtcp::RtcpPacket> packet_bye;
+
+ auto it = report_flags_.begin();
+ while (it != report_flags_.end()) {
+ auto builder_it = builders_.find(it->type);
+ RTC_DCHECK(builder_it != builders_.end())
+ << "Could not find builder for packet type " << it->type;
+ if (it->is_volatile) {
+ report_flags_.erase(it++);
+ } else {
+ ++it;
+ }
+
+ BuilderFunc func = builder_it->second;
+ std::unique_ptr<rtcp::RtcpPacket> packet = (this->*func)(context);
+ if (packet == nullptr)
+ return -1;
+ // If there is a BYE, don't append now - save it and append it
+ // at the end later.
+ if (builder_it->first == kRtcpBye) {
+ packet_bye = std::move(packet);
+ } else {
+ out_packet->Append(packet.release());
+ }
+ }
+
+ // Append the BYE now at the end
+ if (packet_bye) {
+ out_packet->Append(packet_bye.release());
+ }
+
+ if (packet_type_counter_observer_ != nullptr) {
+ packet_type_counter_observer_->RtcpPacketTypesCounterUpdated(
+ remote_ssrc_, packet_type_counter_);
+ }
+
+ RTC_DCHECK(AllVolatileFlagsConsumed());
+ return absl::nullopt;
+}
+
void RTCPSender::PrepareReport(const FeedbackState& feedback_state) {
bool generate_report;
if (IsFlagPresent(kRtcpSr) || IsFlagPresent(kRtcpRr)) {
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index 32c1e1d..7da2546 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -27,6 +27,7 @@
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtcp_nack_stats.h"
#include "modules/rtp_rtcp/source/rtcp_packet.h"
+#include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h"
#include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h"
#include "modules/rtp_rtcp/source/rtcp_packet/report_block.h"
#include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h"
@@ -66,84 +67,124 @@
explicit RTCPSender(const RtpRtcp::Configuration& config);
virtual ~RTCPSender();
- RtcpMode Status() const;
- void SetRTCPStatus(RtcpMode method);
+ RtcpMode Status() const RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
+ void SetRTCPStatus(RtcpMode method)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- bool Sending() const;
+ bool Sending() const RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
int32_t SetSendingStatus(const FeedbackState& feedback_state,
- bool enabled); // combine the functions
+ bool enabled)
+ RTC_LOCKS_EXCLUDED(
+ critical_section_rtcp_sender_); // combine the functions
- int32_t SetNackStatus(bool enable);
+ int32_t SetNackStatus(bool enable)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetTimestampOffset(uint32_t timestamp_offset);
+ void SetTimestampOffset(uint32_t timestamp_offset)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
// TODO(bugs.webrtc.org/6458): Remove default parameter value when all the
// depending projects are updated to correctly set payload type.
void SetLastRtpTime(uint32_t rtp_timestamp,
int64_t capture_time_ms,
- int8_t payload_type = -1);
+ int8_t payload_type = -1)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz);
+ void SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
uint32_t SSRC() const { return ssrc_; }
- void SetRemoteSSRC(uint32_t ssrc);
+ void SetRemoteSSRC(uint32_t ssrc)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- int32_t SetCNAME(const char* cName);
+ int32_t SetCNAME(const char* cName)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- int32_t AddMixedCNAME(uint32_t SSRC, const char* c_name);
+ int32_t AddMixedCNAME(uint32_t SSRC, const char* c_name)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- int32_t RemoveMixedCNAME(uint32_t SSRC);
+ int32_t RemoveMixedCNAME(uint32_t SSRC)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- bool TimeToSendRTCPReport(bool sendKeyframeBeforeRTP = false) const;
+ bool TimeToSendRTCPReport(bool sendKeyframeBeforeRTP = false) const
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
int32_t SendRTCP(const FeedbackState& feedback_state,
RTCPPacketType packetType,
int32_t nackSize = 0,
- const uint16_t* nackList = 0);
+ const uint16_t* nackList = 0)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
int32_t SendCompoundRTCP(const FeedbackState& feedback_state,
const std::set<RTCPPacketType>& packetTypes,
int32_t nackSize = 0,
- const uint16_t* nackList = 0);
+ const uint16_t* nackList = nullptr)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
int32_t SendLossNotification(const FeedbackState& feedback_state,
uint16_t last_decoded_seq_num,
uint16_t last_received_seq_num,
bool decodability_flag,
- bool buffering_allowed);
+ bool buffering_allowed)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs);
+ void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void UnsetRemb();
+ void UnsetRemb() RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- bool TMMBR() const;
+ bool TMMBR() const RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetTMMBRStatus(bool enable);
+ void SetTMMBRStatus(bool enable)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetMaxRtpPacketSize(size_t max_packet_size);
+ void SetMaxRtpPacketSize(size_t max_packet_size)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetTmmbn(std::vector<rtcp::TmmbItem> bounding_set);
+ void SetTmmbn(std::vector<rtcp::TmmbItem> bounding_set)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
int32_t SetApplicationSpecificData(uint8_t subType,
uint32_t name,
const uint8_t* data,
- uint16_t length);
+ uint16_t length)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SendRtcpXrReceiverReferenceTime(bool enable);
+ void SendRtcpXrReceiverReferenceTime(bool enable)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- bool RtcpXrReceiverReferenceTime() const;
+ bool RtcpXrReceiverReferenceTime() const
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetCsrcs(const std::vector<uint32_t>& csrcs);
+ void SetCsrcs(const std::vector<uint32_t>& csrcs)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
- void SetTargetBitrate(unsigned int target_bitrate);
- void SetVideoBitrateAllocation(const VideoBitrateAllocation& bitrate);
+ void SetTargetBitrate(unsigned int target_bitrate)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
+ void SetVideoBitrateAllocation(const VideoBitrateAllocation& bitrate)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
void SendCombinedRtcpPacket(
- std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets);
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets)
+ RTC_LOCKS_EXCLUDED(critical_section_rtcp_sender_);
private:
class RtcpContext;
+ int32_t SendCompoundRTCPLocked(const FeedbackState& feedback_state,
+ const std::set<RTCPPacketType>& packet_types,
+ int32_t nack_size,
+ const uint16_t* nack_list)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
+
+ absl::optional<int32_t> ComputeCompoundRTCPPacket(
+ const FeedbackState& feedback_state,
+ const std::set<RTCPPacketType>& packet_types,
+ int32_t nack_size,
+ const uint16_t* nack_list,
+ rtcp::CompoundPacket* out_packet)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
+
// Determine which RTCP messages should be sent and setup flags.
void PrepareReport(const FeedbackState& feedback_state)
RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
