commit 03e6cccc28d76f28c926ce7cadaeba2c6a6cacf4
author Philipp Hancke <phancke@microsoft.com> Thu Sep 15 11:11:20 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 16 09:40:18 2022
tree deee35e6f99aa28c9ab093d5fd1370060995de11
parent 2720dc5b3b24478a4b1100738b91ab12590ba484

Revert "rtp sender: don't send BYE on deactivating streams"

This reverts commit a22c2a0c581cbe3f612f7a7d9fb9840186cc1e06.

Reason for revert: breaks upstream project

Original change's description:
> 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}

Bug: webrtc:11082
Change-Id: Iaaff0c0d7bb857fe9ce78ebcc716f3c6f1bc5c4a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275640
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#38097}

call/rtp_video_sender.cc

diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 8e582ea..0e4f8d8 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -513,6 +513,7 @@
     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) {

modules/rtp_rtcp/source/rtcp_sender.cc

diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index 626c430..7983371 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -213,8 +213,23 @@
 
 void RTCPSender::SetSendingStatus(const FeedbackState& feedback_state,
                                   bool sending) {
-  MutexLock lock(&mutex_rtcp_sender_);
-  sending_ = 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";
+    }
+  }
 }
 
 void RTCPSender::SetNonSenderRttMeasurement(bool enabled) {

modules/rtp_rtcp/source/rtcp_sender_unittest.cc

diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index f88aacb..ae59dc5 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -329,12 +329,13 @@
   EXPECT_EQ(kSenderSsrc, parser()->bye()->sender_ssrc());
 }
 
-TEST_F(RtcpSenderTest, StopSendingDoesNotTriggersBye) {
+TEST_F(RtcpSenderTest, StopSendingTriggersBye) {
   auto rtcp_sender = CreateRtcpSender(GetDefaultConfig());
   rtcp_sender->SetRTCPStatus(RtcpMode::kReducedSize);
   rtcp_sender->SetSendingStatus(feedback_state(), true);
   rtcp_sender->SetSendingStatus(feedback_state(), false);
-  EXPECT_EQ(0, parser()->bye()->num_packets());
+  EXPECT_EQ(1, parser()->bye()->num_packets());
+  EXPECT_EQ(kSenderSsrc, parser()->bye()->sender_ssrc());
 }
 
 TEST_F(RtcpSenderTest, SendFir) {

modules/rtp_rtcp/source/rtp_rtcp_impl.cc

diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 191a2aa..a3662f1 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -310,6 +310,7 @@
 
 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;

modules/rtp_rtcp/source/rtp_rtcp_interface.h

diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
index 8381b58..8b1d11a 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.
+  // Sets sending status. Sends kRtcpByeCode when going from true to false.
   // Returns -1 on failure else 0.
   virtual int32_t SetSendingStatus(bool sending) = 0;