Cleanup TransportFeedbackObserver from RtpSenderEgress
TransportFeedbackObserver is thus unused from WebRTC except from
DEPRECATED_RtpSender
Change-Id: Ib308f5331a342a4ec4f7c7cfdf6f76c3c4c1807c
Bug: webrtc:15368
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/344721
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42012}
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
index e41a3fd..9fbe93f 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -75,6 +75,11 @@
NetworkLinkRtcpObserver* network_link_rtcp_observer = nullptr;
NetworkStateEstimateObserver* network_state_estimate_observer = nullptr;
+
+ // DEPRECATED, transport_feedback_callback is no longer invoked by the RTP
+ // module except from DEPRECATED_RtpSenderEgress.
+ // TODO: bugs.webrtc.org/15368 - Delete once DEPRECATED_RtpSenderEgress is
+ // deleted.
TransportFeedbackObserver* transport_feedback_callback = nullptr;
VideoBitrateAllocationObserver* bitrate_allocation_observer = nullptr;
RtcpRttStats* rtt_stats = nullptr;
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc
index 705e9d5..665c7ce 100644
--- a/modules/rtp_rtcp/source/rtp_sender_egress.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc
@@ -94,7 +94,6 @@
is_audio_(config.audio),
need_rtp_packet_infos_(config.need_rtp_packet_infos),
fec_generator_(config.fec_generator),
- transport_feedback_observer_(config.transport_feedback_callback),
send_packet_observer_(config.send_packet_observer),
rtp_stats_callback_(config.rtp_stats_callback),
bitrate_callback_(config.send_bitrate_observer),
@@ -107,6 +106,9 @@
kRtpSequenceNumberMapMaxEntries)
: nullptr) {
RTC_DCHECK(worker_queue_);
+ RTC_DCHECK(config.transport_feedback_callback == nullptr)
+ << "transport_feedback_callback is no longer used and will soon be "
+ "deleted.";
if (bitrate_callback_) {
update_task_ = RepeatingTaskHandle::DelayedStart(worker_queue_,
kUpdateInterval, [this]() {
@@ -269,10 +271,6 @@
} else if (packet->transport_sequence_number()) {
options.packet_id = *packet->transport_sequence_number();
}
- if (options.packet_id >= 0 && transport_feedback_observer_) {
- transport_feedback_observer_->OnAddPacket(
- RtpPacketSendInfo::From(*packet, pacing_info));
- }
if (packet->packet_type() != RtpPacketMediaType::kPadding &&
packet->packet_type() != RtpPacketMediaType::kRetransmission &&
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h
index 7f038f6..cb8a4dc 100644
--- a/modules/rtp_rtcp/source/rtp_sender_egress.h
+++ b/modules/rtp_rtcp/source/rtp_sender_egress.h
@@ -143,7 +143,6 @@
absl::optional<uint16_t> last_sent_seq_ RTC_GUARDED_BY(worker_queue_);
absl::optional<uint16_t> last_sent_rtx_seq_ RTC_GUARDED_BY(worker_queue_);
- TransportFeedbackObserver* const transport_feedback_observer_;
SendPacketObserver* const send_packet_observer_;
StreamDataCountersCallback* const rtp_stats_callback_;
BitrateStatisticsObserver* const bitrate_callback_;
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
index 908df95..be79601 100644
--- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
@@ -65,11 +65,6 @@
(override));
};
-class MockTransportFeedbackObserver : public TransportFeedbackObserver {
- public:
- MOCK_METHOD(void, OnAddPacket, (const RtpPacketSendInfo&), (override));
-};
-
class MockStreamDataCountersCallback : public StreamDataCountersCallback {
public:
MOCK_METHOD(void,
@@ -141,7 +136,6 @@
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
config.rtp_stats_callback = &mock_rtp_stats_callback_;
- config.transport_feedback_callback = &feedback_observer_;
config.populate_network2_timestamp = false;
config.field_trials = &trials_;
return config;
@@ -173,7 +167,6 @@
NiceMock<MockRtcEventLog> mock_rtc_event_log_;
NiceMock<MockStreamDataCountersCallback> mock_rtp_stats_callback_;
NiceMock<MockSendPacketObserver> send_packet_observer_;
- NiceMock<MockTransportFeedbackObserver> feedback_observer_;
RtpHeaderExtensionMap header_extensions_;
NiceMock<TestTransport> transport_;
RtpPacketHistory packet_history_;
@@ -181,34 +174,6 @@
uint16_t sequence_number_;
};
-TEST_F(RtpSenderEgressTest, TransportFeedbackObserverGetsCorrectByteCount) {
- constexpr size_t kRtpOverheadBytesPerPacket = 12 + 8;
- constexpr size_t kPayloadSize = 1400;
- const uint16_t kTransportSequenceNumber = 17;
-
- header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
- TransportSequenceNumber::Uri());
-
- const size_t expected_bytes = kPayloadSize + kRtpOverheadBytesPerPacket;
-
- EXPECT_CALL(
- feedback_observer_,
- OnAddPacket(AllOf(
- Field(&RtpPacketSendInfo::media_ssrc, kSsrc),
- Field(&RtpPacketSendInfo::transport_sequence_number,
- kTransportSequenceNumber),
- Field(&RtpPacketSendInfo::rtp_sequence_number, kStartSequenceNumber),
- Field(&RtpPacketSendInfo::length, expected_bytes),
- Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo()))));
-
- std::unique_ptr<RtpPacketToSend> packet = BuildRtpPacket();
- packet->set_transport_sequence_number(kTransportSequenceNumber);
- packet->AllocatePayload(kPayloadSize);
-
- std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
- sender->SendPacket(std::move(packet), PacedPacketInfo());
-}
-
TEST_F(RtpSenderEgressTest, SendsPacketsOneByOneWhenNotBatching) {
std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
EXPECT_CALL(transport_,
@@ -926,115 +891,6 @@
EXPECT_EQ(rtx_stats.retransmitted.packets, 1u);
}
-TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRetransmission) {
- const uint16_t kTransportSequenceNumber = 17;
- header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
- TransportSequenceNumber::Uri());
- std::unique_ptr<RtpPacketToSend> retransmission = BuildRtpPacket();
- retransmission->set_packet_type(RtpPacketMediaType::kRetransmission);
- retransmission->set_transport_sequence_number(kTransportSequenceNumber);
- retransmission->set_original_ssrc(kSsrc);
- uint16_t retransmitted_seq = retransmission->SequenceNumber() - 2;
- retransmission->set_retransmitted_sequence_number(retransmitted_seq);
-
- std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
- EXPECT_CALL(
- feedback_observer_,
- OnAddPacket(AllOf(
- Field(&RtpPacketSendInfo::media_ssrc, kSsrc),
- Field(&RtpPacketSendInfo::rtp_sequence_number, retransmitted_seq),
- Field(&RtpPacketSendInfo::transport_sequence_number,
- kTransportSequenceNumber))));
- sender->SendPacket(std::move(retransmission), PacedPacketInfo());
-}
-
-TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) {
- const uint16_t kTransportSequenceNumber = 17;
- header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
- TransportSequenceNumber::Uri());
-
- std::unique_ptr<RtpPacketToSend> rtx_retransmission = BuildRtpPacket();
- rtx_retransmission->SetSsrc(kRtxSsrc);
- rtx_retransmission->set_transport_sequence_number(kTransportSequenceNumber);
- rtx_retransmission->set_original_ssrc(kSsrc);
- rtx_retransmission->set_packet_type(RtpPacketMediaType::kRetransmission);
- uint16_t rtx_retransmitted_seq = rtx_retransmission->SequenceNumber() - 2;
- rtx_retransmission->set_retransmitted_sequence_number(rtx_retransmitted_seq);
-
- std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
- EXPECT_CALL(
- feedback_observer_,
- OnAddPacket(AllOf(
- Field(&RtpPacketSendInfo::media_ssrc, kSsrc),
- Field(&RtpPacketSendInfo::rtp_sequence_number, rtx_retransmitted_seq),
- Field(&RtpPacketSendInfo::transport_sequence_number,
- kTransportSequenceNumber))));
- sender->SendPacket(std::move(rtx_retransmission), PacedPacketInfo());
-}
-
-TEST_F(RtpSenderEgressTest, TransportFeedbackObserverPadding) {
- const uint16_t kTransportSequenceNumber = 17;
- header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
- TransportSequenceNumber::Uri());
- std::unique_ptr<RtpPacketToSend> padding = BuildRtpPacket();
- padding->SetPadding(224);
- padding->set_packet_type(RtpPacketMediaType::kPadding);
- padding->set_transport_sequence_number(kTransportSequenceNumber);
-
- std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
- EXPECT_CALL(
- feedback_observer_,
- OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt),
- Field(&RtpPacketSendInfo::transport_sequence_number,
- kTransportSequenceNumber))));
- sender->SendPacket(std::move(padding), PacedPacketInfo());
-}
-
-TEST_F(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) {
- const uint16_t kTransportSequenceNumber = 17;
- header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
- TransportSequenceNumber::Uri());
-
- std::unique_ptr<RtpPacketToSend> rtx_padding = BuildRtpPacket();
- rtx_padding->SetPadding(224);
- rtx_padding->SetSsrc(kRtxSsrc);
- rtx_padding->set_packet_type(RtpPacketMediaType::kPadding);
- rtx_padding->set_transport_sequence_number(kTransportSequenceNumber);
-
- std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
- EXPECT_CALL(
- feedback_observer_,
- OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt),
- Field(&RtpPacketSendInfo::transport_sequence_number,
- kTransportSequenceNumber))));
- sender->SendPacket(std::move(rtx_padding), PacedPacketInfo());
-}
-
-TEST_F(RtpSenderEgressTest, TransportFeedbackObserverFec) {
- const uint16_t kTransportSequenceNumber = 17;
- header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
- TransportSequenceNumber::Uri());
-
- std::unique_ptr<RtpPacketToSend> fec_packet = BuildRtpPacket();
- fec_packet->SetSsrc(kFlexFecSsrc);
- fec_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection);
- fec_packet->set_transport_sequence_number(kTransportSequenceNumber);
-
- const rtc::ArrayView<const RtpExtensionSize> kNoRtpHeaderExtensionSizes;
- FlexfecSender flexfec(kFlexfectPayloadType, kFlexFecSsrc, kSsrc, /*mid=*/"",
- /*header_extensions=*/{}, kNoRtpHeaderExtensionSizes,
- /*rtp_state=*/nullptr, time_controller_.GetClock());
- RtpRtcpInterface::Configuration config = DefaultConfig();
- config.fec_generator = &flexfec;
- auto sender = std::make_unique<RtpSenderEgress>(config, &packet_history_);
- EXPECT_CALL(
- feedback_observer_,
- OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt),
- Field(&RtpPacketSendInfo::transport_sequence_number,
- kTransportSequenceNumber))));
- sender->SendPacket(std::move(fec_packet), PacedPacketInfo());
-}
-
TEST_F(RtpSenderEgressTest, SupportsAbortingRetransmissions) {
std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
packet_history_.SetStorePacketsStatus(
