From 08be9baaa343cfd312ba2db20638f3ed809f2e19 Mon Sep 17 00:00:00 2001 From: Tommi Date: Tue, 15 Jun 2021 23:01:57 +0200 Subject: [PATCH] Don't recreate the audio receive stream when updating the local_ssrc. Bug: webrtc:11993 Change-Id: Ic5d8a8a8b7c12fb1d906e0b3cbdf657fd9e8eafc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222042 Commit-Queue: Tommi Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#34299} --- audio/audio_receive_stream.cc | 13 +++ audio/audio_receive_stream.h | 8 +- audio/channel_receive.cc | 15 ++++ audio/channel_receive.h | 3 + audio/mock_voe_channel_proxy.h | 2 + call/call.cc | 15 ++++ call/call.h | 5 ++ call/degraded_call.cc | 5 ++ call/degraded_call.h | 2 + media/engine/fake_webrtc_call.cc | 6 ++ media/engine/fake_webrtc_call.h | 6 ++ media/engine/webrtc_voice_engine.cc | 20 ++--- modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 1 + modules/rtp_rtcp/source/rtcp_receiver.cc | 83 +++++++++++++++++++- modules/rtp_rtcp/source/rtcp_receiver.h | 65 ++++++++++++--- modules/rtp_rtcp/source/rtcp_sender.cc | 10 +++ modules/rtp_rtcp/source/rtcp_sender.h | 9 ++- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 5 ++ modules/rtp_rtcp/source/rtp_rtcp_impl.h | 1 + modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 14 ++++ modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 8 ++ modules/rtp_rtcp/source/rtp_rtcp_interface.h | 4 + 22 files changed, 266 insertions(+), 34 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 623cb268e4..bae0473501 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -445,6 +445,19 @@ void AudioReceiveStream::DeliverRtcp(const uint8_t* packet, size_t length) { channel_receive_->ReceivedRTCPPacket(packet, length); } +void AudioReceiveStream::SetLocalSsrc(uint32_t local_ssrc) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + // TODO(tommi): Consider storing local_ssrc in one place. + config_.rtp.local_ssrc = local_ssrc; + channel_receive_->OnLocalSsrcChange(local_ssrc); +} + +uint32_t AudioReceiveStream::local_ssrc() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + RTC_DCHECK_EQ(config_.rtp.local_ssrc, channel_receive_->GetLocalSsrc()); + return config_.rtp.local_ssrc; +} + const webrtc::AudioReceiveStream::Config& AudioReceiveStream::config() const { RTC_DCHECK_RUN_ON(&worker_thread_checker_); return config_; diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index b038aff79c..85ab8a1bb5 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -122,11 +122,9 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, void AssociateSendStream(AudioSendStream* send_stream); void DeliverRtcp(const uint8_t* packet, size_t length); - uint32_t local_ssrc() const { - // The local_ssrc member variable of config_ will never change and can be - // considered const. - return config_.rtp.local_ssrc; - } + void SetLocalSsrc(uint32_t local_ssrc); + + uint32_t local_ssrc() const; uint32_t remote_ssrc() const { // The remote_ssrc member variable of config_ will never change and can be diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 28568b17c4..150e2074e4 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -180,6 +180,9 @@ class ChannelReceive : public ChannelReceiveInterface { void SetFrameDecryptor(rtc::scoped_refptr frame_decryptor) override; + void OnLocalSsrcChange(uint32_t local_ssrc) override; + uint32_t GetLocalSsrc() const override; + private: void ReceivePacket(const uint8_t* packet, size_t packet_length, @@ -901,6 +904,18 @@ void ChannelReceive::SetFrameDecryptor( frame_decryptor_ = std::move(frame_decryptor); } +void ChannelReceive::OnLocalSsrcChange(uint32_t local_ssrc) { + // TODO(bugs.webrtc.org/11993): Expect to be called on the network thread. + RTC_DCHECK_RUN_ON(&worker_thread_checker_); + rtp_rtcp_->SetLocalSsrc(local_ssrc); +} + +uint32_t ChannelReceive::GetLocalSsrc() const { + // TODO(bugs.webrtc.org/11993): Expect to be called on the network thread. + RTC_DCHECK_RUN_ON(&worker_thread_checker_); + return rtp_rtcp_->local_media_ssrc(); +} + NetworkStatistics ChannelReceive::GetNetworkStatistics( bool get_and_clear_legacy_stats) const { RTC_DCHECK_RUN_ON(&worker_thread_checker_); diff --git a/audio/channel_receive.h b/audio/channel_receive.h index 0a51be6649..196e441fac 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -162,6 +162,9 @@ class ChannelReceiveInterface : public RtpPacketSinkInterface { virtual void SetFrameDecryptor( rtc::scoped_refptr frame_decryptor) = 0; + + virtual void OnLocalSsrcChange(uint32_t local_ssrc) = 0; + virtual uint32_t GetLocalSsrc() const = 0; }; std::unique_ptr CreateChannelReceive( diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h index deef5ae068..ea2a2ac3f0 100644 --- a/audio/mock_voe_channel_proxy.h +++ b/audio/mock_voe_channel_proxy.h @@ -104,6 +104,8 @@ class MockChannelReceive : public voe::ChannelReceiveInterface { SetFrameDecryptor, (rtc::scoped_refptr frame_decryptor), (override)); + MOCK_METHOD(void, OnLocalSsrcChange, (uint32_t local_ssrc), (override)); + MOCK_METHOD(uint32_t, GetLocalSsrc, (), (const, override)); }; class MockChannelSend : public voe::ChannelSendInterface { diff --git a/call/call.cc b/call/call.cc index 273720189f..b9ce0eb8a7 100644 --- a/call/call.cc +++ b/call/call.cc @@ -269,6 +269,9 @@ class Call final : public webrtc::Call, void OnAudioTransportOverheadChanged( int transport_overhead_per_packet) override; + void OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream, + uint32_t local_ssrc) override; + void OnSentPacket(const rtc::SentPacket& sent_packet) override; // Implements TargetTransferRateObserver, @@ -1356,6 +1359,18 @@ void Call::UpdateAggregateNetworkState() { transport_send_->OnNetworkAvailability(aggregate_network_up); } +void Call::OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream, + uint32_t local_ssrc) { + RTC_DCHECK_RUN_ON(worker_thread_); + webrtc::internal::AudioReceiveStream& receive_stream = + static_cast(stream); + + receive_stream.SetLocalSsrc(local_ssrc); + auto it = audio_send_ssrcs_.find(local_ssrc); + receive_stream.AssociateSendStream(it != audio_send_ssrcs_.end() ? it->second + : nullptr); +} + void Call::OnSentPacket(const rtc::SentPacket& sent_packet) { // In production and with most tests, this method will be called on the // network thread. However some test classes such as DirectTransport don't diff --git a/call/call.h b/call/call.h index 86d0620e62..ae53b30593 100644 --- a/call/call.h +++ b/call/call.h @@ -155,6 +155,11 @@ class Call { virtual void OnAudioTransportOverheadChanged( int transport_overhead_per_packet) = 0; + // Called when a receive stream's local ssrc has changed and association with + // send streams needs to be updated. + virtual void OnLocalSsrcUpdated(AudioReceiveStream& stream, + uint32_t local_ssrc) = 0; + virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; virtual void SetClientBitratePreferences( diff --git a/call/degraded_call.cc b/call/degraded_call.cc index 73c236bc0c..a34a9c873d 100644 --- a/call/degraded_call.cc +++ b/call/degraded_call.cc @@ -288,6 +288,11 @@ void DegradedCall::OnAudioTransportOverheadChanged( call_->OnAudioTransportOverheadChanged(transport_overhead_per_packet); } +void DegradedCall::OnLocalSsrcUpdated(AudioReceiveStream& stream, + uint32_t local_ssrc) { + call_->OnLocalSsrcUpdated(stream, local_ssrc); +} + void DegradedCall::OnSentPacket(const rtc::SentPacket& sent_packet) { if (send_config_) { // If we have a degraded send-transport, we have already notified call diff --git a/call/degraded_call.h b/call/degraded_call.h index 03fc14f284..621bbb31ac 100644 --- a/call/degraded_call.h +++ b/call/degraded_call.h @@ -93,6 +93,8 @@ class DegradedCall : public Call, private PacketReceiver { void SignalChannelNetworkState(MediaType media, NetworkState state) override; void OnAudioTransportOverheadChanged( int transport_overhead_per_packet) override; + void OnLocalSsrcUpdated(AudioReceiveStream& stream, + uint32_t local_ssrc) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; protected: diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index fa5125f7a4..0483659910 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -668,6 +668,12 @@ void FakeCall::SignalChannelNetworkState(webrtc::MediaType media, void FakeCall::OnAudioTransportOverheadChanged( int transport_overhead_per_packet) {} +void FakeCall::OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream, + uint32_t local_ssrc) { + auto& fake_stream = static_cast(stream); + fake_stream.SetLocalSsrc(local_ssrc); +} + void FakeCall::OnSentPacket(const rtc::SentPacket& sent_packet) { last_sent_packet_ = sent_packet; if (sent_packet.packet_id >= 0) { diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 874f971c0f..9dec31dbd1 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -100,6 +100,10 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream { return base_mininum_playout_delay_ms_; } + void SetLocalSsrc(uint32_t local_ssrc) { + config_.rtp.local_ssrc = local_ssrc; + } + private: const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { return config_.rtp; @@ -391,6 +395,8 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { webrtc::NetworkState state) override; void OnAudioTransportOverheadChanged( int transport_overhead_per_packet) override; + void OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream, + uint32_t local_ssrc) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; webrtc::TaskQueueBase* const network_thread_; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index f38474bff9..1e896cdb50 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1202,6 +1202,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { config_.frame_decryptor = frame_decryptor; config_.crypto_options = crypto_options; config_.frame_transformer = std::move(frame_transformer); + // TODO(tommi): Remove RecreateAudioReceiveStream() and make stream_ const. RecreateAudioReceiveStream(); } @@ -1214,6 +1215,11 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { call_->DestroyAudioReceiveStream(stream_); } + webrtc::AudioReceiveStream& stream() { + RTC_DCHECK(stream_); + return *stream_; + } + void SetFrameDecryptor( rtc::scoped_refptr frame_decryptor) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); @@ -1221,14 +1227,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { stream_->SetFrameDecryptor(std::move(frame_decryptor)); } - void SetLocalSsrc(uint32_t local_ssrc) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - if (local_ssrc != config_.rtp.local_ssrc) { - config_.rtp.local_ssrc = local_ssrc; - RecreateAudioReceiveStream(); - } - } - void SetUseTransportCcAndRecreateStream(bool use_transport_cc, bool use_nack) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); @@ -1933,10 +1931,8 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) { // same SSRC in order to send receiver reports. if (send_streams_.size() == 1) { receiver_reports_ssrc_ = ssrc; - for (const auto& kv : recv_streams_) { - // TODO(solenberg): Allow applications to set the RTCP SSRC of receive - // streams instead, so we can avoid reconfiguring the streams here. - kv.second->SetLocalSsrc(ssrc); + for (auto& kv : recv_streams_) { + call_->OnLocalSsrcUpdated(kv.second->stream(), ssrc); } } diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index d523128e38..a7707ecc19 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -34,6 +34,7 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { (const uint8_t* incoming_packet, size_t packet_length), (override)); MOCK_METHOD(void, SetRemoteSSRC, (uint32_t ssrc), (override)); + MOCK_METHOD(void, SetLocalSsrc, (uint32_t ssrc), (override)); MOCK_METHOD(void, SetMaxRtpPacketSize, (size_t size), (override)); MOCK_METHOD(size_t, MaxRtpPacketSize, (), (const, override)); MOCK_METHOD(void, diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index e48f20079b..79f24c4779 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -39,6 +39,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet/tmmbr.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" +#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h" #include "modules/rtp_rtcp/source/time_util.h" #include "modules/rtp_rtcp/source/tmmbr_help.h" #include "rtc_base/checks.h" @@ -84,8 +85,14 @@ bool ResetTimestampIfExpired(const Timestamp now, } // namespace +constexpr size_t RTCPReceiver::RegisteredSsrcs::kMediaSsrcIndex; +constexpr size_t RTCPReceiver::RegisteredSsrcs::kMaxSsrcs; + RTCPReceiver::RegisteredSsrcs::RegisteredSsrcs( - const RtpRtcpInterface::Configuration& config) { + bool disable_sequence_checker, + const RtpRtcpInterface::Configuration& config) + : packet_sequence_checker_(disable_sequence_checker) { + packet_sequence_checker_.Detach(); ssrcs_.push_back(config.local_media_ssrc); if (config.rtx_send_ssrc) { ssrcs_.push_back(*config.rtx_send_ssrc); @@ -100,6 +107,21 @@ RTCPReceiver::RegisteredSsrcs::RegisteredSsrcs( RTC_DCHECK_LE(ssrcs_.size(), RTCPReceiver::RegisteredSsrcs::kMaxSsrcs); } +bool RTCPReceiver::RegisteredSsrcs::contains(uint32_t ssrc) const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return absl::c_linear_search(ssrcs_, ssrc); +} + +uint32_t RTCPReceiver::RegisteredSsrcs::media_ssrc() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return ssrcs_[kMediaSsrcIndex]; +} + +void RTCPReceiver::RegisteredSsrcs::set_media_ssrc(uint32_t ssrc) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + ssrcs_[kMediaSsrcIndex] = ssrc; +} + struct RTCPReceiver::PacketInformation { uint32_t packet_type_flags = 0; // RTCPPacketTypeFlags bit field. @@ -117,12 +139,12 @@ struct RTCPReceiver::PacketInformation { }; RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, - ModuleRtpRtcp* owner) + ModuleRtpRtcpImpl2* owner) : clock_(config.clock), receiver_only_(config.receiver_only), rtp_rtcp_(owner), main_ssrc_(config.local_media_ssrc), - registered_ssrcs_(config), + registered_ssrcs_(false, config), rtcp_bandwidth_observer_(config.bandwidth_callback), rtcp_intra_frame_observer_(config.intra_frame_callback), rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), @@ -150,6 +172,53 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, RTC_DCHECK(owner); } +RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, + ModuleRtpRtcp* owner) + : clock_(config.clock), + receiver_only_(config.receiver_only), + rtp_rtcp_(owner), + main_ssrc_(config.local_media_ssrc), + registered_ssrcs_(true, config), + rtcp_bandwidth_observer_(config.bandwidth_callback), + rtcp_intra_frame_observer_(config.intra_frame_callback), + rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), + network_state_estimate_observer_(config.network_state_estimate_observer), + transport_feedback_observer_(config.transport_feedback_callback), + bitrate_allocation_observer_(config.bitrate_allocation_observer), + report_interval_(config.rtcp_report_interval_ms > 0 + ? TimeDelta::Millis(config.rtcp_report_interval_ms) + : (config.audio ? kDefaultAudioReportInterval + : kDefaultVideoReportInterval)), + // TODO(bugs.webrtc.org/10774): Remove fallback. + remote_ssrc_(0), + remote_sender_rtp_time_(0), + remote_sender_packet_count_(0), + remote_sender_octet_count_(0), + remote_sender_reports_count_(0), + xr_rrtr_status_(config.non_sender_rtt_measurement), + xr_rr_rtt_ms_(0), + oldest_tmmbr_info_ms_(0), + cname_callback_(config.rtcp_cname_callback), + report_block_data_observer_(config.report_block_data_observer), + packet_type_counter_observer_(config.rtcp_packet_type_counter_observer), + num_skipped_packets_(0), + last_skipped_packets_warning_ms_(clock_->TimeInMilliseconds()) { + RTC_DCHECK(owner); + // Dear reader - if you're here because of this log statement and are + // wondering what this is about, chances are that you are using an instance + // of RTCPReceiver without using the webrtc APIs. This creates a bit of a + // problem for WebRTC because this class is a part of an internal + // implementation that is constantly changing and being improved. + // The intention of this log statement is to give a heads up that changes + // are coming and encourage you to use the public APIs or be prepared that + // things might break down the line as more changes land. A thing you could + // try out for now is to replace the `CustomSequenceChecker` in the header + // with a regular `SequenceChecker` and see if that triggers an + // error in your code. If it does, chances are you have your own threading + // model that is not the same as WebRTC internally has. + RTC_LOG(LS_INFO) << "************** !!!DEPRECATION WARNING!! **************"; +} + RTCPReceiver::~RTCPReceiver() {} void RTCPReceiver::IncomingPacket(rtc::ArrayView packet) { @@ -178,6 +247,14 @@ void RTCPReceiver::SetRemoteSSRC(uint32_t ssrc) { remote_ssrc_ = ssrc; } +void RTCPReceiver::set_local_media_ssrc(uint32_t ssrc) { + registered_ssrcs_.set_media_ssrc(ssrc); +} + +uint32_t RTCPReceiver::local_media_ssrc() const { + return registered_ssrcs_.media_ssrc(); +} + uint32_t RTCPReceiver::RemoteSSRC() const { MutexLock lock(&rtcp_receiver_lock_); return remote_ssrc_; diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index 429df55d49..57dd1c06b4 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -19,6 +19,7 @@ #include #include "api/array_view.h" +#include "api/sequence_checker.h" #include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/include/rtcp_statistics.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -27,11 +28,15 @@ #include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h" #include "modules/rtp_rtcp/source/rtp_rtcp_interface.h" #include "rtc_base/synchronization/mutex.h" +#include "rtc_base/system/no_unique_address.h" #include "rtc_base/thread_annotations.h" #include "system_wrappers/include/ntp_time.h" namespace webrtc { + +class ModuleRtpRtcpImpl2; class VideoBitrateAllocationObserver; + namespace rtcp { class CommonHeader; class ReportBlock; @@ -57,6 +62,10 @@ class RTCPReceiver final { RTCPReceiver(const RtpRtcpInterface::Configuration& config, ModuleRtpRtcp* owner); + + RTCPReceiver(const RtpRtcpInterface::Configuration& config, + ModuleRtpRtcpImpl2* owner); + ~RTCPReceiver(); void IncomingPacket(const uint8_t* packet, size_t packet_size) { @@ -66,9 +75,14 @@ class RTCPReceiver final { int64_t LastReceivedReportBlockMs() const; + void set_local_media_ssrc(uint32_t ssrc); + uint32_t local_media_ssrc() const; + void SetRemoteSSRC(uint32_t ssrc); uint32_t RemoteSSRC() const; + bool receiver_only() const { return receiver_only_; } + // Get received NTP. // The types for the arguments below derive from the specification: // - `remote_sender_packet_count`: `RTCSentRtpStreamStats.packetsSent` [1] @@ -126,21 +140,48 @@ class RTCPReceiver final { void NotifyTmmbrUpdated(); private: - // A lightweight inlined set of local SSRCs. - class RegisteredSsrcs { +#if RTC_DCHECK_IS_ON + class CustomSequenceChecker : public SequenceChecker { public: - static constexpr size_t kMaxSsrcs = 3; - // Initializes the set of registered local SSRCS by extracting them from the - // provided `config`. - explicit RegisteredSsrcs(const RtpRtcpInterface::Configuration& config); - - // Indicates if `ssrc` is in the set of registered local SSRCs. - bool contains(uint32_t ssrc) const { - return absl::c_linear_search(ssrcs_, ssrc); + explicit CustomSequenceChecker(bool disable_checks) + : disable_checks_(disable_checks) {} + bool IsCurrent() const { + if (disable_checks_) + return true; + return SequenceChecker::IsCurrent(); } private: - absl::InlinedVector ssrcs_; + const bool disable_checks_; + }; +#else + class CustomSequenceChecker : public SequenceChecker { + public: + explicit CustomSequenceChecker(bool) {} + }; +#endif + + // A lightweight inlined set of local SSRCs. + class RegisteredSsrcs { + public: + static constexpr size_t kMediaSsrcIndex = 0; + static constexpr size_t kMaxSsrcs = 3; + // Initializes the set of registered local SSRCS by extracting them from the + // provided `config`. The `disable_sequence_checker` flag is a workaround + // to be able to use a sequence checker without breaking downstream + // code that currently doesn't follow the same threading rules as webrtc. + RegisteredSsrcs(bool disable_sequence_checker, + const RtpRtcpInterface::Configuration& config); + + // Indicates if `ssrc` is in the set of registered local SSRCs. + bool contains(uint32_t ssrc) const; + uint32_t media_ssrc() const; + void set_media_ssrc(uint32_t ssrc); + + private: + RTC_NO_UNIQUE_ADDRESS CustomSequenceChecker packet_sequence_checker_; + absl::InlinedVector ssrcs_ + RTC_GUARDED_BY(packet_sequence_checker_); }; struct PacketInformation; @@ -290,7 +331,7 @@ class RTCPReceiver final { ModuleRtpRtcp* const rtp_rtcp_; const uint32_t main_ssrc_; // The set of registered local SSRCs. - const RegisteredSsrcs registered_ssrcs_; + RegisteredSsrcs registered_ssrcs_; RtcpBandwidthObserver* const rtcp_bandwidth_observer_; RtcpIntraFrameObserver* const rtcp_intra_frame_observer_; diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 287cb9cc91..bacb7675fd 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -308,6 +308,16 @@ void RTCPSender::SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz) { rtp_clock_rates_khz_[payload_type] = rtp_clock_rate_hz / 1000; } +uint32_t RTCPSender::SSRC() const { + MutexLock lock(&mutex_rtcp_sender_); + return ssrc_; +} + +void RTCPSender::SetSsrc(uint32_t ssrc) { + MutexLock lock(&mutex_rtcp_sender_); + ssrc_ = ssrc; +} + void RTCPSender::SetRemoteSSRC(uint32_t ssrc) { MutexLock lock(&mutex_rtcp_sender_); remote_ssrc_ = ssrc; diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index aab2c9051f..67200eef36 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -94,7 +94,8 @@ class RTCPSender final { void SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); - uint32_t SSRC() const { return ssrc_; } + uint32_t SSRC() const; + void SetSsrc(uint32_t ssrc); void SetRemoteSSRC(uint32_t ssrc) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); @@ -187,7 +188,11 @@ class RTCPSender final { RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); const bool audio_; - const uint32_t ssrc_; + // TODO(bugs.webrtc.org/11581): `mutex_rtcp_sender_` shouldn't be required if + // we consistently run network related operations on the network thread. + // This is currently not possible due to callbacks from the process thread in + // ModuleRtpRtcpImpl2. + uint32_t ssrc_ RTC_GUARDED_BY(mutex_rtcp_sender_); Clock* const clock_; Random random_ RTC_GUARDED_BY(mutex_rtcp_sender_); RtcpMode method_ RTC_GUARDED_BY(mutex_rtcp_sender_); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 5a79f55d33..54316a85b5 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -683,6 +683,11 @@ void ModuleRtpRtcpImpl::SetRemoteSSRC(const uint32_t ssrc) { rtcp_receiver_.SetRemoteSSRC(ssrc); } +void ModuleRtpRtcpImpl::SetLocalSsrc(uint32_t local_ssrc) { + rtcp_receiver_.set_local_media_ssrc(local_ssrc); + rtcp_sender_.SetSsrc(local_ssrc); +} + RtpSendRates ModuleRtpRtcpImpl::GetSendRates() const { return rtp_sender_->packet_sender.GetSendRates(); } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 5bcabc57b1..b0e0b41c48 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -63,6 +63,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { size_t incoming_packet_length) override; void SetRemoteSSRC(uint32_t ssrc) override; + void SetLocalSsrc(uint32_t ssrc) override; // Sender part. void RegisterSendPayloadFrequency(int payload_type, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index d5f11f6338..55e4a74817 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -69,6 +69,7 @@ ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration) rtt_ms_(0) { RTC_DCHECK(worker_queue_); process_thread_checker_.Detach(); + packet_sequence_checker_.Detach(); if (!configuration.receiver_only) { rtp_sender_ = std::make_unique(configuration); // Make sure rtcp sender use same timestamp offset as rtp sender. @@ -169,6 +170,7 @@ absl::optional ModuleRtpRtcpImpl2::FlexfecSsrc() const { void ModuleRtpRtcpImpl2::IncomingRtcpPacket(const uint8_t* rtcp_packet, const size_t length) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); rtcp_receiver_.IncomingPacket(rtcp_packet, length); } @@ -219,6 +221,12 @@ RtpState ModuleRtpRtcpImpl2::GetRtxState() const { return rtp_sender_->packet_generator.GetRtxRtpState(); } +uint32_t ModuleRtpRtcpImpl2::local_media_ssrc() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + RTC_DCHECK_EQ(rtcp_receiver_.local_media_ssrc(), rtcp_sender_.SSRC()); + return rtcp_receiver_.local_media_ssrc(); +} + void ModuleRtpRtcpImpl2::SetRid(const std::string& rid) { if (rtp_sender_) { rtp_sender_->packet_generator.SetRid(rid); @@ -650,6 +658,12 @@ void ModuleRtpRtcpImpl2::SetRemoteSSRC(const uint32_t ssrc) { rtcp_receiver_.SetRemoteSSRC(ssrc); } +void ModuleRtpRtcpImpl2::SetLocalSsrc(uint32_t local_ssrc) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + rtcp_receiver_.set_local_media_ssrc(local_ssrc); + rtcp_sender_.SetSsrc(local_ssrc); +} + RtpSendRates ModuleRtpRtcpImpl2::GetSendRates() const { // Typically called on the `rtp_transport_queue_` owned by an // RtpTransportControllerSendInterface instance. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index 00f6ff161d..4c38517e85 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -77,6 +77,8 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, void SetRemoteSSRC(uint32_t ssrc) override; + void SetLocalSsrc(uint32_t local_ssrc) override; + // Sender part. void RegisterSendPayloadFrequency(int payload_type, int payload_frequency) override; @@ -110,6 +112,11 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, uint32_t SSRC() const override { return rtcp_sender_.SSRC(); } + // Semantically identical to `SSRC()` but must be called on the packet + // delivery thread/tq and returns the ssrc that maps to + // RtpRtcpInterface::Configuration::local_media_ssrc. + uint32_t local_media_ssrc() const; + void SetRid(const std::string& rid) override; void SetMid(const std::string& mid) override; @@ -284,6 +291,7 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, TaskQueueBase* const worker_queue_; RTC_NO_UNIQUE_ADDRESS SequenceChecker process_thread_checker_; + RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_; std::unique_ptr rtp_sender_; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 457a993139..dd5744ec54 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -180,6 +180,10 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { virtual void SetRemoteSSRC(uint32_t ssrc) = 0; + // Called when the local ssrc changes (post initialization) for receive + // streams to match with send. Called on the packet receive thread/tq. + virtual void SetLocalSsrc(uint32_t ssrc) = 0; + // ************************************************************************** // Sender // **************************************************************************