From 0fc2843c108240a5eb1585e48f0c5c12ecf073f1 Mon Sep 17 00:00:00 2001 From: Amit Hilbuch Date: Tue, 18 Dec 2018 13:01:47 -0800 Subject: [PATCH] Removing redundant argument for SSRCs from ctor of RtpVideoSender. SSRCs are specified twice in calls to the RtpVideoSender constructor. Once in the first argument of ssrcs, and then again in the RtpConfig ssrcs variable. Resolving to reference the variable in the RtpConfig. Bug: None TBR: stefan@webrtc.org Change-Id: I53528140166a53f3558f950d5662b7d3d6b8c822 Reviewed-on: https://webrtc-review.googlesource.com/c/114910 Commit-Queue: Amit Hilbuch Reviewed-by: Fredrik Solenberg Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#26094} --- call/rtp_transport_controller_send.cc | 3 +-- call/rtp_transport_controller_send.h | 1 - call/rtp_transport_controller_send_interface.h | 1 - call/rtp_video_sender.cc | 16 +++++----------- call/rtp_video_sender.h | 1 - call/rtp_video_sender_unittest.cc | 4 ++-- call/test/mock_rtp_transport_controller_send.h | 5 ++--- video/video_send_stream_impl.cc | 1 - video/video_send_stream_impl_unittest.cc | 2 +- 9 files changed, 11 insertions(+), 23 deletions(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 47477a9c72..f667ca366b 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -152,7 +152,6 @@ RtpTransportControllerSend::~RtpTransportControllerSend() { } RtpVideoSenderInterface* RtpTransportControllerSend::CreateRtpVideoSender( - const std::vector& ssrcs, std::map suspended_ssrcs, const std::map& states, const RtpConfig& rtp_config, @@ -163,7 +162,7 @@ RtpVideoSenderInterface* RtpTransportControllerSend::CreateRtpVideoSender( std::unique_ptr fec_controller, const RtpSenderFrameEncryptionConfig& frame_encryption_config) { video_rtp_senders_.push_back(absl::make_unique( - ssrcs, suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms, + suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms, send_transport, observers, // TODO(holmer): Remove this circular dependency by injecting // the parts of RtpTransportControllerSendInterface that are really used. diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 45836a09e2..d644e740f7 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -52,7 +52,6 @@ class RtpTransportControllerSend final ~RtpTransportControllerSend() override; RtpVideoSenderInterface* CreateRtpVideoSender( - const std::vector& ssrcs, std::map suspended_ssrcs, const std::map& states, // move states into RtpTransportControllerSend diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 9868585cdf..19c1dadd68 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -100,7 +100,6 @@ class RtpTransportControllerSendInterface { virtual PacketRouter* packet_router() = 0; virtual RtpVideoSenderInterface* CreateRtpVideoSender( - const std::vector& ssrcs, std::map suspended_ssrcs, // TODO(holmer): Move states into RtpTransportControllerSend. const std::map& states, diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 9dcc0f6426..1bb2439e96 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -39,7 +39,6 @@ static const int kSendSideSeqNumSetMaxSize = 5500; static const size_t kPathMTU = 1500; std::vector> CreateRtpRtcpModules( - const std::vector& ssrcs, const RtpConfig& rtp_config, int rtcp_report_interval_ms, Transport* send_transport, @@ -59,7 +58,7 @@ std::vector> CreateRtpRtcpModules( RtpKeepAliveConfig keepalive_config, FrameEncryptorInterface* frame_encryptor, const CryptoOptions& crypto_options) { - RTC_DCHECK_GT(ssrcs.size(), 0); + RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0); RtpRtcp::Configuration configuration; configuration.audio = false; @@ -91,7 +90,7 @@ std::vector> CreateRtpRtcpModules( std::vector> modules; const std::vector& flexfec_protected_ssrcs = rtp_config.flexfec.protected_media_ssrcs; - for (uint32_t ssrc : ssrcs) { + for (uint32_t ssrc : rtp_config.ssrcs) { bool enable_flexfec = flexfec_sender != nullptr && std::find(flexfec_protected_ssrcs.begin(), flexfec_protected_ssrcs.end(), @@ -179,7 +178,6 @@ int CalculatePacketRate(uint32_t bitrate_bps, size_t packet_size_bytes) { } // namespace RtpVideoSender::RtpVideoSender( - const std::vector& ssrcs, std::map suspended_ssrcs, const std::map& states, const RtpConfig& rtp_config, @@ -199,8 +197,7 @@ RtpVideoSender::RtpVideoSender( suspended_ssrcs_(std::move(suspended_ssrcs)), flexfec_sender_(MaybeCreateFlexfecSender(rtp_config, suspended_ssrcs_)), fec_controller_(std::move(fec_controller)), - rtp_modules_(CreateRtpRtcpModules(ssrcs, - rtp_config, + rtp_modules_(CreateRtpRtcpModules(rtp_config, rtcp_report_interval_ms, send_transport, observers.intra_frame_callback, @@ -224,13 +221,10 @@ RtpVideoSender::RtpVideoSender( transport_overhead_bytes_per_packet_(0), overhead_bytes_per_packet_(0), encoder_target_rate_bps_(0) { - RTC_DCHECK_EQ(ssrcs.size(), rtp_modules_.size()); - // The same argument for SSRCs is given to this method twice. - // The SSRCs are also accessed in this method through both variables. - RTC_DCHECK(ssrcs == rtp_config.ssrcs); + RTC_DCHECK_EQ(rtp_config.ssrcs.size(), rtp_modules_.size()); module_process_thread_checker_.DetachFromThread(); // SSRCs are assumed to be sorted in the same order as |rtp_modules|. - for (uint32_t ssrc : ssrcs) { + for (uint32_t ssrc : rtp_config.ssrcs) { // Restore state if it previously existed. const RtpPayloadState* state = nullptr; auto it = states.find(ssrc); diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index aff2067396..dac43b1918 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -49,7 +49,6 @@ class RtpVideoSender : public RtpVideoSenderInterface, public: // Rtp modules are assumed to be sorted in simulcast index order. RtpVideoSender( - const std::vector& ssrcs, std::map suspended_ssrcs, const std::map& states, const RtpConfig& rtp_config, diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index d77fe58715..958228d441 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -92,8 +92,8 @@ class RtpVideoSenderTestFixture { config_.rtp.payload_type = payload_type; std::map suspended_ssrcs; router_ = absl::make_unique( - config_.rtp.ssrcs, suspended_ssrcs, suspended_payload_states, - config_.rtp, config_.rtcp_report_interval_ms, &transport_, + suspended_ssrcs, suspended_payload_states, config_.rtp, + config_.rtcp_report_interval_ms, &transport_, CreateObservers(&call_stats_, &encoder_feedback_, &stats_proxy_, &stats_proxy_, &stats_proxy_, &stats_proxy_, &stats_proxy_, &stats_proxy_, &send_delay_stats_), diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 4d34d06e05..c2360417ee 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -32,10 +32,9 @@ namespace webrtc { class MockRtpTransportControllerSend : public RtpTransportControllerSendInterface { public: - MOCK_METHOD10( + MOCK_METHOD9( CreateRtpVideoSender, - RtpVideoSenderInterface*(const std::vector&, - std::map, + RtpVideoSenderInterface*(std::map, const std::map&, const RtpConfig&, int rtcp_report_interval_ms, diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index c6d19d0e26..64ab6e15d8 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -268,7 +268,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( video_stream_encoder), bandwidth_observer_(transport->GetBandwidthObserver()), rtp_video_sender_(transport_->CreateRtpVideoSender( - config_->rtp.ssrcs, suspended_ssrcs, suspended_payload_states, config_->rtp, diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index 32d0df1c2e..2ee375af3b 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -99,7 +99,7 @@ class VideoSendStreamImplTest : public ::testing::Test { EXPECT_CALL(transport_controller_, packet_router()) .WillRepeatedly(Return(&packet_router_)); EXPECT_CALL(transport_controller_, - CreateRtpVideoSender(_, _, _, _, _, _, _, _, _, _)) + CreateRtpVideoSender(_, _, _, _, _, _, _, _, _)) .WillRepeatedly(Return(&rtp_video_sender_)); EXPECT_CALL(rtp_video_sender_, SetActive(_)) .WillRepeatedly(testing::Invoke(