Add ability to set rtp header extensions without recreating streams.

Setting the rtp header extensions on the packet delivery thread
(currently worker, soon to be network), is now possible without
taking the hit of deleting and recreating the receive stream (and
rtp receiver and related state).

Bug: webrtc:11993
Change-Id: I9bbe306844a25d85d79cd216092ead66eaf68960
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223741
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34953}

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<RtpExtension> extensions) = 0;
-
   // Returns true if the stream has been started.
   virtual bool IsRunning() const = 0;
 

call/flexfec_receive_stream_impl.cc

@@ -14,7 +14,7 @@
 
 #include <cstdint>
 #include <string>
-#include <vector>
+#include <utility>
 
 #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<RtpExtension> extensions) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  config_.rtp.extensions = std::move(extensions);
+}
+
 }  // namespace webrtc

call/flexfec_receive_stream_impl.h

@@ -12,6 +12,7 @@
 #define CALL_FLEXFEC_RECEIVE_STREAM_IMPL_H_
 
 #include <memory>
+#include <vector>
 
 #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<RtpExtension> 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<FlexfecReceiver> receiver_;

call/receive_stream.h

@@ -51,6 +51,10 @@ class ReceiveStream {
     std::vector<RtpExtension> extensions;
   };
 
+  // Set/change the rtp header extensions. Must be called on the packet
+  // delivery thread.
+  virtual void SetRtpExtensions(std::vector<RtpExtension> 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

media/engine/fake_webrtc_call.cc

@@ -375,6 +375,11 @@ webrtc::VideoReceiveStream::Stats FakeVideoReceiveStream::GetStats() const {
   return stats_;
 }
 
+void FakeVideoReceiveStream::SetRtpExtensions(
+    std::vector<webrtc::RtpExtension> 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<webrtc::RtpExtension> extensions) {
+  config_.rtp.extensions = std::move(extensions);
+}
+
 const webrtc::FlexfecReceiveStream::Config&
 FakeFlexfecReceiveStream::GetConfig() const {
   return config_;

media/engine/fake_webrtc_call.h

@@ -264,9 +264,12 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream {
 
  private:
   // webrtc::VideoReceiveStream implementation.
+  void SetRtpExtensions(std::vector<webrtc::RtpExtension> 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<webrtc::RtpExtension> extensions) override;
+
   const webrtc::ReceiveStream::RtpConfig& rtp_config() const override {
     return config_.rtp;
   }

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) {

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,

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<const RtpExtension> extensions);
 
+  void Reset(rtc::ArrayView<const RtpExtension> extensions);
+
   template <typename Extension>
   bool Register(int id) {
     return Register(id, Extension::kId, Extension::kUri);

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<const RtpExtension> extensions) = 0;
 };
 }  // namespace webrtc
 #endif  // MODULES_RTP_RTCP_INCLUDE_ULPFEC_RECEIVER_H_

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<const RtpExtension> 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)

modules/rtp_rtcp/source/ulpfec_receiver_impl.cc

@@ -47,6 +47,12 @@ FecPacketCounter UlpfecReceiverImpl::GetPacketCounter() const {
   return packet_counter_;
 }
 
+void UlpfecReceiverImpl::SetRtpExtensions(
+    rtc::ArrayView<const RtpExtension> 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
 //    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

modules/rtp_rtcp/source/ulpfec_receiver_impl.h

@@ -41,9 +41,11 @@ class UlpfecReceiverImpl : public UlpfecReceiver {
 
   FecPacketCounter GetPacketCounter() const override;
 
+  void SetRtpExtensions(rtc::ArrayView<const RtpExtension> 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_;

video/rtp_video_stream_receiver2.cc

@@ -907,6 +907,12 @@ void RtpVideoStreamReceiver2::SetDepacketizerToDecoderFrameTransformer(
   frame_transformer_delegate_->Init();
 }
 
+void RtpVideoStreamReceiver2::SetRtpExtensions(
+    const std::vector<RtpExtension>& 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_)

video/rtp_video_stream_receiver2.h

@@ -176,6 +176,10 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
   void SetDepacketizerToDecoderFrameTransformer(
       rtc::scoped_refptr<FrameTransformerInterface> frame_transformer);
 
+  // Updates the rtp header extensions at runtime. Must be called on the
+  // `packet_sequence_checker_` thread.
+  void SetRtpExtensions(const std::vector<RtpExtension>& 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<int> forced_playout_delay_max_ms_;

video/video_receive_stream.cc

@@ -456,6 +456,12 @@ void VideoReceiveStream::RemoveSecondarySink(
   rtp_video_stream_receiver_.RemoveSecondarySink(sink);
 }
 
+void VideoReceiveStream::SetRtpExtensions(
+    std::vector<RtpExtension> 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) {

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<RtpExtension> extensions) override;
 
   // SetBaseMinimumPlayoutDelayMs and GetBaseMinimumPlayoutDelayMs are called
   // from webrtc/api level and requested by user code. For e.g. blink/js layer

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<RtpExtension> 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<VideoReceiveStream::Config&>(config_);
+  c.rtp.extensions = std::move(extensions);
+}
+
 void VideoReceiveStream2::CreateAndRegisterExternalDecoder(
     const Decoder& decoder) {
   TRACE_EVENT0("webrtc",

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<RtpExtension> extensions) override;
+
+  const RtpConfig& rtp_config() const override { return rtp(); }
 
   webrtc::VideoReceiveStream::Stats GetStats() const override;