Don't recreate the audio receive stream when updating the local_ssrc.

Bug: webrtc:11993
Change-Id: Ic5d8a8a8b7c12fb1d906e0b3cbdf657fd9e8eafc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222042
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34299}

modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -34,6 +34,7 @@ class MockRtpRtcpInterface : public RtpRtcpInterface {
               (const uint8_t* incoming_packet, size_t packet_length),
               (override));
   MOCK_METHOD(void, SetRemoteSSRC, (uint32_t ssrc), (override));
+  MOCK_METHOD(void, SetLocalSsrc, (uint32_t ssrc), (override));
   MOCK_METHOD(void, SetMaxRtpPacketSize, (size_t size), (override));
   MOCK_METHOD(size_t, MaxRtpPacketSize, (), (const, override));
   MOCK_METHOD(void,

modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -39,6 +39,7 @@
 #include "modules/rtp_rtcp/source/rtcp_packet/tmmbr.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
+#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
 #include "modules/rtp_rtcp/source/time_util.h"
 #include "modules/rtp_rtcp/source/tmmbr_help.h"
 #include "rtc_base/checks.h"
@@ -84,8 +85,14 @@ bool ResetTimestampIfExpired(const Timestamp now,
 
 }  // namespace
 
+constexpr size_t RTCPReceiver::RegisteredSsrcs::kMediaSsrcIndex;
+constexpr size_t RTCPReceiver::RegisteredSsrcs::kMaxSsrcs;
+
 RTCPReceiver::RegisteredSsrcs::RegisteredSsrcs(
-    const RtpRtcpInterface::Configuration& config) {
+    bool disable_sequence_checker,
+    const RtpRtcpInterface::Configuration& config)
+    : packet_sequence_checker_(disable_sequence_checker) {
+  packet_sequence_checker_.Detach();
   ssrcs_.push_back(config.local_media_ssrc);
   if (config.rtx_send_ssrc) {
     ssrcs_.push_back(*config.rtx_send_ssrc);
@@ -100,6 +107,21 @@ RTCPReceiver::RegisteredSsrcs::RegisteredSsrcs(
   RTC_DCHECK_LE(ssrcs_.size(), RTCPReceiver::RegisteredSsrcs::kMaxSsrcs);
 }
 
+bool RTCPReceiver::RegisteredSsrcs::contains(uint32_t ssrc) const {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  return absl::c_linear_search(ssrcs_, ssrc);
+}
+
+uint32_t RTCPReceiver::RegisteredSsrcs::media_ssrc() const {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  return ssrcs_[kMediaSsrcIndex];
+}
+
+void RTCPReceiver::RegisteredSsrcs::set_media_ssrc(uint32_t ssrc) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  ssrcs_[kMediaSsrcIndex] = ssrc;
+}
+
 struct RTCPReceiver::PacketInformation {
   uint32_t packet_type_flags = 0;  // RTCPPacketTypeFlags bit field.
 
@@ -117,12 +139,12 @@ struct RTCPReceiver::PacketInformation {
 };
 
 RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
-                           ModuleRtpRtcp* owner)
+                           ModuleRtpRtcpImpl2* owner)
     : clock_(config.clock),
       receiver_only_(config.receiver_only),
       rtp_rtcp_(owner),
       main_ssrc_(config.local_media_ssrc),
-      registered_ssrcs_(config),
+      registered_ssrcs_(false, config),
       rtcp_bandwidth_observer_(config.bandwidth_callback),
       rtcp_intra_frame_observer_(config.intra_frame_callback),
       rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer),
@@ -150,6 +172,53 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
   RTC_DCHECK(owner);
 }
 
+RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
+                           ModuleRtpRtcp* owner)
+    : clock_(config.clock),
+      receiver_only_(config.receiver_only),
+      rtp_rtcp_(owner),
+      main_ssrc_(config.local_media_ssrc),
+      registered_ssrcs_(true, config),
+      rtcp_bandwidth_observer_(config.bandwidth_callback),
+      rtcp_intra_frame_observer_(config.intra_frame_callback),
+      rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer),
+      network_state_estimate_observer_(config.network_state_estimate_observer),
+      transport_feedback_observer_(config.transport_feedback_callback),
+      bitrate_allocation_observer_(config.bitrate_allocation_observer),
+      report_interval_(config.rtcp_report_interval_ms > 0
+                           ? TimeDelta::Millis(config.rtcp_report_interval_ms)
+                           : (config.audio ? kDefaultAudioReportInterval
+                                           : kDefaultVideoReportInterval)),
+      // TODO(bugs.webrtc.org/10774): Remove fallback.
+      remote_ssrc_(0),
+      remote_sender_rtp_time_(0),
+      remote_sender_packet_count_(0),
+      remote_sender_octet_count_(0),
+      remote_sender_reports_count_(0),
+      xr_rrtr_status_(config.non_sender_rtt_measurement),
+      xr_rr_rtt_ms_(0),
+      oldest_tmmbr_info_ms_(0),
+      cname_callback_(config.rtcp_cname_callback),
+      report_block_data_observer_(config.report_block_data_observer),
+      packet_type_counter_observer_(config.rtcp_packet_type_counter_observer),
+      num_skipped_packets_(0),
+      last_skipped_packets_warning_ms_(clock_->TimeInMilliseconds()) {
+  RTC_DCHECK(owner);
+  // Dear reader - if you're here because of this log statement and are
+  // wondering what this is about, chances are that you are using an instance
+  // of RTCPReceiver without using the webrtc APIs. This creates a bit of a
+  // problem for WebRTC because this class is a part of an internal
+  // implementation that is constantly changing and being improved.
+  // The intention of this log statement is to give a heads up that changes
+  // are coming and encourage you to use the public APIs or be prepared that
+  // things might break down the line as more changes land. A thing you could
+  // try out for now is to replace the `CustomSequenceChecker` in the header
+  // with a regular `SequenceChecker` and see if that triggers an
+  // error in your code. If it does, chances are you have your own threading
+  // model that is not the same as WebRTC internally has.
+  RTC_LOG(LS_INFO) << "************** !!!DEPRECATION WARNING!! **************";
+}
+
 RTCPReceiver::~RTCPReceiver() {}
 
 void RTCPReceiver::IncomingPacket(rtc::ArrayView<const uint8_t> packet) {
@@ -178,6 +247,14 @@ void RTCPReceiver::SetRemoteSSRC(uint32_t ssrc) {
   remote_ssrc_ = ssrc;
 }
 
+void RTCPReceiver::set_local_media_ssrc(uint32_t ssrc) {
+  registered_ssrcs_.set_media_ssrc(ssrc);
+}
+
+uint32_t RTCPReceiver::local_media_ssrc() const {
+  return registered_ssrcs_.media_ssrc();
+}
+
 uint32_t RTCPReceiver::RemoteSSRC() const {
   MutexLock lock(&rtcp_receiver_lock_);
   return remote_ssrc_;

modules/rtp_rtcp/source/rtcp_receiver.h

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include "api/array_view.h"
+#include "api/sequence_checker.h"
 #include "modules/rtp_rtcp/include/report_block_data.h"
 #include "modules/rtp_rtcp/include/rtcp_statistics.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -27,11 +28,15 @@
 #include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h"
 #include "modules/rtp_rtcp/source/rtp_rtcp_interface.h"
 #include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/system/no_unique_address.h"
 #include "rtc_base/thread_annotations.h"
 #include "system_wrappers/include/ntp_time.h"
 
 namespace webrtc {
+
+class ModuleRtpRtcpImpl2;
 class VideoBitrateAllocationObserver;
+
 namespace rtcp {
 class CommonHeader;
 class ReportBlock;
@@ -57,6 +62,10 @@ class RTCPReceiver final {
 
   RTCPReceiver(const RtpRtcpInterface::Configuration& config,
                ModuleRtpRtcp* owner);
+
+  RTCPReceiver(const RtpRtcpInterface::Configuration& config,
+               ModuleRtpRtcpImpl2* owner);
+
   ~RTCPReceiver();
 
   void IncomingPacket(const uint8_t* packet, size_t packet_size) {
@@ -66,9 +75,14 @@ class RTCPReceiver final {
 
   int64_t LastReceivedReportBlockMs() const;
 
+  void set_local_media_ssrc(uint32_t ssrc);
+  uint32_t local_media_ssrc() const;
+
   void SetRemoteSSRC(uint32_t ssrc);
   uint32_t RemoteSSRC() const;
 
+  bool receiver_only() const { return receiver_only_; }
+
   // Get received NTP.
   // The types for the arguments below derive from the specification:
   // - `remote_sender_packet_count`: `RTCSentRtpStreamStats.packetsSent` [1]
@@ -126,21 +140,48 @@ class RTCPReceiver final {
   void NotifyTmmbrUpdated();
 
  private:
-  // A lightweight inlined set of local SSRCs.
-  class RegisteredSsrcs {
+#if RTC_DCHECK_IS_ON
+  class CustomSequenceChecker : public SequenceChecker {
    public:
-    static constexpr size_t kMaxSsrcs = 3;
-    // Initializes the set of registered local SSRCS by extracting them from the
-    // provided `config`.
-    explicit RegisteredSsrcs(const RtpRtcpInterface::Configuration& config);
-
-    // Indicates if `ssrc` is in the set of registered local SSRCs.
-    bool contains(uint32_t ssrc) const {
-      return absl::c_linear_search(ssrcs_, ssrc);
+    explicit CustomSequenceChecker(bool disable_checks)
+        : disable_checks_(disable_checks) {}
+    bool IsCurrent() const {
+      if (disable_checks_)
+        return true;
+      return SequenceChecker::IsCurrent();
     }
 
    private:
-    absl::InlinedVector<uint32_t, kMaxSsrcs> ssrcs_;
+    const bool disable_checks_;
+  };
+#else
+  class CustomSequenceChecker : public SequenceChecker {
+   public:
+    explicit CustomSequenceChecker(bool) {}
+  };
+#endif
+
+  // A lightweight inlined set of local SSRCs.
+  class RegisteredSsrcs {
+   public:
+    static constexpr size_t kMediaSsrcIndex = 0;
+    static constexpr size_t kMaxSsrcs = 3;
+    // Initializes the set of registered local SSRCS by extracting them from the
+    // provided `config`. The `disable_sequence_checker` flag is a workaround
+    // to be able to use a sequence checker without breaking downstream
+    // code that currently doesn't follow the same threading rules as webrtc.
+    RegisteredSsrcs(bool disable_sequence_checker,
+                    const RtpRtcpInterface::Configuration& config);
+
+    // Indicates if `ssrc` is in the set of registered local SSRCs.
+    bool contains(uint32_t ssrc) const;
+    uint32_t media_ssrc() const;
+    void set_media_ssrc(uint32_t ssrc);
+
+   private:
+    RTC_NO_UNIQUE_ADDRESS CustomSequenceChecker packet_sequence_checker_;
+    absl::InlinedVector<uint32_t, kMaxSsrcs> ssrcs_
+        RTC_GUARDED_BY(packet_sequence_checker_);
   };
 
   struct PacketInformation;
@@ -290,7 +331,7 @@ class RTCPReceiver final {
   ModuleRtpRtcp* const rtp_rtcp_;
   const uint32_t main_ssrc_;
   // The set of registered local SSRCs.
-  const RegisteredSsrcs registered_ssrcs_;
+  RegisteredSsrcs registered_ssrcs_;
 
   RtcpBandwidthObserver* const rtcp_bandwidth_observer_;
   RtcpIntraFrameObserver* const rtcp_intra_frame_observer_;

modules/rtp_rtcp/source/rtcp_sender.cc

@@ -308,6 +308,16 @@ void RTCPSender::SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz) {
   rtp_clock_rates_khz_[payload_type] = rtp_clock_rate_hz / 1000;
 }
 
+uint32_t RTCPSender::SSRC() const {
+  MutexLock lock(&mutex_rtcp_sender_);
+  return ssrc_;
+}
+
+void RTCPSender::SetSsrc(uint32_t ssrc) {
+  MutexLock lock(&mutex_rtcp_sender_);
+  ssrc_ = ssrc;
+}
+
 void RTCPSender::SetRemoteSSRC(uint32_t ssrc) {
   MutexLock lock(&mutex_rtcp_sender_);
   remote_ssrc_ = ssrc;

modules/rtp_rtcp/source/rtcp_sender.h

@@ -94,7 +94,8 @@ class RTCPSender final {
   void SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz)
       RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_);
 
-  uint32_t SSRC() const { return ssrc_; }
+  uint32_t SSRC() const;
+  void SetSsrc(uint32_t ssrc);
 
   void SetRemoteSSRC(uint32_t ssrc) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_);
 
@@ -187,7 +188,11 @@ class RTCPSender final {
       RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_);
 
   const bool audio_;
-  const uint32_t ssrc_;
+  // TODO(bugs.webrtc.org/11581): `mutex_rtcp_sender_` shouldn't be required if
+  // we consistently run network related operations on the network thread.
+  // This is currently not possible due to callbacks from the process thread in
+  // ModuleRtpRtcpImpl2.
+  uint32_t ssrc_ RTC_GUARDED_BY(mutex_rtcp_sender_);
   Clock* const clock_;
   Random random_ RTC_GUARDED_BY(mutex_rtcp_sender_);
   RtcpMode method_ RTC_GUARDED_BY(mutex_rtcp_sender_);

modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -683,6 +683,11 @@ void ModuleRtpRtcpImpl::SetRemoteSSRC(const uint32_t ssrc) {
   rtcp_receiver_.SetRemoteSSRC(ssrc);
 }
 
+void ModuleRtpRtcpImpl::SetLocalSsrc(uint32_t local_ssrc) {
+  rtcp_receiver_.set_local_media_ssrc(local_ssrc);
+  rtcp_sender_.SetSsrc(local_ssrc);
+}
+
 RtpSendRates ModuleRtpRtcpImpl::GetSendRates() const {
   return rtp_sender_->packet_sender.GetSendRates();
 }

modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -63,6 +63,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
                           size_t incoming_packet_length) override;
 
   void SetRemoteSSRC(uint32_t ssrc) override;
+  void SetLocalSsrc(uint32_t ssrc) override;
 
   // Sender part.
   void RegisterSendPayloadFrequency(int payload_type,

modules/rtp_rtcp/source/rtp_rtcp_impl2.cc

@@ -69,6 +69,7 @@ ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
       rtt_ms_(0) {
   RTC_DCHECK(worker_queue_);
   process_thread_checker_.Detach();
+  packet_sequence_checker_.Detach();
   if (!configuration.receiver_only) {
     rtp_sender_ = std::make_unique<RtpSenderContext>(configuration);
     // Make sure rtcp sender use same timestamp offset as rtp sender.
@@ -169,6 +170,7 @@ absl::optional<uint32_t> ModuleRtpRtcpImpl2::FlexfecSsrc() const {
 
 void ModuleRtpRtcpImpl2::IncomingRtcpPacket(const uint8_t* rtcp_packet,
                                             const size_t length) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
   rtcp_receiver_.IncomingPacket(rtcp_packet, length);
 }
 
@@ -219,6 +221,12 @@ RtpState ModuleRtpRtcpImpl2::GetRtxState() const {
   return rtp_sender_->packet_generator.GetRtxRtpState();
 }
 
+uint32_t ModuleRtpRtcpImpl2::local_media_ssrc() const {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  RTC_DCHECK_EQ(rtcp_receiver_.local_media_ssrc(), rtcp_sender_.SSRC());
+  return rtcp_receiver_.local_media_ssrc();
+}
+
 void ModuleRtpRtcpImpl2::SetRid(const std::string& rid) {
   if (rtp_sender_) {
     rtp_sender_->packet_generator.SetRid(rid);
@@ -650,6 +658,12 @@ void ModuleRtpRtcpImpl2::SetRemoteSSRC(const uint32_t ssrc) {
   rtcp_receiver_.SetRemoteSSRC(ssrc);
 }
 
+void ModuleRtpRtcpImpl2::SetLocalSsrc(uint32_t local_ssrc) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  rtcp_receiver_.set_local_media_ssrc(local_ssrc);
+  rtcp_sender_.SetSsrc(local_ssrc);
+}
+
 RtpSendRates ModuleRtpRtcpImpl2::GetSendRates() const {
   // Typically called on the `rtp_transport_queue_` owned by an
   // RtpTransportControllerSendInterface instance.

modules/rtp_rtcp/source/rtp_rtcp_impl2.h

@@ -77,6 +77,8 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface,
 
   void SetRemoteSSRC(uint32_t ssrc) override;
 
+  void SetLocalSsrc(uint32_t local_ssrc) override;
+
   // Sender part.
   void RegisterSendPayloadFrequency(int payload_type,
                                     int payload_frequency) override;
@@ -110,6 +112,11 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface,
 
   uint32_t SSRC() const override { return rtcp_sender_.SSRC(); }
 
+  // Semantically identical to `SSRC()` but must be called on the packet
+  // delivery thread/tq and returns the ssrc that maps to
+  // RtpRtcpInterface::Configuration::local_media_ssrc.
+  uint32_t local_media_ssrc() const;
+
   void SetRid(const std::string& rid) override;
 
   void SetMid(const std::string& mid) override;
@@ -284,6 +291,7 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface,
 
   TaskQueueBase* const worker_queue_;
   RTC_NO_UNIQUE_ADDRESS SequenceChecker process_thread_checker_;
+  RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_;
 
   std::unique_ptr<RtpSenderContext> rtp_sender_;
 

modules/rtp_rtcp/source/rtp_rtcp_interface.h

@@ -180,6 +180,10 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface {
 
   virtual void SetRemoteSSRC(uint32_t ssrc) = 0;
 
+  // Called when the local ssrc changes (post initialization) for receive
+  // streams to match with send. Called on the packet receive thread/tq.
+  virtual void SetLocalSsrc(uint32_t ssrc) = 0;
+
   // **************************************************************************
   // Sender
   // **************************************************************************