From: Michael Froman Date: Mon, 18 Dec 2023 15:00:00 +0000 Subject: Bug 1867099 - revert libwebrtc 8602f604e0. r=bwc Upstream 8602f604e0 removed code sending BYEs which breaks some of our wpt. They've opened a bug for a real fix here: https://bugs.chromium.org/p/webrtc/issues/detail?id=15664 I've opened Bug 1870643 to track the real fix and upstream bug. Differential Revision: https://phabricator.services.mozilla.com/D196729 Mercurial Revision: https://hg.mozilla.org/mozilla-central/rev/d92a578327f524ec3e1c144c82492a4c76b8266f --- call/rtp_video_sender.cc | 1 + modules/rtp_rtcp/source/rtcp_sender.cc | 19 +++++++++++++++++-- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 5 +++-- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 2 +- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 8a09f25ce0..1166c8b71f 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -528,6 +528,7 @@ void RtpVideoSender::SetModuleIsActive(bool sending, return; } + // Sends a kRtcpByeCode when going from true to false. rtp_module.SetSendingStatus(sending); rtp_module.SetSendingMediaStatus(sending); if (sending) { diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 2b936a151e..43d9fc784d 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -193,8 +193,23 @@ bool RTCPSender::Sending() const { 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) { diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 5d0b47e262..9263f12ab3 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -338,12 +338,13 @@ TEST_F(RtcpSenderTest, SendBye) { 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) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 5c6a9fcb85..13e7765cea 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -273,7 +273,7 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // Returns the FlexFEC SSRC, if there is one. virtual std::optional 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;