From 70efddeceda987c2e598bf8e0d0225d578f566d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 21 Aug 2019 13:36:20 +0200 Subject: [PATCH] Set local ssrc at construction of Rtp module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SetSSRC() method is slated for removal, make sure we set the local SSRC at construction time. Bug: webrtc:10774 Change-Id: I431e828caf60c5e0134adbe82d1d3345745cc6ae Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149827 Commit-Queue: Erik Språng Reviewed-by: Fredrik Solenberg Cr-Commit-Position: refs/heads/master@{#28926} --- audio/audio_receive_stream.cc | 12 +++++------- audio/audio_receive_stream_unittest.cc | 3 --- audio/audio_send_stream.cc | 8 ++------ audio/audio_send_stream_unittest.cc | 2 +- audio/channel_receive.cc | 15 +++++---------- audio/channel_receive.h | 4 +--- audio/channel_send.cc | 18 +++++------------- audio/channel_send.h | 1 - audio/mock_voe_channel_proxy.h | 2 -- media/engine/webrtc_voice_engine.cc | 6 ++++-- 10 files changed, 23 insertions(+), 48 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index c4abea0c4a..c093342c3f 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -78,8 +78,9 @@ std::unique_ptr CreateChannelReceive( return voe::CreateChannelReceive( clock, module_process_thread, internal_audio_state->audio_device_module(), config.media_transport_config, config.rtcp_send_transport, event_log, - config.rtp.remote_ssrc, config.jitter_buffer_max_packets, - config.jitter_buffer_fast_accelerate, config.jitter_buffer_min_delay_ms, + config.rtp.local_ssrc, config.rtp.remote_ssrc, + config.jitter_buffer_max_packets, config.jitter_buffer_fast_accelerate, + config.jitter_buffer_min_delay_ms, config.jitter_buffer_enable_rtx_handling, config.decoder_factory, config.codec_pair_id, config.frame_decryptor, config.crypto_options); } @@ -381,12 +382,9 @@ void AudioReceiveStream::ConfigureStream(AudioReceiveStream* stream, RTC_DCHECK(first_time || old_config.decoder_factory == new_config.decoder_factory); - if (first_time || old_config.rtp.local_ssrc != new_config.rtp.local_ssrc) { - channel_receive->SetLocalSSRC(new_config.rtp.local_ssrc); - } - if (!first_time) { - // Remote ssrc can't be changed mid-stream. + // SSRC can't be changed mid-stream. + RTC_DCHECK_EQ(old_config.rtp.local_ssrc, new_config.rtp.local_ssrc); RTC_DCHECK_EQ(old_config.rtp.remote_ssrc, new_config.rtp.remote_ssrc); } diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc index 12e779d12a..7e1da6deda 100644 --- a/audio/audio_receive_stream_unittest.cc +++ b/audio/audio_receive_stream_unittest.cc @@ -88,7 +88,6 @@ struct ConfigHelper { audio_state_ = AudioState::Create(config); channel_receive_ = new ::testing::StrictMock(); - EXPECT_CALL(*channel_receive_, SetLocalSSRC(kLocalSsrc)).Times(1); EXPECT_CALL(*channel_receive_, SetNACKStatus(true, 15)).Times(1); EXPECT_CALL(*channel_receive_, RegisterReceiverCongestionControlObjects(&packet_router_)) @@ -365,7 +364,6 @@ TEST(AudioReceiveStreamTest, ReconfigureWithUpdatedConfig) { auto recv_stream = helper.CreateAudioReceiveStream(); auto new_config = helper.config(); - new_config.rtp.local_ssrc = kLocalSsrc + 1; new_config.rtp.nack.rtp_history_ms = 300 + 20; new_config.rtp.extensions.clear(); new_config.rtp.extensions.push_back( @@ -376,7 +374,6 @@ TEST(AudioReceiveStreamTest, ReconfigureWithUpdatedConfig) { new_config.decoder_map.emplace(1, SdpAudioFormat("foo", 8000, 1)); MockChannelReceive& channel_receive = *helper.channel_receive(); - EXPECT_CALL(channel_receive, SetLocalSSRC(kLocalSsrc + 1)).Times(1); EXPECT_CALL(channel_receive, SetNACKStatus(true, 15 + 1)).Times(1); EXPECT_CALL(channel_receive, SetReceiveCodecs(new_config.decoder_map)); diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 4ee51090ab..9daefe58b5 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -243,12 +243,8 @@ void AudioSendStream::ConfigureStream( // Configuration parameters which cannot be changed. RTC_DCHECK(first_time || old_config.send_transport == new_config.send_transport); - - if (old_config.rtp.ssrc != new_config.rtp.ssrc) { - channel_send->SetLocalSSRC(new_config.rtp.ssrc); - } - if (stream->suspended_rtp_state_ && - (first_time || old_config.rtp.ssrc != new_config.rtp.ssrc)) { + RTC_DCHECK(first_time || old_config.rtp.ssrc == new_config.rtp.ssrc); + if (stream->suspended_rtp_state_ && first_time) { stream->rtp_rtcp_module_->SetRtpState(*stream->suspended_rtp_state_); } if (first_time || old_config.rtp.c_name != new_config.rtp.c_name) { diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 94bc34cc44..9f0504c709 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -208,7 +208,7 @@ struct ConfigHelper { EXPECT_CALL(*channel_send_, GetRtpRtcp()).WillRepeatedly(Invoke([this]() { return &this->rtp_rtcp_; })); - EXPECT_CALL(*channel_send_, SetLocalSSRC(kSsrc)).Times(1); + EXPECT_CALL(rtp_rtcp_, SSRC).WillRepeatedly(Return(kSsrc)); EXPECT_CALL(*channel_send_, SetRTCP_CNAME(StrEq(kCName))).Times(1); EXPECT_CALL(*channel_send_, SetFrameEncryptor(_)).Times(1); EXPECT_CALL(*channel_send_, SetExtmapAllowMixed(false)).Times(1); diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index d114391469..3b4e9be404 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -102,6 +102,7 @@ class ChannelReceive : public ChannelReceiveInterface, const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, + uint32_t local_ssrc, uint32_t remote_ssrc, size_t jitter_buffer_max_packets, bool jitter_buffer_fast_playout, @@ -155,9 +156,6 @@ class ChannelReceive : public ChannelReceiveInterface, // Produces the transport-related timestamps; current_delay_ms is left unset. absl::optional GetSyncInfo() const override; - // RTP+RTCP - void SetLocalSSRC(unsigned int ssrc) override; - void RegisterReceiverCongestionControlObjects( PacketRouter* packet_router) override; void ResetReceiverCongestionControlObjects() override; @@ -456,6 +454,7 @@ ChannelReceive::ChannelReceive( const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, + uint32_t local_ssrc, uint32_t remote_ssrc, size_t jitter_buffer_max_packets, bool jitter_buffer_fast_playout, @@ -508,8 +507,8 @@ ChannelReceive::ChannelReceive( configuration.receiver_only = true; configuration.outgoing_transport = rtcp_send_transport; configuration.receive_statistics = rtp_receive_statistics_.get(); - configuration.event_log = event_log_; + configuration.local_media_ssrc = local_ssrc; _rtpRtcpModule = RtpRtcp::Create(configuration); _rtpRtcpModule->SetSendingMediaStatus(false); @@ -701,11 +700,6 @@ void ChannelReceive::SetChannelOutputVolumeScaling(float scaling) { _outputGain = scaling; } -void ChannelReceive::SetLocalSSRC(uint32_t ssrc) { - RTC_DCHECK(worker_thread_checker_.IsCurrent()); - _rtpRtcpModule->SetSSRC(ssrc); -} - void ChannelReceive::RegisterReceiverCongestionControlObjects( PacketRouter* packet_router) { RTC_DCHECK(worker_thread_checker_.IsCurrent()); @@ -959,6 +953,7 @@ std::unique_ptr CreateChannelReceive( const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, + uint32_t local_ssrc, uint32_t remote_ssrc, size_t jitter_buffer_max_packets, bool jitter_buffer_fast_playout, @@ -970,7 +965,7 @@ std::unique_ptr CreateChannelReceive( const webrtc::CryptoOptions& crypto_options) { return absl::make_unique( clock, module_process_thread, audio_device_module, media_transport_config, - rtcp_send_transport, rtc_event_log, remote_ssrc, + rtcp_send_transport, rtc_event_log, local_ssrc, remote_ssrc, jitter_buffer_max_packets, jitter_buffer_fast_playout, jitter_buffer_min_delay_ms, jitter_buffer_enable_rtx_handling, decoder_factory, codec_pair_id, frame_decryptor, crypto_options); diff --git a/audio/channel_receive.h b/audio/channel_receive.h index 1fe64b96ce..dadeab316d 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -116,9 +116,6 @@ class ChannelReceiveInterface : public RtpPacketSinkInterface { // Produces the transport-related timestamps; current_delay_ms is left unset. virtual absl::optional GetSyncInfo() const = 0; - // RTP+RTCP - virtual void SetLocalSSRC(uint32_t ssrc) = 0; - virtual void RegisterReceiverCongestionControlObjects( PacketRouter* packet_router) = 0; virtual void ResetReceiverCongestionControlObjects() = 0; @@ -145,6 +142,7 @@ std::unique_ptr CreateChannelReceive( const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, + uint32_t local_ssrc, uint32_t remote_ssrc, size_t jitter_buffer_max_packets, bool jitter_buffer_fast_playout, diff --git a/audio/channel_send.cc b/audio/channel_send.cc index 876ee69095..f57858c344 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -142,7 +142,6 @@ class ChannelSend : public ChannelSendInterface, int payload_frequency) override; // RTP+RTCP - void SetLocalSSRC(uint32_t ssrc) override; void SetRid(const std::string& rid, int extension_id, int repaired_extension_id) override; @@ -279,7 +278,7 @@ class ChannelSend : public ChannelSendInterface, int media_transport_sequence_number_ RTC_GUARDED_BY(encoder_queue_) = 0; rtc::CriticalSection media_transport_lock_; - // Currently set by SetLocalSSRC. + // Currently set to local SSRC at construction. uint64_t media_transport_channel_id_ RTC_GUARDED_BY(&media_transport_lock_) = 0; // Cache payload type and sampling frequency from most recent call to @@ -702,6 +701,10 @@ ChannelSend::ChannelSend(Clock* clock, configuration.rtcp_report_interval_ms = rtcp_report_interval_ms; configuration.local_media_ssrc = ssrc; + if (media_transport_config_.media_transport) { + rtc::CritScope cs(&media_transport_lock_); + media_transport_channel_id_ = ssrc; + } _rtpRtcpModule = RtpRtcp::Create(configuration); _rtpRtcpModule->SetSendingMediaStatus(false); @@ -951,17 +954,6 @@ void ChannelSend::SetSendTelephoneEventPayloadType(int payload_type, payload_frequency, 0, 0); } -void ChannelSend::SetLocalSSRC(uint32_t ssrc) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - RTC_DCHECK(!sending_); - - if (media_transport_config_.media_transport) { - rtc::CritScope cs(&media_transport_lock_); - media_transport_channel_id_ = ssrc; - } - _rtpRtcpModule->SetSSRC(ssrc); -} - void ChannelSend::SetRid(const std::string& rid, int extension_id, int repaired_extension_id) { diff --git a/audio/channel_send.h b/audio/channel_send.h index a9df5e7cd6..575f71febe 100644 --- a/audio/channel_send.h +++ b/audio/channel_send.h @@ -77,7 +77,6 @@ class ChannelSendInterface { rtc::FunctionView*)> modifier) = 0; virtual void CallEncoder(rtc::FunctionView modifier) = 0; - virtual void SetLocalSSRC(uint32_t ssrc) = 0; // Use 0 to indicate that the extension should not be registered. virtual void SetRid(const std::string& rid, int extension_id, diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h index cf2fe8874a..e666bf200b 100644 --- a/audio/mock_voe_channel_proxy.h +++ b/audio/mock_voe_channel_proxy.h @@ -28,7 +28,6 @@ namespace test { class MockChannelReceive : public voe::ChannelReceiveInterface { public: - MOCK_METHOD1(SetLocalSSRC, void(uint32_t ssrc)); MOCK_METHOD2(SetNACKStatus, void(bool enable, int max_packets)); MOCK_METHOD1(RegisterReceiverCongestionControlObjects, void(PacketRouter* packet_router)); @@ -83,7 +82,6 @@ class MockChannelSend : public voe::ChannelSendInterface { int extension_id, int repaired_extension_id)); MOCK_METHOD2(SetMid, void(const std::string& mid, int extension_id)); - MOCK_METHOD1(SetLocalSSRC, void(uint32_t ssrc)); MOCK_METHOD1(SetRTCP_CNAME, void(absl::string_view c_name)); MOCK_METHOD1(SetExtmapAllowMixed, void(bool extmap_allow_mixed)); MOCK_METHOD2(SetSendAudioLevelIndicationStatus, void(bool enable, int id)); diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 07be79333d..72976bf540 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1065,8 +1065,10 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { void SetLocalSsrc(uint32_t local_ssrc) { RTC_DCHECK(worker_thread_checker_.IsCurrent()); - config_.rtp.local_ssrc = local_ssrc; - ReconfigureAudioReceiveStream(); + if (local_ssrc != config_.rtp.local_ssrc) { + config_.rtp.local_ssrc = local_ssrc; + RecreateAudioReceiveStream(); + } } void SetUseTransportCcAndRecreateStream(bool use_transport_cc,