From 94c58fd815f0c7c6429aa53a79621ea9ef39c770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Sat, 6 Jul 2019 12:42:27 +0200 Subject: [PATCH] Add ability to set RTCP sender ssrc at construction time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10774 Change-Id: Iaf5857e24359e9795434bcd0cdbe1658a2f9f5d3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144632 Reviewed-by: Åsa Persson Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#28506} --- modules/rtp_rtcp/source/rtcp_sender.cc | 32 ++++++------ modules/rtp_rtcp/source/rtcp_sender.h | 9 +--- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 50 ++++++++++++------- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 11 +--- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 1 + video/video_send_stream_tests.cc | 25 +++++++--- 6 files changed, 69 insertions(+), 59 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index af5cd270cd..c79cde835f 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -48,6 +48,8 @@ namespace { const uint32_t kRtcpAnyExtendedReports = kRtcpXrReceiverReferenceTime | kRtcpXrDlrrReportBlock | kRtcpXrTargetBitrate; +constexpr int32_t kDefaultVideoReportInterval = 1000; +constexpr int32_t kDefaultAudioReportInterval = 5000; } // namespace RTCPSender::FeedbackState::FeedbackState() @@ -112,29 +114,25 @@ class RTCPSender::RtcpContext { const int64_t now_us_; }; -RTCPSender::RTCPSender( - bool audio, - Clock* clock, - ReceiveStatisticsProvider* receive_statistics, - RtcpPacketTypeCounterObserver* packet_type_counter_observer, - RtcEventLog* event_log, - Transport* outgoing_transport, - int report_interval_ms) - : audio_(audio), - clock_(clock), +RTCPSender::RTCPSender(const RtpRtcp::Configuration& config) + : audio_(config.audio), + clock_(config.clock), random_(clock_->TimeInMicroseconds()), method_(RtcpMode::kOff), - event_log_(event_log), - transport_(outgoing_transport), - report_interval_ms_(report_interval_ms), + event_log_(config.event_log), + transport_(config.outgoing_transport), + report_interval_ms_(config.rtcp_report_interval_ms > 0 + ? config.rtcp_report_interval_ms + : (config.audio ? kDefaultAudioReportInterval + : kDefaultVideoReportInterval)), sending_(false), next_time_to_send_rtcp_(0), timestamp_offset_(0), last_rtp_timestamp_(0), last_frame_capture_time_ms_(-1), - ssrc_(0), + ssrc_(config.media_send_ssrc.value_or(0)), remote_ssrc_(0), - receive_statistics_(receive_statistics), + receive_statistics_(config.receive_statistics), sequence_number_fir_(0), @@ -150,7 +148,7 @@ RTCPSender::RTCPSender( app_length_(0), xr_send_receiver_reference_time_enabled_(false), - packet_type_counter_observer_(packet_type_counter_observer), + packet_type_counter_observer_(config.rtcp_packet_type_counter_observer), send_video_bitrate_allocation_(false), last_payload_type_(-1) { RTC_DCHECK(transport_ != nullptr); @@ -307,7 +305,7 @@ uint32_t RTCPSender::SSRC() const { void RTCPSender::SetSSRC(uint32_t ssrc) { rtc::CritScope lock(&critical_section_rtcp_sender_); - if (ssrc_ != 0) { + if (ssrc != ssrc_) { // not first SetSSRC, probably due to a collision // schedule a new RTCP report // make sure that we send a RTP packet diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index 74f4cc17a6..628121ea4e 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -23,6 +23,7 @@ #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "modules/rtp_rtcp/include/receive_statistics.h" +#include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_nack_stats.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" @@ -62,13 +63,7 @@ class RTCPSender { ModuleRtpRtcpImpl* module; }; - RTCPSender(bool audio, - Clock* clock, - ReceiveStatisticsProvider* receive_statistics, - RtcpPacketTypeCounterObserver* packet_type_counter_observer, - RtcEventLog* event_log, - Transport* outgoing_transport, - int report_interval_ms); + explicit RTCPSender(const RtpRtcp::Configuration& config); virtual ~RTCPSender(); RtcpMode Status() const; diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 01101c0e4b..747e5af6ed 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -81,12 +81,10 @@ class RtcpSenderTest : public ::testing::Test { configuration.outgoing_transport = &test_transport_; configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; configuration.rtcp_report_interval_ms = 1000; - + configuration.receive_statistics = receive_statistics_.get(); + configuration.media_send_ssrc = kSenderSsrc; rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl(configuration)); - rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), - nullptr, nullptr, &test_transport_, - configuration.rtcp_report_interval_ms)); - rtcp_sender_->SetSSRC(kSenderSsrc); + rtcp_sender_.reset(new RTCPSender(configuration)); rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp); rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(), @@ -187,9 +185,13 @@ TEST_F(RtcpSenderTest, SendConsecutiveSrWithExactSlope) { } TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) { - rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), - nullptr, nullptr, &test_transport_, 1000)); - rtcp_sender_->SetSSRC(kSenderSsrc); + RtpRtcp::Configuration config; + config.clock = &clock_; + config.receive_statistics = receive_statistics_.get(); + config.outgoing_transport = &test_transport_; + config.rtcp_report_interval_ms = 1000; + config.media_send_ssrc = kSenderSsrc; + rtcp_sender_.reset(new RTCPSender(config)); rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender_->SetSendingStatus(feedback_state(), true); @@ -205,9 +207,13 @@ TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) { } TEST_F(RtcpSenderTest, DoNotSendCompundBeforeRtp) { - rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), - nullptr, nullptr, &test_transport_, 1000)); - rtcp_sender_->SetSSRC(kSenderSsrc); + RtpRtcp::Configuration config; + config.clock = &clock_; + config.receive_statistics = receive_statistics_.get(); + config.outgoing_transport = &test_transport_; + config.rtcp_report_interval_ms = 1000; + config.media_send_ssrc = kSenderSsrc; + rtcp_sender_.reset(new RTCPSender(config)); rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); rtcp_sender_->SetSendingStatus(feedback_state(), true); @@ -551,9 +557,14 @@ TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfNotEnabled) { TEST_F(RtcpSenderTest, TestRegisterRtcpPacketTypeObserver) { RtcpPacketTypeCounterObserverImpl observer; - rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), - &observer, nullptr, &test_transport_, - 1000)); + RtpRtcp::Configuration config; + config.clock = &clock_; + config.receive_statistics = receive_statistics_.get(); + config.outgoing_transport = &test_transport_; + config.rtcp_packet_type_counter_observer = &observer; + config.rtcp_report_interval_ms = 1000; + rtcp_sender_.reset(new RTCPSender(config)); + rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpPli)); @@ -674,9 +685,14 @@ TEST_F(RtcpSenderTest, ByeMustBeLast) { })); // Re-configure rtcp_sender_ with mock_transport_ - rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), - nullptr, nullptr, &mock_transport, 1000)); - rtcp_sender_->SetSSRC(kSenderSsrc); + RtpRtcp::Configuration config; + config.clock = &clock_; + config.receive_statistics = receive_statistics_.get(); + config.outgoing_transport = &mock_transport; + config.rtcp_report_interval_ms = 1000; + config.media_send_ssrc = kSenderSsrc; + rtcp_sender_.reset(new RTCPSender(config)); + rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp); rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(), diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index aa50227b14..765f76f70f 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -61,16 +61,7 @@ RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) { } ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) - : rtcp_sender_(configuration.audio, - configuration.clock, - configuration.receive_statistics, - configuration.rtcp_packet_type_counter_observer, - configuration.event_log, - configuration.outgoing_transport, - configuration.rtcp_report_interval_ms > 0 - ? configuration.rtcp_report_interval_ms - : (configuration.audio ? kDefaultAudioReportInterval - : kDefaultVideoReportInterval)), + : rtcp_sender_(configuration), rtcp_receiver_(configuration.clock, configuration.receiver_only, configuration.rtcp_packet_type_counter_observer, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 31f9b24965..a0690c1778 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -162,6 +162,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { config.rtcp_packet_type_counter_observer = this; config.rtt_stats = &rtt_stats_; config.rtcp_report_interval_ms = rtcp_report_interval_ms_; + config.media_send_ssrc = kSenderSsrc; impl_.reset(new ModuleRtpRtcpImpl(config)); impl_->SetRTCPStatus(RtcpMode::kCompound); diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 96da991077..75423e5f56 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -914,9 +914,11 @@ void VideoSendStreamTest::TestNackRetransmission( non_padding_sequence_numbers_.end() - kNackedPacketsAtOnceCount, non_padding_sequence_numbers_.end()); - RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), nullptr, - nullptr, nullptr, transport_adapter_.get(), - kRtcpIntervalMs); + RtpRtcp::Configuration config; + config.clock = Clock::GetRealTimeClock(); + config.outgoing_transport = transport_adapter_.get(); + config.rtcp_report_interval_ms = kRtcpIntervalMs; + RTCPSender rtcp_sender(config); rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]); @@ -1127,9 +1129,12 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format, kVideoSendSsrcs[0], header.sequenceNumber, packets_lost_, // Cumulative lost. loss_ratio); // Loss percent. - RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), - &lossy_receive_stats, nullptr, nullptr, - transport_adapter_.get(), kRtcpIntervalMs); + RtpRtcp::Configuration config; + config.clock = Clock::GetRealTimeClock(); + config.receive_statistics = &lossy_receive_stats; + config.outgoing_transport = transport_adapter_.get(); + config.rtcp_report_interval_ms = kRtcpIntervalMs; + RTCPSender rtcp_sender(config); rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]); @@ -1375,8 +1380,12 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) { FakeReceiveStatistics receive_stats(kVideoSendSsrcs[0], last_sequence_number_, rtp_count_, 0); - RTCPSender rtcp_sender(false, clock_, &receive_stats, nullptr, nullptr, - transport_adapter_.get(), kRtcpIntervalMs); + RtpRtcp::Configuration config; + config.clock = clock_; + config.receive_statistics = &receive_stats; + config.outgoing_transport = transport_adapter_.get(); + config.rtcp_report_interval_ms = kRtcpIntervalMs; + RTCPSender rtcp_sender(config); rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]);