Move RtpPacketSender and merge it with RtpPacketPacer.
This interface is intended to only handle packet-sending parts of the
paced sender.
See https://webrtc-review.googlesource.com/c/src/+/145212 for context
Bug: webrtc:10809
Change-Id: I93f0b40e1865665c2d436db67021350a0ed0687b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145216
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28662}
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 552b987..8eeacda 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -332,7 +332,7 @@
if (allocation_settings_.IncludeAudioInAllocationOnStart(
config_.min_bitrate_bps, config_.max_bitrate_bps, config_.has_dscp,
TransportSeqNumId(config_))) {
- rtp_transport_->packet_sender()->SetAccountForAudioPackets(true);
+ rtp_transport_->AccountForAudioPacketsInPacedSender(true);
rtp_rtcp_module_->SetAsPartOfAllocation(true);
rtc::Event thread_sync_event;
worker_queue_->PostTask([&] {
@@ -796,7 +796,7 @@
if (stream->allocation_settings_.IncludeAudioInAllocationOnReconfigure(
new_config.min_bitrate_bps, new_config.max_bitrate_bps,
new_config.has_dscp, TransportSeqNumId(new_config))) {
- stream->rtp_transport_->packet_sender()->SetAccountForAudioPackets(true);
+ stream->rtp_transport_->AccountForAudioPacketsInPacedSender(true);
rtc::Event thread_sync_event;
stream->worker_queue_->PostTask([&] {
RTC_DCHECK_RUN_ON(stream->worker_queue_);
@@ -813,7 +813,7 @@
thread_sync_event.Wait(rtc::Event::kForever);
stream->rtp_rtcp_module_->SetAsPartOfAllocation(true);
} else {
- stream->rtp_transport_->packet_sender()->SetAccountForAudioPackets(false);
+ stream->rtp_transport_->AccountForAudioPacketsInPacedSender(false);
stream->RemoveBitrateObserver();
stream->rtp_rtcp_module_->SetAsPartOfAllocation(false);
}
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index 72eacb3..f00e0dcd 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -365,11 +365,11 @@
TransportSequenceNumberAllocator* seq_num_allocator_ RTC_GUARDED_BY(&crit_);
};
-class RtpPacketSenderProxy : public RtpPacketPacer {
+class RtpPacketSenderProxy : public RtpPacketSender {
public:
RtpPacketSenderProxy() : rtp_packet_pacer_(nullptr) {}
- void SetPacketPacer(RtpPacketPacer* rtp_packet_pacer) {
+ void SetPacketPacer(RtpPacketSender* rtp_packet_pacer) {
RTC_DCHECK(thread_checker_.IsCurrent());
rtc::CritScope lock(&crit_);
rtp_packet_pacer_ = rtp_packet_pacer;
@@ -394,14 +394,10 @@
}
}
- void SetAccountForAudioPackets(bool account_for_audio) override {
- RTC_NOTREACHED();
- }
-
private:
rtc::ThreadChecker thread_checker_;
rtc::CriticalSection crit_;
- RtpPacketPacer* rtp_packet_pacer_ RTC_GUARDED_BY(&crit_);
+ RtpPacketSender* rtp_packet_pacer_ RTC_GUARDED_BY(&crit_);
};
class VoERtcpObserver : public RtcpBandwidthObserver {
@@ -1005,7 +1001,7 @@
RtpTransportControllerSendInterface* transport,
RtcpBandwidthObserver* bandwidth_observer) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
- RtpPacketPacer* rtp_packet_pacer = transport->packet_sender();
+ RtpPacketSender* rtp_packet_pacer = transport->packet_sender();
TransportFeedbackObserver* transport_feedback_observer =
transport->transport_feedback_observer();
PacketRouter* packet_router = transport->packet_router();
diff --git a/call/call.cc b/call/call.cc
index bd9a462..07b29d5 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -503,12 +503,13 @@
call_stats_->DeregisterStatsObserver(&receive_side_cc_);
}
- int64_t first_sent_packet_ms = transport_send_->GetFirstPacketTimeMs();
+ absl::optional<int64_t> first_sent_packet_ms =
+ transport_send_->GetFirstPacketTimeMs();
// Only update histograms after process threads have been shut down, so that
// they won't try to concurrently update stats.
- {
+ if (first_sent_packet_ms) {
rtc::CritScope lock(&bitrate_crit_);
- UpdateSendHistograms(first_sent_packet_ms);
+ UpdateSendHistograms(*first_sent_packet_ms);
}
UpdateReceiveHistograms();
UpdateHistograms();
@@ -619,8 +620,6 @@
}
void Call::UpdateSendHistograms(int64_t first_sent_packet_ms) {
- if (first_sent_packet_ms == -1)
- return;
int64_t elapsed_sec =
(clock_->TimeInMilliseconds() - first_sent_packet_ms) / 1000;
if (elapsed_sec < metrics::kMinRunTimeInSeconds)
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index a1a42e1..e08e0f1 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -167,7 +167,7 @@
return this;
}
-RtpPacketPacer* RtpTransportControllerSend::packet_sender() {
+RtpPacketSender* RtpTransportControllerSend::packet_sender() {
return &pacer_;
}
@@ -398,6 +398,11 @@
}
}
+void RtpTransportControllerSend::AccountForAudioPacketsInPacedSender(
+ bool account_for_audio) {
+ pacer_.SetAccountForAudioPackets(account_for_audio);
+}
+
void RtpTransportControllerSend::OnReceivedEstimatedBitrate(uint32_t bitrate) {
RemoteBitrateReport msg;
msg.receive_time = Timestamp::ms(clock_->TimeInMilliseconds());
diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h
index e7e1610..3a844ea 100644
--- a/call/rtp_transport_controller_send.h
+++ b/call/rtp_transport_controller_send.h
@@ -76,7 +76,7 @@
NetworkStateEstimateObserver* network_state_estimate_observer() override;
TransportFeedbackObserver* transport_feedback_observer() override;
- RtpPacketPacer* packet_sender() override;
+ RtpPacketSender* packet_sender() override;
void SetAllocatedSendBitrateLimits(int min_send_bitrate_bps,
int max_padding_bitrate_bps,
@@ -106,6 +106,8 @@
void OnTransportOverheadChanged(
size_t transport_overhead_per_packet) override;
+ void AccountForAudioPacketsInPacedSender(bool account_for_audio) override;
+
// Implements RtcpBandwidthObserver interface
void OnReceivedEstimatedBitrate(uint32_t bitrate) override;
void OnReceivedRtcpReceiverReport(const ReportBlockList& report_blocks,
diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h
index 0178758..39358d5 100644
--- a/call/rtp_transport_controller_send_interface.h
+++ b/call/rtp_transport_controller_send_interface.h
@@ -27,7 +27,7 @@
#include "logging/rtc_event_log/rtc_event_log.h"
#include "modules/rtp_rtcp/include/report_block_data.h"
#include "modules/rtp_rtcp/include/rtcp_statistics.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
@@ -119,7 +119,7 @@
virtual NetworkStateEstimateObserver* network_state_estimate_observer() = 0;
virtual TransportFeedbackObserver* transport_feedback_observer() = 0;
- virtual RtpPacketPacer* packet_sender() = 0;
+ virtual RtpPacketSender* packet_sender() = 0;
// SetAllocatedSendBitrateLimits sets bitrates limits imposed by send codec
// settings.
@@ -160,6 +160,8 @@
virtual void OnTransportOverheadChanged(
size_t transport_overhead_per_packet) = 0;
+
+ virtual void AccountForAudioPacketsInPacedSender(bool account_for_audio) = 0;
};
} // namespace webrtc
diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h
index 81db587..74041b3 100644
--- a/call/test/mock_rtp_transport_controller_send.h
+++ b/call/test/mock_rtp_transport_controller_send.h
@@ -48,7 +48,7 @@
MOCK_METHOD0(network_state_estimate_observer,
NetworkStateEstimateObserver*());
MOCK_METHOD0(transport_feedback_observer, TransportFeedbackObserver*());
- MOCK_METHOD0(packet_sender, RtpPacketPacer*());
+ MOCK_METHOD0(packet_sender, RtpPacketSender*());
MOCK_METHOD3(SetAllocatedSendBitrateLimits, void(int, int, int));
MOCK_METHOD1(SetPacingFactor, void(float));
MOCK_METHOD1(SetQueueTimeLimit, void(int));
@@ -67,6 +67,7 @@
MOCK_METHOD1(SetSdpBitrateParameters, void(const BitrateConstraints&));
MOCK_METHOD1(SetClientBitratePreferences, void(const BitrateSettings&));
MOCK_METHOD1(OnTransportOverheadChanged, void(size_t));
+ MOCK_METHOD1(AccountForAudioPacketsInPacedSender, void(bool));
MOCK_METHOD1(OnReceivedPacket, void(const ReceivedPacket&));
};
} // namespace webrtc
diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc
index 58ae5fa..63e3156 100644
--- a/modules/pacing/paced_sender.cc
+++ b/modules/pacing/paced_sender.cc
@@ -212,10 +212,10 @@
RtpPacketToSend::Type type;
switch (priority) {
- case RtpPacketPacer::kHighPriority:
+ case RtpPacketSender::kHighPriority:
type = RtpPacketToSend::Type::kAudio;
break;
- case RtpPacketPacer::kNormalPriority:
+ case RtpPacketSender::kNormalPriority:
type = RtpPacketToSend::Type::kRetransmission;
break;
default:
diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h
index 3bc628d..85a49ec 100644
--- a/modules/pacing/paced_sender.h
+++ b/modules/pacing/paced_sender.h
@@ -27,8 +27,7 @@
#include "modules/pacing/interval_budget.h"
#include "modules/pacing/packet_router.h"
#include "modules/pacing/round_robin_packet_queue.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
-#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
#include "modules/utility/include/process_thread.h"
#include "rtc_base/critical_section.h"
@@ -39,7 +38,7 @@
class Clock;
class RtcEventLog;
-class PacedSender : public Module, public RtpPacketPacer {
+class PacedSender : public Module, public RtpPacketSender {
public:
static constexpr int64_t kNoCongestionWindow = -1;
@@ -98,7 +97,7 @@
// With the introduction of audio BWE audio traffic will be accounted for
// the pacer budget calculation. The audio traffic still will be injected
// at high priority.
- void SetAccountForAudioPackets(bool account_for_audio) override;
+ void SetAccountForAudioPackets(bool account_for_audio);
// Returns the time since the oldest queued packet was enqueued.
virtual int64_t QueueInMs() const;
diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
index 98a512e..95d3801 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -15,7 +15,7 @@
"include/rtcp_statistics.h",
"include/rtp_cvo.h",
"include/rtp_header_extension_map.h",
- "include/rtp_packet_pacer.h",
+ "include/rtp_packet_sender.h",
"include/rtp_rtcp_defines.h",
"source/byte_io.h",
"source/rtcp_packet.h",
diff --git a/modules/rtp_rtcp/include/rtp_packet_pacer.h b/modules/rtp_rtcp/include/rtp_packet_pacer.h
deleted file mode 100644
index 180ddf7..0000000
--- a/modules/rtp_rtcp/include/rtp_packet_pacer.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#ifndef MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_PACER_H_
-#define MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_PACER_H_
-
-#include <memory>
-
-#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
-namespace webrtc {
-
-// Interface for a paced sender, as implemented in the pacing module.
-// This intended to replace the RtpPacketSender interface defined in
-// rtp_rtcp_defines.h
-// TODO(bugs.webrtc.org/10633): Add things missing to this interface so that we
-// can use multiple different pacer implementations, and stop inheriting from
-// RtpPacketSender.
-class RtpPacketPacer : public RtpPacketSender {
- public:
- RtpPacketPacer() = default;
- ~RtpPacketPacer() override = default;
-
- // Insert packet into queue, for eventual transmission. Based on the type of
- // the packet, it will prioritized and scheduled relative to other packets and
- // the current target send rate.
- virtual void EnqueuePacket(std::unique_ptr<RtpPacketToSend> packet) = 0;
-};
-
-} // namespace webrtc
-
-#endif // MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_PACER_H_
diff --git a/modules/rtp_rtcp/include/rtp_packet_sender.h b/modules/rtp_rtcp/include/rtp_packet_sender.h
new file mode 100644
index 0000000..493ec1b
--- /dev/null
+++ b/modules/rtp_rtcp/include/rtp_packet_sender.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_SENDER_H_
+#define MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_SENDER_H_
+
+#include <memory>
+
+#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
+
+namespace webrtc {
+
+// TODO(bugs.webrtc.org/10633): Remove Priority and InsertPacket when old pacer
+// code path is gone.
+class RtpPacketSender {
+ public:
+ virtual ~RtpPacketSender() = default;
+
+ // These are part of the legacy PacedSender interface and will be removed.
+ enum Priority {
+ kHighPriority = 0, // Pass through; will be sent immediately.
+ kNormalPriority = 2, // Put in back of the line.
+ kLowPriority = 3, // Put in back of the low priority line.
+ };
+
+ // Adds the packet information to the queue and call TimeToSendPacket when
+ // it's time to send.
+ virtual void InsertPacket(Priority priority,
+ uint32_t ssrc,
+ uint16_t sequence_number,
+ int64_t capture_time_ms,
+ size_t bytes,
+ bool retransmission) = 0;
+
+ // Insert packet into queue, for eventual transmission. Based on the type of
+ // the packet, it will be prioritized and scheduled relative to other packets
+ // and the current target send rate.
+ virtual void EnqueuePacket(std::unique_ptr<RtpPacketToSend> packet) = 0;
+};
+
+} // namespace webrtc
+
+#endif // MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_SENDER_H_
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index f4a8c9d..0ff6753 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -25,7 +25,7 @@
#include "modules/rtp_rtcp/include/flexfec_sender.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/include/report_block_data.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
#include "rtc_base/constructor_magic.h"
@@ -89,7 +89,7 @@
RemoteBitrateEstimator* remote_bitrate_estimator = nullptr;
// Spread any bursts of packets into smaller bursts to minimize packet loss.
- RtpPacketPacer* paced_sender = nullptr;
+ RtpPacketSender* paced_sender = nullptr;
// Generate FlexFEC packets.
// TODO(brandtr): Remove when FlexfecSender is wired up to PacedSender.
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index a779633..81c1a98 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -340,37 +340,6 @@
virtual ~RtcpRttStats() {}
};
-// This class will be deprecated and replaced with RtpPacketPacer.
-class RtpPacketSender {
- public:
- RtpPacketSender() {}
- virtual ~RtpPacketSender() {}
-
- // These are part of the legacy PacedSender interface and will be removed.
- enum Priority {
- kHighPriority = 0, // Pass through; will be sent immediately.
- kNormalPriority = 2, // Put in back of the line.
- kLowPriority = 3, // Put in back of the low priority line.
- };
-
- // Adds the packet information to the queue and call TimeToSendPacket when
- // it's time to send.
- virtual void InsertPacket(Priority priority,
- uint32_t ssrc,
- uint16_t sequence_number,
- int64_t capture_time_ms,
- size_t bytes,
- bool retransmission) = 0;
-
- // Currently audio traffic is not accounted by pacer and passed through.
- // With the introduction of audio BWE audio traffic will be accounted for
- // the pacer budget calculation. The audio traffic still will be injected
- // at high priority.
- // TODO(alexnarest): Make it pure virtual after rtp_sender_unittest will be
- // updated to support it.
- virtual void SetAccountForAudioPackets(bool account_for_audio) {}
-};
-
class TransportSequenceNumberAllocator {
public:
TransportSequenceNumberAllocator() {}
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 98f93cc..62fe25c 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -213,7 +213,7 @@
bool audio,
Clock* clock,
Transport* transport,
- RtpPacketPacer* paced_sender,
+ RtpPacketSender* paced_sender,
absl::optional<uint32_t> flexfec_ssrc,
TransportSequenceNumberAllocator* sequence_number_allocator,
TransportFeedbackObserver* transport_feedback_observer,
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index f79b71d..50bbd30 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -24,7 +24,7 @@
#include "api/transport/webrtc_key_value_config.h"
#include "modules/rtp_rtcp/include/flexfec_sender.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_packet_history.h"
@@ -52,7 +52,7 @@
RTPSender(bool audio,
Clock* clock,
Transport* transport,
- RtpPacketPacer* paced_sender,
+ RtpPacketSender* paced_sender,
absl::optional<uint32_t> flexfec_ssrc,
TransportSequenceNumberAllocator* sequence_number_allocator,
TransportFeedbackObserver* transport_feedback_callback,
@@ -249,7 +249,7 @@
const absl::optional<uint32_t> flexfec_ssrc_;
- RtpPacketPacer* const paced_sender_;
+ RtpPacketSender* const paced_sender_;
TransportSequenceNumberAllocator* const transport_sequence_number_allocator_;
TransportFeedbackObserver* const transport_feedback_observer_;
rtc::CriticalSection send_critsect_;
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index a93a1a6..c338255 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -22,7 +22,7 @@
#include "modules/rtp_rtcp/include/rtp_cvo.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_header_parser.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "modules/rtp_rtcp/source/rtp_format_video_generic.h"
@@ -166,7 +166,7 @@
} // namespace
-class MockRtpPacketPacer : public RtpPacketPacer {
+class MockRtpPacketPacer : public RtpPacketSender {
public:
MockRtpPacketPacer() {}
virtual ~MockRtpPacketPacer() {}
@@ -180,6 +180,15 @@
int64_t capture_time_ms,
size_t bytes,
bool retransmission));
+
+ MOCK_METHOD2(CreateProbeCluster, void(int bitrate_bps, int cluster_id));
+
+ MOCK_METHOD0(Pause, void());
+ MOCK_METHOD0(Resume, void());
+ MOCK_METHOD1(SetCongestionWindow,
+ void(absl::optional<int64_t> congestion_window_bytes));
+ MOCK_METHOD1(UpdateOutstandingData, void(int64_t outstanding_bytes));
+ MOCK_METHOD1(SetAccountForAudioPackets, void(bool account_for_audio));
};
class MockTransportSequenceNumberAllocator