From 6be3e788f579ded8adff9a1783b1d4fda526a323 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 9 May 2022 15:20:24 +0000 Subject: [PATCH] Add getter for rtp header extensions for receiver classes. This is to avoid accessing the array via the config struct. Moving forward we might want to consider using the RtpHeaderExtensionMap instead of a std::vector of RtpExtension. Bug: webrtc:11993 Change-Id: I8469dbbd9bb95a69f87b5912bfc4bf8b8f603beb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261317 Reviewed-by: Niels Moller Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#36820} --- audio/audio_receive_stream.cc | 5 +++++ audio/audio_receive_stream.h | 1 + call/call.cc | 20 ++++++++++---------- call/flexfec_receive_stream_impl.cc | 6 ++++++ call/flexfec_receive_stream_impl.h | 1 + call/receive_stream.h | 5 +++++ media/engine/fake_webrtc_call.cc | 15 +++++++++++++++ media/engine/fake_webrtc_call.h | 3 +++ media/engine/webrtc_voice_engine.cc | 3 +-- video/video_receive_stream2.cc | 6 ++++++ video/video_receive_stream2.h | 1 + 11 files changed, 54 insertions(+), 12 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index d4c7910f56..f45f608a2a 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -265,6 +265,11 @@ void AudioReceiveStream::SetRtpExtensions( config_.rtp.extensions = std::move(extensions); } +const std::vector& AudioReceiveStream::GetRtpExtensions() const { + RTC_DCHECK_RUN_ON(&worker_thread_checker_); + return config_.rtp.extensions; +} + webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats( bool get_and_clear_legacy_stats) const { RTC_DCHECK_RUN_ON(&worker_thread_checker_); diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index 252ab13f39..6a4c0222c3 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -95,6 +95,7 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, void SetFrameDecryptor(rtc::scoped_refptr frame_decryptor) override; void SetRtpExtensions(std::vector extensions) override; + const std::vector& GetRtpExtensions() const override; webrtc::AudioReceiveStream::Stats GetStats( bool get_and_clear_legacy_stats) const override; diff --git a/call/call.cc b/call/call.cc index 0703fd4d29..ae6c767a39 100644 --- a/call/call.cc +++ b/call/call.cc @@ -79,10 +79,10 @@ bool SendPeriodicFeedback(const std::vector& extensions) { return true; } -bool UseSendSideBwe(const ReceiveStream::RtpConfig& rtp) { - if (!rtp.transport_cc) +bool UseSendSideBwe(const ReceiveStream* stream) { + if (!stream->rtp_config().transport_cc) return false; - for (const auto& extension : rtp.extensions) { + for (const auto& extension : stream->GetRtpExtensions()) { if (extension.uri == RtpExtension::kTransportSequenceNumberUri || extension.uri == RtpExtension::kTransportSequenceNumberV2Uri) return true; @@ -1010,8 +1010,7 @@ void Call::DestroyAudioReceiveStream( uint32_t ssrc = audio_receive_stream->remote_ssrc(); receive_side_cc_ - .GetRemoteBitrateEstimator( - UseSendSideBwe(audio_receive_stream->rtp_config())) + .GetRemoteBitrateEstimator(UseSendSideBwe(audio_receive_stream)) ->RemoveStream(ssrc); audio_receive_streams_.erase(audio_receive_stream); @@ -1189,6 +1188,7 @@ void Call::DestroyVideoReceiveStream( // TODO(bugs.webrtc.org/11993): Unregister on the network thread. receive_stream_impl->UnregisterFromTransport(); + // TODO(tommi): Remove `rtp()` accessor. const webrtc::VideoReceiveStream::Config::Rtp& rtp = receive_stream_impl->rtp(); @@ -1201,7 +1201,8 @@ void Call::DestroyVideoReceiveStream( video_receive_streams_.erase(receive_stream_impl); ConfigureSync(receive_stream_impl->sync_group()); - receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(rtp)) + receive_side_cc_ + .GetRemoteBitrateEstimator(UseSendSideBwe(receive_stream_impl)) ->RemoveStream(rtp.remote_ssrc); UpdateAggregateNetworkState(); @@ -1251,8 +1252,7 @@ void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) { // Remove all SSRCs pointing to the FlexfecReceiveStreamImpl to be // destroyed. receive_side_cc_ - .GetRemoteBitrateEstimator( - UseSendSideBwe(receive_stream_impl->rtp_config())) + .GetRemoteBitrateEstimator(UseSendSideBwe(receive_stream_impl)) ->RemoveStream(ssrc); delete receive_stream_impl; @@ -1694,10 +1694,10 @@ bool Call::IdentifyReceivedPacket(RtpPacketReceived& packet, } packet.IdentifyExtensions( - RtpHeaderExtensionMap(it->second->rtp_config().extensions)); + RtpHeaderExtensionMap(it->second->GetRtpExtensions())); if (use_send_side_bwe) { - *use_send_side_bwe = UseSendSideBwe(it->second->rtp_config()); + *use_send_side_bwe = UseSendSideBwe(it->second); } return true; diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc index 6c8378d4b2..24e2120ece 100644 --- a/call/flexfec_receive_stream_impl.cc +++ b/call/flexfec_receive_stream_impl.cc @@ -210,4 +210,10 @@ void FlexfecReceiveStreamImpl::SetRtpExtensions( std::move(extensions); } +const std::vector& FlexfecReceiveStreamImpl::GetRtpExtensions() + const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return config_.rtp.extensions; +} + } // namespace webrtc diff --git a/call/flexfec_receive_stream_impl.h b/call/flexfec_receive_stream_impl.h index c640ac6879..0bc9faaf3d 100644 --- a/call/flexfec_receive_stream_impl.h +++ b/call/flexfec_receive_stream_impl.h @@ -60,6 +60,7 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream { // ReceiveStream impl. void SetRtpExtensions(std::vector extensions) override; + const std::vector& GetRtpExtensions() const override; const RtpConfig& rtp_config() const override { return config_.rtp; } uint32_t remote_ssrc() const { return config_.rtp.remote_ssrc; } diff --git a/call/receive_stream.h b/call/receive_stream.h index a6756fc5c1..5413387a3b 100644 --- a/call/receive_stream.h +++ b/call/receive_stream.h @@ -55,6 +55,11 @@ class ReceiveStream { // delivery thread. virtual void SetRtpExtensions(std::vector extensions) = 0; + // Access the currently set rtp extensions. Must be called on the packet + // delivery thread. + // TODO(tommi): Consider using `RtpHeaderExtensionMap` instead. + virtual const std::vector& GetRtpExtensions() const = 0; + // Called on the packet delivery thread since some members of the config may // change mid-stream (e.g. the local ssrc). All mutation must also happen on // the packet delivery thread. Return value can be assumed to diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index 20651d1fa1..d235e20750 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -130,6 +130,11 @@ void FakeAudioReceiveStream::SetRtpExtensions( config_.rtp.extensions = std::move(extensions); } +const std::vector& +FakeAudioReceiveStream::GetRtpExtensions() const { + return config_.rtp.extensions; +} + webrtc::AudioReceiveStream::Stats FakeAudioReceiveStream::GetStats( bool get_and_clear_legacy_stats) const { return stats_; @@ -380,6 +385,11 @@ void FakeVideoReceiveStream::SetRtpExtensions( config_.rtp.extensions = std::move(extensions); } +const std::vector& +FakeVideoReceiveStream::GetRtpExtensions() const { + return config_.rtp.extensions; +} + void FakeVideoReceiveStream::Start() { receiving_ = true; } @@ -402,6 +412,11 @@ void FakeFlexfecReceiveStream::SetRtpExtensions( config_.rtp.extensions = std::move(extensions); } +const std::vector& +FakeFlexfecReceiveStream::GetRtpExtensions() const { + return config_.rtp.extensions; +} + const webrtc::FlexfecReceiveStream::Config& FakeFlexfecReceiveStream::GetConfig() const { return config_; diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 338996a625..47d7b183c7 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -128,6 +128,7 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream { void SetFrameDecryptor(rtc::scoped_refptr frame_decryptor) override; void SetRtpExtensions(std::vector extensions) override; + const std::vector& GetRtpExtensions() const override; webrtc::AudioReceiveStream::Stats GetStats( bool get_and_clear_legacy_stats) const override; @@ -266,6 +267,7 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { private: // webrtc::VideoReceiveStream implementation. void SetRtpExtensions(std::vector extensions) override; + const std::vector& GetRtpExtensions() const override; const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { return config_.rtp; @@ -298,6 +300,7 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { const webrtc::FlexfecReceiveStream::Config& config); void SetRtpExtensions(std::vector extensions) override; + const std::vector& GetRtpExtensions() const override; const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { return config_.rtp; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 1672a718b1..82a30ffbf2 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1262,9 +1262,8 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { webrtc::RtpParameters GetRtpParameters() const { webrtc::RtpParameters rtp_parameters; rtp_parameters.encodings.emplace_back(); - const auto& config = stream_->rtp_config(); rtp_parameters.encodings[0].ssrc = stream_->remote_ssrc(); - rtp_parameters.header_extensions = config.extensions; + rtp_parameters.header_extensions = stream_->GetRtpExtensions(); return rtp_parameters; } diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 890edc621d..dcc2f80853 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -487,6 +487,12 @@ void VideoReceiveStream2::SetRtpExtensions( c.rtp.extensions = std::move(extensions); } +const std::vector& VideoReceiveStream2::GetRtpExtensions() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + // TODO(tommi): return the state held by `rtp_video_stream_receiver_`. + return config_.rtp.extensions; +} + void VideoReceiveStream2::CreateAndRegisterExternalDecoder( const Decoder& decoder) { TRACE_EVENT0("webrtc", diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 726e91be54..e5acf307e2 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -135,6 +135,7 @@ class VideoReceiveStream2 void Stop() override; void SetRtpExtensions(std::vector extensions) override; + const std::vector& GetRtpExtensions() const override; const RtpConfig& rtp_config() const override { return rtp(); }