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}
diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
index d523128..a7707ec 100644
--- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
+++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -34,6 +34,7 @@
(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,
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index e48f200..79f24c4 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/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 @@
} // 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 @@
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 @@
};
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 @@
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 @@
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_;
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h
index 429df55..57dd1c0 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/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 @@
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 @@
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 @@
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 @@
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_;
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index 287cb9c..bacb767 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -308,6 +308,16 @@
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;
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index aab2c90..67200ee 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -94,7 +94,8 @@
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 @@
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_);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 5a79f55..54316a8 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -683,6 +683,11 @@
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();
}
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 5bcabc5..b0e0b41 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -63,6 +63,7 @@
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,
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
index d5f11f6..55e4a74 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
@@ -69,6 +69,7 @@
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 @@
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 @@
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 @@
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.
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
index 00f6ff1..4c38517 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
@@ -77,6 +77,8 @@
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 @@
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 @@
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_;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
index 457a993..dd5744e 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -180,6 +180,10 @@
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
// **************************************************************************