From 913ea5d98b0464fb3147f7a5fb93eee0862e2527 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Mon, 1 Jun 2020 23:28:44 +0000 Subject: [PATCH] Reland "Add trace of enqueued and sent RTP packets" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 45bb717a2866c2d836b5332a24af0d09f2b30714. Reason for revert: Use #if RTC_TRACE_EVENTS_ENABLED to avoid unused variable. Original change's description: > Revert "Add trace of enqueued and sent RTP packets" > > This reverts commit 45b9192ad981dcdc12ad4aef087fff2195bd030c. > > Reason for revert: When tracing is disabled, this results in a clang warning (unused variable), which results in a build error since Werror is enabled by default. > > Original change's description: > > Add trace of enqueued and sent RTP packets > > > > This is useful in debugging the latency from a packet > > is enqueued until it's sent. > > > > Bug: webrtc:11617 > > Change-Id: Ic2f194334a2e178de221df3a0838481035bb3505 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176231 > > Reviewed-by: Erik Språng > > Commit-Queue: Johannes Kron > > Cr-Commit-Position: refs/heads/master@{#31381} > > TBR=sprang@webrtc.org,kron@webrtc.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: webrtc:11617 > Change-Id: I854c17e587c624691a0e5e3ec9fd38c2607eda84 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176380 > Commit-Queue: Casey Fischer > Reviewed-by: Adam Nathan > Cr-Commit-Position: refs/heads/master@{#31399} TBR=sprang@webrtc.org,yujo@chromium.org,adamnathan@google.com,kron@webrtc.org,caseyfischer@google.com # Not skipping CQ checks because this is a reland. Bug: webrtc:11617 Change-Id: I9de7f7ed290481a51c161a693f5b2d5df7d2eae3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176367 Commit-Queue: Johannes Kron Reviewed-by: Erik Språng Reviewed-by: Johannes Kron Cr-Commit-Position: refs/heads/master@{#31407} --- modules/pacing/paced_sender.cc | 8 +++++ modules/pacing/packet_router.cc | 40 ++++++++++++++++------- modules/pacing/task_queue_paced_sender.cc | 12 +++++++ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 88effe4b6a..a0e76761e7 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -22,6 +22,7 @@ #include "rtc_base/location.h" #include "rtc_base/logging.h" #include "rtc_base/time_utils.h" +#include "rtc_base/trace_event.h" #include "system_wrappers/include/clock.h" namespace webrtc { @@ -114,8 +115,15 @@ void PacedSender::SetPacingRates(DataRate pacing_rate, DataRate padding_rate) { void PacedSender::EnqueuePackets( std::vector> packets) { { + TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("webrtc"), + "PacedSender::EnqueuePackets"); rtc::CritScope cs(&critsect_); for (auto& packet : packets) { + TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc"), + "PacedSender::EnqueuePackets::Loop", "sequence_number", + packet->SequenceNumber(), "rtp_timestamp", + packet->Timestamp()); + pacing_controller_.EnqueuePacket(std::move(packet)); } } diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index e9e8d4bd23..36b1e8eeec 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -24,6 +24,7 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/time_utils.h" +#include "rtc_base/trace_event.h" namespace webrtc { namespace { @@ -136,6 +137,10 @@ void PacketRouter::RemoveReceiveRtpModule( void PacketRouter::SendPacket(std::unique_ptr packet, const PacedPacketInfo& cluster_info) { + TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc"), "PacketRouter::SendPacket", + "sequence_number", packet->SequenceNumber(), "rtp_timestamp", + packet->Timestamp()); + rtc::CritScope cs(&modules_crit_); // With the new pacer code path, transport sequence numbers are only set here, // on the pacer thread. Therefore we don't need atomics/synchronization. @@ -168,6 +173,9 @@ void PacketRouter::SendPacket(std::unique_ptr packet, std::vector> PacketRouter::GeneratePadding( DataSize size) { + TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("webrtc"), + "PacketRouter::GeneratePadding", "bytes", size.bytes()); + rtc::CritScope cs(&modules_crit_); // First try on the last rtp module to have sent media. This increases the // the chance that any payload based padding will be useful as it will be @@ -179,24 +187,32 @@ std::vector> PacketRouter::GeneratePadding( if (last_send_module_ != nullptr && last_send_module_->SupportsRtxPayloadPadding()) { padding_packets = last_send_module_->GeneratePadding(size.bytes()); - if (!padding_packets.empty()) { - return padding_packets; - } } - // Iterate over all modules send module. Video modules will be at the front - // and so will be prioritized. This is important since audio packets may not - // be taken into account by the bandwidth estimator, e.g. in FF. - for (RtpRtcp* rtp_module : send_modules_list_) { - if (rtp_module->SupportsPadding()) { - padding_packets = rtp_module->GeneratePadding(size.bytes()); - if (!padding_packets.empty()) { - last_send_module_ = rtp_module; - break; + if (padding_packets.empty()) { + // Iterate over all modules send module. Video modules will be at the front + // and so will be prioritized. This is important since audio packets may not + // be taken into account by the bandwidth estimator, e.g. in FF. + for (RtpRtcp* rtp_module : send_modules_list_) { + if (rtp_module->SupportsPadding()) { + padding_packets = rtp_module->GeneratePadding(size.bytes()); + if (!padding_packets.empty()) { + last_send_module_ = rtp_module; + break; + } } } } +#if RTC_TRACE_EVENTS_ENABLED + for (auto& packet : padding_packets) { + TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc"), + "PacketRouter::GeneratePadding::Loop", "sequence_number", + packet->SequenceNumber(), "rtp_timestamp", + packet->Timestamp()); + } +#endif + return padding_packets; } diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index 2e8b72eb85..fccc1a6e64 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -17,6 +17,7 @@ #include "rtc_base/event.h" #include "rtc_base/logging.h" #include "rtc_base/task_utils/to_queued_task.h" +#include "rtc_base/trace_event.h" namespace webrtc { namespace { @@ -121,6 +122,17 @@ void TaskQueuePacedSender::SetPacingRates(DataRate pacing_rate, void TaskQueuePacedSender::EnqueuePackets( std::vector> packets) { +#if RTC_TRACE_EVENTS_ENABLED + TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("webrtc"), + "TaskQueuePacedSender::EnqueuePackets"); + for (auto& packet : packets) { + TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc"), + "TaskQueuePacedSender::EnqueuePackets::Loop", + "sequence_number", packet->SequenceNumber(), "rtp_timestamp", + packet->Timestamp()); + } +#endif + task_queue_.PostTask([this, packets_ = std::move(packets)]() mutable { RTC_DCHECK_RUN_ON(&task_queue_); for (auto& packet : packets_) {