Move legacy RtpRtcpModule to deferred sequencing.
Bug: webrtc:11340
Change-Id: I45ba6c37fe7fec8de756dee1eb914aceebbeae93
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228437
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34734}
diff --git a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
index c542557..201a1ce 100644
--- a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
+++ b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
@@ -36,14 +36,24 @@
} // namespace
DEPRECATED_RtpSenderEgress::NonPacedPacketSender::NonPacedPacketSender(
- DEPRECATED_RtpSenderEgress* sender)
- : transport_sequence_number_(0), sender_(sender) {}
+ DEPRECATED_RtpSenderEgress* sender,
+ PacketSequencer* sequence_number_assigner)
+ : transport_sequence_number_(0),
+ sender_(sender),
+ sequence_number_assigner_(sequence_number_assigner) {
+ RTC_DCHECK(sequence_number_assigner_);
+}
DEPRECATED_RtpSenderEgress::NonPacedPacketSender::~NonPacedPacketSender() =
default;
void DEPRECATED_RtpSenderEgress::NonPacedPacketSender::EnqueuePackets(
std::vector<std::unique_ptr<RtpPacketToSend>> packets) {
for (auto& packet : packets) {
+ // Assign sequence numbers, but not for flexfec which is already running on
+ // an internally maintained sequence number series.
+ if (packet->Ssrc() != sender_->FlexFecSsrc()) {
+ sequence_number_assigner_->Sequence(*packet);
+ }
if (!packet->SetExtension<TransportSequenceNumber>(
++transport_sequence_number_)) {
--transport_sequence_number_;
diff --git a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h
index 4aeb430..fc440d1 100644
--- a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h
+++ b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h
@@ -20,6 +20,7 @@
#include "api/rtc_event_log/rtc_event_log.h"
#include "api/units/data_rate.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/packet_sequencer.h"
#include "modules/rtp_rtcp/source/rtp_packet_history.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_interface.h"
@@ -36,7 +37,8 @@
// without passing through an actual paced sender.
class NonPacedPacketSender : public RtpPacketSender {
public:
- explicit NonPacedPacketSender(DEPRECATED_RtpSenderEgress* sender);
+ NonPacedPacketSender(DEPRECATED_RtpSenderEgress* sender,
+ PacketSequencer* sequence_number_assigner);
virtual ~NonPacedPacketSender();
void EnqueuePackets(
@@ -45,6 +47,7 @@
private:
uint16_t transport_sequence_number_;
DEPRECATED_RtpSenderEgress* const sender_;
+ PacketSequencer* sequence_number_assigner_;
};
DEPRECATED_RtpSenderEgress(const RtpRtcpInterface::Configuration& config,
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index e3fd8ab..f6f9afb 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -49,12 +49,12 @@
/*require_marker_before_media_padding=*/!config.audio,
config.clock),
packet_sender(config, &packet_history),
- non_paced_sender(&packet_sender),
+ non_paced_sender(&packet_sender, &sequencer_),
packet_generator(
config,
&packet_history,
config.paced_sender ? config.paced_sender : &non_paced_sender,
- &sequencer_) {}
+ /*packet_sequencer=*/nullptr) {}
std::unique_ptr<RtpRtcp> RtpRtcp::DEPRECATED_Create(
const Configuration& configuration) {
@@ -255,30 +255,41 @@
}
uint16_t ModuleRtpRtcpImpl::SequenceNumber() const {
- return rtp_sender_->packet_generator.SequenceNumber();
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
+ return rtp_sender_->sequencer_.media_sequence_number();
}
// Set SequenceNumber, default is a random number.
void ModuleRtpRtcpImpl::SetSequenceNumber(const uint16_t seq_num) {
- rtp_sender_->packet_generator.SetSequenceNumber(seq_num);
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
+ rtp_sender_->sequencer_.set_media_sequence_number(seq_num);
}
void ModuleRtpRtcpImpl::SetRtpState(const RtpState& rtp_state) {
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
rtp_sender_->packet_generator.SetRtpState(rtp_state);
+ rtp_sender_->sequencer_.SetRtpState(rtp_state);
rtcp_sender_.SetTimestampOffset(rtp_state.start_timestamp);
}
void ModuleRtpRtcpImpl::SetRtxState(const RtpState& rtp_state) {
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
rtp_sender_->packet_generator.SetRtxRtpState(rtp_state);
+ rtp_sender_->sequencer_.set_rtx_sequence_number(rtp_state.sequence_number);
}
RtpState ModuleRtpRtcpImpl::GetRtpState() const {
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
RtpState state = rtp_sender_->packet_generator.GetRtpState();
+ rtp_sender_->sequencer_.PopulateRtpState(state);
return state;
}
RtpState ModuleRtpRtcpImpl::GetRtxState() const {
- return rtp_sender_->packet_generator.GetRtxRtpState();
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
+ RtpState state = rtp_sender_->packet_generator.GetRtxRtpState();
+ state.sequence_number = rtp_sender_->sequencer_.rtx_sequence_number();
+ return state;
}
void ModuleRtpRtcpImpl::SetRid(const std::string& rid) {
@@ -410,6 +421,21 @@
if (!rtp_sender_->packet_generator.SendingMedia()) {
return false;
}
+ {
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
+ if (packet->packet_type() == RtpPacketMediaType::kPadding &&
+ packet->Ssrc() == rtp_sender_->packet_generator.SSRC() &&
+ !rtp_sender_->sequencer_.CanSendPaddingOnMediaSsrc()) {
+ // New media packet preempted this generated padding packet, discard it.
+ return false;
+ }
+ bool is_flexfec =
+ packet->packet_type() == RtpPacketMediaType::kForwardErrorCorrection &&
+ packet->Ssrc() == rtp_sender_->packet_generator.FlexfecSsrc();
+ if (!is_flexfec) {
+ rtp_sender_->sequencer_.Sequence(*packet);
+ }
+ }
rtp_sender_->packet_sender.SendPacket(packet, pacing_info);
return true;
}
@@ -444,12 +470,10 @@
std::vector<std::unique_ptr<RtpPacketToSend>>
ModuleRtpRtcpImpl::GeneratePadding(size_t target_size_bytes) {
RTC_DCHECK(rtp_sender_);
- // `can_send_padding_on_media_ssrc` set to false but is ignored at this
- // point, RTPSender will internally query `sequencer_` while holding the
- // send lock.
+ MutexLock lock(&rtp_sender_->sequencer_mutex);
return rtp_sender_->packet_generator.GeneratePadding(
target_size_bytes, rtp_sender_->packet_sender.MediaHasBeenSent(),
- /*can_send_padding_on_media_ssrc=*/false);
+ rtp_sender_->sequencer_.CanSendPaddingOnMediaSsrc());
}
std::vector<RtpSequenceNumberMap::Info>
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index c5d0b3a..81b1170 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -275,7 +275,8 @@
// Storage of packets, for retransmissions and padding, if applicable.
RtpPacketHistory packet_history;
// Handles sequence number assignment and padding timestamp generation.
- PacketSequencer sequencer_;
+ mutable Mutex sequencer_mutex;
+ PacketSequencer sequencer_ RTC_GUARDED_BY(sequencer_mutex);
// Handles final time timestamping/stats/etc and handover to Transport.
DEPRECATED_RtpSenderEgress packet_sender;
// If no paced sender configured, this class will be used to pass packets
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index 0902e90..70c9a1c 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -566,6 +566,7 @@
const uint32_t kStartTimestamp = 1u;
SetUp();
sender_.impl_->SetStartTimestamp(kStartTimestamp);
+ sender_.impl_->SetSequenceNumber(1);
PacedPacketInfo pacing_info;
RtpPacketToSend packet(nullptr);
@@ -574,7 +575,6 @@
// Single-packet frame.
packet.SetTimestamp(1);
- packet.SetSequenceNumber(1);
packet.set_first_packet_of_frame(true);
packet.SetMarker(true);
sender_.impl_->TrySendPacket(&packet, pacing_info);
@@ -589,16 +589,13 @@
// Three-packet frame.
packet.SetTimestamp(2);
- packet.SetSequenceNumber(2);
packet.set_first_packet_of_frame(true);
packet.SetMarker(false);
sender_.impl_->TrySendPacket(&packet, pacing_info);
- packet.SetSequenceNumber(3);
packet.set_first_packet_of_frame(false);
sender_.impl_->TrySendPacket(&packet, pacing_info);
- packet.SetSequenceNumber(4);
packet.SetMarker(true);
sender_.impl_->TrySendPacket(&packet, pacing_info);
