admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove lock from UlpfecReceiverImpl and replace with a sequence checker.

Browse Source 
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}
This commit is contained in:
Tomas Gunnarsson 2021-01-19 13:30:23 +01:00 committed by Commit Bot
parent 5eb527cf7f
commit 1e75df26e3
2 changed files with 16 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
View File

@ -37,12 +37,13 @@ UlpfecReceiverImpl::UlpfecReceiverImpl(
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 @@ FecPacketCounter UlpfecReceiverImpl::GetPacketCounter() const {
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 @@ bool UlpfecReceiverImpl::AddReceivedRedPacket(
"packet size; dropping.";
return false;
}
MutexLock lock(&mutex_);
static constexpr uint8_t kRedHeaderLength = 1;
@ -151,7 +155,7 @@ bool UlpfecReceiverImpl::AddReceivedRedPacket(
// 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 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
// 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 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
// 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;
}

16
modules/rtp_rtcp/source/ulpfec_receiver_impl.h
View File

@ -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 @@ class UlpfecReceiverImpl : public UlpfecReceiver {
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