Add void::RtcpFeedbackSenderInterface::SendCombinedRtcpPacket
This method sends arbitrary number rtp::RcpPackets into one or more IP packets.
It is implemented both in RtcpTranceiver and in RtpRtcp.
Change-Id: I00424ee2f1730ff98626f768846f4ac1ad864933
BUG: webrtc:10742
Change-Id: I00424ee2f1730ff98626f768846f4ac1ad864933
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156240
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29430}
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index 7492217..0b9284b 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -14,6 +14,7 @@
#include <stddef.h>
#include <list>
+#include <memory>
#include <vector>
#include "absl/strings/string_view.h"
@@ -317,6 +318,8 @@
virtual bool SendFeedbackPacket(const rtcp::TransportFeedback& feedback) = 0;
virtual bool SendNetworkStateEstimatePacket(
const rtcp::RemoteEstimate& packet) = 0;
+ virtual void SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) = 0;
virtual void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs) = 0;
virtual void UnsetRemb() = 0;
};
diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
index 347d4ce..bf280f3 100644
--- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
+++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -150,6 +150,9 @@
MOCK_METHOD1(SendFeedbackPacket, bool(const rtcp::TransportFeedback& packet));
MOCK_METHOD1(SendNetworkStateEstimatePacket,
bool(const rtcp::RemoteEstimate& packet));
+ MOCK_METHOD1(
+ SendCombinedRtcpPacket,
+ void(std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets));
MOCK_METHOD1(SetTargetSendBitrate, void(uint32_t bitrate_bps));
MOCK_METHOD4(SendLossNotification,
int32_t(uint16_t last_decoded_seq_num,
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index 1c6d154..15325d1 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -50,22 +50,6 @@
kRtcpXrTargetBitrate;
constexpr int32_t kDefaultVideoReportInterval = 1000;
constexpr int32_t kDefaultAudioReportInterval = 5000;
-} // namespace
-
-RTCPSender::FeedbackState::FeedbackState()
- : packets_sent(0),
- media_bytes_sent(0),
- send_bitrate(0),
- last_rr_ntp_secs(0),
- last_rr_ntp_frac(0),
- remote_sr(0),
- module(nullptr) {}
-
-RTCPSender::FeedbackState::FeedbackState(const FeedbackState&) = default;
-
-RTCPSender::FeedbackState::FeedbackState(FeedbackState&&) = default;
-
-RTCPSender::FeedbackState::~FeedbackState() = default;
class PacketContainer : public rtcp::CompoundPacket {
public:
@@ -96,6 +80,57 @@
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(PacketContainer);
};
+// Helper to put several RTCP packets into lower layer datagram RTCP packet.
+// Prefer to use this class instead of PacketContainer.
+class PacketSender {
+ public:
+ PacketSender(rtcp::RtcpPacket::PacketReadyCallback callback,
+ size_t max_packet_size)
+ : callback_(callback), max_packet_size_(max_packet_size) {
+ RTC_CHECK_LE(max_packet_size, IP_PACKET_SIZE);
+ }
+ ~PacketSender() { RTC_DCHECK_EQ(index_, 0) << "Unsent rtcp packet."; }
+
+ // Appends a packet to pending compound packet.
+ // Sends rtcp packet if buffer is full and resets the buffer.
+ void AppendPacket(const rtcp::RtcpPacket& packet) {
+ packet.Create(buffer_, &index_, max_packet_size_, callback_);
+ }
+
+ // Sends pending rtcp packet.
+ void Send() {
+ if (index_ > 0) {
+ callback_(rtc::ArrayView<const uint8_t>(buffer_, index_));
+ index_ = 0;
+ }
+ }
+
+ bool IsEmpty() const { return index_ == 0; }
+
+ private:
+ const rtcp::RtcpPacket::PacketReadyCallback callback_;
+ const size_t max_packet_size_;
+ size_t index_ = 0;
+ uint8_t buffer_[IP_PACKET_SIZE];
+};
+
+} // namespace
+
+RTCPSender::FeedbackState::FeedbackState()
+ : packets_sent(0),
+ media_bytes_sent(0),
+ send_bitrate(0),
+ last_rr_ntp_secs(0),
+ last_rr_ntp_frac(0),
+ remote_sr(0),
+ module(nullptr) {}
+
+RTCPSender::FeedbackState::FeedbackState(const FeedbackState&) = default;
+
+RTCPSender::FeedbackState::FeedbackState(FeedbackState&&) = default;
+
+RTCPSender::FeedbackState::~FeedbackState() = default;
+
class RTCPSender::RtcpContext {
public:
RtcpContext(const FeedbackState& feedback_state,
@@ -1014,4 +1049,33 @@
return packet.Build(max_packet_size, callback) && send_success;
}
+void RTCPSender::SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) {
+ size_t max_packet_size;
+ uint32_t ssrc;
+ {
+ rtc::CritScope lock(&critical_section_rtcp_sender_);
+ if (method_ == RtcpMode::kOff) {
+ RTC_LOG(LS_WARNING) << "Can't send rtcp if it is disabled.";
+ return;
+ }
+
+ max_packet_size = max_packet_size_;
+ ssrc = ssrc_;
+ }
+ RTC_DCHECK_LE(max_packet_size, IP_PACKET_SIZE);
+ auto callback = [&](rtc::ArrayView<const uint8_t> packet) {
+ if (transport_->SendRtcp(packet.data(), packet.size())) {
+ if (event_log_)
+ event_log_->Log(std::make_unique<RtcEventRtcpPacketOutgoing>(packet));
+ }
+ };
+ PacketSender sender(callback, max_packet_size);
+ for (auto& rtcp_packet : rtcp_packets) {
+ rtcp_packet->SetSenderSsrc(ssrc);
+ sender.AppendPacket(*rtcp_packet);
+ }
+ sender.Send();
+}
+
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index 33db97a..6deee87 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -142,6 +142,8 @@
void SetVideoBitrateAllocation(const VideoBitrateAllocation& bitrate);
bool SendFeedbackPacket(const rtcp::TransportFeedback& packet);
bool SendNetworkStateEstimatePacket(const rtcp::RemoteEstimate& packet);
+ void SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets);
private:
class RtcpContext;
diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index a077836..c3f3920 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -11,6 +11,7 @@
#include "modules/rtp_rtcp/source/rtcp_sender.h"
#include <memory>
+#include <utility>
#include "absl/base/macros.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -849,4 +850,21 @@
EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false));
}
+TEST_F(RtcpSenderTest, SendsCombinedRtcpPacket) {
+ rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
+
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets;
+ auto transport_feedback = std::make_unique<rtcp::TransportFeedback>();
+ transport_feedback->AddReceivedPacket(321, 10000);
+ packets.push_back(std::move(transport_feedback));
+ auto remote_estimate = std::make_unique<rtcp::RemoteEstimate>();
+ packets.push_back(std::move(remote_estimate));
+ rtcp_sender_->SendCombinedRtcpPacket(std::move(packets));
+
+ EXPECT_EQ(parser()->transport_feedback()->num_packets(), 1);
+ EXPECT_EQ(parser()->transport_feedback()->sender_ssrc(), kSenderSsrc);
+ EXPECT_EQ(parser()->app()->num_packets(), 1);
+ EXPECT_EQ(parser()->app()->sender_ssrc(), kSenderSsrc);
+}
+
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.cc b/modules/rtp_rtcp/source/rtcp_transceiver.cc
index 4538301..46e222c 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver.cc
@@ -12,6 +12,7 @@
#include <memory>
#include <utility>
+#include <vector>
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "rtc_base/checks.h"
@@ -131,6 +132,16 @@
return true;
}
+void RtcpTransceiver::SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) {
+ RTC_CHECK(rtcp_transceiver_);
+ RtcpTransceiverImpl* ptr = rtcp_transceiver_.get();
+ task_queue_->PostTask(
+ [ptr, rtcp_packets = std::move(rtcp_packets)]() mutable {
+ ptr->SendCombinedRtcpPacket(std::move(rtcp_packets));
+ });
+}
+
void RtcpTransceiver::SendNack(uint32_t ssrc,
std::vector<uint16_t> sequence_numbers) {
RTC_CHECK(rtcp_transceiver_);
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.h b/modules/rtp_rtcp/source/rtcp_transceiver.h
index 8b70c6d..5468e25 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver.h
+++ b/modules/rtp_rtcp/source/rtcp_transceiver.h
@@ -82,6 +82,12 @@
bool SendNetworkStateEstimatePacket(
const rtcp::RemoteEstimate& packet) override;
+ // TODO(bugs.webrtc.org/8239): Remove SendCombinedRtcpPacket
+ // and move generating of the TransportFeedback message inside
+ // RtcpTransceiverImpl when there is one RtcpTransceiver per rtp transport.
+ void SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) override;
+
// Reports missing packets, https://tools.ietf.org/html/rfc4585#section-6.2.1
void SendNack(uint32_t ssrc, std::vector<uint16_t> sequence_numbers);
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
index e982421..977fc8b 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
@@ -380,6 +380,20 @@
sender.Send();
}
+void RtcpTransceiverImpl::SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) {
+ auto send_packet = [this](rtc::ArrayView<const uint8_t> packet) {
+ config_.outgoing_transport->SendRtcp(packet.data(), packet.size());
+ };
+ PacketSender sender(send_packet, config_.max_packet_size);
+
+ for (auto& rtcp_packet : rtcp_packets) {
+ rtcp_packet->SetSenderSsrc(config_.feedback_ssrc);
+ sender.AppendPacket(*rtcp_packet);
+ }
+ sender.Send();
+}
+
void RtcpTransceiverImpl::SendImmediateFeedback(
const rtcp::RtcpPacket& rtcp_packet) {
auto send_packet = [this](rtc::ArrayView<const uint8_t> packet) {
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
index 083f77e..8039f2b 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
@@ -63,6 +63,11 @@
void SendPictureLossIndication(uint32_t ssrc);
void SendFullIntraRequest(rtc::ArrayView<const uint32_t> ssrcs);
+ // SendCombinedRtcpPacket ignores rtcp mode and does not send a compound
+ // message. https://tools.ietf.org/html/rfc4585#section-3.1
+ void SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets);
+
private:
class PacketSender;
struct RemoteSenderState;
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
index 8be17ca..568d348 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
@@ -11,7 +11,9 @@
#include "modules/rtp_rtcp/source/rtcp_transceiver.h"
#include <memory>
+#include <utility>
+#include "modules/rtp_rtcp/source/rtcp_packet/remote_estimate.h"
#include "modules/rtp_rtcp/source/rtcp_packet/sender_report.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "rtc_base/event.h"
@@ -19,6 +21,7 @@
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/mock_transport.h"
+#include "test/rtcp_packet_parser.h"
namespace {
@@ -32,7 +35,10 @@
using ::webrtc::RtcpTransceiver;
using ::webrtc::RtcpTransceiverConfig;
using ::webrtc::TaskQueueForTest;
+using ::webrtc::rtcp::RemoteEstimate;
+using ::webrtc::rtcp::RtcpPacket;
using ::webrtc::rtcp::TransportFeedback;
+using ::webrtc::test::RtcpPacketParser;
class MockMediaReceiverRtcpObserver : public webrtc::MediaReceiverRtcpObserver {
public:
@@ -283,4 +289,41 @@
WaitPostedTasks(&queue);
}
+TEST(RtcpTransceiverTest, SendsCombinedRtcpPacketOnTaskQueue) {
+ static constexpr uint32_t kSenderSsrc = 12345;
+
+ MockTransport outgoing_transport;
+ TaskQueueForTest 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([&](const uint8_t* buffer, size_t size) {
+ EXPECT_TRUE(queue.IsCurrent());
+ RtcpPacketParser rtcp_parser;
+ rtcp_parser.Parse(buffer, size);
+ EXPECT_EQ(rtcp_parser.transport_feedback()->num_packets(), 1);
+ EXPECT_EQ(rtcp_parser.transport_feedback()->sender_ssrc(), kSenderSsrc);
+ EXPECT_EQ(rtcp_parser.app()->num_packets(), 1);
+ EXPECT_EQ(rtcp_parser.app()->sender_ssrc(), kSenderSsrc);
+ return true;
+ });
+
+ // Create minimalistic transport feedback packet.
+ std::vector<std::unique_ptr<RtcpPacket>> packets;
+ auto transport_feedback = std::make_unique<TransportFeedback>();
+ transport_feedback->AddReceivedPacket(321, 10000);
+ packets.push_back(std::move(transport_feedback));
+
+ auto remote_estimate = std::make_unique<RemoteEstimate>();
+ packets.push_back(std::move(remote_estimate));
+
+ rtcp_transceiver.SendCombinedRtcpPacket(std::move(packets));
+ WaitPostedTasks(&queue);
+}
+
} // namespace
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index cb826f6..7d8e338 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -657,6 +657,11 @@
return rtcp_sender_.SendNetworkStateEstimatePacket(packet);
}
+void ModuleRtpRtcpImpl::SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) {
+ rtcp_sender_.SendCombinedRtcpPacket(std::move(rtcp_packets));
+}
+
int32_t ModuleRtpRtcpImpl::SendLossNotification(uint16_t last_decoded_seq_num,
uint16_t last_received_seq_num,
bool decodability_flag,
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 2359fec..9ec481c 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -237,6 +237,9 @@
bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) override;
bool SendNetworkStateEstimatePacket(
const rtcp::RemoteEstimate& packet) override;
+ void SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) override;
+
// (APP) Application specific data.
int32_t SetRTCPApplicationSpecificData(uint8_t sub_type,
uint32_t name,
