Send rtcp::RemoteEstimate and rtcp::TransportFeedback in one packet
Change-Id: I53912f4e82a9fd795f8886d6b2cdb313bde08c4d
BUG: webrtc:10742
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156380
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29437}
diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc
index 229cdb3..2946b5c 100644
--- a/modules/pacing/packet_router.cc
+++ b/modules/pacing/packet_router.cc
@@ -13,11 +13,13 @@
#include <algorithm>
#include <cstdint>
#include <limits>
+#include <memory>
#include <utility>
#include "absl/types/optional.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtcp_packet.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "rtc_base/atomic_ops.h"
#include "rtc_base/checks.h"
@@ -274,33 +276,25 @@
return true;
}
-bool PacketRouter::SendTransportFeedback(rtcp::TransportFeedback* packet) {
+bool PacketRouter::SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets) {
rtc::CritScope cs(&modules_crit_);
+
// Prefer send modules.
for (auto* rtp_module : rtp_send_modules_) {
- packet->SetSenderSsrc(rtp_module->SSRC());
- if (rtp_module->SendFeedbackPacket(*packet)) {
- return true;
+ if (rtp_module->RTCP() == RtcpMode::kOff) {
+ continue;
}
+ rtp_module->SendCombinedRtcpPacket(std::move(packets));
+ return true;
}
- for (auto* rtcp_sender : rtcp_feedback_senders_) {
- packet->SetSenderSsrc(rtcp_sender->SSRC());
- if (rtcp_sender->SendFeedbackPacket(*packet)) {
- return true;
- }
- }
- return false;
-}
-void PacketRouter::SendNetworkStateEstimatePacket(
- rtcp::RemoteEstimate* packet) {
- rtc::CritScope cs(&modules_crit_);
- for (auto* rtcp_sender : rtcp_feedback_senders_) {
- packet->SetSenderSsrc(rtcp_sender->SSRC());
- if (rtcp_sender->SendNetworkStateEstimatePacket(*packet)) {
- break;
- }
+ if (rtcp_feedback_senders_.empty()) {
+ return false;
}
+ auto* rtcp_sender = rtcp_feedback_senders_[0];
+ rtcp_sender->SendCombinedRtcpPacket(std::move(packets));
+ return true;
}
void PacketRouter::AddRembModuleCandidate(
diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h
index be535fe..85aa003 100644
--- a/modules/pacing/packet_router.h
+++ b/modules/pacing/packet_router.h
@@ -22,6 +22,7 @@
#include "api/transport/network_types.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtcp_packet.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/critical_section.h"
@@ -30,9 +31,6 @@
namespace webrtc {
class RtpRtcp;
-namespace rtcp {
-class TransportFeedback;
-} // namespace rtcp
// PacketRouter keeps track of rtp send modules to support the pacer.
// In addition, it handles feedback messages, which are sent on a send
@@ -76,10 +74,9 @@
// Send REMB feedback.
bool SendRemb(int64_t bitrate_bps, const std::vector<uint32_t>& ssrcs);
- // Send transport feedback packet to send-side.
- bool SendTransportFeedback(rtcp::TransportFeedback* packet) override;
- // Send RemoteEstimate packet to send-side.
- void SendNetworkStateEstimatePacket(rtcp::RemoteEstimate* packet) override;
+ // Sends |packets| in one or more IP packets.
+ bool SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets) override;
private:
RtpRtcp* FindRtpModule(uint32_t ssrc)
diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc
index 22de617..3fd9882 100644
--- a/modules/pacing/packet_router_unittest.cc
+++ b/modules/pacing/packet_router_unittest.cc
@@ -89,9 +89,10 @@
}
TEST_F(PacketRouterTest, Sanity_NoModuleRegistered_SendTransportFeedback) {
- rtcp::TransportFeedback feedback;
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback;
+ feedback.push_back(std::make_unique<rtcp::TransportFeedback>());
- EXPECT_FALSE(packet_router_.SendTransportFeedback(&feedback));
+ EXPECT_FALSE(packet_router_.SendCombinedRtcpPacket(std::move(feedback)));
}
TEST_F(PacketRouterTest, GeneratePaddingPicksCorrectModule) {
@@ -242,15 +243,21 @@
NiceMock<MockRtpRtcp> rtp_1;
NiceMock<MockRtpRtcp> rtp_2;
+ ON_CALL(rtp_1, RTCP()).WillByDefault(Return(RtcpMode::kCompound));
+ ON_CALL(rtp_2, RTCP()).WillByDefault(Return(RtcpMode::kCompound));
+
packet_router_.AddSendRtpModule(&rtp_1, false);
packet_router_.AddReceiveRtpModule(&rtp_2, false);
- rtcp::TransportFeedback feedback;
- EXPECT_CALL(rtp_1, SendFeedbackPacket(_)).Times(1).WillOnce(Return(true));
- packet_router_.SendTransportFeedback(&feedback);
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback;
+ feedback.push_back(std::make_unique<rtcp::TransportFeedback>());
+ EXPECT_CALL(rtp_1, SendCombinedRtcpPacket).Times(1);
+ packet_router_.SendCombinedRtcpPacket(std::move(feedback));
packet_router_.RemoveSendRtpModule(&rtp_1);
- EXPECT_CALL(rtp_2, SendFeedbackPacket(_)).Times(1).WillOnce(Return(true));
- packet_router_.SendTransportFeedback(&feedback);
+ EXPECT_CALL(rtp_2, SendCombinedRtcpPacket).Times(1);
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> new_feedback;
+ new_feedback.push_back(std::make_unique<rtcp::TransportFeedback>());
+ packet_router_.SendCombinedRtcpPacket(std::move(new_feedback));
packet_router_.RemoveReceiveRtpModule(&rtp_2);
}
diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn
index cec58a1..aa046d5 100644
--- a/modules/remote_bitrate_estimator/BUILD.gn
+++ b/modules/remote_bitrate_estimator/BUILD.gn
@@ -115,6 +115,7 @@
"../..:webrtc_common",
"../../api/transport:field_trial_based_config",
"../../api/transport:mock_network_control",
+ "../../api/transport:network_control",
"../../rtc_base",
"../../rtc_base:checks",
"../../rtc_base:rtc_base_approved",
diff --git a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
index a6bba70..c60c030 100644
--- a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
+++ b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
@@ -14,16 +14,15 @@
#define MODULES_REMOTE_BITRATE_ESTIMATOR_INCLUDE_REMOTE_BITRATE_ESTIMATOR_H_
#include <map>
+#include <memory>
#include <vector>
#include "modules/include/module.h"
#include "modules/include/module_common_types.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtcp_packet.h"
namespace webrtc {
-namespace rtcp {
-class TransportFeedback;
-} // namespace rtcp
class Clock;
@@ -42,8 +41,9 @@
class TransportFeedbackSenderInterface {
public:
virtual ~TransportFeedbackSenderInterface() = default;
- virtual bool SendTransportFeedback(rtcp::TransportFeedback* packet) = 0;
- virtual void SendNetworkStateEstimatePacket(rtcp::RemoteEstimate* packet) = 0;
+
+ virtual bool SendCombinedRtcpPacket(
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets) = 0;
};
// TODO(holmer): Remove when all implementations have been updated.
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
index e9b6100..f66b370 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
@@ -12,6 +12,8 @@
#include <algorithm>
#include <limits>
+#include <memory>
+#include <utility>
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "rtc_base/checks.h"
@@ -187,13 +189,13 @@
if (!periodic_window_start_seq_)
return;
+ std::unique_ptr<rtcp::RemoteEstimate> remote_estimate;
if (network_state_estimator_) {
absl::optional<NetworkStateEstimate> state_estimate =
network_state_estimator_->GetCurrentEstimate();
if (state_estimate) {
- rtcp::RemoteEstimate rtcp_estimate;
- rtcp_estimate.SetEstimate(state_estimate.value());
- feedback_sender_->SendNetworkStateEstimatePacket(&rtcp_estimate);
+ remote_estimate = std::make_unique<rtcp::RemoteEstimate>();
+ remote_estimate->SetEstimate(state_estimate.value());
}
}
@@ -202,13 +204,20 @@
begin_iterator != packet_arrival_times_.cend();
begin_iterator =
packet_arrival_times_.lower_bound(*periodic_window_start_seq_)) {
- rtcp::TransportFeedback feedback_packet;
+ auto feedback_packet = std::make_unique<rtcp::TransportFeedback>();
periodic_window_start_seq_ = BuildFeedbackPacket(
feedback_packet_count_++, media_ssrc_, *periodic_window_start_seq_,
- begin_iterator, packet_arrival_times_.cend(), &feedback_packet);
+ begin_iterator, packet_arrival_times_.cend(), feedback_packet.get());
RTC_DCHECK(feedback_sender_ != nullptr);
- feedback_sender_->SendTransportFeedback(&feedback_packet);
+
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets;
+ if (remote_estimate) {
+ packets.push_back(std::move(remote_estimate));
+ }
+ packets.push_back(std::move(feedback_packet));
+
+ feedback_sender_->SendCombinedRtcpPacket(std::move(packets));
// Note: Don't erase items from packet_arrival_times_ after sending, in case
// they need to be re-sent after a reordering. Removal will be handled
// by OnPacketArrival once packets are too old.
@@ -221,7 +230,9 @@
if (feedback_request.sequence_count == 0) {
return;
}
- rtcp::TransportFeedback feedback_packet(feedback_request.include_timestamps);
+
+ auto feedback_packet = std::make_unique<rtcp::TransportFeedback>(
+ feedback_request.include_timestamps);
int64_t first_sequence_number =
sequence_number - feedback_request.sequence_count + 1;
@@ -231,13 +242,15 @@
BuildFeedbackPacket(feedback_packet_count_++, media_ssrc_,
first_sequence_number, begin_iterator, end_iterator,
- &feedback_packet);
+ feedback_packet.get());
// Clear up to the first packet that is included in this feedback packet.
packet_arrival_times_.erase(packet_arrival_times_.begin(), begin_iterator);
RTC_DCHECK(feedback_sender_ != nullptr);
- feedback_sender_->SendTransportFeedback(&feedback_packet);
+ std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets;
+ packets.push_back(std::move(feedback_packet));
+ feedback_sender_->SendCombinedRtcpPacket(std::move(packets));
}
int64_t RemoteEstimatorProxy::BuildFeedbackPacket(
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
index 30e6ef4..2d2d8af 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
@@ -10,7 +10,11 @@
#include "modules/remote_bitrate_estimator/remote_estimator_proxy.h"
+#include <memory>
+#include <utility>
+
#include "api/transport/field_trial_based_config.h"
+#include "api/transport/network_types.h"
#include "api/transport/test/mock_network_control.h"
#include "modules/pacing/packet_router.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
@@ -22,6 +26,7 @@
using ::testing::ElementsAre;
using ::testing::Invoke;
using ::testing::Return;
+using ::testing::SizeIs;
namespace webrtc {
namespace {
@@ -60,10 +65,9 @@
class MockTransportFeedbackSender : public TransportFeedbackSenderInterface {
public:
- MOCK_METHOD1(SendTransportFeedback,
- bool(rtcp::TransportFeedback* feedback_packet));
- MOCK_METHOD1(SendNetworkStateEstimatePacket,
- void(rtcp::RemoteEstimate* packet));
+ MOCK_METHOD1(
+ SendCombinedRtcpPacket,
+ bool(std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets));
};
class RemoteEstimatorProxyTest : public ::testing::Test {
@@ -116,15 +120,21 @@
TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) {
IncomingPacket(kBaseSeq, kBaseTimeMs);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq));
- EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs));
+ return true;
+ }));
Process();
}
@@ -133,15 +143,21 @@
IncomingPacket(kBaseSeq, kBaseTimeMs);
IncomingPacket(kBaseSeq, kBaseTimeMs + 1000);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq));
- EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs));
+ return true;
+ }));
Process();
}
@@ -150,23 +166,27 @@
// First feedback.
IncomingPacket(kBaseSeq, kBaseTimeMs);
IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1000);
- EXPECT_CALL(router_, SendTransportFeedback(_)).WillOnce(Return(true));
+ EXPECT_CALL(router_, SendCombinedRtcpPacket).WillOnce(Return(true));
Process();
// Second feedback starts with a missing packet (DROP kBaseSeq + 2).
IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3000);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 3));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + 3000));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 3));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + 3000));
+ return true;
+ }));
Process();
}
@@ -176,18 +196,22 @@
IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs);
IncomingPacket(kBaseSeq + 2, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 2));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs, kBaseTimeMs + kMaxSmallDeltaMs,
- kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 2));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs, kBaseTimeMs + kMaxSmallDeltaMs,
+ kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1));
+ return true;
+ }));
Process();
}
@@ -199,25 +223,35 @@
IncomingPacket(kBaseSeq, kBaseTimeMs);
IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kTooLargeDelta);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq));
- EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs));
- return true;
- }))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs));
+ return true;
+ }))
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 1));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + kTooLargeDelta));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 1));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + kTooLargeDelta));
+ return true;
+ }));
Process();
}
@@ -228,15 +262,19 @@
IncomingPacket(kBaseSeq, kBaseTimeMs);
IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([&](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kLargeSeq, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [&](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kLargeSeq, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs));
- return true;
- }));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs));
+ return true;
+ }));
Process();
}
@@ -254,17 +292,21 @@
}
// Only expect feedback for the last two packets.
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([&](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 20000 + 9, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 20009, kBaseSeq + 40009));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + 28 * kDeltaMs,
- kBaseTimeMs + 29 * kDeltaMs));
- return true;
- }));
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [&](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 20000 + 9, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 20009, kBaseSeq + 40009));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + 28 * kDeltaMs,
+ kBaseTimeMs + 29 * kDeltaMs));
+ return true;
+ }));
Process();
}
@@ -281,16 +323,20 @@
}
// Only expect feedback for the first two packets.
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([&](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 40000, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 40000, kBaseSeq));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs));
- return true;
- }));
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [&](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 40000, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 40000, kBaseSeq));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs));
+ return true;
+ }));
Process();
}
@@ -299,33 +345,41 @@
IncomingPacket(kBaseSeq, kBaseTimeMs);
IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq, kBaseSeq + 2));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs, kBaseTimeMs + 2));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq, kBaseSeq + 2));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs, kBaseTimeMs + 2));
+ return true;
+ }));
Process();
IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 1, kBaseSeq + 2));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + 1, kBaseTimeMs + 2));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 1, kBaseSeq + 2));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + 1, kBaseTimeMs + 2));
+ return true;
+ }));
Process();
}
@@ -335,21 +389,29 @@
IncomingPacket(kBaseSeq + 2, kBaseTimeMs);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence());
- EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs));
- return true;
- }));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs));
+ return true;
+ }));
Process();
IncomingPacket(kBaseSeq + 3, kTimeoutTimeMs); // kBaseSeq + 2 times out here.
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(
- Invoke([kTimeoutTimeMs](rtcp::TransportFeedback* feedback_packet) {
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [&](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
EXPECT_EQ(kBaseSeq + 3, feedback_packet->GetBaseSequence());
EXPECT_THAT(TimestampsMs(*feedback_packet),
@@ -364,9 +426,12 @@
IncomingPacket(kBaseSeq, kBaseTimeMs - 1);
IncomingPacket(kBaseSeq + 1, kTimeoutTimeMs - 1);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(
- Invoke([kTimeoutTimeMs](rtcp::TransportFeedback* feedback_packet) {
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [&](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence());
EXPECT_THAT(SequenceNumbers(*feedback_packet),
@@ -430,7 +495,7 @@
TEST_F(RemoteEstimatorProxyOnRequestTest, ProcessDoesNotSendFeedback) {
proxy_.SetSendPeriodicFeedback(false);
IncomingPacket(kBaseSeq, kBaseTimeMs);
- EXPECT_CALL(router_, SendTransportFeedback(_)).Times(0);
+ EXPECT_CALL(router_, SendCombinedRtcpPacket).Times(0);
Process();
}
@@ -440,17 +505,21 @@
IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs);
IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2 * kMaxSmallDeltaMs);
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 3, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 3, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 3));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + 3 * kMaxSmallDeltaMs));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 3));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + 3 * kMaxSmallDeltaMs));
+ return true;
+ }));
constexpr FeedbackRequest kSinglePacketFeedbackRequest = {
/*include_timestamps=*/true, /*sequence_count=*/1};
@@ -465,22 +534,26 @@
IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs);
}
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 6, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 6, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 6, kBaseSeq + 7, kBaseSeq + 8,
- kBaseSeq + 9, kBaseSeq + 10));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs,
- kBaseTimeMs + 7 * kMaxSmallDeltaMs,
- kBaseTimeMs + 8 * kMaxSmallDeltaMs,
- kBaseTimeMs + 9 * kMaxSmallDeltaMs,
- kBaseTimeMs + 10 * kMaxSmallDeltaMs));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 6, kBaseSeq + 7, kBaseSeq + 8,
+ kBaseSeq + 9, kBaseSeq + 10));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs,
+ kBaseTimeMs + 7 * kMaxSmallDeltaMs,
+ kBaseTimeMs + 8 * kMaxSmallDeltaMs,
+ kBaseTimeMs + 9 * kMaxSmallDeltaMs,
+ kBaseTimeMs + 10 * kMaxSmallDeltaMs));
+ return true;
+ }));
constexpr FeedbackRequest kFivePacketsFeedbackRequest = {
/*include_timestamps=*/true, /*sequence_count=*/5};
@@ -497,19 +570,23 @@
IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs);
}
- EXPECT_CALL(router_, SendTransportFeedback(_))
- .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) {
- EXPECT_EQ(kBaseSeq + 6, feedback_packet->GetBaseSequence());
- EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(Invoke(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ rtcp::TransportFeedback* feedback_packet =
+ static_cast<rtcp::TransportFeedback*>(
+ feedback_packets[0].get());
+ EXPECT_EQ(kBaseSeq + 6, feedback_packet->GetBaseSequence());
+ EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc());
- EXPECT_THAT(SequenceNumbers(*feedback_packet),
- ElementsAre(kBaseSeq + 6, kBaseSeq + 8, kBaseSeq + 10));
- EXPECT_THAT(TimestampsMs(*feedback_packet),
- ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs,
- kBaseTimeMs + 8 * kMaxSmallDeltaMs,
- kBaseTimeMs + 10 * kMaxSmallDeltaMs));
- return true;
- }));
+ EXPECT_THAT(SequenceNumbers(*feedback_packet),
+ ElementsAre(kBaseSeq + 6, kBaseSeq + 8, kBaseSeq + 10));
+ EXPECT_THAT(TimestampsMs(*feedback_packet),
+ ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs,
+ kBaseTimeMs + 8 * kMaxSmallDeltaMs,
+ kBaseTimeMs + 10 * kMaxSmallDeltaMs));
+ return true;
+ }));
constexpr FeedbackRequest kFivePacketsFeedbackRequest = {
/*include_timestamps=*/true, /*sequence_count=*/5};
@@ -578,11 +655,15 @@
kBaseTimeMs, kDefaultPacketSize,
CreateHeader(kBaseSeq, absl::nullopt,
AbsoluteSendTime::MsTo24Bits(kBaseTimeMs - 1)));
-
- EXPECT_CALL(router_, SendTransportFeedback(_));
EXPECT_CALL(network_state_estimator_, GetCurrentEstimate())
.WillOnce(Return(NetworkStateEstimate()));
- EXPECT_CALL(router_, SendNetworkStateEstimatePacket(_));
+ EXPECT_CALL(router_, SendCombinedRtcpPacket)
+ .WillOnce(
+ [](std::vector<std::unique_ptr<rtcp::RtcpPacket>> feedback_packets) {
+ EXPECT_THAT(feedback_packets, SizeIs(2));
+ return true;
+ });
+
Process();
}
