From ef6b028c928e52d66c49f80e1a1c833ab9affdbb Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 18 Mar 2022 14:49:12 +0100 Subject: [PATCH] Implement recieving NACK in RtcpTranceiver Bug: webrtc:8239 Change-Id: I41d6c3252bbffeab66ded7ed294f82134351541a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/254800 Reviewed-by: Emil Lundmark Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#36254} --- .../rtp_rtcp/source/rtcp_transceiver_config.h | 3 ++ .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 32 ++++++++++++++++--- .../rtp_rtcp/source/rtcp_transceiver_impl.h | 3 ++ .../source/rtcp_transceiver_impl_unittest.cc | 29 +++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index 34789dbd0b..8b0a8fad62 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.h @@ -97,6 +97,9 @@ class RtpStreamRtcpHandler { int last_clock_rate_ = 90'000; }; virtual RtpStats SentStats() = 0; + + virtual void OnNack(uint32_t sender_ssrc, + rtc::ArrayView sequence_numbers) {} }; struct RtcpTransceiverConfig { diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index 382e9f19ff..a43edbde17 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -351,10 +351,34 @@ void RtcpTransceiverImpl::HandlePayloadSpecificFeedback( void RtcpTransceiverImpl::HandleRtpFeedback( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now) { - // Transport feedback is the only message handled right now. - if (rtcp_packet_header.fmt() != - rtcp::TransportFeedback::kFeedbackMessageType || - config_.network_link_observer == nullptr) { + switch (rtcp_packet_header.fmt()) { + case rtcp::Nack::kFeedbackMessageType: + HandleNack(rtcp_packet_header); + break; + case rtcp::TransportFeedback::kFeedbackMessageType: + HandleTransportFeedback(rtcp_packet_header, now); + break; + } +} + +void RtcpTransceiverImpl::HandleNack( + const rtcp::CommonHeader& rtcp_packet_header) { + rtcp::Nack nack; + if (local_senders_.empty() || !nack.Parse(rtcp_packet_header)) { + return; + } + auto it = local_senders_by_ssrc_.find(nack.media_ssrc()); + if (it != local_senders_by_ssrc_.end()) { + it->second->handler->OnNack(nack.sender_ssrc(), nack.packet_ids()); + } +} + +void RtcpTransceiverImpl::HandleTransportFeedback( + const rtcp::CommonHeader& rtcp_packet_header, + Timestamp now) { + RTC_DCHECK_EQ(rtcp_packet_header.fmt(), + rtcp::TransportFeedback::kFeedbackMessageType); + if (config_.network_link_observer == nullptr) { return; } rtcp::TransportFeedback feedback; diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index be9a98188e..9e0da6a42d 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -105,6 +105,9 @@ class RtcpTransceiverImpl { Timestamp now); void HandleRtpFeedback(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now); + void HandleNack(const rtcp::CommonHeader& rtcp_packet_header); + void HandleTransportFeedback(const rtcp::CommonHeader& rtcp_packet_header, + Timestamp now); void HandleExtendedReports(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now); // Extended Reports blocks handlers. diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 744b4eb6a2..e90cf047f2 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -40,6 +40,7 @@ namespace { using ::testing::_; using ::testing::ElementsAre; +using ::testing::ElementsAreArray; using ::testing::Ge; using ::testing::NiceMock; using ::testing::Return; @@ -82,6 +83,10 @@ class MockRtpStreamRtcpHandler : public RtpStreamRtcpHandler { } MOCK_METHOD(RtpStats, SentStats, (), (override)); + MOCK_METHOD(void, + OnNack, + (uint32_t, rtc::ArrayView), + (override)); private: int num_calls_ = 0; @@ -1044,6 +1049,30 @@ TEST(RtcpTransceiverImplTest, SendsNack) { EXPECT_EQ(rtcp_parser.nack()->packet_ids(), kMissingSequenceNumbers); } +TEST(RtcpTransceiverImplTest, ReceivesNack) { + static constexpr uint32_t kRemoteSsrc = 4321; + static constexpr uint32_t kMediaSsrc1 = 1234; + static constexpr uint32_t kMediaSsrc2 = 1235; + std::vector kMissingSequenceNumbers = {34, 37, 38}; + RtcpTransceiverConfig config = DefaultTestConfig(); + RtcpTransceiverImpl rtcp_transceiver(config); + + MockRtpStreamRtcpHandler local_stream1; + MockRtpStreamRtcpHandler local_stream2; + EXPECT_CALL(local_stream1, + OnNack(kRemoteSsrc, ElementsAreArray(kMissingSequenceNumbers))); + EXPECT_CALL(local_stream2, OnNack).Times(0); + + EXPECT_TRUE(rtcp_transceiver.AddMediaSender(kMediaSsrc1, &local_stream1)); + EXPECT_TRUE(rtcp_transceiver.AddMediaSender(kMediaSsrc2, &local_stream2)); + + rtcp::Nack nack; + nack.SetSenderSsrc(kRemoteSsrc); + nack.SetMediaSsrc(kMediaSsrc1); + nack.SetPacketIds(kMissingSequenceNumbers); + rtcp_transceiver.ReceivePacket(nack.Build(), config.clock->CurrentTime()); +} + TEST(RtcpTransceiverImplTest, RequestKeyFrameWithPictureLossIndication) { const uint32_t kSenderSsrc = 1234; const uint32_t kRemoteSsrc = 4321;