rtp sender: don't send BYE on deactivating streams
as this breaks RTCP assumptions about SSRCs being no longer
active as defined in
https://www.rfc-editor.org/rfc/rfc3550#section-6.6
This should not be sent in reaction to temporarily disabling
a stream via RTCRtpParameters.active as this does not mean that
the participant is leaving the session as defined in
https://www.rfc-editor.org/rfc/rfc3550#section-6.3.7
and does not indicate end of participation as defined in
https://www.rfc-editor.org/rfc/rfc3550#section-6.1
which stipulates BYE should be the last packet sent from this SSRC.
BUG=webrtc:11082
Change-Id: Ia5144857f85303643146b0759184f0f3f50b66e4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273348
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#38059}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 0e4f8d8..8e582ea 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -513,7 +513,6 @@
const bool was_active = rtp_module.Sending();
const bool should_be_active = active_modules[i];
- // Sends a kRtcpByeCode when going from true to false.
rtp_module.SetSendingStatus(active_modules[i]);
if (was_active && !should_be_active) {
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index 7983371..626c430 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -213,23 +213,8 @@
void RTCPSender::SetSendingStatus(const FeedbackState& feedback_state,
bool sending) {
- bool sendRTCPBye = false;
- {
- MutexLock lock(&mutex_rtcp_sender_);
-
- if (method_ != RtcpMode::kOff) {
- if (sending == false && sending_ == true) {
- // Trigger RTCP bye
- sendRTCPBye = true;
- }
- }
- sending_ = sending;
- }
- if (sendRTCPBye) {
- if (SendRTCP(feedback_state, kRtcpBye) != 0) {
- RTC_LOG(LS_WARNING) << "Failed to send RTCP BYE";
- }
- }
+ MutexLock lock(&mutex_rtcp_sender_);
+ sending_ = sending;
}
void RTCPSender::SetNonSenderRttMeasurement(bool enabled) {
diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index ae59dc5..f88aacb 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -329,13 +329,12 @@
EXPECT_EQ(kSenderSsrc, parser()->bye()->sender_ssrc());
}
-TEST_F(RtcpSenderTest, StopSendingTriggersBye) {
+TEST_F(RtcpSenderTest, StopSendingDoesNotTriggersBye) {
auto rtcp_sender = CreateRtcpSender(GetDefaultConfig());
rtcp_sender->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender->SetSendingStatus(feedback_state(), true);
rtcp_sender->SetSendingStatus(feedback_state(), false);
- EXPECT_EQ(1, parser()->bye()->num_packets());
- EXPECT_EQ(kSenderSsrc, parser()->bye()->sender_ssrc());
+ EXPECT_EQ(0, parser()->bye()->num_packets());
}
TEST_F(RtcpSenderTest, SendFir) {
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index a3662f1..191a2aa 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -310,7 +310,6 @@
int32_t ModuleRtpRtcpImpl::SetSendingStatus(const bool sending) {
if (rtcp_sender_.Sending() != sending) {
- // Sends RTCP BYE when going from true to false
rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending);
}
return 0;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
index 8b1d11a..8381b58 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -281,7 +281,7 @@
// Returns the FlexFEC SSRC, if there is one.
virtual absl::optional<uint32_t> FlexfecSsrc() const = 0;
- // Sets sending status. Sends kRtcpByeCode when going from true to false.
+ // Sets sending status.
// Returns -1 on failure else 0.
virtual int32_t SetSendingStatus(bool sending) = 0;
