Revert "Reland "RtpRtcp modules and below: Make media, RTX and FEC SSRCs const""
This reverts commit c9348218cfe0cff6d0d3a383f7d1d6cfce4b1262.
Reason for revert: Downstream tests are relying on incorrect behavior which this CL explicitly checks...
Original change's description:
> Reland "RtpRtcp modules and below: Make media, RTX and FEC SSRCs const"
>
> This is a reland of 17608dc4592fe25c1effdd75bf856f4af251942e
>
> Downstream fixed, relanding.
>
> Original change's description:
> > RtpRtcp modules and below: Make media, RTX and FEC SSRCs const
> >
> > Downstream usage of SetSsrc() / SetRtxSsrc() should now be gone. Let's
> > remove them, make the members const, and remove now unnecessary locking.
> >
> > Bug: webrtc:10774
> > Change-Id: Ie4c1b3935508cf329c5553030f740c565d32e04b
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/155660
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Niels Moller <nisse@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#29475}
>
> TBR=nisse@webrtc.org
>
> Bug: webrtc:10774
> Change-Id: I759bed3ff1909857696c6d1b13df595a5e552f03
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157049
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29486}
TBR=nisse@webrtc.org,sprang@webrtc.org
Change-Id: I168fb3738a04dfdbd1581ddd8c3276ede9f72322
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10774
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157080
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29488}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index fbfdc09..73e356d 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -177,7 +177,7 @@
bool enable_flexfec = flexfec_sender != nullptr &&
std::find(flexfec_protected_ssrcs.begin(),
flexfec_protected_ssrcs.end(),
- configuration.local_media_ssrc) !=
+ *configuration.local_media_ssrc) !=
flexfec_protected_ssrcs.end();
configuration.flexfec_sender = enable_flexfec ? flexfec_sender : nullptr;
auto playout_delay_oracle = std::make_unique<PlayoutDelayOracle>();
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index a046f64..69ca8f8 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -122,7 +122,7 @@
// SSRCs for media and retransmission, respectively.
// FlexFec SSRC is fetched from |flexfec_sender|.
- uint32_t local_media_ssrc;
+ absl::optional<uint32_t> local_media_ssrc;
absl::optional<uint32_t> rtx_send_ssrc;
private:
@@ -200,6 +200,10 @@
// Returns SSRC.
uint32_t SSRC() const override = 0;
+ // Sets SSRC, default is a random number.
+ // TODO(bugs.webrtc.org/10774): Remove.
+ virtual void SetSSRC(uint32_t ssrc) = 0;
+
// Sets the value for sending in the RID (and Repaired) RTP header extension.
// RIDs are used to identify an RTP stream if SSRCs are not negotiated.
// If the RID and Repaired RID extensions are not registered, the RID will
@@ -223,6 +227,11 @@
// a combination of values of the enumerator RtxMode.
virtual int RtxSendStatus() const = 0;
+ // Sets the SSRC to use when sending RTX packets. This doesn't enable RTX,
+ // only the SSRC is set.
+ // TODO(bugs.webrtc.org/10774): Remove.
+ virtual void SetRtxSsrc(uint32_t ssrc) = 0;
+
// Sets the payload type to use when sending RTX packets. Note that this
// doesn't enable RTX, only the payload type is set.
virtual void SetRtxSendPayloadType(int payload_type,
diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
index 17601dd..a75fd6e 100644
--- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc
+++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
@@ -134,7 +134,6 @@
configuration.outgoing_transport = &transport_;
configuration.retransmission_rate_limiter = &retransmission_rate_limiter_;
configuration.local_media_ssrc = kTestSsrc;
- configuration.rtx_send_ssrc = kTestRtxSsrc;
rtp_rtcp_module_ = RtpRtcp::Create(configuration);
FieldTrialBasedConfig field_trials;
RTPSenderVideo::Config video_config;
@@ -201,6 +200,7 @@
rtx_receiver_ = transport_.stream_receiver_controller_.CreateReceiver(
kTestRtxSsrc, &rtx_stream_);
rtp_rtcp_module_->SetRtxSendStatus(rtx_method);
+ rtp_rtcp_module_->SetRtxSsrc(kTestRtxSsrc);
transport_.DropEveryNthPacket(loss);
uint32_t timestamp = 3000;
uint16_t nack_list[kVideoNackListSize];
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index 6b64473..f06fd1c 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -65,18 +65,6 @@
constexpr int32_t kDefaultVideoReportInterval = 1000;
constexpr int32_t kDefaultAudioReportInterval = 5000;
-
-std::set<uint32_t> GetRegisteredSsrcs(const RtpRtcp::Configuration& config) {
- std::set<uint32_t> ssrcs;
- ssrcs.insert(config.local_media_ssrc);
- if (config.rtx_send_ssrc) {
- ssrcs.insert(*config.rtx_send_ssrc);
- }
- if (config.flexfec_sender) {
- ssrcs.insert(config.flexfec_sender->ssrc());
- }
- return ssrcs;
-}
} // namespace
struct RTCPReceiver::PacketInformation {
@@ -138,8 +126,6 @@
: clock_(config.clock),
receiver_only_(config.receiver_only),
rtp_rtcp_(owner),
- main_ssrc_(config.local_media_ssrc),
- registered_ssrcs_(GetRegisteredSsrcs(config)),
rtcp_bandwidth_observer_(config.bandwidth_callback),
rtcp_intra_frame_observer_(config.intra_frame_callback),
rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer),
@@ -151,6 +137,7 @@
: (config.audio ? kDefaultAudioReportInterval
: kDefaultVideoReportInterval)),
// TODO(bugs.webrtc.org/10774): Remove fallback.
+ main_ssrc_(config.local_media_ssrc.value_or(0)),
remote_ssrc_(0),
remote_sender_rtp_time_(0),
xr_rrtr_status_(false),
@@ -165,6 +152,15 @@
num_skipped_packets_(0),
last_skipped_packets_warning_ms_(clock_->TimeInMilliseconds()) {
RTC_DCHECK(owner);
+ if (config.local_media_ssrc) {
+ registered_ssrcs_.insert(*config.local_media_ssrc);
+ }
+ if (config.rtx_send_ssrc) {
+ registered_ssrcs_.insert(*config.rtx_send_ssrc);
+ }
+ if (config.flexfec_sender) {
+ registered_ssrcs_.insert(config.flexfec_sender->ssrc());
+ }
}
RTCPReceiver::~RTCPReceiver() {}
@@ -198,6 +194,13 @@
return remote_ssrc_;
}
+void RTCPReceiver::SetSsrcs(uint32_t main_ssrc,
+ const std::set<uint32_t>& registered_ssrcs) {
+ rtc::CritScope lock(&rtcp_receiver_lock_);
+ main_ssrc_ = main_ssrc;
+ registered_ssrcs_ = registered_ssrcs;
+}
+
int32_t RTCPReceiver::RTT(uint32_t remote_ssrc,
int64_t* last_rtt_ms,
int64_t* avg_rtt_ms,
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h
index 5b92d55..3056711 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -59,6 +59,7 @@
int64_t LastReceivedReportBlockMs() const;
+ void SetSsrcs(uint32_t main_ssrc, const std::set<uint32_t>& registered_ssrcs);
void SetRemoteSSRC(uint32_t ssrc);
uint32_t RemoteSSRC() const;
@@ -214,8 +215,6 @@
Clock* const clock_;
const bool receiver_only_;
ModuleRtpRtcp* const rtp_rtcp_;
- const uint32_t main_ssrc_;
- const std::set<uint32_t> registered_ssrcs_;
rtc::CriticalSection feedbacks_lock_;
RtcpBandwidthObserver* const rtcp_bandwidth_observer_;
@@ -227,7 +226,9 @@
const int report_interval_ms_;
rtc::CriticalSection rtcp_receiver_lock_;
+ uint32_t main_ssrc_ RTC_GUARDED_BY(rtcp_receiver_lock_);
uint32_t remote_ssrc_ RTC_GUARDED_BY(rtcp_receiver_lock_);
+ std::set<uint32_t> registered_ssrcs_ RTC_GUARDED_BY(rtcp_receiver_lock_);
// Received sender report.
NtpTime remote_sender_ntp_time_ RTC_GUARDED_BY(rtcp_receiver_lock_);
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index fba9b45..15325d1 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -150,7 +150,6 @@
RTCPSender::RTCPSender(const RtpRtcp::Configuration& config)
: audio_(config.audio),
- ssrc_(config.local_media_ssrc),
clock_(config.clock),
random_(clock_->TimeInMicroseconds()),
method_(RtcpMode::kOff),
@@ -165,6 +164,7 @@
timestamp_offset_(0),
last_rtp_timestamp_(0),
last_frame_capture_time_ms_(-1),
+ ssrc_(config.local_media_ssrc.value_or(0)),
remote_ssrc_(0),
receive_statistics_(config.receive_statistics),
@@ -331,6 +331,23 @@
rtp_clock_rates_khz_[payload_type] = rtp_clock_rate_hz / 1000;
}
+uint32_t RTCPSender::SSRC() const {
+ rtc::CritScope lock(&critical_section_rtcp_sender_);
+ return ssrc_;
+}
+
+void RTCPSender::SetSSRC(uint32_t ssrc) {
+ rtc::CritScope lock(&critical_section_rtcp_sender_);
+
+ if (ssrc_ != 0 && ssrc != ssrc_) {
+ // not first SetSSRC, probably due to a collision
+ // schedule a new RTCP report
+ // make sure that we send a RTP packet
+ next_time_to_send_rtcp_ = clock_->TimeInMilliseconds() + 100;
+ }
+ ssrc_ = ssrc;
+}
+
void RTCPSender::SetRemoteSSRC(uint32_t ssrc) {
rtc::CritScope lock(&critical_section_rtcp_sender_);
remote_ssrc_ = ssrc;
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index 97b4b70..6deee87 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -85,7 +85,9 @@
void SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz);
- uint32_t SSRC() const { return ssrc_; }
+ uint32_t SSRC() const;
+
+ void SetSSRC(uint32_t ssrc);
void SetRemoteSSRC(uint32_t ssrc);
@@ -185,7 +187,6 @@
private:
const bool audio_;
- const uint32_t ssrc_;
Clock* const clock_;
Random random_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
RtcpMode method_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
@@ -204,6 +205,7 @@
uint32_t last_rtp_timestamp_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
int64_t last_frame_capture_time_ms_
RTC_GUARDED_BY(critical_section_rtcp_sender_);
+ uint32_t ssrc_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
// SSRC that we receive on our RTP channel
uint32_t remote_ssrc_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
std::string cname_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index c732a35..c3f3920 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -825,6 +825,31 @@
EXPECT_FALSE(rtcp_sender_->TimeToSendRTCPReport(false));
}
+TEST_F(RtcpSenderTest, DoesntSchedulesInitialReportOnFirstSetSsrc) {
+ // Set up without first SSRC not set at construction.
+ RtpRtcp::Configuration configuration = GetDefaultConfig();
+ configuration.local_media_ssrc = absl::nullopt;
+
+ rtcp_sender_.reset(new RTCPSender(configuration));
+ rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
+ rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
+ rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(),
+ /*payload_type=*/0);
+ rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
+
+ // Set SSRC for the first time. New report should not be scheduled.
+ rtcp_sender_->SetSSRC(kSenderSsrc);
+ clock_.AdvanceTimeMilliseconds(100);
+ EXPECT_FALSE(rtcp_sender_->TimeToSendRTCPReport(false));
+}
+
+TEST_F(RtcpSenderTest, SchedulesReportOnSsrcChange) {
+ rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
+ rtcp_sender_->SetSSRC(kSenderSsrc + 1);
+ clock_.AdvanceTimeMilliseconds(100);
+ EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false));
+}
+
TEST_F(RtcpSenderTest, SendsCombinedRtcpPacket) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 7938396..7d8e338 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -175,6 +175,10 @@
return rtp_sender_ ? rtp_sender_->RtxStatus() : kRtxOff;
}
+void ModuleRtpRtcpImpl::SetRtxSsrc(uint32_t ssrc) {
+ rtp_sender_->SetRtxSsrc(ssrc);
+}
+
void ModuleRtpRtcpImpl::SetRtxSendPayloadType(int payload_type,
int associated_payload_type) {
rtp_sender_->SetRtxPayloadType(payload_type, associated_payload_type);
@@ -236,6 +240,18 @@
return rtp_sender_->GetRtxRtpState();
}
+uint32_t ModuleRtpRtcpImpl::SSRC() const {
+ return rtcp_sender_.SSRC();
+}
+
+void ModuleRtpRtcpImpl::SetSSRC(const uint32_t ssrc) {
+ if (rtp_sender_) {
+ rtp_sender_->SetSSRC(ssrc);
+ }
+ rtcp_sender_.SetSSRC(ssrc);
+ SetRtcpReceiverSsrcs(ssrc);
+}
+
void ModuleRtpRtcpImpl::SetRid(const std::string& rid) {
if (rtp_sender_) {
rtp_sender_->SetRid(rid);
@@ -290,6 +306,11 @@
if (rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending) != 0) {
RTC_LOG(LS_WARNING) << "Failed to send RTCP BYE";
}
+ if (sending && rtp_sender_) {
+ // Update Rtcp receiver config, to track Rtx config changes from
+ // the SetRtxStatus and SetRtxSsrc methods.
+ SetRtcpReceiverSsrcs(rtp_sender_->SSRC());
+ }
}
return 0;
}
@@ -734,6 +755,17 @@
return rtcp_receiver_.BoundingSet(tmmbr_owner);
}
+void ModuleRtpRtcpImpl::SetRtcpReceiverSsrcs(uint32_t main_ssrc) {
+ std::set<uint32_t> ssrcs;
+ ssrcs.insert(main_ssrc);
+ if (RtxSendStatus() != kRtxOff)
+ ssrcs.insert(rtp_sender_->RtxSsrc());
+ absl::optional<uint32_t> flexfec_ssrc = FlexfecSsrc();
+ if (flexfec_ssrc)
+ ssrcs.insert(*flexfec_ssrc);
+ rtcp_receiver_.SetSsrcs(main_ssrc, ssrcs);
+}
+
void ModuleRtpRtcpImpl::set_rtt_ms(int64_t rtt_ms) {
rtc::CritScope cs(&critical_section_rtt_);
rtt_ms_ = rtt_ms;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 312f9d6..9ec481c 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -94,7 +94,10 @@
RtpState GetRtpState() const override;
RtpState GetRtxState() const override;
- uint32_t SSRC() const override { return rtcp_sender_.SSRC(); }
+ uint32_t SSRC() const override;
+
+ // Configure SSRC, default is a random number.
+ void SetSSRC(uint32_t ssrc) override;
void SetRid(const std::string& rid) override;
@@ -107,6 +110,8 @@
void SetRtxSendStatus(int mode) override;
int RtxSendStatus() const override;
+ void SetRtxSsrc(uint32_t ssrc) override;
+
void SetRtxSendPayloadType(int payload_type,
int associated_payload_type) override;
@@ -297,6 +302,7 @@
private:
FRIEND_TEST_ALL_PREFIXES(RtpRtcpImplTest, Rtt);
FRIEND_TEST_ALL_PREFIXES(RtpRtcpImplTest, RttForReceiverOnly);
+ void SetRtcpReceiverSsrcs(uint32_t main_ssrc);
void set_rtt_ms(int64_t rtt_ms);
int64_t rtt_ms() const;
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 5aa707f..c88e0e2 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -124,8 +124,6 @@
: clock_(config.clock),
random_(clock_->TimeInMicroseconds()),
audio_configured_(config.audio),
- ssrc_(config.local_media_ssrc),
- rtx_ssrc_(config.rtx_send_ssrc),
flexfec_ssrc_(config.flexfec_sender
? absl::make_optional(config.flexfec_sender->ssrc())
: absl::nullopt),
@@ -156,6 +154,7 @@
bitrate_callback_(config.send_bitrate_observer),
// RTP variables
sequence_number_forced_(false),
+ ssrc_(config.local_media_ssrc),
ssrc_has_acked_(false),
rtx_ssrc_has_acked_(false),
last_rtp_timestamp_(0),
@@ -165,6 +164,7 @@
last_packet_marker_bit_(false),
csrcs_(),
rtx_(kRtxOff),
+ ssrc_rtx_(config.rtx_send_ssrc),
rtp_overhead_bytes_per_packet_(0),
supports_bwe_extension_(false),
retransmission_rate_limiter_(config.retransmission_rate_limiter),
@@ -267,6 +267,17 @@
return rtx_;
}
+void RTPSender::SetRtxSsrc(uint32_t ssrc) {
+ rtc::CritScope lock(&send_critsect_);
+ ssrc_rtx_.emplace(ssrc);
+}
+
+uint32_t RTPSender::RtxSsrc() const {
+ rtc::CritScope lock(&send_critsect_);
+ RTC_DCHECK(ssrc_rtx_);
+ return *ssrc_rtx_;
+}
+
void RTPSender::SetRtxPayloadType(int payload_type,
int associated_payload_type) {
rtc::CritScope lock(&send_critsect_);
@@ -417,7 +428,7 @@
case RtpPacketToSend::Type::kPadding:
// Both padding and retransmission must be on either the media or the
// RTX stream.
- if (packet_ssrc == rtx_ssrc_) {
+ if (packet_ssrc == ssrc_rtx_) {
is_rtx = true;
} else if (packet_ssrc != ssrc_) {
return false;
@@ -610,7 +621,7 @@
}
RTC_DCHECK(ssrc_);
- padding_packet->SetSsrc(ssrc_);
+ padding_packet->SetSsrc(*ssrc_);
padding_packet->SetPayloadType(last_payload_type_);
padding_packet->SetSequenceNumber(sequence_number_++);
} else {
@@ -634,8 +645,8 @@
padding_packet->set_capture_time_ms(padding_packet->capture_time_ms() +
(now_ms - last_timestamp_time_ms_));
}
- RTC_DCHECK(rtx_ssrc_);
- padding_packet->SetSsrc(*rtx_ssrc_);
+ RTC_DCHECK(ssrc_rtx_);
+ padding_packet->SetSsrc(*ssrc_rtx_);
padding_packet->SetSequenceNumber(sequence_number_rtx_++);
padding_packet->SetPayloadType(rtx_payload_type_map_.begin()->second);
}
@@ -791,10 +802,17 @@
if (!bitrate_callback_)
return;
int64_t now_ms = clock_->TimeInMilliseconds();
+ uint32_t ssrc;
+ {
+ rtc::CritScope lock(&send_critsect_);
+ if (!ssrc_)
+ return;
+ ssrc = *ssrc_;
+ }
rtc::CritScope lock(&statistics_crit_);
bitrate_callback_->Notify(total_bitrate_sent_.Rate(now_ms).value_or(0),
- nack_bitrate_sent_.Rate(now_ms).value_or(0), ssrc_);
+ nack_bitrate_sent_.Rate(now_ms).value_or(0), ssrc);
}
size_t RTPSender::RtpHeaderLength() const {
@@ -832,7 +850,7 @@
auto packet = std::make_unique<RtpPacketToSend>(
&rtp_header_extension_map_, max_packet_size_ + kExtraCapacity);
RTC_DCHECK(ssrc_);
- packet->SetSsrc(ssrc_);
+ packet->SetSsrc(*ssrc_);
packet->SetCsrcs(csrcs_);
// Reserve extensions, if registered, RtpSender set in SendToNetwork.
packet->ReserveExtension<AbsoluteSendTime>();
@@ -905,6 +923,30 @@
return timestamp_offset_;
}
+void RTPSender::SetSSRC(uint32_t ssrc) {
+ {
+ rtc::CritScope lock(&send_critsect_);
+ if (ssrc_ == ssrc) {
+ return; // Since it's the same SSRC, don't reset anything.
+ }
+
+ ssrc_.emplace(ssrc);
+ if (!sequence_number_forced_) {
+ sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber);
+ }
+ }
+
+ // Clear RTP packet history, since any packets there belong to the old SSRC
+ // and they may conflict with packets from the new one.
+ packet_history_.Clear();
+}
+
+uint32_t RTPSender::SSRC() const {
+ rtc::CritScope lock(&send_critsect_);
+ RTC_DCHECK(ssrc_);
+ return *ssrc_;
+}
+
void RTPSender::SetRid(const std::string& rid) {
// RID is used in simulcast scenario when multiple layers share the same mid.
rtc::CritScope lock(&send_critsect_);
@@ -919,6 +961,10 @@
mid_ = mid;
}
+absl::optional<uint32_t> RTPSender::FlexfecSsrc() const {
+ return flexfec_ssrc_;
+}
+
void RTPSender::SetCsrcs(const std::vector<uint32_t>& csrcs) {
RTC_DCHECK_LE(csrcs.size(), kRtpCsrcSize);
rtc::CritScope lock(&send_critsect_);
@@ -1006,7 +1052,7 @@
if (!sending_media_)
return nullptr;
- RTC_DCHECK(rtx_ssrc_);
+ RTC_DCHECK(ssrc_rtx_);
// Replace payload type.
auto kv = rtx_payload_type_map_.find(packet.PayloadType());
@@ -1022,7 +1068,7 @@
rtx_packet->SetSequenceNumber(sequence_number_rtx_++);
// Replace SSRC.
- rtx_packet->SetSsrc(*rtx_ssrc_);
+ rtx_packet->SetSsrc(*ssrc_rtx_);
CopyHeaderAndExtensionsToRtxPacket(packet, rtx_packet.get());
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 9194d44..d0a8396 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -67,6 +67,9 @@
uint32_t TimestampOffset() const;
void SetTimestampOffset(uint32_t timestamp);
+ // TODO(bugs.webrtc.org/10774): Remove.
+ void SetSSRC(uint32_t ssrc);
+
void SetRid(const std::string& rid);
void SetMid(const std::string& mid);
@@ -113,10 +116,10 @@
// RTX.
void SetRtxStatus(int mode);
int RtxStatus() const;
- uint32_t RtxSsrc() const {
- RTC_DCHECK(rtx_ssrc_);
- return *rtx_ssrc_;
- }
+ uint32_t RtxSsrc() const;
+
+ // TODO(bugs.webrtc.org/10774): Remove.
+ void SetRtxSsrc(uint32_t ssrc);
void SetRtxPayloadType(int payload_type, int associated_payload_type);
@@ -140,9 +143,9 @@
// Including RTP headers.
size_t MaxRtpPacketSize() const;
- uint32_t SSRC() const { return ssrc_; }
+ uint32_t SSRC() const;
- absl::optional<uint32_t> FlexfecSsrc() const { return flexfec_ssrc_; }
+ absl::optional<uint32_t> FlexfecSsrc() const;
// Sends packet to |transport_| or to the pacer, depending on configuration.
// TODO(bugs.webrtc.org/XXX): Remove in favor of EnqueuePackets().
@@ -222,8 +225,6 @@
const bool audio_configured_;
- const uint32_t ssrc_;
- const absl::optional<uint32_t> rtx_ssrc_;
const absl::optional<uint32_t> flexfec_ssrc_;
const std::unique_ptr<NonPacedPacketSender> non_paced_packet_sender_;
@@ -267,6 +268,9 @@
bool sequence_number_forced_ RTC_GUARDED_BY(send_critsect_);
uint16_t sequence_number_ RTC_GUARDED_BY(send_critsect_);
uint16_t sequence_number_rtx_ RTC_GUARDED_BY(send_critsect_);
+ // Must be explicitly set by the application, use of absl::optional
+ // only to keep track of correct use.
+ absl::optional<uint32_t> ssrc_ RTC_GUARDED_BY(send_critsect_);
// RID value to send in the RID or RepairedRID header extension.
std::string rid_ RTC_GUARDED_BY(send_critsect_);
// MID value to send in the MID header extension.
@@ -282,6 +286,7 @@
bool last_packet_marker_bit_ RTC_GUARDED_BY(send_critsect_);
std::vector<uint32_t> csrcs_ RTC_GUARDED_BY(send_critsect_);
int rtx_ RTC_GUARDED_BY(send_critsect_);
+ absl::optional<uint32_t> ssrc_rtx_ RTC_GUARDED_BY(send_critsect_);
// Mapping rtx_payload_type_map_[associated] = rtx.
std::map<int8_t, int8_t> rtx_payload_type_map_ RTC_GUARDED_BY(send_critsect_);
size_t rtp_overhead_bytes_per_packet_ RTC_GUARDED_BY(send_critsect_);
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 0b2d48e..da7ba4f 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -2562,6 +2562,34 @@
EXPECT_EQ(*transmission_time_extension, 2 * kOffsetMs * kTimestampTicksPerMs);
}
+TEST_P(RtpSenderTestWithoutPacer, ClearHistoryOnSsrcChange) {
+ const int64_t kRtt = 10;
+
+ rtp_sender_->SetSendingMediaStatus(true);
+ rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads);
+ rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload);
+ rtp_sender_->SetStorePacketsStatus(true, 10);
+ rtp_sender_->SetRtt(kRtt);
+
+ // Send a packet and record its sequence numbers.
+ SendGenericPacket();
+ ASSERT_EQ(1u, transport_.sent_packets_.size());
+ const uint16_t packet_seqence_number =
+ transport_.sent_packets_.back().SequenceNumber();
+
+ // Advance time and make sure it can be retransmitted, even if we try to set
+ // the ssrc the what it already is.
+ rtp_sender_->SetSSRC(kSsrc);
+ fake_clock_.AdvanceTimeMilliseconds(kRtt);
+ EXPECT_GT(rtp_sender_->ReSendPacket(packet_seqence_number), 0);
+
+ // Change the SSRC, then move the time and try to retransmit again. The old
+ // packet should now be gone.
+ rtp_sender_->SetSSRC(kSsrc + 1);
+ fake_clock_.AdvanceTimeMilliseconds(kRtt);
+ EXPECT_EQ(rtp_sender_->ReSendPacket(packet_seqence_number), 0);
+}
+
TEST_P(RtpSenderTestWithoutPacer, ClearHistoryOnSequenceNumberCange) {
const int64_t kRtt = 10;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index d769bfe..0e4c114 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -933,7 +933,6 @@
config.clock = Clock::GetRealTimeClock();
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
- config.local_media_ssrc = kReceiverLocalVideoSsrc;
RTCPSender rtcp_sender(config);
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
@@ -1150,7 +1149,6 @@
config.receive_statistics = &lossy_receive_stats;
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
- config.local_media_ssrc = kVideoSendSsrcs[0];
RTCPSender rtcp_sender(config);
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
@@ -1402,7 +1400,6 @@
config.receive_statistics = &receive_stats;
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
- config.local_media_ssrc = kVideoSendSsrcs[0];
RTCPSender rtcp_sender(config);
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
