diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h index 1c2d11b1af..17691f7021 100644 --- a/call/audio_receive_stream.h +++ b/call/audio_receive_stream.h @@ -166,10 +166,6 @@ class AudioReceiveStream : public MediaReceiveStream { int history_ms) = 0; virtual void SetNonSenderRttMeasurement(bool enabled) = 0; - // Set/change the rtp header extensions. Must be called on the packet - // delivery thread. - virtual void SetRtpExtensions(std::vector extensions) = 0; - // Returns true if the stream has been started. virtual bool IsRunning() const = 0; diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc index 7230a4a56d..eda5c7f05d 100644 --- a/call/flexfec_receive_stream_impl.cc +++ b/call/flexfec_receive_stream_impl.cc @@ -14,7 +14,7 @@ #include #include -#include +#include #include "api/array_view.h" #include "api/call/transport.h" @@ -184,6 +184,7 @@ void FlexfecReceiveStreamImpl::UnregisterFromTransport() { } void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); if (!receiver_) return; @@ -201,4 +202,10 @@ FlexfecReceiveStreamImpl::Stats FlexfecReceiveStreamImpl::GetStats() const { return FlexfecReceiveStream::Stats(); } +void FlexfecReceiveStreamImpl::SetRtpExtensions( + std::vector extensions) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + config_.rtp.extensions = std::move(extensions); +} + } // namespace webrtc diff --git a/call/flexfec_receive_stream_impl.h b/call/flexfec_receive_stream_impl.h index 285a33f7bb..c2407cd419 100644 --- a/call/flexfec_receive_stream_impl.h +++ b/call/flexfec_receive_stream_impl.h @@ -12,6 +12,7 @@ #define CALL_FLEXFEC_RECEIVE_STREAM_IMPL_H_ #include +#include #include "call/flexfec_receive_stream.h" #include "call/rtp_packet_sink_interface.h" @@ -58,13 +59,14 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream { Stats GetStats() const override; // ReceiveStream impl. + void SetRtpExtensions(std::vector extensions) override; const RtpConfig& rtp_config() const override { return config_.rtp; } private: RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_; - // Config. - const Config config_; + // Config. Mostly const, header extensions may change. + Config config_ RTC_GUARDED_BY(packet_sequence_checker_); // Erasure code interfacing. const std::unique_ptr receiver_; diff --git a/call/receive_stream.h b/call/receive_stream.h index 0f59b37ae3..a6756fc5c1 100644 --- a/call/receive_stream.h +++ b/call/receive_stream.h @@ -51,6 +51,10 @@ class ReceiveStream { std::vector extensions; }; + // Set/change the rtp header extensions. Must be called on the packet + // delivery thread. + virtual void SetRtpExtensions(std::vector extensions) = 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 f61aab04f1..62bcab9bfe 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -375,6 +375,11 @@ webrtc::VideoReceiveStream::Stats FakeVideoReceiveStream::GetStats() const { return stats_; } +void FakeVideoReceiveStream::SetRtpExtensions( + std::vector extensions) { + config_.rtp.extensions = std::move(extensions); +} + void FakeVideoReceiveStream::Start() { receiving_ = true; } @@ -392,6 +397,11 @@ FakeFlexfecReceiveStream::FakeFlexfecReceiveStream( const webrtc::FlexfecReceiveStream::Config& config) : config_(config) {} +void FakeFlexfecReceiveStream::SetRtpExtensions( + std::vector extensions) { + config_.rtp.extensions = std::move(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 5f6b8643db..e732379cbd 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -264,9 +264,12 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { private: // webrtc::VideoReceiveStream implementation. + void SetRtpExtensions(std::vector extensions) override; + const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { return config_.rtp; } + void Start() override; void Stop() override; @@ -293,6 +296,8 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { explicit FakeFlexfecReceiveStream( const webrtc::FlexfecReceiveStream::Config& config); + void SetRtpExtensions(std::vector extensions) override; + const webrtc::ReceiveStream::RtpConfig& rtp_config() const override { return config_.rtp; } diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 72903c627a..a630b45c9c 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3018,13 +3018,20 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters( if (params.rtp_header_extensions) { if (config_.rtp.extensions != *params.rtp_header_extensions) { config_.rtp.extensions = *params.rtp_header_extensions; - video_needs_recreation = true; + if (stream_) { + stream_->SetRtpExtensions(config_.rtp.extensions); + } else { + video_needs_recreation = true; + } } if (flexfec_config_.rtp.extensions != *params.rtp_header_extensions) { flexfec_config_.rtp.extensions = *params.rtp_header_extensions; - if (flexfec_stream_ || flexfec_config_.IsCompleteAndEnabled()) + if (flexfec_stream_) { + flexfec_stream_->SetRtpExtensions(flexfec_config_.rtp.extensions); + } else if (flexfec_config_.IsCompleteAndEnabled()) { video_needs_recreation = true; + } } } if (params.flexfec_payload_type) { diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 025a553040..be431cb8d2 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -3071,11 +3071,11 @@ TEST_F(WebRtcVideoChannelTest, IdenticalRecvExtensionsDoesntRecreateStream) { EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams()); - // Setting different extensions should recreate the stream. + // Setting different extensions should not require the stream to be recreated. recv_parameters_.extensions.resize(1); EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_)); - EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams()); + EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams()); } TEST_F(WebRtcVideoChannelTest, diff --git a/modules/rtp_rtcp/include/rtp_header_extension_map.h b/modules/rtp_rtcp/include/rtp_header_extension_map.h index 72e5541d37..a746d8a076 100644 --- a/modules/rtp_rtcp/include/rtp_header_extension_map.h +++ b/modules/rtp_rtcp/include/rtp_header_extension_map.h @@ -31,6 +31,8 @@ class RtpHeaderExtensionMap { explicit RtpHeaderExtensionMap(bool extmap_allow_mixed); explicit RtpHeaderExtensionMap(rtc::ArrayView extensions); + void Reset(rtc::ArrayView extensions); + template bool Register(int id) { return Register(id, Extension::kId, Extension::kUri); diff --git a/modules/rtp_rtcp/include/ulpfec_receiver.h b/modules/rtp_rtcp/include/ulpfec_receiver.h index bf1c8264c2..6cbae52c99 100644 --- a/modules/rtp_rtcp/include/ulpfec_receiver.h +++ b/modules/rtp_rtcp/include/ulpfec_receiver.h @@ -53,6 +53,9 @@ class UlpfecReceiver { // Returns a counter describing the added and recovered packets. virtual FecPacketCounter GetPacketCounter() const = 0; + + virtual void SetRtpExtensions( + rtc::ArrayView extensions) = 0; }; } // namespace webrtc #endif // MODULES_RTP_RTCP_INCLUDE_ULPFEC_RECEIVER_H_ diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc index d1eee22a55..ddd579344f 100644 --- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc +++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc @@ -80,6 +80,14 @@ RtpHeaderExtensionMap::RtpHeaderExtensionMap( RegisterByUri(extension.id, extension.uri); } +void RtpHeaderExtensionMap::Reset( + rtc::ArrayView extensions) { + for (auto& id : ids_) + id = kInvalidId; + for (const RtpExtension& extension : extensions) + RegisterByUri(extension.id, extension.uri); +} + bool RtpHeaderExtensionMap::RegisterByType(int id, RTPExtensionType type) { for (const ExtensionInfo& extension : kExtensions) if (type == extension.type) diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc index c993923a42..159e21f9d2 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc @@ -47,6 +47,12 @@ FecPacketCounter UlpfecReceiverImpl::GetPacketCounter() const { return packet_counter_; } +void UlpfecReceiverImpl::SetRtpExtensions( + rtc::ArrayView extensions) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + extensions_.Reset(extensions); +} + // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h index f59251f848..92e51530b8 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h @@ -41,9 +41,11 @@ class UlpfecReceiverImpl : public UlpfecReceiver { FecPacketCounter GetPacketCounter() const override; + void SetRtpExtensions(rtc::ArrayView extensions) override; + private: const uint32_t ssrc_; - const RtpHeaderExtensionMap extensions_; + RtpHeaderExtensionMap extensions_ RTC_GUARDED_BY(&sequence_checker_); RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; RecoveredPacketReceiver* const recovered_packet_callback_; diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index b4a139edb6..309d158873 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -907,6 +907,12 @@ void RtpVideoStreamReceiver2::SetDepacketizerToDecoderFrameTransformer( frame_transformer_delegate_->Init(); } +void RtpVideoStreamReceiver2::SetRtpExtensions( + const std::vector& extensions) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + rtp_header_extensions_.Reset(extensions); +} + void RtpVideoStreamReceiver2::UpdateRtt(int64_t max_rtt_ms) { RTC_DCHECK_RUN_ON(&worker_task_checker_); if (nack_module_) diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index 3a4f58e8fe..ff9e43fa6d 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -176,6 +176,10 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, void SetDepacketizerToDecoderFrameTransformer( rtc::scoped_refptr frame_transformer); + // Updates the rtp header extensions at runtime. Must be called on the + // `packet_sequence_checker_` thread. + void SetRtpExtensions(const std::vector& extensions); + // Called by VideoReceiveStream when stats are updated. void UpdateRtt(int64_t max_rtt_ms); @@ -290,7 +294,8 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, RemoteNtpTimeEstimator ntp_estimator_; - RtpHeaderExtensionMap rtp_header_extensions_; + RtpHeaderExtensionMap rtp_header_extensions_ + RTC_GUARDED_BY(packet_sequence_checker_); // Set by the field trial WebRTC-ForcePlayoutDelay to override any playout // delay that is specified in the received packets. FieldTrialOptional forced_playout_delay_max_ms_; diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index e2e2bb8175..6823a1eac2 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -456,6 +456,12 @@ void VideoReceiveStream::RemoveSecondarySink( rtp_video_stream_receiver_.RemoveSecondarySink(sink); } +void VideoReceiveStream::SetRtpExtensions( + std::vector extensions) { + // VideoReceiveStream is deprecated and this function not supported. + RTC_NOTREACHED(); +} + bool VideoReceiveStream::SetBaseMinimumPlayoutDelayMs(int delay_ms) { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); if (delay_ms < kMinBaseMinimumDelayMs || delay_ms > kMaxBaseMinimumDelayMs) { diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index 637b91a924..bbf036766c 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -93,6 +93,7 @@ class VideoReceiveStream void AddSecondarySink(RtpPacketSinkInterface* sink) override; void RemoveSecondarySink(const RtpPacketSinkInterface* sink) override; + void SetRtpExtensions(std::vector extensions) override; // SetBaseMinimumPlayoutDelayMs and GetBaseMinimumPlayoutDelayMs are called // from webrtc/api level and requested by user code. For e.g. blink/js layer diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 3127069a1c..725a9bb594 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -219,8 +219,8 @@ VideoReceiveStream2::VideoReceiveStream2( this, // NackSender nullptr, // Use default KeyFrameRequestSender this, // OnCompleteFrameCallback - config_.frame_decryptor, - config_.frame_transformer), + std::move(config_.frame_decryptor), + std::move(config_.frame_transformer)), rtp_stream_sync_(call->worker_thread(), this), max_wait_for_keyframe_ms_(DetermineMaxWaitForFrame(config, true)), max_wait_for_frame_ms_(DetermineMaxWaitForFrame(config, false)), @@ -450,6 +450,25 @@ void VideoReceiveStream2::Stop() { transport_adapter_.Disable(); } +void VideoReceiveStream2::SetRtpExtensions( + std::vector extensions) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + rtp_video_stream_receiver_.SetRtpExtensions(extensions); + // TODO(tommi): We don't use the `c.rtp.extensions` member in the + // VideoReceiveStream2 class, so this const_cast<> is a temporary hack to keep + // things consistent between VideoReceiveStream2 and RtpVideoStreamReceiver2 + // for debugging purposes. The `packet_sequence_checker_` gives us assurances + // that from a threading perspective, this is still safe. The accessors that + // give read access to this state, run behind the same check. + // The alternative to the const_cast<> would be to make `config_` non-const + // and guarded by `packet_sequence_checker_`. However the scope of that state + // is huge (the whole Config struct), and would require all methods that touch + // the struct to abide the needs of the `extensions` member. + VideoReceiveStream::Config& c = + const_cast(config_); + c.rtp.extensions = std::move(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 ace5de2dd4..cf637f8c0e 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -134,7 +134,9 @@ class VideoReceiveStream2 void Start() override; void Stop() override; - const RtpConfig& rtp_config() const override { return config_.rtp; } + void SetRtpExtensions(std::vector extensions) override; + + const RtpConfig& rtp_config() const override { return rtp(); } webrtc::VideoReceiveStream::Stats GetStats() const override;