From bb90497eaada6ea725eb7622536956b722bb8a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 6 Aug 2021 13:10:11 +0200 Subject: [PATCH] Add support for deferred sequence numbering. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this turned on, packets will be sequence number after the pacing stage rather that during packetization. This avoids a race where packets may be sent out of order, and paves the way for the ability to cull packets from the pacer queue without causing sequence number gaps. For now, the feature is off by default. Follow-ups will enable it for video and audio separately. Bug: webrtc:11340, webrtc:12470 Change-Id: I6d411d8c85b9047e3e9b05ff4c2c3ed97c579aa1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/208584 Commit-Queue: Erik Språng Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#34661} --- modules/rtp_rtcp/source/packet_sequencer.cc | 2 +- modules/rtp_rtcp/source/packet_sequencer.h | 2 +- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 81 ++++++++++++++++--- modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 4 + .../source/rtp_rtcp_impl2_unittest.cc | 43 +++++++--- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 4 + modules/rtp_rtcp/source/rtp_sender.cc | 2 +- modules/rtp_rtcp/source/rtp_sender.h | 9 ++- modules/rtp_rtcp/source/rtp_sender_egress.cc | 45 ++++++++--- modules/rtp_rtcp/source/rtp_sender_egress.h | 7 +- modules/rtp_rtcp/source/rtp_sender_video.cc | 6 +- 11 files changed, 164 insertions(+), 41 deletions(-) diff --git a/modules/rtp_rtcp/source/packet_sequencer.cc b/modules/rtp_rtcp/source/packet_sequencer.cc index a4e7b46fef..db108d4493 100644 --- a/modules/rtp_rtcp/source/packet_sequencer.cc +++ b/modules/rtp_rtcp/source/packet_sequencer.cc @@ -77,7 +77,7 @@ void PacketSequencer::SetRtpState(const RtpState& state) { last_timestamp_time_ms_ = state.last_timestamp_time_ms; } -void PacketSequencer::PupulateRtpState(RtpState& state) const { +void PacketSequencer::PopulateRtpState(RtpState& state) const { state.sequence_number = media_sequence_number_; state.timestamp = last_rtp_timestamp_; state.capture_time_ms = last_capture_time_ms_; diff --git a/modules/rtp_rtcp/source/packet_sequencer.h b/modules/rtp_rtcp/source/packet_sequencer.h index 5c3ca1a326..7c0dee7a5b 100644 --- a/modules/rtp_rtcp/source/packet_sequencer.h +++ b/modules/rtp_rtcp/source/packet_sequencer.h @@ -44,7 +44,7 @@ class PacketSequencer { } void SetRtpState(const RtpState& state); - void PupulateRtpState(RtpState& state) const; + void PopulateRtpState(RtpState& state) const; uint16_t media_sequence_number() const { return media_sequence_number_; } uint16_t rtx_sequence_number() const { return rtx_sequence_number_; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index af64ddb269..536e5d43a8 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -63,20 +63,25 @@ int DelayMillisForDuration(TimeDelta duration) { ModuleRtpRtcpImpl2::RtpSenderContext::RtpSenderContext( const RtpRtcpInterface::Configuration& config) : packet_history(config.clock, config.enable_rtx_padding_prioritization), + deferred_sequencing_(config.use_deferred_sequencing), sequencer_(config.local_media_ssrc, config.rtx_send_ssrc, /*require_marker_before_media_padding=*/!config.audio, config.clock), packet_sender(config, &packet_history), - non_paced_sender(&packet_sender, this), + non_paced_sender(&packet_sender, this, config.use_deferred_sequencing), packet_generator( config, &packet_history, config.paced_sender ? config.paced_sender : &non_paced_sender, - &sequencer_) {} + config.use_deferred_sequencing ? nullptr : &sequencer_) {} void ModuleRtpRtcpImpl2::RtpSenderContext::AssignSequenceNumber( RtpPacketToSend* packet) { - packet_generator.AssignSequenceNumber(packet); + if (deferred_sequencing_) { + sequencer_.Sequence(*packet); + } else { + packet_generator.AssignSequenceNumber(packet); + } } ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration) @@ -96,6 +101,7 @@ ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration) rtt_ms_(0) { RTC_DCHECK(worker_queue_); packet_sequence_checker_.Detach(); + pacer_thread_checker_.Detach(); if (!configuration.receiver_only) { rtp_sender_ = std::make_unique(configuration); // Make sure rtcp sender use same timestamp offset as rtp sender. @@ -180,30 +186,54 @@ void ModuleRtpRtcpImpl2::SetStartTimestamp(const uint32_t timestamp) { } uint16_t ModuleRtpRtcpImpl2::SequenceNumber() const { + if (rtp_sender_->deferred_sequencing_) { + return rtp_sender_->sequencer_.media_sequence_number(); + } return rtp_sender_->packet_generator.SequenceNumber(); } // Set SequenceNumber, default is a random number. void ModuleRtpRtcpImpl2::SetSequenceNumber(const uint16_t seq_num) { - rtp_sender_->packet_generator.SetSequenceNumber(seq_num); + if (rtp_sender_->deferred_sequencing_) { + RTC_DCHECK_RUN_ON(&pacer_thread_checker_); + if (rtp_sender_->sequencer_.media_sequence_number() != seq_num) { + rtp_sender_->sequencer_.set_media_sequence_number(seq_num); + rtp_sender_->packet_history.Clear(); + } + } else { + rtp_sender_->packet_generator.SetSequenceNumber(seq_num); + } } void ModuleRtpRtcpImpl2::SetRtpState(const RtpState& rtp_state) { rtp_sender_->packet_generator.SetRtpState(rtp_state); + if (rtp_sender_->deferred_sequencing_) { + rtp_sender_->sequencer_.SetRtpState(rtp_state); + } rtcp_sender_.SetTimestampOffset(rtp_state.start_timestamp); } void ModuleRtpRtcpImpl2::SetRtxState(const RtpState& rtp_state) { rtp_sender_->packet_generator.SetRtxRtpState(rtp_state); + if (rtp_sender_->deferred_sequencing_) { + rtp_sender_->sequencer_.set_rtx_sequence_number(rtp_state.sequence_number); + } } RtpState ModuleRtpRtcpImpl2::GetRtpState() const { RtpState state = rtp_sender_->packet_generator.GetRtpState(); + if (rtp_sender_->deferred_sequencing_) { + rtp_sender_->sequencer_.PopulateRtpState(state); + } return state; } RtpState ModuleRtpRtcpImpl2::GetRtxState() const { - return rtp_sender_->packet_generator.GetRtxRtpState(); + RtpState state = rtp_sender_->packet_generator.GetRtxRtpState(); + if (rtp_sender_->deferred_sequencing_) { + state.sequence_number = rtp_sender_->sequencer_.rtx_sequence_number(); + } + return state; } void ModuleRtpRtcpImpl2::SetNonSenderRttMeasurement(bool enabled) { @@ -298,6 +328,12 @@ bool ModuleRtpRtcpImpl2::Sending() const { // updated. void ModuleRtpRtcpImpl2::SetSendingMediaStatus(const bool sending) { if (rtp_sender_) { + // Turning on or off sending status indicates module being set + // up or torn down, detach thread checker since subsequent calls + // may be from a different thread. + if (rtp_sender_->packet_generator.SendingMedia() != sending) { + pacer_thread_checker_.Detach(); + } rtp_sender_->packet_generator.SetSendingMediaStatus(sending); } else { RTC_DCHECK(!sending); @@ -346,8 +382,22 @@ bool ModuleRtpRtcpImpl2::OnSendingRtpFrame(uint32_t timestamp, bool ModuleRtpRtcpImpl2::TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK(rtp_sender_); - // TODO(sprang): Consider if we can remove this check. - if (!rtp_sender_->packet_generator.SendingMedia()) { + RTC_DCHECK_RUN_ON(&pacer_thread_checker_); + if (rtp_sender_->deferred_sequencing_) { + RTC_DCHECK(rtp_sender_->packet_generator.SendingMedia()); + 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); + } + } else if (!rtp_sender_->packet_generator.SendingMedia()) { return false; } rtp_sender_->packet_sender.SendPacket(packet, pacing_info); @@ -365,9 +415,11 @@ void ModuleRtpRtcpImpl2::SetFecProtectionParams( std::vector> ModuleRtpRtcpImpl2::FetchFecPackets() { RTC_DCHECK(rtp_sender_); + RTC_DCHECK_RUN_ON(&pacer_thread_checker_); auto fec_packets = rtp_sender_->packet_sender.FetchFecPackets(); - if (!fec_packets.empty()) { - // Don't assign sequence numbers for FlexFEC packets. + if (!fec_packets.empty() && !rtp_sender_->deferred_sequencing_) { + // Only assign sequence numbers for FEC packets in non-deferred mode, and + // never for FlexFEC which has as separate sequence number series. const bool generate_sequence_numbers = !rtp_sender_->packet_sender.FlexFecSsrc().has_value(); if (generate_sequence_numbers) { @@ -398,12 +450,21 @@ bool ModuleRtpRtcpImpl2::SupportsRtxPayloadPadding() const { std::vector> ModuleRtpRtcpImpl2::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. + RTC_DCHECK_RUN_ON(&pacer_thread_checker_); + + // `can_send_padding_on_media_ssrc` set to false when deferred sequencing + // is off. It will be ignored in that case, RTPSender will internally query + // `sequencer_` while holding the send lock instead. return rtp_sender_->packet_generator.GeneratePadding( target_size_bytes, rtp_sender_->packet_sender.MediaHasBeenSent(), - /*can_send_padding_on_media_ssrc=*/false); + + rtp_sender_->deferred_sequencing_ + ? rtp_sender_->sequencer_.CanSendPaddingOnMediaSsrc() + : false); } std::vector diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index fb1facb2ae..2d8de02bcb 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -266,6 +266,9 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, void AssignSequenceNumber(RtpPacketToSend* packet) override; // Storage of packets, for retransmissions and padding, if applicable. RtpPacketHistory packet_history; + // If false, sequencing is owned by `packet_generator` and can happen on + // several threads. If true, sequencing always happens on the pacer thread. + const bool deferred_sequencing_; // Handles sequence number assignment and padding timestamp generation. PacketSequencer sequencer_; // Handles final time timestamping/stats/etc and handover to Transport. @@ -308,6 +311,7 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, TaskQueueBase* const worker_queue_; RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_; + RTC_NO_UNIQUE_ADDRESS SequenceChecker pacer_thread_checker_; std::unique_ptr rtp_sender_; RTCPSender rtcp_sender_; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 01085d1a33..9e8e824158 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -152,9 +152,12 @@ class SendTransport : public Transport, }; struct TestConfig { - explicit TestConfig(bool with_overhead) : with_overhead(with_overhead) {} + explicit TestConfig(bool with_overhead, bool with_deferred_sequencing) + : with_overhead(with_overhead), + with_deferred_sequencing(with_deferred_sequencing) {} bool with_overhead = false; + bool with_deferred_sequencing = false; }; class FieldTrialConfig : public WebRtcKeyValueConfig { @@ -201,10 +204,12 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, RtpRtcpModule(GlobalSimulatedTimeController* time_controller, bool is_sender, - const FieldTrialConfig& trials) + const FieldTrialConfig& trials, + bool deferred_sequencing) : time_controller_(time_controller), is_sender_(is_sender), trials_(trials), + deferred_sequencing_(deferred_sequencing), receive_statistics_( ReceiveStatistics::Create(time_controller->GetClock())), transport_(kOneWayNetworkDelay, time_controller) { @@ -214,6 +219,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, TimeController* const time_controller_; const bool is_sender_; const FieldTrialConfig& trials_; + const bool deferred_sequencing_; RtcpPacketTypeCounter packets_sent_; RtcpPacketTypeCounter packets_received_; std::unique_ptr receive_statistics_; @@ -283,6 +289,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, config.field_trials = &trials_; config.send_packet_observer = this; config.fec_generator = fec_generator_; + config.use_deferred_sequencing = deferred_sequencing_; impl_.reset(new ModuleRtpRtcpImpl2(config)); impl_->SetRemoteSSRC(is_sender_ ? kReceiverSsrc : kSenderSsrc); impl_->SetRTCPStatus(RtcpMode::kCompound); @@ -303,10 +310,12 @@ class RtpRtcpImpl2Test : public ::testing::TestWithParam { field_trials_(FieldTrialConfig::GetFromTestConfig(GetParam())), sender_(&time_controller_, /*is_sender=*/true, - field_trials_), + field_trials_, + GetParam().with_deferred_sequencing), receiver_(&time_controller_, /*is_sender=*/false, - field_trials_) {} + field_trials_, + GetParam().with_deferred_sequencing) {} void SetUp() override { // Send module. @@ -408,6 +417,12 @@ class RtpRtcpImpl2Test : public ::testing::TestWithParam { rtc::Buffer packet = nack.Build(); module->impl_->IncomingRtcpPacket(packet.data(), packet.size()); } + + void MaybeAssignSequenceNumber(RtpPacketToSend* packet) { + if (!GetParam().with_deferred_sequencing) { + sender_.impl_->RtpSender()->AssignSequenceNumber(packet); + } + } }; TEST_P(RtpRtcpImpl2Test, RetransmitsAllLayers) { @@ -738,7 +753,7 @@ TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { // Single-packet frame. packet.SetTimestamp(1); - packet.SetSequenceNumber(1); + MaybeAssignSequenceNumber(&packet); packet.set_first_packet_of_frame(true); packet.SetMarker(true); sender_.impl_->TrySendPacket(&packet, pacing_info); @@ -754,16 +769,16 @@ TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { // Three-packet frame. packet.SetTimestamp(2); - packet.SetSequenceNumber(2); + MaybeAssignSequenceNumber(&packet); packet.set_first_packet_of_frame(true); packet.SetMarker(false); sender_.impl_->TrySendPacket(&packet, pacing_info); - packet.SetSequenceNumber(3); + MaybeAssignSequenceNumber(&packet); packet.set_first_packet_of_frame(false); sender_.impl_->TrySendPacket(&packet, pacing_info); - packet.SetSequenceNumber(4); + MaybeAssignSequenceNumber(&packet); packet.SetMarker(true); sender_.impl_->TrySendPacket(&packet, pacing_info); @@ -922,7 +937,7 @@ TEST_P(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { packet->set_packet_type(RtpPacketToSend::Type::kVideo); packet->set_first_packet_of_frame(true); packet->SetMarker(false); // Marker false - not last packet of frame. - sender_.impl_->RtpSender()->AssignSequenceNumber(packet.get()); + MaybeAssignSequenceNumber(packet.get()); EXPECT_TRUE(sender_.impl_->TrySendPacket(packet.get(), pacing_info)); @@ -933,7 +948,7 @@ TEST_P(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { packet->set_packet_type(RtpPacketToSend::Type::kVideo); packet->set_first_packet_of_frame(true); packet->SetMarker(true); - sender_.impl_->RtpSender()->AssignSequenceNumber(packet.get()); + MaybeAssignSequenceNumber(packet.get()); EXPECT_TRUE(sender_.impl_->TrySendPacket(packet.get(), pacing_info)); @@ -1172,9 +1187,11 @@ TEST_P(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { EXPECT_EQ(rtx_state.sequence_number, rtx_packet.SequenceNumber() + 1); } -INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, +INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverheadAndDeferredSequencing, RtpRtcpImpl2Test, - ::testing::Values(TestConfig{false}, - TestConfig{true})); + ::testing::Values(TestConfig{false, false}, + TestConfig{false, true}, + TestConfig{true, false}, + TestConfig{true, true})); } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 5961b3d787..7e644befce 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -146,6 +146,10 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // https://tools.ietf.org/html/rfc3611#section-4.4 and #section-4.5 bool non_sender_rtt_measurement = false; + // If true, sequence numbers are not assigned until after the pacer stage, + // in RtpSenderEgress. + bool use_deferred_sequencing = false; + private: RTC_DISALLOW_COPY_AND_ASSIGN(Configuration); }; diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index a0ea1e84e8..a47d7740ab 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -806,7 +806,7 @@ RtpState RTPSender::GetRtpState() const { state.start_timestamp = timestamp_offset_; state.ssrc_has_acked = ssrc_has_acked_; if (sequencer_) { - sequencer_->PupulateRtpState(state); + sequencer_->PopulateRtpState(state); } return state; } diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 6d5826e07d..0eb6558280 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -144,6 +144,13 @@ class RTPSender { bool AssignSequenceNumbersAndStoreLastPacketState( rtc::ArrayView> packets) RTC_LOCKS_EXCLUDED(send_mutex_); + // If true, packet sequence numbering is expected to happen outside this + // class: media packetizers should not call AssignSequenceNumber(), and any + // generated padding will not have assigned sequence numbers. If false, + // packetizers do need to ecplixitly sequence number the packets and + // GeneratePadding() will return sequence numbered packets. + // TODO(bugs.webrtc.org/11340): Remove when legacy behavior is gone. + bool deferred_sequence_numbering() const { return sequencer_ == nullptr; } // Maximum header overhead per fec/padding packet. size_t FecOrPaddingPacketMaxRtpHeaderLength() const RTC_LOCKS_EXCLUDED(send_mutex_); @@ -211,7 +218,7 @@ class RTPSender { // RTP variables uint32_t timestamp_offset_ RTC_GUARDED_BY(send_mutex_); - PacketSequencer* const sequencer_ RTC_GUARDED_BY(send_mutex_); + PacketSequencer* const sequencer_ RTC_PT_GUARDED_BY(send_mutex_); // RID value to send in the RID or RepairedRID header extension. std::string rid_ RTC_GUARDED_BY(send_mutex_); // MID value to send in the MID header extension. diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 126b89c8c8..53e4adbfde 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -41,8 +41,10 @@ bool IsTrialSetTo(const WebRtcKeyValueConfig* field_trials, RtpSenderEgress::NonPacedPacketSender::NonPacedPacketSender( RtpSenderEgress* sender, - SequenceNumberAssigner* sequence_number_assigner) - : transport_sequence_number_(0), + SequenceNumberAssigner* sequence_number_assigner, + bool deferred_sequencing) + : deferred_sequencing_(deferred_sequencing), + transport_sequence_number_(0), sender_(sender), sequence_number_assigner_(sequence_number_assigner) { RTC_DCHECK(sequence_number_assigner_); @@ -57,22 +59,21 @@ void RtpSenderEgress::NonPacedPacketSender::EnqueuePackets( } auto fec_packets = sender_->FetchFecPackets(); if (!fec_packets.empty()) { - // Don't generate sequence numbers for flexfec, they are already running on - // an internally maintained sequence. - const bool generate_sequence_numbers = !sender_->FlexFecSsrc().has_value(); - - for (auto& packet : fec_packets) { - if (generate_sequence_numbers) { - sequence_number_assigner_->AssignSequenceNumber(packet.get()); - } - PrepareForSend(packet.get()); - } EnqueuePackets(std::move(fec_packets)); } } void RtpSenderEgress::NonPacedPacketSender::PrepareForSend( RtpPacketToSend* packet) { + // Assign sequence numbers if deferred sequencing is used, but don't generate + // sequence numbers for flexfec, which is already running on an internally + // maintained sequence number series. + const bool is_flexfec = packet->Ssrc() == sender_->FlexFecSsrc(); + if ((deferred_sequencing_ || + packet->packet_type() == RtpPacketMediaType::kForwardErrorCorrection) && + !is_flexfec) { + sequence_number_assigner_->AssignSequenceNumber(packet); + } if (!packet->SetExtension( ++transport_sequence_number_)) { --transport_sequence_number_; @@ -93,6 +94,7 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcpInterface::Configuration& config, !IsTrialSetTo(config.field_trials, "WebRTC-SendSideBwe-WithOverhead", "Disabled")), + deferred_sequencing_(config.use_deferred_sequencing), clock_(config.clock), packet_history_(packet_history), transport_(config.outgoing_transport), @@ -140,6 +142,25 @@ void RtpSenderEgress::SendPacket(RtpPacketToSend* packet, RTC_DCHECK_RUN_ON(&pacer_checker_); RTC_DCHECK(packet); + if (deferred_sequencing_) { + // Strict increasing of sequence numbers can only be guaranteed with + // deferred sequencing due to raciness with the pacer. + if (packet->Ssrc() == ssrc_ && + packet->packet_type() != RtpPacketMediaType::kRetransmission) { + if (last_sent_seq_.has_value()) { + RTC_DCHECK_EQ(static_cast(*last_sent_seq_ + 1), + packet->SequenceNumber()); + } + last_sent_seq_ = packet->SequenceNumber(); + } else if (packet->Ssrc() == rtx_ssrc_) { + if (last_sent_rtx_seq_.has_value()) { + RTC_DCHECK_EQ(static_cast(*last_sent_rtx_seq_ + 1), + packet->SequenceNumber()); + } + last_sent_rtx_seq_ = packet->SequenceNumber(); + } + } + RTC_DCHECK(packet->packet_type().has_value()); RTC_DCHECK(HasCorrectSsrc(*packet)); if (packet->packet_type() == RtpPacketMediaType::kRetransmission) { diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index c767a1fe1b..9c1baea034 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -44,7 +44,8 @@ class RtpSenderEgress { class NonPacedPacketSender : public RtpPacketSender { public: NonPacedPacketSender(RtpSenderEgress* sender, - SequenceNumberAssigner* sequence_number_assigner); + SequenceNumberAssigner* sequence_number_assigner, + bool deferred_sequencing); virtual ~NonPacedPacketSender(); void EnqueuePackets( @@ -52,6 +53,7 @@ class RtpSenderEgress { private: void PrepareForSend(RtpPacketToSend* packet); + const bool deferred_sequencing_; uint16_t transport_sequence_number_; RtpSenderEgress* const sender_; SequenceNumberAssigner* sequence_number_assigner_; @@ -134,6 +136,7 @@ class RtpSenderEgress { const absl::optional flexfec_ssrc_; const bool populate_network2_timestamp_; const bool send_side_bwe_with_overhead_; + const bool deferred_sequencing_; Clock* const clock_; RtpPacketHistory* const packet_history_; Transport* const transport_; @@ -143,6 +146,8 @@ class RtpSenderEgress { #endif const bool need_rtp_packet_infos_; VideoFecGenerator* const fec_generator_ RTC_GUARDED_BY(pacer_checker_); + absl::optional last_sent_seq_ RTC_GUARDED_BY(pacer_checker_); + absl::optional last_sent_rtx_seq_ RTC_GUARDED_BY(pacer_checker_); TransportFeedbackObserver* const transport_feedback_observer_; SendSideDelayObserver* const send_side_delay_observer_; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index fa9c5ad8b1..d265cc6faa 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -476,6 +476,9 @@ bool RTPSenderVideo::SendVideo( if (payload.empty()) return false; + if (!rtp_sender_->SendingMedia()) { + return false; + } int32_t retransmission_settings = retransmission_settings_; if (codec_type == VideoCodecType::kVideoCodecH264) { @@ -705,7 +708,8 @@ bool RTPSenderVideo::SendVideo( } } - if (!rtp_sender_->AssignSequenceNumbersAndStoreLastPacketState(rtp_packets)) { + if (!rtp_sender_->deferred_sequence_numbering() && + !rtp_sender_->AssignSequenceNumbersAndStoreLastPacketState(rtp_packets)) { // Media not being sent. return false; }