From 54abf984ccba10fb5daa61fa811f339edb5bfd61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 13 Aug 2021 17:25:44 +0200 Subject: [PATCH] Remove the now unused non-deferred sequencing code path. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The config flag will be removed once downstream usage is gone. Bug: webrtc:11340 Change-Id: Iee8816660009211540d9b09bb3cba514455d709b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228431 Commit-Queue: Erik Språng Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#34757} --- modules/rtp_rtcp/include/rtp_rtcp_defines.h | 9 - modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 3 +- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 98 +++----- modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 6 +- .../source/rtp_rtcp_impl2_unittest.cc | 37 +-- modules/rtp_rtcp/source/rtp_sender.cc | 82 +------ modules/rtp_rtcp/source/rtp_sender.h | 23 +- modules/rtp_rtcp/source/rtp_sender_audio.cc | 9 - modules/rtp_rtcp/source/rtp_sender_egress.cc | 22 +- modules/rtp_rtcp/source/rtp_sender_egress.h | 8 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 210 +++++------------- modules/rtp_rtcp/source/rtp_sender_video.cc | 6 - 12 files changed, 111 insertions(+), 402 deletions(-) diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 5a80cd0cc7..dfdf641ed2 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -478,14 +478,5 @@ class SendPacketObserver { uint32_t ssrc) = 0; }; -// Interface for a class that can assign RTP sequence numbers for a packet -// to be sent. -class SequenceNumberAssigner { - public: - SequenceNumberAssigner() = default; - virtual ~SequenceNumberAssigner() = default; - - virtual void AssignSequenceNumber(RtpPacketToSend* packet) = 0; -}; } // namespace webrtc #endif // MODULES_RTP_RTCP_INCLUDE_RTP_RTCP_DEFINES_H_ diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 34e8429f87..a9bd671a34 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -53,8 +53,7 @@ ModuleRtpRtcpImpl::RtpSenderContext::RtpSenderContext( packet_generator( config, &packet_history, - config.paced_sender ? config.paced_sender : &non_paced_sender, - /*packet_sequencer=*/nullptr) {} + config.paced_sender ? config.paced_sender : &non_paced_sender) {} std::unique_ptr RtpRtcp::DEPRECATED_Create( const Configuration& configuration) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 7d33ca08b9..88de8db387 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -63,27 +63,16 @@ 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, config.use_deferred_sequencing), + non_paced_sender(&packet_sender, &sequencer), packet_generator( config, &packet_history, - config.paced_sender ? config.paced_sender : &non_paced_sender, - config.use_deferred_sequencing ? nullptr : &sequencer) {} -void ModuleRtpRtcpImpl2::RtpSenderContext::AssignSequenceNumber( - RtpPacketToSend* packet) { - RTC_DCHECK_RUN_ON(&sequencing_checker); - if (deferred_sequencing_) { - sequencer.Sequence(*packet); - } else { - packet_generator.AssignSequenceNumber(packet); - } -} + config.paced_sender ? config.paced_sender : &non_paced_sender) {} ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration) : worker_queue_(TaskQueueBase::Current()), @@ -188,57 +177,42 @@ void ModuleRtpRtcpImpl2::SetStartTimestamp(const uint32_t timestamp) { uint16_t ModuleRtpRtcpImpl2::SequenceNumber() const { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); - if (rtp_sender_->deferred_sequencing_) { - return rtp_sender_->sequencer.media_sequence_number(); - } - return rtp_sender_->packet_generator.SequenceNumber(); + return rtp_sender_->sequencer.media_sequence_number(); } // Set SequenceNumber, default is a random number. void ModuleRtpRtcpImpl2::SetSequenceNumber(const uint16_t seq_num) { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); - if (rtp_sender_->deferred_sequencing_) { - 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); + if (rtp_sender_->sequencer.media_sequence_number() != seq_num) { + rtp_sender_->sequencer.set_media_sequence_number(seq_num); + rtp_sender_->packet_history.Clear(); } } void ModuleRtpRtcpImpl2::SetRtpState(const RtpState& rtp_state) { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); rtp_sender_->packet_generator.SetRtpState(rtp_state); - if (rtp_sender_->deferred_sequencing_) { - rtp_sender_->sequencer.SetRtpState(rtp_state); - } + rtp_sender_->sequencer.SetRtpState(rtp_state); rtcp_sender_.SetTimestampOffset(rtp_state.start_timestamp); } void ModuleRtpRtcpImpl2::SetRtxState(const RtpState& rtp_state) { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); rtp_sender_->packet_generator.SetRtxRtpState(rtp_state); - if (rtp_sender_->deferred_sequencing_) { - rtp_sender_->sequencer.set_rtx_sequence_number(rtp_state.sequence_number); - } + rtp_sender_->sequencer.set_rtx_sequence_number(rtp_state.sequence_number); } RtpState ModuleRtpRtcpImpl2::GetRtpState() const { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); RtpState state = rtp_sender_->packet_generator.GetRtpState(); - if (rtp_sender_->deferred_sequencing_) { - rtp_sender_->sequencer.PopulateRtpState(state); - } + rtp_sender_->sequencer.PopulateRtpState(state); return state; } RtpState ModuleRtpRtcpImpl2::GetRtxState() const { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); RtpState state = rtp_sender_->packet_generator.GetRtxRtpState(); - if (rtp_sender_->deferred_sequencing_) { - state.sequence_number = rtp_sender_->sequencer.rtx_sequence_number(); - } + state.sequence_number = rtp_sender_->sequencer.rtx_sequence_number(); return state; } @@ -383,25 +357,22 @@ bool ModuleRtpRtcpImpl2::TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK(rtp_sender_); RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); - if (rtp_sender_->deferred_sequencing_) { - if (!rtp_sender_->packet_generator.SendingMedia()) { - return false; - } - 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()) { + if (!rtp_sender_->packet_generator.SendingMedia()) { return false; } + 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; } @@ -418,19 +389,7 @@ std::vector> ModuleRtpRtcpImpl2::FetchFecPackets() { RTC_DCHECK(rtp_sender_); RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); - auto fec_packets = rtp_sender_->packet_sender.FetchFecPackets(); - 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) { - for (auto& fec_packet : fec_packets) { - rtp_sender_->packet_generator.AssignSequenceNumber(fec_packet.get()); - } - } - } - return fec_packets; + return rtp_sender_->packet_sender.FetchFecPackets(); } void ModuleRtpRtcpImpl2::OnPacketsAcknowledged( @@ -454,14 +413,9 @@ ModuleRtpRtcpImpl2::GeneratePadding(size_t target_size_bytes) { RTC_DCHECK(rtp_sender_); RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_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(), - rtp_sender_->deferred_sequencing_ - ? rtp_sender_->sequencer.CanSendPaddingOnMediaSsrc() - : false); + rtp_sender_->sequencer.CanSendPaddingOnMediaSsrc()); } std::vector diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index f01c0c066b..a5038956ed 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -263,14 +263,10 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, FRIEND_TEST_ALL_PREFIXES(RtpRtcpImpl2Test, Rtt); FRIEND_TEST_ALL_PREFIXES(RtpRtcpImpl2Test, RttForReceiverOnly); - struct RtpSenderContext : public SequenceNumberAssigner { + struct RtpSenderContext { explicit RtpSenderContext(const RtpRtcpInterface::Configuration& config); - 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_; SequenceChecker sequencing_checker; // Handles sequence number assignment and padding timestamp generation. PacketSequencer sequencer RTC_GUARDED_BY(sequencing_checker); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 9e8e824158..599264e7c5 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -152,12 +152,9 @@ class SendTransport : public Transport, }; struct TestConfig { - explicit TestConfig(bool with_overhead, bool with_deferred_sequencing) - : with_overhead(with_overhead), - with_deferred_sequencing(with_deferred_sequencing) {} + explicit TestConfig(bool with_overhead) : with_overhead(with_overhead) {} bool with_overhead = false; - bool with_deferred_sequencing = false; }; class FieldTrialConfig : public WebRtcKeyValueConfig { @@ -204,12 +201,10 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, RtpRtcpModule(GlobalSimulatedTimeController* time_controller, bool is_sender, - const FieldTrialConfig& trials, - bool deferred_sequencing) + const FieldTrialConfig& trials) : 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) { @@ -219,7 +214,6 @@ 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_; @@ -289,7 +283,6 @@ 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); @@ -310,12 +303,10 @@ class RtpRtcpImpl2Test : public ::testing::TestWithParam { field_trials_(FieldTrialConfig::GetFromTestConfig(GetParam())), sender_(&time_controller_, /*is_sender=*/true, - field_trials_, - GetParam().with_deferred_sequencing), + field_trials_), receiver_(&time_controller_, /*is_sender=*/false, - field_trials_, - GetParam().with_deferred_sequencing) {} + field_trials_) {} void SetUp() override { // Send module. @@ -417,12 +408,6 @@ 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) { @@ -753,7 +738,6 @@ TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { // Single-packet frame. packet.SetTimestamp(1); - MaybeAssignSequenceNumber(&packet); packet.set_first_packet_of_frame(true); packet.SetMarker(true); sender_.impl_->TrySendPacket(&packet, pacing_info); @@ -769,16 +753,13 @@ TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { // Three-packet frame. packet.SetTimestamp(2); - MaybeAssignSequenceNumber(&packet); packet.set_first_packet_of_frame(true); packet.SetMarker(false); sender_.impl_->TrySendPacket(&packet, pacing_info); - MaybeAssignSequenceNumber(&packet); packet.set_first_packet_of_frame(false); sender_.impl_->TrySendPacket(&packet, pacing_info); - MaybeAssignSequenceNumber(&packet); packet.SetMarker(true); sender_.impl_->TrySendPacket(&packet, pacing_info); @@ -937,7 +918,6 @@ 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. - MaybeAssignSequenceNumber(packet.get()); EXPECT_TRUE(sender_.impl_->TrySendPacket(packet.get(), pacing_info)); @@ -948,7 +928,6 @@ TEST_P(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { packet->set_packet_type(RtpPacketToSend::Type::kVideo); packet->set_first_packet_of_frame(true); packet->SetMarker(true); - MaybeAssignSequenceNumber(packet.get()); EXPECT_TRUE(sender_.impl_->TrySendPacket(packet.get(), pacing_info)); @@ -1187,11 +1166,9 @@ TEST_P(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { EXPECT_EQ(rtx_state.sequence_number, rtx_packet.SequenceNumber() + 1); } -INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverheadAndDeferredSequencing, +INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpRtcpImpl2Test, - ::testing::Values(TestConfig{false, false}, - TestConfig{false, true}, - TestConfig{true, false}, - TestConfig{true, true})); + ::testing::Values(TestConfig{false}, + TestConfig{true})); } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 707973f2a6..80f32046d6 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -158,7 +158,12 @@ double GetMaxPaddingSizeFactor(const WebRtcKeyValueConfig* field_trials) { RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config, RtpPacketHistory* packet_history, RtpPacketSender* packet_sender, - PacketSequencer* packet_sequencer) + PacketSequencer*) + : RTPSender(config, packet_history, packet_sender) {} + +RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config, + RtpPacketHistory* packet_history, + RtpPacketSender* packet_sender) : clock_(config.clock), random_(clock_->TimeInMicroseconds()), audio_configured_(config.audio), @@ -173,7 +178,6 @@ RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config, max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. rtp_header_extension_map_(config.extmap_allow_mixed), // RTP variables - sequencer_(packet_sequencer), always_send_mid_and_rid_(config.always_send_mid_and_rid), ssrc_has_acked_(false), rtx_ssrc_has_acked_(false), @@ -441,16 +445,10 @@ std::vector> RTPSender::GeneratePadding( padding_packet->set_packet_type(RtpPacketMediaType::kPadding); padding_packet->SetMarker(false); if (rtx_ == kRtxOff) { - bool can_send_padding = sequencer_ - ? sequencer_->CanSendPaddingOnMediaSsrc() - : can_send_padding_on_media_ssrc; - if (!can_send_padding) { + if (!can_send_padding_on_media_ssrc) { break; } padding_packet->SetSsrc(ssrc_); - if (sequencer_) { - sequencer_->Sequence(*padding_packet); - } } else { // Without abs-send-time or transport sequence number a media packet // must be sent before padding so that the timestamps used for @@ -465,9 +463,6 @@ std::vector> RTPSender::GeneratePadding( RTC_DCHECK(rtx_ssrc_); padding_packet->SetSsrc(*rtx_ssrc_); padding_packet->SetPayloadType(rtx_payload_type_map_.begin()->second); - if (sequencer_) { - sequencer_->Sequence(*padding_packet); - } } if (rtp_header_extension_map_.IsRegistered(TransportSequenceNumber::kId)) { @@ -574,28 +569,6 @@ std::unique_ptr RTPSender::AllocatePacket() const { return packet; } -bool RTPSender::AssignSequenceNumber(RtpPacketToSend* packet) { - MutexLock lock(&send_mutex_); - RTC_DCHECK(sequencer_); - if (!sending_media_) - return false; - sequencer_->Sequence(*packet); - return true; -} - -bool RTPSender::AssignSequenceNumbersAndStoreLastPacketState( - rtc::ArrayView> packets) { - RTC_DCHECK(!packets.empty()); - MutexLock lock(&send_mutex_); - RTC_DCHECK(sequencer_); - if (!sending_media_) - return false; - for (auto& packet : packets) { - sequencer_->Sequence(*packet); - } - return true; -} - void RTPSender::SetSendingMediaStatus(bool enabled) { MutexLock lock(&send_mutex_); sending_media_ = enabled; @@ -643,30 +616,6 @@ void RTPSender::SetCsrcs(const std::vector& csrcs) { UpdateHeaderSizes(); } -void RTPSender::SetSequenceNumber(uint16_t seq) { - bool updated_sequence_number = false; - { - MutexLock lock(&send_mutex_); - RTC_DCHECK(sequencer_); - if (sequencer_->media_sequence_number() != seq) { - updated_sequence_number = true; - } - sequencer_->set_media_sequence_number(seq); - } - - if (updated_sequence_number) { - // Sequence number series has been reset to a new value, clear RTP packet - // history, since any packets there may conflict with new ones. - packet_history_->Clear(); - } -} - -uint16_t RTPSender::SequenceNumber() const { - MutexLock lock(&send_mutex_); - RTC_DCHECK(sequencer_); - return sequencer_->media_sequence_number(); -} - static void CopyHeaderAndExtensionsToRtxPacket(const RtpPacketToSend& packet, RtpPacketToSend* rtx_packet) { // Set the relevant fixed packet headers. The following are not set: @@ -740,11 +689,6 @@ std::unique_ptr RTPSender::BuildRtxPacket( // Replace SSRC. rtx_packet->SetSsrc(*rtx_ssrc_); - // Replace sequence number. - if (sequencer_) { - sequencer_->Sequence(*rtx_packet); - } - CopyHeaderAndExtensionsToRtxPacket(packet, rtx_packet.get()); // RTX packets are sent on an SSRC different from the main media, so the @@ -792,9 +736,6 @@ void RTPSender::SetRtpState(const RtpState& rtp_state) { MutexLock lock(&send_mutex_); timestamp_offset_ = rtp_state.start_timestamp; - if (sequencer_) { - sequencer_->SetRtpState(rtp_state); - } ssrc_has_acked_ = rtp_state.ssrc_has_acked; UpdateHeaderSizes(); } @@ -805,17 +746,11 @@ RtpState RTPSender::GetRtpState() const { RtpState state; state.start_timestamp = timestamp_offset_; state.ssrc_has_acked = ssrc_has_acked_; - if (sequencer_) { - sequencer_->PopulateRtpState(state); - } return state; } void RTPSender::SetRtxRtpState(const RtpState& rtp_state) { MutexLock lock(&send_mutex_); - if (sequencer_) { - sequencer_->set_rtx_sequence_number(rtp_state.sequence_number); - } rtx_ssrc_has_acked_ = rtp_state.ssrc_has_acked; } @@ -823,9 +758,6 @@ RtpState RTPSender::GetRtxRtpState() const { MutexLock lock(&send_mutex_); RtpState state; - if (sequencer_) { - state.sequence_number = sequencer_->rtx_sequence_number(); - } state.start_timestamp = timestamp_offset_; state.ssrc_has_acked = rtx_ssrc_has_acked_; diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 4919b40252..3374f2b993 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -44,6 +44,11 @@ class RtpPacketToSend; class RTPSender { public: + RTPSender(const RtpRtcpInterface::Configuration& config, + RtpPacketHistory* packet_history, + RtpPacketSender* packet_sender); + + ABSL_DEPRECATED("bugs.webrtc.org/11340") RTPSender(const RtpRtcpInterface::Configuration& config, RtpPacketHistory* packet_history, RtpPacketSender* packet_sender, @@ -134,23 +139,6 @@ class RTPSender { // extensions RtpSender updates before sending. std::unique_ptr AllocatePacket() const RTC_LOCKS_EXCLUDED(send_mutex_); - // Allocate sequence number for provided packet. - // Save packet's fields to generate padding that doesn't break media stream. - // Return false if sending was turned off. - bool AssignSequenceNumber(RtpPacketToSend* packet) - RTC_LOCKS_EXCLUDED(send_mutex_); - // Same as AssignSequenceNumber(), but applies sequence numbers atomically to - // a batch of packets. - 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_); @@ -218,7 +206,6 @@ class RTPSender { // RTP variables uint32_t timestamp_offset_ 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_audio.cc b/modules/rtp_rtcp/source/rtp_sender_audio.cc index c23e81a06a..2125295886 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio.cc @@ -304,11 +304,6 @@ bool RTPSenderAudio::SendAudio(AudioFrameType frame_type, return false; memcpy(payload, payload_data, payload_size); - if (!rtp_sender_->deferred_sequence_numbering() && - !rtp_sender_->AssignSequenceNumber(packet.get())) { - return false; - } - { MutexLock lock(&send_audio_mutex_); last_payload_type_ = payload_type; @@ -376,10 +371,6 @@ bool RTPSenderAudio::SendTelephoneEventPacket(bool ended, packet->SetSsrc(rtp_sender_->SSRC()); packet->SetTimestamp(dtmf_timestamp); packet->set_capture_time_ms(clock_->TimeInMilliseconds()); - if (!rtp_sender_->deferred_sequence_numbering() && - !rtp_sender_->AssignSequenceNumber(packet.get())) { - return false; - } // Create DTMF data. uint8_t* dtmfbuffer = packet->AllocatePayload(kDtmfSize); diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 9c3688826d..f2a74e196c 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -41,13 +41,9 @@ bool IsTrialSetTo(const WebRtcKeyValueConfig* field_trials, RtpSenderEgress::NonPacedPacketSender::NonPacedPacketSender( RtpSenderEgress* sender, - 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_); + PacketSequencer* sequencer) + : transport_sequence_number_(0), sender_(sender), sequencer_(sequencer) { + RTC_DCHECK(sequencer); } RtpSenderEgress::NonPacedPacketSender::~NonPacedPacketSender() = default; @@ -65,14 +61,10 @@ void RtpSenderEgress::NonPacedPacketSender::EnqueuePackets( 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); + // Assign sequence numbers, but not for flexfec which is already running on + // an internally maintained sequence number series. + if (packet->Ssrc() != sender_->FlexFecSsrc()) { + sequencer_->Sequence(*packet); } if (!packet->SetExtension( ++transport_sequence_number_)) { diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index 747471ca3c..bf9076d9f2 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -24,6 +24,7 @@ #include "api/units/data_rate.h" #include "modules/remote_bitrate_estimator/test/bwe_test_logging.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" @@ -43,9 +44,7 @@ class RtpSenderEgress { // without passing through an actual paced sender. class NonPacedPacketSender : public RtpPacketSender { public: - NonPacedPacketSender(RtpSenderEgress* sender, - SequenceNumberAssigner* sequence_number_assigner, - bool deferred_sequencing); + NonPacedPacketSender(RtpSenderEgress* sender, PacketSequencer* sequencer); virtual ~NonPacedPacketSender(); void EnqueuePackets( @@ -53,10 +52,9 @@ class RtpSenderEgress { private: void PrepareForSend(RtpPacketToSend* packet); - const bool deferred_sequencing_; uint16_t transport_sequence_number_; RtpSenderEgress* const sender_; - SequenceNumberAssigner* sequence_number_assigner_; + PacketSequencer* sequencer_; }; RtpSenderEgress(const RtpRtcpInterface::Configuration& config, diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 8181cd068f..2724cec8ff 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -137,7 +137,6 @@ class RtpSenderTest : public ::testing::Test { std::vector(), nullptr, clock_), - deferred_sequencing_(false), kMarkerBit(true) {} void SetUp() override { SetUpRtpSender(true, false, nullptr); } @@ -165,21 +164,6 @@ class RtpSenderTest : public ::testing::Test { } void CreateSender(const RtpRtcpInterface::Configuration& config) { - packet_history_ = std::make_unique( - config.clock, config.enable_rtx_padding_prioritization); - sequencer_.emplace(kSsrc, kRtxSsrc, - /*require_marker_before_media_padding=*/!config.audio, - clock_); - rtp_sender_ = - std::make_unique(config, packet_history_.get(), - config.paced_sender, &sequencer_.value()); - rtp_sender_->SetSequenceNumber(kSeqNum); - rtp_sender_->SetTimestampOffset(0); - deferred_sequencing_ = false; - } - - void CreateSenderWithDeferredSequencing( - const RtpRtcpInterface::Configuration& config) { packet_history_ = std::make_unique( config.clock, config.enable_rtx_padding_prioritization); sequencer_.emplace(kSsrc, kRtxSsrc, @@ -189,7 +173,6 @@ class RtpSenderTest : public ::testing::Test { config.paced_sender, nullptr); sequencer_->set_media_sequence_number(kSeqNum); rtp_sender_->SetTimestampOffset(0); - deferred_sequencing_ = true; } GlobalSimulatedTimeController time_controller_; @@ -199,7 +182,6 @@ class RtpSenderTest : public ::testing::Test { RateLimiter retransmission_rate_limiter_; FlexfecSender flexfec_sender_; - bool deferred_sequencing_; absl::optional sequencer_; std::unique_ptr packet_history_; std::unique_ptr rtp_sender_; @@ -217,9 +199,6 @@ class RtpSenderTest : public ::testing::Test { packet->SetMarker(marker_bit); packet->SetTimestamp(timestamp); packet->set_capture_time_ms(capture_time_ms); - if (!deferred_sequencing_) { - EXPECT_TRUE(rtp_sender_->AssignSequenceNumber(packet.get())); - } return packet; } @@ -254,6 +233,14 @@ class RtpSenderTest : public ::testing::Test { sequencer_->CanSendPaddingOnMediaSsrc()); } + std::vector> Sequence( + std::vector> packets) { + for (auto& packet : packets) { + sequencer_->Sequence(*packet); + } + return packets; + } + size_t GenerateAndSendPadding(size_t target_size_bytes) { size_t generated_bytes = 0; for (auto& packet : GeneratePadding(target_size_bytes)) { @@ -341,7 +328,7 @@ TEST_F(RtpSenderTest, PaddingAlwaysAllowedOnAudio) { // Padding on audio stream allowed regardless of marker in the last packet. audio_packet->SetMarker(false); audio_packet->SetPayloadType(kPayload); - rtp_sender_->AssignSequenceNumber(audio_packet.get()); + sequencer_->Sequence(*audio_packet); const size_t kPaddingSize = 59; @@ -372,7 +359,6 @@ TEST_F(RtpSenderTest, SendToNetworkForwardsPacketsToPacer) { mock_paced_sender_, EnqueuePackets(ElementsAre(AllOf( Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), - Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum)), Pointee(Property(&RtpPacketToSend::capture_time_ms, now_ms)))))); EXPECT_TRUE( rtp_sender_->SendToNetwork(std::make_unique(*packet))); @@ -383,7 +369,7 @@ TEST_F(RtpSenderTest, ReSendPacketForwardsPacketsToPacer) { RtpPacketHistory::StorageMode::kStoreAndCull, 10); int64_t now_ms = clock_->TimeInMilliseconds(); auto packet = BuildRtpPacket(kPayload, kMarkerBit, kTimestamp, now_ms); - uint16_t seq_no = packet->SequenceNumber(); + packet->SetSequenceNumber(kSeqNum); packet->set_allow_retransmission(true); packet_history_->PutRtpPacket(std::move(packet), now_ms); @@ -394,7 +380,7 @@ TEST_F(RtpSenderTest, ReSendPacketForwardsPacketsToPacer) { Pointee(Property(&RtpPacketToSend::capture_time_ms, now_ms)), Pointee(Property(&RtpPacketToSend::packet_type, RtpPacketMediaType::kRetransmission)))))); - EXPECT_TRUE(rtp_sender_->ReSendPacket(seq_no)); + EXPECT_TRUE(rtp_sender_->ReSendPacket(kSeqNum)); } // This test sends 1 regular video packet, then 4 padding packets, and then @@ -405,6 +391,7 @@ TEST_F(RtpSenderTest, SendPadding) { std::unique_ptr media_packet = SendPacket(/*capture_time_ms=*/clock_->TimeInMilliseconds(), /*payload_size=*/100); + sequencer_->Sequence(*media_packet); // Wait 50 ms before generating each padding packet. for (int i = 0; i < kNumPaddingPackets; ++i) { @@ -415,30 +402,24 @@ TEST_F(RtpSenderTest, SendPadding) { // number range. Size will be forced to full pack size and the timestamp // shall be that of the last media packet. EXPECT_CALL(mock_paced_sender_, - EnqueuePackets(ElementsAre(AllOf( - Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), - Pointee(Property(&RtpPacketToSend::SequenceNumber, - media_packet->SequenceNumber() + i + 1)), - Pointee(Property(&RtpPacketToSend::padding_size, - kMaxPaddingLength)), - Pointee(Property(&RtpPacketToSend::Timestamp, - media_packet->Timestamp())))))); + EnqueuePackets(ElementsAre(Pointee(AllOf( + Property(&RtpPacketToSend::Ssrc, kSsrc), + Property(&RtpPacketToSend::padding_size, kMaxPaddingLength), + Property(&RtpPacketToSend::SequenceNumber, + media_packet->SequenceNumber() + i + 1), + Property(&RtpPacketToSend::Timestamp, + media_packet->Timestamp())))))); std::vector> padding_packets = - rtp_sender_->GeneratePadding(kPaddingTargetBytes, - /*media_has_been_sent=*/true, - /*can_send_padding_on_media_ssrc=*/true); + Sequence(GeneratePadding(kPaddingTargetBytes)); ASSERT_THAT(padding_packets, SizeIs(1)); rtp_sender_->SendToNetwork(std::move(padding_packets[0])); } // Send a regular video packet again. - EXPECT_CALL(mock_paced_sender_, - EnqueuePackets(ElementsAre(AllOf( - Pointee(Property( - &RtpPacketToSend::SequenceNumber, - media_packet->SequenceNumber() + kNumPaddingPackets + 1)), - Pointee(Property(&RtpPacketToSend::Timestamp, - Gt(media_packet->Timestamp()))))))); + EXPECT_CALL( + mock_paced_sender_, + EnqueuePackets(ElementsAre(Pointee(Property( + &RtpPacketToSend::Timestamp, Gt(media_packet->Timestamp())))))); std::unique_ptr next_media_packet = SendPacket(/*capture_time_ms=*/clock_->TimeInMilliseconds(), @@ -518,6 +499,7 @@ TEST_F(RtpSenderTest, UpdatesTimestampsOnPlainRtxPadding) { Pointee(Property(&RtpPacketToSend::capture_time_ms, start_time)))))); std::unique_ptr media_packet = SendPacket(start_time, /*payload_size=*/600); + sequencer_->Sequence(*media_packet); // Advance time before sending padding. const TimeDelta kTimeDiff = TimeDelta::Millis(17); @@ -525,14 +507,13 @@ TEST_F(RtpSenderTest, UpdatesTimestampsOnPlainRtxPadding) { // Timestamps on padding should be offset from the sent media. EXPECT_THAT( - GeneratePadding(/*target_size_bytes=*/100), - Each(AllOf( - Pointee(Property(&RtpPacketToSend::padding_size, kMaxPaddingLength)), - Pointee(Property( - &RtpPacketToSend::Timestamp, - start_timestamp + (kTimestampTicksPerMs * kTimeDiff.ms()))), - Pointee(Property(&RtpPacketToSend::capture_time_ms, - start_time + kTimeDiff.ms()))))); + Sequence(GeneratePadding(/*target_size_bytes=*/100)), + Each(Pointee(AllOf( + Property(&RtpPacketToSend::padding_size, kMaxPaddingLength), + Property(&RtpPacketToSend::Timestamp, + start_timestamp + (kTimestampTicksPerMs * kTimeDiff.ms())), + Property(&RtpPacketToSend::capture_time_ms, + start_time + kTimeDiff.ms()))))); } TEST_F(RtpSenderTest, KeepsTimestampsOnPayloadPadding) { @@ -607,6 +588,7 @@ TEST_F(RtpSenderTest, RidIncludedOnRtxSentPackets) { Property(&RtpPacketToSend::HasExtension, false)))))) .WillOnce([&](std::vector> packets) { + sequencer_->Sequence(*packets[0]); packet_history_->PutRtpPacket(std::move(packets[0]), clock_->TimeInMilliseconds()); }); @@ -712,21 +694,19 @@ TEST_F(RtpSenderTest, MidAndRidNotIncludedOnRtxPacketsAfterAck) { EnableRidSending(kRid); // This first packet will include both MID and RID. - EXPECT_CALL(mock_paced_sender_, EnqueuePackets(SizeIs(1))) - .WillOnce([&](std::vector> packets) { - packet_history_->PutRtpPacket(std::move(packets[0]), - clock_->TimeInMilliseconds()); - }); auto first_built_packet = SendGenericPacket(); + sequencer_->Sequence(*first_built_packet); + packet_history_->PutRtpPacket( + std::make_unique(*first_built_packet), + /*send_time=*/clock_->TimeInMilliseconds()); rtp_sender_->OnReceivedAckOnSsrc(first_built_packet->SequenceNumber()); // The second packet will include neither since an ack was received. - EXPECT_CALL(mock_paced_sender_, EnqueuePackets(SizeIs(1))) - .WillOnce([&](std::vector> packets) { - packet_history_->PutRtpPacket(std::move(packets[0]), - clock_->TimeInMilliseconds()); - }); auto second_built_packet = SendGenericPacket(); + sequencer_->Sequence(*second_built_packet); + packet_history_->PutRtpPacket( + std::make_unique(*second_built_packet), + /*send_time=*/clock_->TimeInMilliseconds()); // The first RTX packet will include MID and RRID. EXPECT_CALL(mock_paced_sender_, EnqueuePackets(SizeIs(1))) @@ -849,22 +829,20 @@ TEST_F(RtpSenderTest, MidAndRridNotIncludedOnRtxPacketsAfterRtpStateRestored) { TEST_F(RtpSenderTest, RespectsNackBitrateLimit) { const int32_t kPacketSize = 1400; const int32_t kNumPackets = 30; - retransmission_rate_limiter_.SetMaxRate(kPacketSize * kNumPackets * 8); + EnableRtx(); - packet_history_->SetStorePacketsStatus( - RtpPacketHistory::StorageMode::kStoreAndCull, kNumPackets); - const uint16_t kStartSequenceNumber = rtp_sender_->SequenceNumber(); std::vector sequence_numbers; for (int32_t i = 0; i < kNumPackets; ++i) { - sequence_numbers.push_back(kStartSequenceNumber + i); + std::unique_ptr packet = + BuildRtpPacket(kPayload, /*marker_bit=*/true, /*timestamp=*/0, + /*capture_time_ms=*/clock_->TimeInMilliseconds()); + packet->set_allow_retransmission(true); + sequencer_->Sequence(*packet); + sequence_numbers.push_back(packet->SequenceNumber()); + packet_history_->PutRtpPacket(std::move(packet), + /*send_time=*/clock_->TimeInMilliseconds()); time_controller_.AdvanceTime(TimeDelta::Millis(1)); - EXPECT_CALL(mock_paced_sender_, EnqueuePackets(SizeIs(1))) - .WillOnce([&](std::vector> packets) { - packet_history_->PutRtpPacket(std::move(packets[0]), - clock_->TimeInMilliseconds()); - }); - SendPacket(clock_->TimeInMilliseconds(), kPacketSize); } time_controller_.AdvanceTime(TimeDelta::Millis(1000 - kNumPackets)); @@ -1175,6 +1153,7 @@ TEST_F(RtpSenderTest, GeneratePaddingCreatesPurePaddingWithoutRtx) { packet->set_allow_retransmission(true); packet->SetPayloadSize(kPayloadPacketSize); packet->set_packet_type(RtpPacketMediaType::kVideo); + sequencer_->Sequence(*packet); packet_history_->PutRtpPacket(std::move(packet), clock_->TimeInMilliseconds()); @@ -1251,6 +1230,7 @@ TEST_F(RtpSenderTest, SetsCaptureTimeOnRtxRetransmissions) { BuildRtpPacket(kPayload, kMarkerBit, start_time_ms, /*capture_time_ms=*/start_time_ms); packet->set_allow_retransmission(true); + sequencer_->Sequence(*packet); packet_history_->PutRtpPacket(std::move(packet), start_time_ms); // Advance time, request an RTX retransmission. Capture timestamp should be @@ -1263,24 +1243,6 @@ TEST_F(RtpSenderTest, SetsCaptureTimeOnRtxRetransmissions) { EXPECT_GT(rtp_sender_->ReSendPacket(kSeqNum), 0); } -TEST_F(RtpSenderTest, ClearHistoryOnSequenceNumberCange) { - EnableRtx(); - - // Put a packet in the packet history. - const int64_t now_ms = clock_->TimeInMilliseconds(); - std::unique_ptr packet = - BuildRtpPacket(kPayload, kMarkerBit, now_ms, now_ms); - packet->set_allow_retransmission(true); - packet_history_->PutRtpPacket(std::move(packet), now_ms); - - EXPECT_TRUE(packet_history_->GetPacketState(kSeqNum)); - - // Update the sequence number of the RTP module, verify packet has been - // removed. - rtp_sender_->SetSequenceNumber(rtp_sender_->SequenceNumber() - 1); - EXPECT_FALSE(packet_history_->GetPacketState(kSeqNum)); -} - TEST_F(RtpSenderTest, IgnoresNackAfterDisablingMedia) { const int64_t kRtt = 10; @@ -1293,6 +1255,7 @@ TEST_F(RtpSenderTest, IgnoresNackAfterDisablingMedia) { BuildRtpPacket(kPayload, kMarkerBit, start_time_ms, /*capture_time_ms=*/start_time_ms); packet->set_allow_retransmission(true); + sequencer_->Sequence(*packet); packet_history_->PutRtpPacket(std::move(packet), start_time_ms); // Disable media sending and try to retransmit the packet, it should fail. @@ -1318,6 +1281,7 @@ TEST_F(RtpSenderTest, DoesntFecProtectRetransmissions) { /*capture_time_ms=*/start_time_ms); packet->set_allow_retransmission(true); packet->set_fec_protect_packet(true); + sequencer_->Sequence(*packet); packet_history_->PutRtpPacket(std::move(packet), start_time_ms); // Re-send packet, the retransmitted packet should not have the FEC protection @@ -1377,70 +1341,4 @@ TEST_F(RtpSenderTest, MarksPacketsWithKeyframeStatus) { } } -TEST_F(RtpSenderTest, PlainPaddingWithDeferredSequencing) { - CreateSenderWithDeferredSequencing(GetDefaultConfig()); - - EXPECT_THAT( - rtp_sender_->GeneratePadding( - /*target_size_bytes=*/500, - /*media_has_been_sent=*/true, - /*can_send_padding_on_media_ssrc=*/true), - Each(Pointee(AllOf(Property(&RtpPacketToSend::SequenceNumber, 0), - Property(&RtpPacketToSend::padding_size, Gt(0u)), - Property(&RtpPacketToSend::Ssrc, kSsrc))))); -} - -TEST_F(RtpSenderTest, PlainRtxPaddingWithDeferredSequencing) { - CreateSenderWithDeferredSequencing(GetDefaultConfig()); - EnableRtx(); - - EXPECT_THAT( - rtp_sender_->GeneratePadding( - /*target_size_bytes=*/500, - /*media_has_been_sent=*/true, - /*can_send_padding_on_media_ssrc=*/true), - Each(Pointee(AllOf(Property(&RtpPacketToSend::SequenceNumber, 0), - Property(&RtpPacketToSend::padding_size, Gt(0u)), - Property(&RtpPacketToSend::Ssrc, kRtxSsrc))))); -} - -TEST_F(RtpSenderTest, PayloadPaddingWithDeferredSequencing) { - CreateSenderWithDeferredSequencing(GetDefaultConfig()); - EnableRtx(); - ASSERT_TRUE(rtp_sender_->RegisterRtpHeaderExtension( - TransportSequenceNumber::kUri, kTransportSequenceNumberExtensionId)); - - EXPECT_CALL(mock_paced_sender_, EnqueuePackets); - std::unique_ptr media_packet = - SendPacket(clock_->TimeInMilliseconds(), /*payload_size=*/500); - packet_history_->PutRtpPacket(std::move(media_packet), - clock_->TimeInMilliseconds()); - - EXPECT_THAT( - rtp_sender_->GeneratePadding( - /*target_size_bytes=*/500, - /*media_has_been_sent=*/true, - /*can_send_padding_on_media_ssrc=*/true), - Each(Pointee(AllOf(Property(&RtpPacketToSend::SequenceNumber, 0), - Property(&RtpPacketToSend::payload_size, Gt(0u)), - Property(&RtpPacketToSend::Ssrc, kRtxSsrc))))); -} - -TEST_F(RtpSenderTest, RtxRetransmissionWithDeferredSequencing) { - CreateSenderWithDeferredSequencing(GetDefaultConfig()); - EnableRtx(); - - int64_t now_ms = clock_->TimeInMilliseconds(); - auto packet = BuildRtpPacket(kPayload, kMarkerBit, kTimestamp, now_ms); - packet->SetSequenceNumber(kSeqNum); - packet->set_allow_retransmission(true); - packet_history_->PutRtpPacket(std::move(packet), now_ms); - - EXPECT_CALL(mock_paced_sender_, - EnqueuePackets(ElementsAre(Pointee( - AllOf(Property(&RtpPacketToSend::Ssrc, kRtxSsrc), - Property(&RtpPacketToSend::SequenceNumber, 0u)))))); - EXPECT_TRUE(rtp_sender_->ReSendPacket(kSeqNum)); -} - } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index e7ac1e486b..b8d9b65546 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -708,12 +708,6 @@ bool RTPSenderVideo::SendVideo( } } - if (!rtp_sender_->deferred_sequence_numbering() && - !rtp_sender_->AssignSequenceNumbersAndStoreLastPacketState(rtp_packets)) { - // Media not being sent. - return false; - } - LogAndSendToNetwork(std::move(rtp_packets), payload.size()); // Update details about the last sent frame.