Move towards always using packet type instead of priority in RTPSender
Bug: webrtc:10633
Change-Id: I835686f58f9edcf0c7cec8f0b3d54eb93f2920df
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143176
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28349}
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 68a8e21..ae811a2 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -87,6 +87,42 @@
RtpGenericFrameDescriptorExtension01::kMaxSizeBytes},
};
+// TODO(bugs.webrtc.org/10633): Remove when downstream code stops using
+// priority. At the time of writing, the priority can be directly mapped to a
+// packet type. This is only for a transition period.
+RtpPacketToSend::Type PacketPriorityToType(RtpPacketSender::Priority priority) {
+ switch (priority) {
+ case RtpPacketSender::Priority::kLowPriority:
+ return RtpPacketToSend::Type::kVideo;
+ case RtpPacketSender::Priority::kNormalPriority:
+ return RtpPacketToSend::Type::kRetransmission;
+ case RtpPacketSender::Priority::kHighPriority:
+ return RtpPacketToSend::Type::kAudio;
+ default:
+ RTC_NOTREACHED() << "Unexpected priority: " << priority;
+ return RtpPacketToSend::Type::kVideo;
+ }
+}
+
+// TODO(bugs.webrtc.org/10633): Remove when packets are always owned by pacer.
+RtpPacketSender::Priority PacketTypeToPriority(RtpPacketToSend::Type type) {
+ switch (type) {
+ case RtpPacketToSend::Type::kAudio:
+ return RtpPacketSender::Priority::kHighPriority;
+ case RtpPacketToSend::Type::kVideo:
+ return RtpPacketSender::Priority::kLowPriority;
+ case RtpPacketToSend::Type::kRetransmission:
+ return RtpPacketSender::Priority::kNormalPriority;
+ case RtpPacketToSend::Type::kForwardErrorCorrection:
+ return RtpPacketSender::Priority::kLowPriority;
+ break;
+ case RtpPacketToSend::Type::kPadding:
+ RTC_NOTREACHED() << "Unexpected type for legacy path: kPadding";
+ break;
+ }
+ return RtpPacketSender::Priority::kLowPriority;
+}
+
} // namespace
RTPSender::RTPSender(
@@ -802,8 +838,7 @@
}
bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
- StorageType storage,
- RtpPacketSender::Priority priority) {
+ StorageType storage) {
RTC_DCHECK(packet);
int64_t now_ms = clock_->TimeInMilliseconds();
@@ -813,6 +848,8 @@
int64_t capture_time_ms = packet->capture_time_ms();
size_t packet_size =
send_side_bwe_with_overhead_ ? packet->size() : packet->payload_size();
+ auto packet_type = packet->packet_type();
+ RTC_DCHECK(packet_type.has_value());
if (ssrc == FlexfecSsrc()) {
// Store FlexFEC packets in the history here, so they can be found
// when the pacer calls TimeToSendPacket.
@@ -822,8 +859,8 @@
packet_history_.PutRtpPacket(std::move(packet), storage, absl::nullopt);
}
- paced_sender_->InsertPacket(priority, ssrc, seq_no, capture_time_ms,
- packet_size, false);
+ paced_sender_->InsertPacket(PacketTypeToPriority(*packet_type), ssrc,
+ seq_no, capture_time_ms, packet_size, false);
return true;
}
@@ -884,6 +921,13 @@
return sent;
}
+bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
+ StorageType storage,
+ RtpPacketSender::Priority priority) {
+ packet->set_packet_type(PacketPriorityToType(priority));
+ return SendToNetwork(std::move(packet), storage);
+}
+
void RTPSender::RecomputeMaxSendDelay() {
max_delay_it_ = send_delays_.begin();
for (auto it = send_delays_.begin(); it != send_delays_.end(); ++it) {
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 94ef809..5e86742 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -159,6 +159,10 @@
// Sends packet to |transport_| or to the pacer, depending on configuration.
bool SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
+ StorageType storage);
+
+ // Fallback that infers PacketType from Priority.
+ bool SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
StorageType storage,
RtpPacketSender::Priority priority);
diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.cc b/modules/rtp_rtcp/source/rtp_sender_audio.cc
index c54cb91..9e5e71c 100644
--- a/modules/rtp_rtcp/source/rtp_sender_audio.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_audio.cc
@@ -255,8 +255,9 @@
TRACE_EVENT_ASYNC_END2("webrtc", "Audio", rtp_timestamp, "timestamp",
packet->Timestamp(), "seqnum",
packet->SequenceNumber());
- bool send_result = LogAndSendToNetwork(
- std::move(packet), kAllowRetransmission, RtpPacketSender::kHighPriority);
+ packet->set_packet_type(RtpPacketToSend::Type::kAudio);
+ bool send_result =
+ LogAndSendToNetwork(std::move(packet), kAllowRetransmission);
if (first_packet_sent_()) {
RTC_LOG(LS_INFO) << "First audio RTP packet sent to pacer";
}
@@ -339,8 +340,8 @@
dtmfbuffer[1] = E | R | volume;
ByteWriter<uint16_t>::WriteBigEndian(dtmfbuffer + 2, duration);
- result = LogAndSendToNetwork(std::move(packet), kAllowRetransmission,
- RtpPacketSender::kHighPriority);
+ packet->set_packet_type(RtpPacketToSend::Type::kAudio);
+ result = LogAndSendToNetwork(std::move(packet), kAllowRetransmission);
send_count--;
} while (send_count > 0 && result);
@@ -349,8 +350,7 @@
bool RTPSenderAudio::LogAndSendToNetwork(
std::unique_ptr<RtpPacketToSend> packet,
- StorageType storage,
- RtpPacketSender::Priority priority) {
+ StorageType storage) {
#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE
int64_t now_ms = clock_->TimeInMilliseconds();
BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "AudioTotBitrate_kbps", now_ms,
@@ -360,8 +360,7 @@
rtp_sender_->NackOverheadRate() / 1000,
packet->Ssrc());
#endif
-
- return rtp_sender_->SendToNetwork(std::move(packet), storage, priority);
+ return rtp_sender_->SendToNetwork(std::move(packet), storage);
}
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.h b/modules/rtp_rtcp/source/rtp_sender_audio.h
index 1c78e92..d8a61fd 100644
--- a/modules/rtp_rtcp/source/rtp_sender_audio.h
+++ b/modules/rtp_rtcp/source/rtp_sender_audio.h
@@ -64,8 +64,7 @@
private:
bool LogAndSendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
- StorageType storage,
- RtpPacketSender::Priority priority);
+ StorageType storage);
Clock* const clock_ = nullptr;
RTPSender* const rtp_sender_ = nullptr;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index 419e938..bbadda8 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -251,8 +251,8 @@
// Remember some values about the packet before sending it away.
size_t packet_size = packet->size();
uint16_t seq_num = packet->SequenceNumber();
- if (!LogAndSendToNetwork(std::move(packet), storage,
- RtpPacketSender::kLowPriority)) {
+ packet->set_packet_type(RtpPacketToSend::Type::kVideo);
+ if (!LogAndSendToNetwork(std::move(packet), storage)) {
RTC_LOG(LS_WARNING) << "Failed to send video packet " << seq_num;
return;
}
@@ -293,8 +293,8 @@
}
// Send |red_packet| instead of |packet| for allocated sequence number.
size_t red_packet_size = red_packet->size();
- if (LogAndSendToNetwork(std::move(red_packet), media_packet_storage,
- RtpPacketSender::kLowPriority)) {
+ red_packet->set_packet_type(RtpPacketToSend::Type::kVideo);
+ if (LogAndSendToNetwork(std::move(red_packet), media_packet_storage)) {
rtc::CritScope cs(&stats_crit_);
video_bitrate_.Update(red_packet_size, clock_->TimeInMilliseconds());
} else {
@@ -309,8 +309,7 @@
rtp_packet->set_capture_time_ms(media_packet->capture_time_ms());
rtp_packet->set_packet_type(RtpPacketToSend::Type::kForwardErrorCorrection);
uint16_t fec_sequence_number = rtp_packet->SequenceNumber();
- if (LogAndSendToNetwork(std::move(rtp_packet), kDontRetransmit,
- RtpPacketSender::kLowPriority)) {
+ if (LogAndSendToNetwork(std::move(rtp_packet), kDontRetransmit)) {
rtc::CritScope cs(&stats_crit_);
fec_bitrate_.Update(fec_packet->length(), clock_->TimeInMilliseconds());
} else {
@@ -337,8 +336,9 @@
for (auto& fec_packet : fec_packets) {
size_t packet_length = fec_packet->size();
uint16_t seq_num = fec_packet->SequenceNumber();
- if (LogAndSendToNetwork(std::move(fec_packet), kDontRetransmit,
- RtpPacketSender::kLowPriority)) {
+ fec_packet->set_packet_type(
+ RtpPacketToSend::Type::kForwardErrorCorrection);
+ if (LogAndSendToNetwork(std::move(fec_packet), kDontRetransmit)) {
rtc::CritScope cs(&stats_crit_);
fec_bitrate_.Update(packet_length, clock_->TimeInMilliseconds());
} else {
@@ -350,8 +350,7 @@
bool RTPSenderVideo::LogAndSendToNetwork(
std::unique_ptr<RtpPacketToSend> packet,
- StorageType storage,
- RtpPacketSender::Priority priority) {
+ StorageType storage) {
#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE
int64_t now_ms = clock_->TimeInMilliseconds();
BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms,
@@ -363,7 +362,7 @@
rtp_sender_->NackOverheadRate() / 1000,
packet->Ssrc());
#endif
- return rtp_sender_->SendToNetwork(std::move(packet), storage, priority);
+ return rtp_sender_->SendToNetwork(std::move(packet), storage);
}
void RTPSenderVideo::SetUlpfecConfig(int red_payload_type,
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index 94377bf..3555958 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -143,8 +143,7 @@
bool protect_media_packet);
bool LogAndSendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
- StorageType storage,
- RtpPacketSender::Priority priority);
+ StorageType storage);
bool red_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) {
return red_payload_type_ >= 0;
