From 02d7d353a90bccc6fdd0e517af31fab50944d31c Mon Sep 17 00:00:00 2001 From: Amit Hilbuch Date: Tue, 2 Jul 2019 21:04:57 +0000 Subject: [PATCH] Revert "Add ability to set ssrcs of RtpSender at construction time" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit e9d6e658c307fc0241d622756703d5c0d5388d80. Reason for revert: breaks downstream project Original change's description: > Add ability to set ssrcs of RtpSender at construction time > > Bug: webrtc:10774 > Change-Id: I7147a75ccbcd1093dcd2e08047da8900843fdd8d > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144037 > Commit-Queue: Erik Språng > Reviewed-by: Åsa Persson > Cr-Commit-Position: refs/heads/master@{#28447} TBR=asapersson@webrtc.org,sprang@webrtc.org Change-Id: I8b0cba0836e7d86ae1718055196c2c89860b97ff No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10774 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144368 Reviewed-by: Amit Hilbuch Commit-Queue: Amit Hilbuch Cr-Commit-Position: refs/heads/master@{#28453} --- modules/rtp_rtcp/include/rtp_rtcp.h | 10 +- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 22 +- modules/rtp_rtcp/source/rtp_sender.cc | 85 +++-- modules/rtp_rtcp/source/rtp_sender.h | 24 +- .../source/rtp_sender_audio_unittest.cc | 27 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 333 +++++++----------- .../source/rtp_sender_video_unittest.cc | 29 +- 7 files changed, 257 insertions(+), 273 deletions(-) diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 25be175cd7..fcad876a4a 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -28,6 +28,7 @@ #include "modules/rtp_rtcp/include/rtp_packet_pacer.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtp_packet_to_send.h" +#include "modules/rtp_rtcp/source/rtp_sender.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/deprecation.h" @@ -40,7 +41,6 @@ class RateLimiter; class ReceiveStatisticsProvider; class RemoteBitrateEstimator; class RtcEventLog; -class RTPSender; class Transport; class VideoBitrateAllocationObserver; @@ -52,7 +52,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { public: struct Configuration { Configuration(); - Configuration(Configuration&& rhs); // True for a audio version of the RTP/RTCP module object false will create // a video version. @@ -121,11 +120,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { // defaults to webrtc::FieldTrialBasedConfig. const WebRtcKeyValueConfig* field_trials = nullptr; - // SSRCs for sending media and retransmission, respectively. - // FlexFec SSRC is fetched from |flexfec_sender|. - absl::optional media_send_ssrc; - absl::optional rtx_send_ssrc; - private: RTC_DISALLOW_COPY_AND_ASSIGN(Configuration); }; @@ -199,7 +193,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { uint32_t SSRC() const override = 0; // Sets SSRC, default is a random number. - // TODO(bugs.webrtc.org/10774): Remove. virtual void SetSSRC(uint32_t ssrc) = 0; // Sets the value for sending in the RID (and Repaired) RTP header extension. @@ -227,7 +220,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { // Sets the SSRC to use when sending RTX packets. This doesn't enable RTX, // only the SSRC is set. - // TODO(bugs.webrtc.org/10774): Remove. virtual void SetRtxSsrc(uint32_t ssrc) = 0; // Sets the payload type to use when sending RTX packets. Note that this diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 21b85a19d5..f8a0d709ae 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -40,7 +40,6 @@ constexpr int32_t kDefaultAudioReportInterval = 5000; } // namespace RtpRtcp::Configuration::Configuration() = default; -RtpRtcp::Configuration::Configuration(Configuration&& rhs) = default; std::unique_ptr RtpRtcp::Create(const Configuration& configuration) { RTC_DCHECK(configuration.clock); @@ -95,8 +94,27 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) ack_observer_(configuration.ack_observer), rtt_stats_(configuration.rtt_stats), rtt_ms_(0) { + FieldTrialBasedConfig default_trials; if (!configuration.receiver_only) { - rtp_sender_.reset(new RTPSender(configuration)); + rtp_sender_.reset(new RTPSender( + configuration.audio, configuration.clock, + configuration.outgoing_transport, configuration.paced_sender, + configuration.flexfec_sender + ? absl::make_optional(configuration.flexfec_sender->ssrc()) + : absl::nullopt, + configuration.transport_sequence_number_allocator, + configuration.transport_feedback_callback, + configuration.send_bitrate_observer, + configuration.send_side_delay_observer, configuration.event_log, + configuration.send_packet_observer, + configuration.retransmission_rate_limiter, + configuration.overhead_observer, + configuration.populate_network2_timestamp, + configuration.frame_encryptor, configuration.require_frame_encryption, + configuration.extmap_allow_mixed, + configuration.field_trials ? *configuration.field_trials + : default_trials)); + // Make sure rtcp sender use same timestamp offset as rtp sender. rtcp_sender_.SetTimestampOffset(rtp_sender_->TimestampOffset()); } diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 9f42287f03..ac82262928 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -18,7 +18,6 @@ #include "absl/memory/memory.h" #include "absl/strings/match.h" #include "api/array_view.h" -#include "api/transport/field_trial_based_config.h" #include "logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "modules/rtp_rtcp/include/rtp_cvo.h" @@ -124,41 +123,42 @@ RtpPacketSender::Priority PacketTypeToPriority(RtpPacketToSend::Type type) { return RtpPacketSender::Priority::kLowPriority; } -bool IsEnabled(absl::string_view name, - const WebRtcKeyValueConfig* field_trials) { - FieldTrialBasedConfig default_trials; - auto& trials = field_trials ? *field_trials : default_trials; - return trials.Lookup(name).find("Enabled") == 0; -} - -bool IsDisabled(absl::string_view name, - const WebRtcKeyValueConfig* field_trials) { - FieldTrialBasedConfig default_trials; - auto& trials = field_trials ? *field_trials : default_trials; - return trials.Lookup(name).find("Disabled") == 0; -} - } // namespace -RTPSender::RTPSender(const RtpRtcp::Configuration& config) - : clock_(config.clock), +RTPSender::RTPSender( + bool audio, + Clock* clock, + Transport* transport, + RtpPacketPacer* paced_sender, + absl::optional flexfec_ssrc, + TransportSequenceNumberAllocator* sequence_number_allocator, + TransportFeedbackObserver* transport_feedback_observer, + BitrateStatisticsObserver* bitrate_callback, + SendSideDelayObserver* send_side_delay_observer, + RtcEventLog* event_log, + SendPacketObserver* send_packet_observer, + RateLimiter* retransmission_rate_limiter, + OverheadObserver* overhead_observer, + bool populate_network2_timestamp, + FrameEncryptorInterface* frame_encryptor, + bool require_frame_encryption, + bool extmap_allow_mixed, + const WebRtcKeyValueConfig& field_trials) + : clock_(clock), random_(clock_->TimeInMicroseconds()), - audio_configured_(config.audio), - flexfec_ssrc_(config.flexfec_sender - ? absl::make_optional(config.flexfec_sender->ssrc()) - : absl::nullopt), - paced_sender_(config.paced_sender), - transport_sequence_number_allocator_( - config.transport_sequence_number_allocator), - transport_feedback_observer_(config.transport_feedback_callback), - transport_(config.outgoing_transport), + audio_configured_(audio), + flexfec_ssrc_(flexfec_ssrc), + paced_sender_(paced_sender), + transport_sequence_number_allocator_(sequence_number_allocator), + transport_feedback_observer_(transport_feedback_observer), + transport_(transport), sending_media_(true), // Default to sending media. force_part_of_allocation_(false), max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. last_payload_type_(-1), - rtp_header_extension_map_(config.extmap_allow_mixed), - packet_history_(clock_), - flexfec_packet_history_(clock_), + rtp_header_extension_map_(extmap_allow_mixed), + packet_history_(clock), + flexfec_packet_history_(clock), // Statistics send_delays_(), max_delay_it_(send_delays_.end()), @@ -168,13 +168,12 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) total_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale), nack_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale), - send_side_delay_observer_(config.send_side_delay_observer), - event_log_(config.event_log), - send_packet_observer_(config.send_packet_observer), - bitrate_callback_(config.send_bitrate_observer), + send_side_delay_observer_(send_side_delay_observer), + event_log_(event_log), + send_packet_observer_(send_packet_observer), + bitrate_callback_(bitrate_callback), // RTP variables sequence_number_forced_(false), - ssrc_(config.media_send_ssrc), last_rtp_timestamp_(0), capture_time_ms_(0), last_timestamp_time_ms_(0), @@ -182,19 +181,19 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) last_packet_marker_bit_(false), csrcs_(), rtx_(kRtxOff), - ssrc_rtx_(config.rtx_send_ssrc), rtp_overhead_bytes_per_packet_(0), - retransmission_rate_limiter_(config.retransmission_rate_limiter), - overhead_observer_(config.overhead_observer), - populate_network2_timestamp_(config.populate_network2_timestamp), + retransmission_rate_limiter_(retransmission_rate_limiter), + overhead_observer_(overhead_observer), + populate_network2_timestamp_(populate_network2_timestamp), send_side_bwe_with_overhead_( - IsEnabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)), + field_trials.Lookup("WebRTC-SendSideBwe-WithOverhead") + .find("Enabled") == 0), legacy_packet_history_storage_mode_( - IsEnabled("WebRTC-UseRtpPacketHistoryLegacyStorageMode", - config.field_trials)), + field_trials.Lookup("WebRTC-UseRtpPacketHistoryLegacyStorageMode") + .find("Enabled") == 0), payload_padding_prefer_useful_packets_( - !IsDisabled("WebRTC-PayloadPadding-UseMostUsefulPacket", - config.field_trials)) { + field_trials.Lookup("WebRTC-PayloadPadding-UseMostUsefulPacket") + .find("Disabled") != 0) { // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); // Random start, 16 bits. Can't be 0. diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 39c0e25996..c931a15c9a 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -25,7 +25,6 @@ #include "modules/rtp_rtcp/include/flexfec_sender.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_packet_pacer.h" -#include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtp_packet_history.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" @@ -46,7 +45,24 @@ class RtpPacketToSend; class RTPSender { public: - explicit RTPSender(const RtpRtcp::Configuration& config); + RTPSender(bool audio, + Clock* clock, + Transport* transport, + RtpPacketPacer* paced_sender, + absl::optional flexfec_ssrc, + TransportSequenceNumberAllocator* sequence_number_allocator, + TransportFeedbackObserver* transport_feedback_callback, + BitrateStatisticsObserver* bitrate_callback, + SendSideDelayObserver* send_side_delay_observer, + RtcEventLog* event_log, + SendPacketObserver* send_packet_observer, + RateLimiter* nack_rate_limiter, + OverheadObserver* overhead_observer, + bool populate_network2_timestamp, + FrameEncryptorInterface* frame_encryptor, + bool require_frame_encryption, + bool extmap_allow_mixed, + const WebRtcKeyValueConfig& field_trials); ~RTPSender(); @@ -67,7 +83,6 @@ class RTPSender { uint32_t TimestampOffset() const; void SetTimestampOffset(uint32_t timestamp); - // TODO(bugs.webrtc.org/10774): Remove. void SetSSRC(uint32_t ssrc); void SetRid(const std::string& rid); @@ -114,9 +129,8 @@ class RTPSender { // RTX. void SetRtxStatus(int mode); int RtxStatus() const; - uint32_t RtxSsrc() const; - // TODO(bugs.webrtc.org/10774): Remove. + uint32_t RtxSsrc() const; void SetRtxSsrc(uint32_t ssrc); void SetRtxPayloadType(int payload_type, int associated_payload_type); diff --git a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc index 22289946bf..d698bd7c09 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc @@ -64,15 +64,26 @@ class RtpSenderAudioTest : public ::testing::Test { public: RtpSenderAudioTest() : fake_clock_(kStartTime), - rtp_sender_([&] { - RtpRtcp::Configuration config; - config.audio = true; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - return config; - }()), + rtp_sender_(true, + &fake_clock_, + &transport_, + nullptr, + absl::nullopt, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + false, + nullptr, + false, + false, + FieldTrialBasedConfig()), rtp_sender_audio_(&fake_clock_, &rtp_sender_) { + rtp_sender_.SetSSRC(kSsrc); rtp_sender_.SetSequenceNumber(kSeqNum); } diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index dad0d74102..fd60b73aa3 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -195,14 +195,6 @@ class RtpSenderTest : public ::testing::TestWithParam { mock_rtc_event_log_(), mock_paced_sender_(), retransmission_rate_limiter_(&fake_clock_, 1000), - flexfec_sender_(0, - kFlexFecSsrc, - kSsrc, - "", - std::vector(), - std::vector(), - nullptr, - &fake_clock_), rtp_sender_(), transport_(), kMarkerBit(true), @@ -212,21 +204,16 @@ class RtpSenderTest : public ::testing::TestWithParam { void SetUp() override { SetUpRtpSender(true, false); } void SetUpRtpSender(bool pacer, bool populate_network2) { - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.rtx_send_ssrc = kRtxSsrc; - config.flexfec_sender = &flexfec_sender_; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.paced_sender = pacer ? &mock_paced_sender_ : nullptr; - config.populate_network2_timestamp = populate_network2; - rtp_sender_.reset(new RTPSender(config)); + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, pacer ? &mock_paced_sender_ : nullptr, + kFlexFecSsrc, &seq_num_allocator_, nullptr, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, populate_network2, nullptr, + false, false, FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetTimestampOffset(0); + rtp_sender_->SetSSRC(kSsrc); + rtp_sender_->SetRtxSsrc(kRtxSsrc); } SimulatedClock fake_clock_; @@ -236,7 +223,6 @@ class RtpSenderTest : public ::testing::TestWithParam { StrictMock send_packet_observer_; StrictMock feedback_observer_; RateLimiter retransmission_rate_limiter_; - FlexfecSender flexfec_sender_; std::unique_ptr rtp_sender_; LoopbackTransportTest transport_; const bool kMarkerBit; @@ -359,17 +345,14 @@ TEST_P(RtpSenderTestWithoutPacer, AssignSequenceNumberMayAllowPaddingOnVideo) { TEST_P(RtpSenderTest, AssignSequenceNumberAllowsPaddingOnAudio) { MockTransport transport; - RtpRtcp::Configuration config; - config.audio = true; - config.clock = &fake_clock_; - config.outgoing_transport = &transport; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.event_log = &mock_rtc_event_log_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + const bool kEnableAudio = true; + rtp_sender_.reset(new RTPSender( + kEnableAudio, &fake_clock_, &transport, &mock_paced_sender_, + absl::nullopt, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, + nullptr, &retransmission_rate_limiter_, nullptr, false, nullptr, false, + false, FieldTrialBasedConfig())); rtp_sender_->SetTimestampOffset(0); + rtp_sender_->SetSSRC(kSsrc); std::unique_ptr audio_packet = rtp_sender_->AllocatePacket(); // Padding on audio stream allowed regardless of marker in the last packet. @@ -410,18 +393,13 @@ TEST_P(RtpSenderTestWithoutPacer, TransportFeedbackObserverGetsCorrectByteCount) { constexpr int kRtpOverheadBytesPerPacket = 12 + 8; NiceMock mock_overhead_observer; - - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.transport_feedback_callback = &feedback_observer_; - config.event_log = &mock_rtc_event_log_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.overhead_observer = &mock_overhead_observer; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, nullptr, + &retransmission_rate_limiter_, &mock_overhead_observer, + false, nullptr, false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -449,17 +427,13 @@ TEST_P(RtpSenderTestWithoutPacer, } TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.transport_feedback_callback = &feedback_observer_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, + false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -491,16 +465,13 @@ TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { } TEST_P(RtpSenderTestWithoutPacer, PacketOptionsNoRetransmission) { - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.transport_feedback_callback = &feedback_observer_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, + false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); SendGenericPacket(); @@ -550,15 +521,12 @@ TEST_P(RtpSenderTestWithoutPacer, DoesnSetIncludedInAllocationByDefault) { TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { StrictMock send_side_delay_observer_; - - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.send_side_delay_observer = &send_side_delay_observer_; - config.event_log = &mock_rtc_event_log_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + nullptr, nullptr, nullptr, &send_side_delay_observer_, + &mock_rtc_event_log_, nullptr, nullptr, nullptr, false, + nullptr, false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, &playout_delay_oracle, nullptr, false, false, @@ -640,19 +608,14 @@ TEST_P(RtpSenderTestWithoutPacer, OnSendPacketUpdated) { } TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.transport_feedback_callback = &feedback_observer_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, &mock_paced_sender_, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetStorePacketsStatus(true, 10); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, @@ -1041,16 +1004,13 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { } TEST_P(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, &mock_paced_sender_, absl::nullopt, + nullptr /* TransportSequenceNumberAllocator */, nullptr, nullptr, nullptr, + nullptr, &send_packet_observer_, &retransmission_rate_limiter_, nullptr, + false, nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -1075,17 +1035,13 @@ TEST_P(RtpSenderTest, SendRedundantPayloads) { test::ScopedFieldTrials field_trials( "WebRTC-PayloadPadding-UseMostUsefulPacket/Disabled/"); MockTransport transport; - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.rtx_send_ssrc = kRtxSsrc; - config.event_log = &mock_rtc_event_log_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport, &mock_paced_sender_, absl::nullopt, + nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); uint16_t seq_num = kSeqNum; @@ -1098,6 +1054,7 @@ TEST_P(RtpSenderTest, SendRedundantPayloads) { rtp_header_len += 4; // 4 extra bytes common to all extension headers. rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); + rtp_sender_->SetRtxSsrc(1234); const size_t kNumPayloadSizes = 10; const size_t kPayloadSizes[kNumPayloadSizes] = {500, 550, 600, 650, 700, @@ -1156,17 +1113,13 @@ TEST_P(RtpSenderTest, SendRedundantPayloadsUsefulPadding) { test::ScopedFieldTrials field_trials( "WebRTC-PayloadPadding-UseMostUsefulPacket/Enabled/"); MockTransport transport; - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.rtx_send_ssrc = kRtxSsrc; - config.event_log = &mock_rtc_event_log_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_ = absl::make_unique( + false, &fake_clock_, &transport, &mock_paced_sender_, absl::nullopt, + nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig()); rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); uint16_t seq_num = kSeqNum; @@ -1179,6 +1132,7 @@ TEST_P(RtpSenderTest, SendRedundantPayloadsUsefulPadding) { rtp_header_len += 4; // 4 extra bytes common to all extension headers. rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); + rtp_sender_->SetRtxSsrc(1234); const size_t kNumPayloadSizes = 10; const size_t kPayloadSizes[kNumPayloadSizes] = {500, 550, 600, 650, 700, @@ -1305,25 +1259,21 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { constexpr uint32_t kTimestamp = 1234; constexpr int kMediaPayloadType = 127; constexpr int kFlexfecPayloadType = 118; + constexpr uint32_t kMediaSsrc = 1234; + constexpr uint32_t kFlexfecSsrc = 5678; const std::vector kNoRtpExtensions; const std::vector kNoRtpExtensionSizes; - FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexFecSsrc, kSsrc, kNoMid, - kNoRtpExtensions, kNoRtpExtensionSizes, + FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, + kNoMid, kNoRtpExtensions, kNoRtpExtensionSizes, nullptr /* rtp_state */, &fake_clock_); // Reset |rtp_sender_| to use FlexFEC. - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.flexfec_sender = &flexfec_sender_; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, &mock_paced_sender_, kFlexfecSsrc, + &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, + &send_packet_observer_, &retransmission_rate_limiter_, nullptr, false, + nullptr, false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetStorePacketsStatus(true, 10); @@ -1341,11 +1291,12 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { params.fec_mask_type = kFecMaskRandom; rtp_sender_video.SetFecParameters(params, params); - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, - kSsrc, kSeqNum, _, _, false)); + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kLowPriority, kMediaSsrc, kSeqNum, + _, _, false)); uint16_t flexfec_seq_num; EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, - kFlexFecSsrc, _, _, _, false)) + kFlexfecSsrc, _, _, _, false)) .WillOnce(SaveArg<2>(&flexfec_seq_num)); RTPVideoHeader video_header; @@ -1358,47 +1309,43 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) .Times(2); EXPECT_EQ(RtpPacketSendResult::kSuccess, - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + rtp_sender_->TimeToSendPacket(kMediaSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), false, PacedPacketInfo())); EXPECT_EQ(RtpPacketSendResult::kSuccess, - rtp_sender_->TimeToSendPacket(kFlexFecSsrc, flexfec_seq_num, + rtp_sender_->TimeToSendPacket(kFlexfecSsrc, flexfec_seq_num, fake_clock_.TimeInMilliseconds(), false, PacedPacketInfo())); ASSERT_EQ(2, transport_.packets_sent()); const RtpPacketReceived& media_packet = transport_.sent_packets_[0]; EXPECT_EQ(kMediaPayloadType, media_packet.PayloadType()); EXPECT_EQ(kSeqNum, media_packet.SequenceNumber()); - EXPECT_EQ(kSsrc, media_packet.Ssrc()); + EXPECT_EQ(kMediaSsrc, media_packet.Ssrc()); const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[1]; EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); EXPECT_EQ(flexfec_seq_num, flexfec_packet.SequenceNumber()); - EXPECT_EQ(kFlexFecSsrc, flexfec_packet.Ssrc()); + EXPECT_EQ(kFlexfecSsrc, flexfec_packet.Ssrc()); } TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { constexpr uint32_t kTimestamp = 1234; constexpr int kMediaPayloadType = 127; constexpr int kFlexfecPayloadType = 118; + constexpr uint32_t kMediaSsrc = 1234; constexpr uint32_t kFlexfecSsrc = 5678; const std::vector kNoRtpExtensions; const std::vector kNoRtpExtensionSizes; - FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kSsrc, kNoMid, - kNoRtpExtensions, kNoRtpExtensionSizes, + FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, + kNoMid, kNoRtpExtensions, kNoRtpExtensionSizes, nullptr /* rtp_state */, &fake_clock_); // Reset |rtp_sender_| to use FlexFEC. - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.flexfec_sender = &flexfec_sender; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, nullptr, flexfec_sender.ssrc(), + &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, + &send_packet_observer_, &retransmission_rate_limiter_, nullptr, false, + nullptr, false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); PlayoutDelayOracle playout_delay_oracle; @@ -1427,7 +1374,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { ASSERT_EQ(2, transport_.packets_sent()); const RtpPacketReceived& media_packet = transport_.sent_packets_[0]; EXPECT_EQ(kMediaPayloadType, media_packet.PayloadType()); - EXPECT_EQ(kSsrc, media_packet.Ssrc()); + EXPECT_EQ(kMediaSsrc, media_packet.Ssrc()); const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[1]; EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); EXPECT_EQ(kFlexfecSsrc, flexfec_packet.Ssrc()); @@ -1487,6 +1434,7 @@ TEST_P(RtpSenderTestWithoutPacer, RidIncludedOnRtxSentPackets) { rtp_sender_->SetSendingMediaStatus(true); rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); + rtp_sender_->SetRtxSsrc(kRtxSsrc); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); rtp_sender_->SetStorePacketsStatus(true, 10); @@ -1513,25 +1461,22 @@ TEST_P(RtpSenderTest, FecOverheadRate) { constexpr uint32_t kTimestamp = 1234; constexpr int kMediaPayloadType = 127; constexpr int kFlexfecPayloadType = 118; + constexpr uint32_t kMediaSsrc = 1234; + constexpr uint32_t kFlexfecSsrc = 5678; const std::vector kNoRtpExtensions; const std::vector kNoRtpExtensionSizes; - FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexFecSsrc, kSsrc, kNoMid, - kNoRtpExtensions, kNoRtpExtensionSizes, + FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, + kNoMid, kNoRtpExtensions, kNoRtpExtensionSizes, nullptr /* rtp_state */, &fake_clock_); // Reset |rtp_sender_| to use FlexFEC. - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.paced_sender = &mock_paced_sender_; - config.media_send_ssrc = kSsrc; - config.flexfec_sender = &flexfec_sender; - config.transport_sequence_number_allocator = &seq_num_allocator_; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, &mock_paced_sender_, + flexfec_sender.ssrc(), &seq_num_allocator_, nullptr, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); PlayoutDelayOracle playout_delay_oracle; @@ -1598,14 +1543,12 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { uint32_t total_bitrate_; uint32_t retransmit_bitrate_; } callback; - - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.send_bitrate_observer = &callback; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + nullptr, nullptr, &callback, nullptr, nullptr, nullptr, + &retransmission_rate_limiter_, nullptr, false, nullptr, + false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, @@ -1769,6 +1712,8 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { TEST_P(RtpSenderTestWithoutPacer, BytesReportedCorrectly) { // XXX const char* kPayloadName = "GENERIC"; const uint8_t kPayloadType = 127; + rtp_sender_->SetSSRC(1234); + rtp_sender_->SetRtxSsrc(kRtxSsrc); rtp_sender_->SetRtxPayloadType(kPayloadType - 1, kPayloadType); rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); @@ -1838,13 +1783,12 @@ TEST_P(RtpSenderTestWithoutPacer, RespectsNackBitrateLimit) { TEST_P(RtpSenderTest, OnOverheadChanged) { MockOverheadObserver mock_overhead_observer; - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.overhead_observer = &mock_overhead_observer; - rtp_sender_ = absl::make_unique(config); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, + &retransmission_rate_limiter_, &mock_overhead_observer, + false, nullptr, false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); // RTP overhead is 12B. EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(12)).Times(1); @@ -1861,13 +1805,12 @@ TEST_P(RtpSenderTest, OnOverheadChanged) { TEST_P(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) { MockOverheadObserver mock_overhead_observer; - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.overhead_observer = &mock_overhead_observer; - rtp_sender_ = absl::make_unique(config); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, + &retransmission_rate_limiter_, &mock_overhead_observer, + false, nullptr, false, false, FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(_)).Times(1); SendGenericPacket(); @@ -2083,17 +2026,13 @@ TEST_P(RtpSenderTest, TrySendPacketUpdatesStats) { const size_t kPayloadSize = 1000; StrictMock send_side_delay_observer; - - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.media_send_ssrc = kSsrc; - config.rtx_send_ssrc = kRtxSsrc; - config.flexfec_sender = &flexfec_sender_; - config.send_side_delay_observer = &send_side_delay_observer; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - rtp_sender_ = absl::make_unique(config); + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, nullptr, kFlexFecSsrc, nullptr, nullptr, + nullptr, &send_side_delay_observer, &mock_rtc_event_log_, + &send_packet_observer_, nullptr, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); + rtp_sender_->SetRtxSsrc(kRtxSsrc); ASSERT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 2589ad2715..e6468aaeb1 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -140,18 +140,29 @@ class RtpSenderVideoTest : public ::testing::TestWithParam { : field_trials_(GetParam()), fake_clock_(kStartTime), retransmission_rate_limiter_(&fake_clock_, 1000), - rtp_sender_([&] { - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.field_trials = &field_trials_; - config.media_send_ssrc = kSsrc; - return config; - }()), + // TODO(pbos): Set up to use pacer. + rtp_sender_(false, + &fake_clock_, + &transport_, + nullptr, + absl::nullopt, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + &retransmission_rate_limiter_, + nullptr, + false, + nullptr, + false, + false, + field_trials_), rtp_sender_video_(&fake_clock_, &rtp_sender_, nullptr, field_trials_) { rtp_sender_.SetSequenceNumber(kSeqNum); rtp_sender_.SetTimestampOffset(0); + rtp_sender_.SetSSRC(kSsrc); rtp_sender_video_.RegisterPayloadType(kPayload, "generic", /*raw_payload=*/false);