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}

modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc

@@ -36,14 +36,24 @@ bool IsDisabled(absl::string_view name,
 }  // 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_;

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 @@ class DEPRECATED_RtpSenderEgress {
   // 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 @@ class DEPRECATED_RtpSenderEgress {
    private:
     uint16_t transport_sequence_number_;
     DEPRECATED_RtpSenderEgress* const sender_;
+    PacketSequencer* sequence_number_assigner_;
   };
 
   DEPRECATED_RtpSenderEgress(const RtpRtcpInterface::Configuration& config,

modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -49,12 +49,12 @@ ModuleRtpRtcpImpl::RtpSenderContext::RtpSenderContext(
                  /*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 @@ void ModuleRtpRtcpImpl::SetStartTimestamp(const uint32_t timestamp) {
 }
 
 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 @@ bool ModuleRtpRtcpImpl::TrySendPacket(RtpPacketToSend* packet,
   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 @@ bool ModuleRtpRtcpImpl::SupportsRtxPayloadPadding() const {
 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>

modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -275,7 +275,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
     // 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

modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc

@@ -566,6 +566,7 @@ TEST_F(RtpRtcpImplTest, StoresPacketInfoForSentPackets) {
   const uint32_t kStartTimestamp = 1u;
   SetUp();
   sender_.impl_->SetStartTimestamp(kStartTimestamp);
+  sender_.impl_->SetSequenceNumber(1);
 
   PacedPacketInfo pacing_info;
   RtpPacketToSend packet(nullptr);
@@ -574,7 +575,6 @@ TEST_F(RtpRtcpImplTest, StoresPacketInfoForSentPackets) {
 
   // 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 @@ TEST_F(RtpRtcpImplTest, StoresPacketInfoForSentPackets) {
 
   // 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);