From d5cae4d59c6af691c2efa7d513f71ee4c0eea814 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 14 Dec 2017 11:14:35 +0100 Subject: [PATCH] Add hacky way to send TransportFeedback in RtcpTransceiver With an extra interface it will allow to add both RtpRtcp module and RtcpTransceiver as feedback sender to PacketRouter Though hacky, this is very similar to currently used implementation in the RTCPSender::SendFeedbackPacket Bug: webrtc:8239 Change-Id: I237b422ae1594dede78cb63daa4aa42b6774d6fe Reviewed-on: https://webrtc-review.googlesource.com/32680 Commit-Queue: Danil Chapovalov Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#21274} --- modules/rtp_rtcp/source/rtcp_transceiver.cc | 19 ++++++++++ modules/rtp_rtcp/source/rtcp_transceiver.h | 8 +++++ .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 8 +++++ .../rtp_rtcp/source/rtcp_transceiver_impl.h | 3 ++ .../source/rtcp_transceiver_unittest.cc | 35 +++++++++++++++++++ 5 files changed, 73 insertions(+) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.cc b/modules/rtp_rtcp/source/rtcp_transceiver.cc index 52b2a683ce..28b95c0202 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver.cc @@ -12,6 +12,7 @@ #include +#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "rtc_base/checks.h" #include "rtc_base/event.h" #include "rtc_base/ptr_util.h" @@ -91,6 +92,24 @@ void RtcpTransceiver::UnsetRemb() { }); } +uint32_t RtcpTransceiver::SSRC() const { + return rtcp_transceiver_->sender_ssrc(); +} + +bool RtcpTransceiver::SendFeedbackPacket( + const rtcp::TransportFeedback& packet) { + struct Closure { + void operator()() { + if (ptr) + ptr->SendRawPacket(raw_packet); + } + rtc::WeakPtr ptr; + rtc::Buffer raw_packet; + }; + task_queue_->PostTask(Closure{ptr_, packet.Build()}); + return true; +} + void RtcpTransceiver::SendNack(uint32_t ssrc, std::vector sequence_numbers) { // TODO(danilchap): Replace with lambda with move capture when available. diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.h b/modules/rtp_rtcp/source/rtcp_transceiver.h index 09840e1721..26091b57a5 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver.h @@ -44,6 +44,14 @@ class RtcpTransceiver { // Stops sending REMB in following compound packets. void UnsetRemb(); + // TODO(bugs.webrtc.org/8239): Remove SendFeedbackPacket and SSRC functions + // and move generating of the TransportFeedback message inside + // RtcpTransceiverImpl when there is one RtcpTransceiver per rtp transport. + + // Returns ssrc to put as sender ssrc into rtcp::TransportFeedback. + uint32_t SSRC() const; + bool SendFeedbackPacket(const rtcp::TransportFeedback& packet); + // Reports missing packets, https://tools.ietf.org/html/rfc4585#section-6.2.1 void SendNack(uint32_t ssrc, std::vector sequence_numbers); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index e8de54aa7d..28d90bc463 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -149,6 +149,14 @@ void RtcpTransceiverImpl::UnsetRemb() { remb_.reset(); } +void RtcpTransceiverImpl::SendRawPacket(rtc::ArrayView packet) { + // Unlike other senders, this functions just tries to send packet away and + // disregard rtcp_mode, max_packet_size or anything else. + // TODO(bugs.webrtc.org/8239): respect config_ by creating the + // TransportFeedback inside this class when there is one per rtp transport. + config_.outgoing_transport->SendRtcp(packet.data(), packet.size()); +} + void RtcpTransceiverImpl::SendNack(uint32_t ssrc, std::vector sequence_numbers) { RTC_DCHECK(!sequence_numbers.empty()); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index 83e6087780..35fee75ba8 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -49,6 +49,9 @@ class RtcpTransceiverImpl { void SetRemb(int64_t bitrate_bps, std::vector ssrcs); void UnsetRemb(); + // Temporary helpers to send pre-built TransportFeedback rtcp packet. + uint32_t sender_ssrc() const { return config_.feedback_ssrc; } + void SendRawPacket(rtc::ArrayView packet); void SendNack(uint32_t ssrc, std::vector sequence_numbers); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc index dea91dba86..0a374e63b9 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc @@ -10,6 +10,7 @@ #include "modules/rtp_rtcp/source/rtcp_transceiver.h" +#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "rtc_base/event.h" #include "rtc_base/ptr_util.h" #include "test/gmock.h" @@ -19,12 +20,14 @@ namespace { using ::testing::AtLeast; +using ::testing::Invoke; using ::testing::InvokeWithoutArgs; using ::testing::NiceMock; using ::testing::_; using ::webrtc::MockTransport; using ::webrtc::RtcpTransceiver; using ::webrtc::RtcpTransceiverConfig; +using ::webrtc::rtcp::TransportFeedback; void WaitPostedTasks(rtc::TaskQueue* queue) { rtc::Event done(false, false); @@ -132,4 +135,36 @@ TEST(RtcpTransceiverTest, DoesntSendPacketsAfterDestruction) { EXPECT_FALSE(rtcp_transceiver); } +TEST(RtcpTransceiverTest, SendsTransportFeedbackOnTaskQueue) { + static constexpr uint32_t kSenderSsrc = 12345; + MockTransport outgoing_transport; + rtc::TaskQueue queue("rtcp"); + RtcpTransceiverConfig config; + config.feedback_ssrc = kSenderSsrc; + config.outgoing_transport = &outgoing_transport; + config.task_queue = &queue; + config.schedule_periodic_compound_packets = false; + RtcpTransceiver rtcp_transceiver(config); + + EXPECT_CALL(outgoing_transport, SendRtcp(_, _)) + .WillOnce(Invoke([&](const uint8_t* buffer, size_t size) { + EXPECT_TRUE(queue.IsCurrent()); + + std::unique_ptr transport_feedback = + TransportFeedback::ParseFrom(buffer, size); + EXPECT_TRUE(transport_feedback); + EXPECT_EQ(transport_feedback->sender_ssrc(), kSenderSsrc); + return true; + })); + + // Create minimalistic transport feedback packet. + TransportFeedback transport_feedback; + transport_feedback.SetSenderSsrc(rtcp_transceiver.SSRC()); + transport_feedback.AddReceivedPacket(321, 10000); + + EXPECT_TRUE(rtcp_transceiver.SendFeedbackPacket(transport_feedback)); + + WaitPostedTasks(&queue); +} + } // namespace