Remove lock from UlpfecReceiverImpl and replace with a sequence checker.
Also making some more state const.
Instances of this class are currently constructed and used on the
"worker thread" but as part of the work for bug webrtc:11993, the
instances will be moved over to the network thread. Since the
class as is does not require synchronization, that is a good property
to make explicit now and then make sure we maintain it in the
transition.
Bug: webrtc:11993
Change-Id: Id587a746ce0a4363b9e9871ae1749549f8ef3e24
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/202681
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33035}
diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
index fee0b9c..16b87ba7 100644
--- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
+++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
@@ -37,12 +37,13 @@
fec_(ForwardErrorCorrection::CreateUlpfec(ssrc_)) {}
UlpfecReceiverImpl::~UlpfecReceiverImpl() {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
received_packets_.clear();
fec_->ResetState(&recovered_packets_);
}
FecPacketCounter UlpfecReceiverImpl::GetPacketCounter() const {
- MutexLock lock(&mutex_);
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
return packet_counter_;
}
@@ -77,6 +78,10 @@
bool UlpfecReceiverImpl::AddReceivedRedPacket(
const RtpPacketReceived& rtp_packet,
uint8_t ulpfec_payload_type) {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ // TODO(bugs.webrtc.org/11993): We get here via Call::DeliverRtp, so should be
+ // moved to the network thread.
+
if (rtp_packet.Ssrc() != ssrc_) {
RTC_LOG(LS_WARNING)
<< "Received RED packet with different SSRC than expected; dropping.";
@@ -87,7 +92,6 @@
"packet size; dropping.";
return false;
}
- MutexLock lock(&mutex_);
static constexpr uint8_t kRedHeaderLength = 1;
@@ -151,7 +155,7 @@
// TODO(nisse): Drop always-zero return value.
int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
- mutex_.Lock();
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
// If we iterate over |received_packets_| and it contains a packet that cause
// us to recurse back to this function (for example a RED packet encapsulating
@@ -168,10 +172,8 @@
// Send received media packet to VCM.
if (!received_packet->is_fec) {
ForwardErrorCorrection::Packet* packet = received_packet->pkt;
- mutex_.Unlock();
recovered_packet_callback_->OnRecoveredPacket(packet->data.data(),
packet->data.size());
- mutex_.Lock();
// Create a packet with the buffer to modify it.
RtpPacketReceived rtp_packet;
const uint8_t* const original_data = packet->data.cdata();
@@ -208,13 +210,10 @@
// Set this flag first; in case the recovered packet carries a RED
// header, OnRecoveredPacket will recurse back here.
recovered_packet->returned = true;
- mutex_.Unlock();
recovered_packet_callback_->OnRecoveredPacket(packet->data.data(),
packet->data.size());
- mutex_.Lock();
}
- mutex_.Unlock();
return 0;
}
diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
index 2bed042..fc7fe38 100644
--- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
+++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
@@ -22,7 +22,8 @@
#include "modules/rtp_rtcp/include/ulpfec_receiver.h"
#include "modules/rtp_rtcp/source/forward_error_correction.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
-#include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/synchronization/sequence_checker.h"
+#include "rtc_base/system/no_unique_address.h"
namespace webrtc {
@@ -44,17 +45,18 @@
const uint32_t ssrc_;
const RtpHeaderExtensionMap extensions_;
- mutable Mutex mutex_;
- RecoveredPacketReceiver* recovered_packet_callback_;
- std::unique_ptr<ForwardErrorCorrection> fec_;
+ RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_;
+ RecoveredPacketReceiver* const recovered_packet_callback_;
+ const std::unique_ptr<ForwardErrorCorrection> fec_;
// TODO(nisse): The AddReceivedRedPacket method adds one or two packets to
// this list at a time, after which it is emptied by ProcessReceivedFec. It
// will make things simpler to merge AddReceivedRedPacket and
// ProcessReceivedFec into a single method, and we can then delete this list.
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>
- received_packets_;
- ForwardErrorCorrection::RecoveredPacketList recovered_packets_;
- FecPacketCounter packet_counter_;
+ received_packets_ RTC_GUARDED_BY(&sequence_checker_);
+ ForwardErrorCorrection::RecoveredPacketList recovered_packets_
+ RTC_GUARDED_BY(&sequence_checker_);
+ FecPacketCounter packet_counter_ RTC_GUARDED_BY(&sequence_checker_);
};
} // namespace webrtc
