From 45bb717a2866c2d836b5332a24af0d09f2b30714 Mon Sep 17 00:00:00 2001 From: Casey Fischer Date: Mon, 1 Jun 2020 18:38:55 +0000 Subject: [PATCH] Revert "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 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} --- modules/pacing/paced_sender.cc | 8 ----- modules/pacing/packet_router.cc | 40 ++++++++--------------- modules/pacing/task_queue_paced_sender.cc | 10 ------ 3 files changed, 13 insertions(+), 45 deletions(-) diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index a0e76761e7..88effe4b6a 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -22,7 +22,6 @@ #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 { @@ -115,15 +114,8 @@ 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 02befc91ec..e9e8d4bd23 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -24,7 +24,6 @@ #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 { @@ -137,10 +136,6 @@ 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. @@ -173,9 +168,6 @@ 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 @@ -187,28 +179,22 @@ 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()) { - // 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()) { + return padding_packets; } } - for (auto& packet : padding_packets) { - TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc"), - "PacketRouter::GeneratePadding::Loop", "sequence_number", - packet->SequenceNumber(), "rtp_timestamp", - packet->Timestamp()); + // 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; + } + } } return padding_packets; diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index e817f1b708..2e8b72eb85 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -17,7 +17,6 @@ #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 { @@ -122,15 +121,6 @@ void TaskQueuePacedSender::SetPacingRates(DataRate pacing_rate, void TaskQueuePacedSender::EnqueuePackets( std::vector> packets) { - 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()); - } - task_queue_.PostTask([this, packets_ = std::move(packets)]() mutable { RTC_DCHECK_RUN_ON(&task_queue_); for (auto& packet : packets_) {