From cc50b04c021ae16f57a0b2dd11bb1ec62a2a6db1 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 9 May 2022 10:22:48 +0000 Subject: [PATCH] Remove config() getter from AudioReceiveStream(). This reduces the surface of externally accessible state that belongs to the class, which makes it easier to control what state belongs to what thread. In this CL enforcing remote_ssrc() to be conceptually const and sync_group to conceptually belong to the packet delivery thread. Bug: webrtc:11993 Change-Id: I7de9366dc0c2bf451b5c58595c2d073b4016f2e7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261450 Reviewed-by: Niels Moller Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#36813} --- audio/audio_receive_stream.cc | 20 ++++++++++---------- audio/audio_receive_stream.h | 5 ++++- audio/audio_receive_stream_unittest.cc | 2 ++ call/call.cc | 9 +++++---- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 6717878abb..d4c7910f56 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -146,7 +146,7 @@ AudioReceiveStream::AudioReceiveStream( AudioReceiveStream::~AudioReceiveStream() { RTC_DCHECK_RUN_ON(&worker_thread_checker_); - RTC_LOG(LS_INFO) << "~AudioReceiveStream: " << config_.rtp.remote_ssrc; + RTC_LOG(LS_INFO) << "~AudioReceiveStream: " << remote_ssrc(); Stop(); channel_receive_->SetAssociatedSendChannel(nullptr); channel_receive_->ResetReceiverCongestionControlObjects(); @@ -157,7 +157,7 @@ void AudioReceiveStream::RegisterWithTransport( RTC_DCHECK_RUN_ON(&packet_sequence_checker_); RTC_DCHECK(!rtp_stream_receiver_); rtp_stream_receiver_ = receiver_controller->CreateReceiver( - config_.rtp.remote_ssrc, channel_receive_.get()); + remote_ssrc(), channel_receive_.get()); } void AudioReceiveStream::UnregisterFromTransport() { @@ -170,8 +170,8 @@ void AudioReceiveStream::ReconfigureForTesting( RTC_DCHECK_RUN_ON(&worker_thread_checker_); // SSRC can't be changed mid-stream. - RTC_DCHECK_EQ(config_.rtp.remote_ssrc, config.rtp.remote_ssrc); - RTC_DCHECK_EQ(config_.rtp.local_ssrc, config.rtp.local_ssrc); + RTC_DCHECK_EQ(remote_ssrc(), config.rtp.remote_ssrc); + RTC_DCHECK_EQ(local_ssrc(), config.rtp.local_ssrc); // Configuration parameters which cannot be changed. RTC_DCHECK_EQ(config_.rtcp_send_transport, config.rtcp_send_transport); @@ -269,7 +269,7 @@ webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats( bool get_and_clear_legacy_stats) const { RTC_DCHECK_RUN_ON(&worker_thread_checker_); webrtc::AudioReceiveStream::Stats stats; - stats.remote_ssrc = config_.rtp.remote_ssrc; + stats.remote_ssrc = remote_ssrc(); webrtc::CallReceiveStatistics call_stats = channel_receive_->GetRTCPStatistics(); @@ -398,7 +398,7 @@ AudioMixer::Source::AudioFrameInfo AudioReceiveStream::GetAudioFrameWithInfo( } int AudioReceiveStream::Ssrc() const { - return config_.rtp.remote_ssrc; + return remote_ssrc(); } int AudioReceiveStream::PreferredSampleRate() const { @@ -407,7 +407,7 @@ int AudioReceiveStream::PreferredSampleRate() const { uint32_t AudioReceiveStream::id() const { RTC_DCHECK_RUN_ON(&worker_thread_checker_); - return config_.rtp.remote_ssrc; + return remote_ssrc(); } absl::optional AudioReceiveStream::GetInfo() const { @@ -471,9 +471,9 @@ uint32_t AudioReceiveStream::local_ssrc() const { return config_.rtp.local_ssrc; } -const webrtc::AudioReceiveStream::Config& AudioReceiveStream::config() const { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - return config_; +const std::string& AudioReceiveStream::sync_group() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return config_.sync_group; } const AudioSendStream* AudioReceiveStream::GetAssociatedSendStreamForTesting() diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index 3f4675cd72..6fc9f7fe62 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -134,7 +134,10 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, return config_.rtp.remote_ssrc; } - const webrtc::AudioReceiveStream::Config& config() const; + // Returns a reference to the currently set sync group of the stream. + // Must be called on the packet delivery thread. + const std::string& sync_group() const; + const AudioSendStream* GetAssociatedSendStreamForTesting() const; // TODO(tommi): Remove this method. diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc index 1d6183a1d4..a1e8c1f61f 100644 --- a/audio/audio_receive_stream_unittest.cc +++ b/audio/audio_receive_stream_unittest.cc @@ -131,6 +131,8 @@ struct ConfigHelper { EXPECT_THAT(codecs, ::testing::IsEmpty()); })); EXPECT_CALL(*channel_receive_, SetSourceTracker(_)); + EXPECT_CALL(*channel_receive_, GetLocalSsrc()) + .WillRepeatedly(Return(kLocalSsrc)); stream_config_.rtp.local_ssrc = kLocalSsrc; stream_config_.rtp.remote_ssrc = kRemoteSsrc; diff --git a/call/call.cc b/call/call.cc index 5b306610ea..1a0357fc75 100644 --- a/call/call.cc +++ b/call/call.cc @@ -1009,8 +1009,9 @@ void Call::DestroyAudioReceiveStream( audio_receive_stream->UnregisterFromTransport(); uint32_t ssrc = audio_receive_stream->remote_ssrc(); - const AudioReceiveStream::Config& config = audio_receive_stream->config(); - receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(config.rtp)) + receive_side_cc_ + .GetRemoteBitrateEstimator( + UseSendSideBwe(audio_receive_stream->rtp_config())) ->RemoveStream(ssrc); audio_receive_streams_.erase(audio_receive_stream); @@ -1018,7 +1019,7 @@ void Call::DestroyAudioReceiveStream( // After calling erase(), call ConfigureSync. This will clear associated // video streams or associate them with a different audio stream if one exists // for this sync_group. - ConfigureSync(audio_receive_stream->config().sync_group); + ConfigureSync(audio_receive_stream->sync_group()); UnregisterReceiveStream(ssrc); @@ -1471,7 +1472,7 @@ AudioReceiveStream* Call::FindAudioStreamForSyncGroup( RTC_DCHECK_RUN_ON(&receive_11993_checker_); if (!sync_group.empty()) { for (AudioReceiveStream* stream : audio_receive_streams_) { - if (stream->config().sync_group == sync_group) + if (stream->sync_group() == sync_group) return stream; } }