Add GetRtpExtensionMap to ReceiveStream and remove GetRtpExtensions.

GetRtpExtensions() is still used in one corner case for audio receive
streams, so GetRtpExtensions has migrated to AudioReceiveStream.

Updated FlexfecReceiveStream config management (incl. pass by value) and
now store an RtpHeaderExtensionMap in FlexfecReceiveStreamImpl.

Call GetRtpExtensionMap() from call.cc instead of constructing one on
the fly for each rtp packet (for video packets at least).

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

audio/audio_receive_stream.cc

@@ -275,6 +275,10 @@ const std::vector<RtpExtension>& AudioReceiveStream::GetRtpExtensions() const {
   return config_.rtp.extensions;
 }
 
+RtpHeaderExtensionMap AudioReceiveStream::GetRtpExtensionMap() const {
+  return RtpHeaderExtensionMap(config_.rtp.extensions);
+}
+
 webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats(
     bool get_and_clear_legacy_stats) const {
   RTC_DCHECK_RUN_ON(&worker_thread_checker_);

audio/audio_receive_stream.h

@@ -96,6 +96,7 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream,
                              frame_decryptor) override;
   void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
   const std::vector<RtpExtension>& GetRtpExtensions() const override;
+  RtpHeaderExtensionMap GetRtpExtensionMap() const override;
 
   webrtc::AudioReceiveStream::Stats GetStats(
       bool get_and_clear_legacy_stats) const override;

call/BUILD.gn

@@ -351,6 +351,7 @@ rtc_source_set("receive_stream_interface") {
     "../api:scoped_refptr",
     "../api/crypto:frame_decryptor_interface",
     "../api/transport/rtp:rtp_source",
+    "../modules/rtp_rtcp:rtp_rtcp_format",
   ]
 }
 

call/audio_receive_stream.h

@@ -199,6 +199,12 @@ class AudioReceiveStream : public MediaReceiveStream {
   // post initialization.
   virtual uint32_t remote_ssrc() const = 0;
 
+  // Access the currently set rtp extensions. Must be called on the packet
+  // delivery thread.
+  // TODO(tommi): This is currently only called from
+  // `WebRtcAudioReceiveStream::GetRtpParameters()`. See if we can remove it.
+  virtual const std::vector<RtpExtension>& GetRtpExtensions() const = 0;
+
  protected:
   virtual ~AudioReceiveStream() {}
 };

call/call.cc

@@ -79,15 +79,14 @@ bool SendPeriodicFeedback(const std::vector<RtpExtension>& extensions) {
   return true;
 }
 
+bool HasTransportSequenceNumber(const RtpHeaderExtensionMap& map) {
+  return map.IsRegistered(kRtpExtensionTransportSequenceNumber) ||
+         map.IsRegistered(kRtpExtensionTransportSequenceNumber02);
+}
+
 bool UseSendSideBwe(const ReceiveStream* stream) {
-  if (!stream->transport_cc())
-    return false;
-  for (const auto& extension : stream->GetRtpExtensions()) {
-    if (extension.uri == RtpExtension::kTransportSequenceNumberUri ||
-        extension.uri == RtpExtension::kTransportSequenceNumberV2Uri)
-      return true;
-  }
-  return false;
+  return stream->transport_cc() &&
+         HasTransportSequenceNumber(stream->GetRtpExtensionMap());
 }
 
 const int* FindKeyByValue(const std::map<int, int>& m, int v) {
@@ -237,7 +236,7 @@ class Call final : public webrtc::Call,
       webrtc::VideoReceiveStream* receive_stream) override;
 
   FlexfecReceiveStream* CreateFlexfecReceiveStream(
-      const FlexfecReceiveStream::Config& config) override;
+      const FlexfecReceiveStream::Config config) override;
   void DestroyFlexfecReceiveStream(
       FlexfecReceiveStream* receive_stream) override;
 
