Set RtcpSender transport at construction. BUG= Review URL: https://codereview.webrtc.org/1365043002 Cr-Commit-Position: refs/heads/master@{#10106}
diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 38c1c04..ee5fe13 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp
@@ -93,6 +93,7 @@ '<(webrtc_root)/test/test.gyp:frame_generator', '<(webrtc_root)/test/test.gyp:rtp_test_utils', '<(webrtc_root)/test/test.gyp:test_support_main', + '<(webrtc_root)/test/webrtc_test_common.gyp:webrtc_test_common', '<(webrtc_root)/tools/tools.gyp:agc_test_utils', ], 'sources': [
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc index 3f0b047..9652a53 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc
@@ -18,6 +18,7 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_sender.h" #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" +#include "webrtc/test/null_transport.h" #include "webrtc/typedefs.h" namespace { @@ -79,6 +80,7 @@ RTCPSender* rtcp_sender_; RTCPReceiver* rtcp_receiver_; TestTransport* test_transport_; + test::NullTransport null_transport_; MockRemoteBitrateObserver remote_bitrate_observer_; rtc::scoped_ptr<RemoteBitrateEstimator> remote_bitrate_estimator_; }; @@ -88,14 +90,13 @@ configuration.audio = false; configuration.clock = system_clock_; configuration.remote_bitrate_estimator = remote_bitrate_estimator_.get(); + configuration.outgoing_transport = &null_transport_; dummy_rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration); - rtcp_sender_ = - new RTCPSender(false, system_clock_, receive_statistics_.get(), nullptr); rtcp_receiver_ = new RTCPReceiver(system_clock_, false, nullptr, nullptr, nullptr, nullptr, dummy_rtp_rtcp_impl_); test_transport_ = new TestTransport(rtcp_receiver_); - - EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(test_transport_)); + rtcp_sender_ = new RTCPSender(false, system_clock_, receive_statistics_.get(), + nullptr, test_transport_); } void RtcpFormatRembTest::TearDown() {
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index 111757b..f250e29 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -135,13 +135,12 @@ bool audio, Clock* clock, ReceiveStatistics* receive_statistics, - RtcpPacketTypeCounterObserver* packet_type_counter_observer) + RtcpPacketTypeCounterObserver* packet_type_counter_observer, + Transport* outgoing_transport) : audio_(audio), clock_(clock), method_(kRtcpOff), - critical_section_transport_( - CriticalSectionWrapper::CreateCriticalSection()), - cbTransport_(nullptr), + transport_(outgoing_transport), critical_section_rtcp_sender_( CriticalSectionWrapper::CreateCriticalSection()), @@ -173,6 +172,7 @@ packet_type_counter_observer_(packet_type_counter_observer) { memset(last_send_report_, 0, sizeof(last_send_report_)); memset(last_rtcp_time_, 0, sizeof(last_rtcp_time_)); + RTC_DCHECK(transport_ != nullptr); builders_[kRtcpSr] = &RTCPSender::BuildSR; builders_[kRtcpRr] = &RTCPSender::BuildRR; @@ -196,12 +196,6 @@ RTCPSender::~RTCPSender() { } -int32_t RTCPSender::RegisterSendTransport(Transport* outgoingTransport) { - CriticalSectionScoped lock(critical_section_transport_.get()); - cbTransport_ = outgoingTransport; - return 0; -} - RTCPMethod RTCPSender::Status() const { CriticalSectionScoped lock(critical_section_rtcp_sender_.get()); return method_; @@ -1115,11 +1109,8 @@ } int32_t RTCPSender::SendToNetwork(const uint8_t* dataBuffer, size_t length) { - CriticalSectionScoped lock(critical_section_transport_.get()); - if (cbTransport_) { - if (cbTransport_->SendRtcp(dataBuffer, length)) - return 0; - } + if (transport_->SendRtcp(dataBuffer, length)) + return 0; return -1; } @@ -1210,10 +1201,6 @@ } bool RTCPSender::SendFeedbackPacket(const rtcp::TransportFeedback& packet) { - CriticalSectionScoped lock(critical_section_transport_.get()); - if (!cbTransport_) - return false; - class Sender : public rtcp::RtcpPacket::PacketReadyCallback { public: Sender(Transport* transport) @@ -1226,7 +1213,7 @@ Transport* const transport_; bool send_failure_; - } sender(cbTransport_); + } sender(transport_); uint8_t buffer[IP_PACKET_SIZE]; return packet.BuildExternalBuffer(buffer, IP_PACKET_SIZE, &sender) &&
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index 2051102..2f27989 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
@@ -74,11 +74,10 @@ RTCPSender(bool audio, Clock* clock, ReceiveStatistics* receive_statistics, - RtcpPacketTypeCounterObserver* packet_type_counter_observer); + RtcpPacketTypeCounterObserver* packet_type_counter_observer, + Transport* outgoing_transport); virtual ~RTCPSender(); - int32_t RegisterSendTransport(Transport* outgoingTransport); - RTCPMethod Status() const; void SetRTCPStatus(RTCPMethod method); @@ -228,8 +227,7 @@ Clock* const clock_; RTCPMethod method_ GUARDED_BY(critical_section_rtcp_sender_); - rtc::scoped_ptr<CriticalSectionWrapper> critical_section_transport_; - Transport* cbTransport_ GUARDED_BY(critical_section_transport_); + Transport* const transport_; rtc::scoped_ptr<CriticalSectionWrapper> critical_section_rtcp_sender_; bool using_nack_ GUARDED_BY(critical_section_rtcp_sender_);
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 33c3f84..19dbc4d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -230,11 +230,10 @@ configuration.outgoing_transport = &test_transport_; rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl(configuration)); - rtcp_sender_.reset( - new RTCPSender(false, &clock_, receive_statistics_.get(), nullptr)); + rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), + nullptr, &test_transport_)); rtcp_sender_->SetSSRC(kSenderSsrc); rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); - EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(&test_transport_)); } void InsertIncomingPacket(uint32_t remote_ssrc, uint16_t seq_num) { @@ -667,10 +666,9 @@ TEST_F(RtcpSenderTest, TestRegisterRtcpPacketTypeObserver) { RtcpPacketTypeCounterObserverImpl observer; - rtcp_sender_.reset( - new RTCPSender(false, &clock_, receive_statistics_.get(), &observer)); + rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), + &observer, &test_transport_)); rtcp_sender_->SetRemoteSSRC(kRemoteSsrc); - EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(&test_transport_)); rtcp_sender_->SetRTCPStatus(kRtcpNonCompound); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpPli)); EXPECT_EQ(1, parser()->pli()->num_packets());
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index ae4caf7..2e65452 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -72,7 +72,8 @@ rtcp_sender_(configuration.audio, configuration.clock, configuration.receive_statistics, - configuration.rtcp_packet_type_counter_observer), + configuration.rtcp_packet_type_counter_observer, + configuration.outgoing_transport), rtcp_receiver_(configuration.clock, configuration.receiver_only, configuration.rtcp_packet_type_counter_observer, @@ -99,9 +100,6 @@ rtt_ms_(0) { send_video_codec_.codecType = kVideoCodecUnknown; - // TODO(pwestin) move to constructors of each rtp/rtcp sender/receiver object. - rtcp_sender_.RegisterSendTransport(configuration.outgoing_transport); - // Make sure that RTCP objects are aware of our SSRC. uint32_t SSRC = rtp_sender_.SSRC(); rtcp_sender_.SetSSRC(SSRC);
diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index 1540e55..f75113a 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc
@@ -9,6 +9,7 @@ */ #include "webrtc/modules/rtp_rtcp/test/testAPI/test_api.h" +#include "webrtc/test/null_transport.h" #include <algorithm> #include <vector> @@ -90,6 +91,7 @@ RtpRtcp::Configuration configuration; configuration.audio = true; configuration.clock = &fake_clock_; + configuration.outgoing_transport = &null_transport_; module_.reset(RtpRtcp::CreateRtpRtcp(configuration)); rtp_payload_registry_.reset(new RTPPayloadRegistry( RTPPayloadStrategy::CreateStrategy(true))); @@ -105,6 +107,7 @@ uint16_t test_sequence_number_; std::vector<uint32_t> test_csrcs_; SimulatedClock fake_clock_; + test::NullTransport null_transport_; }; TEST_F(RtpRtcpAPITest, Basic) {
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 688278e..f4fabde 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc
@@ -329,8 +329,8 @@ FakeReceiveStatistics lossy_receive_stats( kSendSsrcs[0], header.sequenceNumber, send_count_ / 2, 127); RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), - &lossy_receive_stats, nullptr); - EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); + &lossy_receive_stats, nullptr, + &transport_adapter_); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); rtcp_sender.SetRemoteSSRC(kSendSsrcs[0]); @@ -411,8 +411,7 @@ nacked_sequence_number_ = nack_sequence_number; NullReceiveStatistics null_stats; RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), &null_stats, - nullptr); - EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); + nullptr, &transport_adapter_); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); rtcp_sender.SetRemoteSSRC(kSendSsrcs[0]); @@ -598,8 +597,8 @@ FakeReceiveStatistics lossy_receive_stats( kSendSsrcs[0], header.sequenceNumber, packet_count_ / 2, 127); RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), - &lossy_receive_stats, nullptr); - EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); + &lossy_receive_stats, nullptr, + &transport_adapter_); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); rtcp_sender.SetRemoteSSRC(kSendSsrcs[0]); @@ -828,8 +827,8 @@ EXCLUSIVE_LOCKS_REQUIRED(crit_) { FakeReceiveStatistics receive_stats( kSendSsrcs[0], last_sequence_number_, rtp_count_, 0); - RTCPSender rtcp_sender(false, clock_, &receive_stats, nullptr); - EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); + RTCPSender rtcp_sender(false, clock_, &receive_stats, nullptr, + &transport_adapter_); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); rtcp_sender.SetRemoteSSRC(kSendSsrcs[0]); @@ -888,8 +887,7 @@ // Receive statistics reporting having lost 50% of the packets. FakeReceiveStatistics receive_stats(kSendSsrcs[0], 1, 1, 0); RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), &receive_stats, - nullptr); - EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); + nullptr, &transport_adapter_); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); rtcp_sender.SetRemoteSSRC(kSendSsrcs[0]);