diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index 108794cd92..27e99f0ff2 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -84,6 +84,7 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, // webrtc::AudioReceiveStream implementation. void Start() override; void Stop() override; + const RtpConfig& rtp_config() const override { return config_.rtp; } bool IsRunning() const override; void SetDepacketizerToDecoderFrameTransformer( rtc::scoped_refptr frame_transformer) diff --git a/call/call.cc b/call/call.cc index ffe7a25e7f..273720189f 100644 --- a/call/call.cc +++ b/call/call.cc @@ -81,12 +81,10 @@ bool SendPeriodicFeedback(const std::vector& extensions) { return true; } -// TODO(nisse): This really begs for a shared context struct. -bool UseSendSideBwe(const std::vector& extensions, - bool transport_cc) { - if (!transport_cc) +bool UseSendSideBwe(const ReceiveStream::RtpConfig& rtp) { + if (!rtp.transport_cc) return false; - for (const auto& extension : extensions) { + for (const auto& extension : rtp.extensions) { if (extension.uri == RtpExtension::kTransportSequenceNumberUri || extension.uri == RtpExtension::kTransportSequenceNumberV2Uri) return true; @@ -94,18 +92,6 @@ bool UseSendSideBwe(const std::vector& extensions, return false; } -bool UseSendSideBwe(const VideoReceiveStream::Config& config) { - return UseSendSideBwe(config.rtp.extensions, config.rtp.transport_cc); -} - -bool UseSendSideBwe(const AudioReceiveStream::Config& config) { - return UseSendSideBwe(config.rtp.extensions, config.rtp.transport_cc); -} - -bool UseSendSideBwe(const FlexfecReceiveStream::Config& config) { - return UseSendSideBwe(config.rtp.extensions, config.rtp.transport_cc); -} - const int* FindKeyByValue(const std::map& m, int v) { for (const auto& kv : m) { if (kv.second == v) @@ -410,33 +396,9 @@ class Call final : public webrtc::Call, // This extra map is used for receive processing which is // independent of media type. - // TODO(nisse): In the RTP transport refactoring, we should have a - // single mapping from ssrc to a more abstract receive stream, with - // accessor methods for all configuration we need at this level. - struct ReceiveRtpConfig { - explicit ReceiveRtpConfig(const webrtc::AudioReceiveStream::Config& config) - : extensions(config.rtp.extensions), - use_send_side_bwe(UseSendSideBwe(config)) {} - explicit ReceiveRtpConfig(const webrtc::VideoReceiveStream::Config& config) - : extensions(config.rtp.extensions), - use_send_side_bwe(UseSendSideBwe(config)) {} - explicit ReceiveRtpConfig(const FlexfecReceiveStream::Config& config) - : extensions(config.rtp.extensions), - use_send_side_bwe(UseSendSideBwe(config)) {} - - // Registered RTP header extensions for each stream. Note that RTP header - // extensions are negotiated per track ("m= line") in the SDP, but we have - // no notion of tracks at the Call level. We therefore store the RTP header - // extensions per SSRC instead, which leads to some storage overhead. - const RtpHeaderExtensionMap extensions; - // Set if both RTP extension the RTCP feedback message needed for - // send side BWE are negotiated. - const bool use_send_side_bwe; - }; - // TODO(bugs.webrtc.org/11993): Move receive_rtp_config_ over to the // network thread. - std::map receive_rtp_config_ + std::map receive_rtp_config_ RTC_GUARDED_BY(worker_thread_); // Audio and Video send streams are owned by the client that creates them. @@ -995,7 +957,7 @@ webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream( // TODO(bugs.webrtc.org/11993): Update the below on the network thread. // We could possibly set up the audio_receiver_controller_ association up // as part of the async setup. - receive_rtp_config_.emplace(config.rtp.remote_ssrc, ReceiveRtpConfig(config)); + receive_rtp_config_.emplace(config.rtp.remote_ssrc, receive_stream); ConfigureSync(config.sync_group); @@ -1023,9 +985,7 @@ void Call::DestroyAudioReceiveStream( uint32_t ssrc = audio_receive_stream->remote_ssrc(); const AudioReceiveStream::Config& config = audio_receive_stream->config(); - receive_side_cc_ - .GetRemoteBitrateEstimator( - UseSendSideBwe(config.rtp.extensions, config.rtp.transport_cc)) + receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(config.rtp)) ->RemoveStream(ssrc); audio_receive_streams_.erase(audio_receive_stream); @@ -1177,9 +1137,9 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( // stream. Since the transport_send_cc negotiation is per payload // type, we may get an incorrect value for the rtx stream, but // that is unlikely to matter in practice. - receive_rtp_config_.emplace(config.rtp.rtx_ssrc, ReceiveRtpConfig(config)); + receive_rtp_config_.emplace(config.rtp.rtx_ssrc, receive_stream); } - receive_rtp_config_.emplace(config.rtp.remote_ssrc, ReceiveRtpConfig(config)); + receive_rtp_config_.emplace(config.rtp.remote_ssrc, receive_stream); video_receive_streams_.insert(receive_stream); ConfigureSync(config.sync_group); @@ -1211,7 +1171,7 @@ void Call::DestroyVideoReceiveStream( video_receive_streams_.erase(receive_stream_impl); ConfigureSync(config.sync_group); - receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(config)) + receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(config.rtp)) ->RemoveStream(config.rtp.remote_ssrc); UpdateAggregateNetworkState(); @@ -1243,7 +1203,7 @@ FlexfecReceiveStream* Call::CreateFlexfecReceiveStream( RTC_DCHECK(receive_rtp_config_.find(config.rtp.remote_ssrc) == receive_rtp_config_.end()); - receive_rtp_config_.emplace(config.rtp.remote_ssrc, ReceiveRtpConfig(config)); + receive_rtp_config_.emplace(config.rtp.remote_ssrc, receive_stream); // TODO(brandtr): Store config in RtcEventLog here. @@ -1260,14 +1220,13 @@ void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) { receive_stream_impl->UnregisterFromTransport(); RTC_DCHECK(receive_stream != nullptr); - const FlexfecReceiveStream::Config& config = receive_stream->GetConfig(); - uint32_t ssrc = config.rtp.remote_ssrc; - receive_rtp_config_.erase(ssrc); + const FlexfecReceiveStream::RtpConfig& rtp = receive_stream->rtp_config(); + receive_rtp_config_.erase(rtp.remote_ssrc); // Remove all SSRCs pointing to the FlexfecReceiveStreamImpl to be // destroyed. - receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(config)) - ->RemoveStream(ssrc); + receive_side_cc_.GetRemoteBitrateEstimator(UseSendSideBwe(rtp)) + ->RemoveStream(rtp.remote_ssrc); delete receive_stream; } @@ -1591,7 +1550,8 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type, return DELIVERY_UNKNOWN_SSRC; } - parsed_packet.IdentifyExtensions(it->second.extensions); + parsed_packet.IdentifyExtensions( + RtpHeaderExtensionMap(it->second->rtp_config().extensions)); NotifyBweOfReceivedPacket(parsed_packet, media_type); @@ -1656,7 +1616,8 @@ void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) { // which is being torn down. return; } - parsed_packet.IdentifyExtensions(it->second.extensions); + parsed_packet.IdentifyExtensions( + RtpHeaderExtensionMap(it->second->rtp_config().extensions)); // TODO(brandtr): Update here when we support protecting audio packets too. parsed_packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); @@ -1667,8 +1628,8 @@ void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) { void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet, MediaType media_type) { auto it = receive_rtp_config_.find(packet.Ssrc()); - bool use_send_side_bwe = - (it != receive_rtp_config_.end()) && it->second.use_send_side_bwe; + bool use_send_side_bwe = (it != receive_rtp_config_.end()) && + UseSendSideBwe(it->second->rtp_config()); RTPHeader header; packet.GetHeader(&header); diff --git a/call/flexfec_receive_stream.h b/call/flexfec_receive_stream.h index 0c9d5519e5..72e544e7ec 100644 --- a/call/flexfec_receive_stream.h +++ b/call/flexfec_receive_stream.h @@ -68,8 +68,6 @@ class FlexfecReceiveStream : public RtpPacketSinkInterface, }; virtual Stats GetStats() const = 0; - - virtual const Config& GetConfig() const = 0; }; } // namespace webrtc diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc index 8368ff495f..ab82a6d71a 100644 --- a/call/flexfec_receive_stream_impl.cc +++ b/call/flexfec_receive_stream_impl.cc @@ -205,9 +205,4 @@ FlexfecReceiveStreamImpl::Stats FlexfecReceiveStreamImpl::GetStats() const { return FlexfecReceiveStream::Stats(); } -const FlexfecReceiveStream::Config& FlexfecReceiveStreamImpl::GetConfig() - const { - return config_; -} - } // namespace webrtc diff --git a/call/flexfec_receive_stream_impl.h b/call/flexfec_receive_stream_impl.h index 8826aedad9..12c4b04332 100644 --- a/call/flexfec_receive_stream_impl.h +++ b/call/flexfec_receive_stream_impl.h @@ -59,7 +59,9 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream { void OnRtpPacket(const RtpPacketReceived& packet) override; Stats GetStats() const override; - const Config& GetConfig() const override; + + // ReceiveStream impl. + const RtpConfig& rtp_config() const override { return config_.rtp; } private: RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_; diff --git a/call/receive_stream.h b/call/receive_stream.h index 9f1f718dec..0f59b37ae3 100644 --- a/call/receive_stream.h +++ b/call/receive_stream.h @@ -28,21 +28,35 @@ class ReceiveStream { // Receive-stream specific RTP settings. struct RtpConfig { // Synchronization source (stream identifier) to be received. + // This member will not change mid-stream and can be assumed to be const + // post initialization. uint32_t remote_ssrc = 0; // Sender SSRC used for sending RTCP (such as receiver reports). + // This value may change mid-stream and must be done on the same thread + // that the value is read on (i.e. packet delivery). uint32_t local_ssrc = 0; // Enable feedback for send side bandwidth estimation. // See // https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions // for details. + // This value may change mid-stream and must be done on the same thread + // that the value is read on (i.e. packet delivery). bool transport_cc = false; // RTP header extensions used for the received stream. + // This value may change mid-stream and must be done on the same thread + // that the value is read on (i.e. packet delivery). std::vector extensions; }; + // 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 + // only be used in the calling context (on the stack basically). + virtual const RtpConfig& rtp_config() const = 0; + protected: virtual ~ReceiveStream() {} }; diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index a2273c4002..5a14157082 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -101,6 +101,9 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream { } private: + const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { + return config_.rtp; + } void Start() override { started_ = true; } void Stop() override { started_ = false; } bool IsRunning() const override { return started_; } @@ -250,6 +253,9 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { private: // webrtc::VideoReceiveStream implementation. + const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { + return config_.rtp; + } void Start() override; void Stop() override; @@ -276,7 +282,11 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { explicit FakeFlexfecReceiveStream( const webrtc::FlexfecReceiveStream::Config& config); - const webrtc::FlexfecReceiveStream::Config& GetConfig() const override; + const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { + return config_.rtp; + } + + const webrtc::FlexfecReceiveStream::Config& GetConfig() const; private: webrtc::FlexfecReceiveStream::Stats GetStats() const override; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 1e9fb6edfe..89cc1f4d9f 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -4164,7 +4164,7 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetDefaultRecvCodecsWithSsrc) { const std::vector& streams = fake_call_->GetFlexfecReceiveStreams(); ASSERT_EQ(1U, streams.size()); - const FakeFlexfecReceiveStream* stream = streams.front(); + const auto* stream = streams.front(); const webrtc::FlexfecReceiveStream::Config& config = stream->GetConfig(); EXPECT_EQ(GetEngineCodec("flexfec-03").id, config.payload_type); EXPECT_EQ(kFlexfecSsrc, config.rtp.remote_ssrc); @@ -4280,7 +4280,7 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, DuplicateFlexfecCodecIsDropped) { const std::vector& streams = fake_call_->GetFlexfecReceiveStreams(); ASSERT_EQ(1U, streams.size()); - const FakeFlexfecReceiveStream* stream = streams.front(); + const auto* stream = streams.front(); const webrtc::FlexfecReceiveStream::Config& config = stream->GetConfig(); EXPECT_EQ(GetEngineCodec("flexfec-03").id, config.payload_type); } @@ -5126,7 +5126,7 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetRecvParamsWithoutFecDisablesFec) { ASSERT_EQ(1U, streams.size()); const FakeFlexfecReceiveStream* stream = streams.front(); EXPECT_EQ(GetEngineCodec("flexfec-03").id, stream->GetConfig().payload_type); - EXPECT_EQ(kFlexfecSsrc, stream->GetConfig().rtp.remote_ssrc); + EXPECT_EQ(kFlexfecSsrc, stream->rtp_config().remote_ssrc); ASSERT_EQ(1U, stream->GetConfig().protected_media_ssrcs.size()); EXPECT_EQ(kSsrcs1[0], stream->GetConfig().protected_media_ssrcs[0]); diff --git a/modules/rtp_rtcp/source/rtp_packet.cc b/modules/rtp_rtcp/source/rtp_packet.cc index 1c2331dec9..dcf773952b 100644 --- a/modules/rtp_rtcp/source/rtp_packet.cc +++ b/modules/rtp_rtcp/source/rtp_packet.cc @@ -71,8 +71,8 @@ RtpPacket::RtpPacket(const ExtensionManager* extensions, size_t capacity) RtpPacket::~RtpPacket() {} -void RtpPacket::IdentifyExtensions(const ExtensionManager& extensions) { - extensions_ = extensions; +void RtpPacket::IdentifyExtensions(ExtensionManager extensions) { + extensions_ = std::move(extensions); } bool RtpPacket::Parse(const uint8_t* buffer, size_t buffer_size) { diff --git a/modules/rtp_rtcp/source/rtp_packet.h b/modules/rtp_rtcp/source/rtp_packet.h index aa854f35ab..1c31f92ce9 100644 --- a/modules/rtp_rtcp/source/rtp_packet.h +++ b/modules/rtp_rtcp/source/rtp_packet.h @@ -51,7 +51,7 @@ class RtpPacket { bool Parse(rtc::CopyOnWriteBuffer packet); // Maps extensions id to their types. - void IdentifyExtensions(const ExtensionManager& extensions); + void IdentifyExtensions(ExtensionManager extensions); // Header. bool Marker() const { return marker_; } diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index 9bea0b8fb9..c778d74558 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -87,6 +87,8 @@ class VideoReceiveStream void Start() override; void Stop() override; + const RtpConfig& rtp_config() const override { return config_.rtp; } + webrtc::VideoReceiveStream::Stats GetStats() const override; void AddSecondarySink(RtpPacketSinkInterface* sink) override; diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index dbd46cc66e..8fd995084e 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -127,6 +127,8 @@ class VideoReceiveStream2 void Start() override; void Stop() override; + const RtpConfig& rtp_config() const override { return config_.rtp; } + webrtc::VideoReceiveStream::Stats GetStats() const override; // SetBaseMinimumPlayoutDelayMs and GetBaseMinimumPlayoutDelayMs are called