@@ -1206,27 +1205,23 @@ void Call::DestroyVideoReceiveStream(
 }
 
 FlexfecReceiveStream* Call::CreateFlexfecReceiveStream(
-    const FlexfecReceiveStream::Config& config) {
+    const FlexfecReceiveStream::Config config) {
   TRACE_EVENT0("webrtc", "Call::CreateFlexfecReceiveStream");
   RTC_DCHECK_RUN_ON(worker_thread_);
 
-  RecoveredPacketReceiver* recovered_packet_receiver = this;
-
-  FlexfecReceiveStreamImpl* receive_stream;
-
   // Unlike the video and audio receive streams, FlexfecReceiveStream implements
   // RtpPacketSinkInterface itself, and hence its constructor passes its `this`
   // pointer to video_receiver_controller_->CreateStream(). Calling the
   // constructor while on the worker thread ensures that we don't call
   // OnRtpPacket until the constructor is finished and the object is
   // in a valid state, since OnRtpPacket runs on the same thread.
-  receive_stream = new FlexfecReceiveStreamImpl(
-      clock_, config, recovered_packet_receiver, call_stats_->AsRtcpRttStats());
+  FlexfecReceiveStreamImpl* receive_stream = new FlexfecReceiveStreamImpl(
+      clock_, std::move(config), this, call_stats_->AsRtcpRttStats());
 
   // TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network
   // thread.
   receive_stream->RegisterWithTransport(&video_receiver_controller_);
-  RegisterReceiveStream(config.rtp.remote_ssrc, receive_stream);
+  RegisterReceiveStream(receive_stream->remote_ssrc(), receive_stream);
 
   // TODO(brandtr): Store config in RtcEventLog here.
 
@@ -1689,8 +1684,7 @@ bool Call::IdentifyReceivedPacket(RtpPacketReceived& packet,
     return false;
   }
 
-  packet.IdentifyExtensions(
-      RtpHeaderExtensionMap(it->second->GetRtpExtensions()));
+  packet.IdentifyExtensions(it->second->GetRtpExtensionMap());
 
   if (use_send_side_bwe) {
     *use_send_side_bwe = UseSendSideBwe(it->second);

call/call.h

@@ -127,7 +127,7 @@ class Call {
   // protected by a FlexfecReceiveStream, the latter should be created before
   // the former.
   virtual FlexfecReceiveStream* CreateFlexfecReceiveStream(
-      const FlexfecReceiveStream::Config& config) = 0;
+      const FlexfecReceiveStream::Config config) = 0;
   virtual void DestroyFlexfecReceiveStream(
       FlexfecReceiveStream* receive_stream) = 0;
 

call/degraded_call.cc

@@ -247,8 +247,8 @@ void DegradedCall::DestroyVideoReceiveStream(
 }
 
 FlexfecReceiveStream* DegradedCall::CreateFlexfecReceiveStream(
-    const FlexfecReceiveStream::Config& config) {
-  return call_->CreateFlexfecReceiveStream(config);
+    const FlexfecReceiveStream::Config config) {
+  return call_->CreateFlexfecReceiveStream(std::move(config));
 }
 
 void DegradedCall::DestroyFlexfecReceiveStream(

call/degraded_call.h

@@ -78,7 +78,7 @@ class DegradedCall : public Call, private PacketReceiver {
   void DestroyVideoReceiveStream(VideoReceiveStream* receive_stream) override;
 
   FlexfecReceiveStream* CreateFlexfecReceiveStream(
-      const FlexfecReceiveStream::Config& config) override;
+      const FlexfecReceiveStream::Config config) override;
   void DestroyFlexfecReceiveStream(
       FlexfecReceiveStream* receive_stream) override;
 

call/flexfec_receive_stream_impl.cc

@@ -137,10 +137,11 @@ std::unique_ptr<ModuleRtpRtcpImpl2> CreateRtpRtcpModule(
 
 FlexfecReceiveStreamImpl::FlexfecReceiveStreamImpl(
     Clock* clock,
-    const Config& config,
+    Config config,
     RecoveredPacketReceiver* recovered_packet_receiver,
     RtcpRttStats* rtt_stats)
-    : config_(config),
+    : extension_map_(std::move(config.rtp.extensions)),
+      config_(std::move(config)),
       receiver_(MaybeCreateFlexfecReceiver(clock,
                                            config_,
                                            recovered_packet_receiver)),
@@ -174,7 +175,7 @@ void FlexfecReceiveStreamImpl::RegisterWithTransport(
   // here at all, we'd then delete the OnRtpPacket method and instead register
   // `receiver_` as the RtpPacketSinkInterface for this stream.
   rtp_stream_receiver_ =
-      receiver_controller->CreateReceiver(config_.rtp.remote_ssrc, this);
+      receiver_controller->CreateReceiver(remote_ssrc(), this);
 }
 
 void FlexfecReceiveStreamImpl::UnregisterFromTransport() {
@@ -190,7 +191,7 @@ void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) {
   receiver_->OnRtpPacket(packet);
 
   // Do not report media packets in the RTCP RRs generated by `rtp_rtcp_`.
-  if (packet.Ssrc() == config_.rtp.remote_ssrc) {
+  if (packet.Ssrc() == remote_ssrc()) {
     rtp_receive_statistics_->OnRtpPacket(packet);
   }
 }
@@ -204,16 +205,12 @@ FlexfecReceiveStreamImpl::Stats FlexfecReceiveStreamImpl::GetStats() const {
 void FlexfecReceiveStreamImpl::SetRtpExtensions(
     std::vector<RtpExtension> extensions) {
   RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
-  // TODO(tommi): Remove this cast once header extensions are managed outside
-  // of the config struct.
-  const_cast<std::vector<RtpExtension>&>(config_.rtp.extensions) =
-      std::move(extensions);
+  extension_map_.Reset(extensions);
 }
 
-const std::vector<RtpExtension>& FlexfecReceiveStreamImpl::GetRtpExtensions()
-    const {
+RtpHeaderExtensionMap FlexfecReceiveStreamImpl::GetRtpExtensionMap() const {
   RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
-  return config_.rtp.extensions;
+  return extension_map_;
 }
 
 }  // namespace webrtc

call/flexfec_receive_stream_impl.h

@@ -34,7 +34,7 @@ class RtpStreamReceiverInterface;
 class FlexfecReceiveStreamImpl : public FlexfecReceiveStream {
  public:
   FlexfecReceiveStreamImpl(Clock* clock,
-                           const Config& config,
+                           Config config,
                            RecoveredPacketReceiver* recovered_packet_receiver,
                            RtcpRttStats* rtt_stats);
   // Destruction happens on the worker thread. Prior to destruction the caller
@@ -60,7 +60,8 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream {
 
   // ReceiveStream impl.
   void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
-  const std::vector<RtpExtension>& GetRtpExtensions() const override;
+  RtpHeaderExtensionMap GetRtpExtensionMap() const override;
+
   uint32_t remote_ssrc() const { return config_.rtp.remote_ssrc; }
   bool transport_cc() const override {
     RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
@@ -70,6 +71,8 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream {
  private:
   RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_;
 
+  RtpHeaderExtensionMap extension_map_;
+
   // Config. Mostly const, header extensions may change, which is an exception
   // case that's specifically handled in `SetRtpExtensions`, which must be
   // called on the `packet_sequence_checker` thread.

call/receive_stream.h

@@ -18,6 +18,7 @@
 #include "api/media_types.h"
 #include "api/scoped_refptr.h"
 #include "api/transport/rtp/rtp_source.h"
+#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
 
 namespace webrtc {
 
@@ -56,11 +57,7 @@ class ReceiveStream {
   // Set/change the rtp header extensions. Must be called on the packet
   // delivery thread.
   virtual void SetRtpExtensions(std::vector<RtpExtension> 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<RtpExtension>& GetRtpExtensions() const = 0;
+  virtual RtpHeaderExtensionMap GetRtpExtensionMap() const = 0;
 
   // Returns a bool for whether feedback for send side bandwidth estimation is
   // enabled. See

media/engine/fake_webrtc_call.cc

@@ -135,6 +135,11 @@ FakeAudioReceiveStream::GetRtpExtensions() const {
   return config_.rtp.extensions;
 }
 
+webrtc::RtpHeaderExtensionMap FakeAudioReceiveStream::GetRtpExtensionMap()
+    const {
+  return webrtc::RtpHeaderExtensionMap(config_.rtp.extensions);
+}
+
 webrtc::AudioReceiveStream::Stats FakeAudioReceiveStream::GetStats(
     bool get_and_clear_legacy_stats) const {
   return stats_;
@@ -385,9 +390,9 @@ void FakeVideoReceiveStream::SetRtpExtensions(
   config_.rtp.extensions = std::move(extensions);
 }
 
-const std::vector<webrtc::RtpExtension>&
-FakeVideoReceiveStream::GetRtpExtensions() const {
-  return config_.rtp.extensions;
+webrtc::RtpHeaderExtensionMap FakeVideoReceiveStream::GetRtpExtensionMap()
+    const {
+  return webrtc::RtpHeaderExtensionMap(config_.rtp.extensions);
 }
 
 void FakeVideoReceiveStream::Start() {
@@ -404,17 +409,17 @@ void FakeVideoReceiveStream::SetStats(
 }
 
 FakeFlexfecReceiveStream::FakeFlexfecReceiveStream(
-    const webrtc::FlexfecReceiveStream::Config& config)
-    : config_(config) {}
+    const webrtc::FlexfecReceiveStream::Config config)
+    : config_(std::move(config)) {}
 
 void FakeFlexfecReceiveStream::SetRtpExtensions(
     std::vector<webrtc::RtpExtension> extensions) {
   config_.rtp.extensions = std::move(extensions);
 }
 
-const std::vector<webrtc::RtpExtension>&
-FakeFlexfecReceiveStream::GetRtpExtensions() const {
-  return config_.rtp.extensions;
+webrtc::RtpHeaderExtensionMap FakeFlexfecReceiveStream::GetRtpExtensionMap()
+    const {
+  return webrtc::RtpHeaderExtensionMap(config_.rtp.extensions);
 }
 
 const webrtc::FlexfecReceiveStream::Config&
@@ -600,8 +605,9 @@ void FakeCall::DestroyVideoReceiveStream(
 }
 
 webrtc::FlexfecReceiveStream* FakeCall::CreateFlexfecReceiveStream(
-    const webrtc::FlexfecReceiveStream::Config& config) {
-  FakeFlexfecReceiveStream* fake_stream = new FakeFlexfecReceiveStream(config);
+    const webrtc::FlexfecReceiveStream::Config config) {
+  FakeFlexfecReceiveStream* fake_stream =
+      new FakeFlexfecReceiveStream(std::move(config));
   flexfec_receive_streams_.push_back(fake_stream);
   ++num_created_receive_streams_;
   return fake_stream;

media/engine/fake_webrtc_call.h

@@ -127,6 +127,7 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream {
                              frame_decryptor) override;
   void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
   const std::vector<webrtc::RtpExtension>& GetRtpExtensions() const override;
+  webrtc::RtpHeaderExtensionMap GetRtpExtensionMap() const override;
 
   webrtc::AudioReceiveStream::Stats GetStats(
       bool get_and_clear_legacy_stats) const override;
@@ -265,7 +266,7 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream {
  private:
   // webrtc::VideoReceiveStream implementation.
   void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
-  const std::vector<webrtc::RtpExtension>& GetRtpExtensions() const override;
+  webrtc::RtpHeaderExtensionMap GetRtpExtensionMap() const override;
   bool transport_cc() const override { return config_.rtp.transport_cc; }
 
   void Start() override;
@@ -292,10 +293,10 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream {
 class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream {
  public:
   explicit FakeFlexfecReceiveStream(
-      const webrtc::FlexfecReceiveStream::Config& config);
+      const webrtc::FlexfecReceiveStream::Config config);
 
   void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
-  const std::vector<webrtc::RtpExtension>& GetRtpExtensions() const override;
+  webrtc::RtpHeaderExtensionMap GetRtpExtensionMap() const override;
   bool transport_cc() const override { return config_.rtp.transport_cc; }
 
   const webrtc::FlexfecReceiveStream::Config& GetConfig() const;
@@ -381,7 +382,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
       webrtc::VideoReceiveStream* receive_stream) override;
 
   webrtc::FlexfecReceiveStream* CreateFlexfecReceiveStream(
-      const webrtc::FlexfecReceiveStream::Config& config) override;
+      const webrtc::FlexfecReceiveStream::Config config) override;
   void DestroyFlexfecReceiveStream(
       webrtc::FlexfecReceiveStream* receive_stream) override;
 

video/rtp_video_stream_receiver2.cc

@@ -912,6 +912,11 @@ void RtpVideoStreamReceiver2::SetRtpExtensions(
   rtp_header_extensions_.Reset(extensions);
 }
 
+const RtpHeaderExtensionMap& RtpVideoStreamReceiver2::GetRtpExtensions() const {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  return rtp_header_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

@@ -179,6 +179,7 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
   // Updates the rtp header extensions at runtime. Must be called on the
   // `packet_sequence_checker_` thread.
   void SetRtpExtensions(const std::vector<RtpExtension>& extensions);
+  const RtpHeaderExtensionMap& GetRtpExtensions() const;
 
   // Called by VideoReceiveStream when stats are updated.
   void UpdateRtt(int64_t max_rtt_ms);

video/video_receive_stream2.cc

@@ -481,10 +481,9 @@ void VideoReceiveStream2::SetRtpExtensions(
   c.rtp.extensions = std::move(extensions);
 }
 
-const std::vector<RtpExtension>& VideoReceiveStream2::GetRtpExtensions() const {
+RtpHeaderExtensionMap VideoReceiveStream2::GetRtpExtensionMap() const {
   RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
-  // TODO(tommi): return the state held by `rtp_video_stream_receiver_`.
-  return config_.rtp.extensions;
+  return rtp_video_stream_receiver_.GetRtpExtensions();
 }
 
 void VideoReceiveStream2::CreateAndRegisterExternalDecoder(

video/video_receive_stream2.h

@@ -136,7 +136,7 @@ class VideoReceiveStream2
   void Stop() override;
 
   void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
-  const std::vector<RtpExtension>& GetRtpExtensions() const override;
+  RtpHeaderExtensionMap GetRtpExtensionMap() const override;
   bool transport_cc() const override { return config_.rtp.transport_cc; }
 
   webrtc::VideoReceiveStream::Stats GetStats() const override;